8000 Fixes bytes convertion method, last episode by jfsimon · Pull Request #7489 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

Fixes bytes convertion method, last episode #7489

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 27, 2013

Conversation

jfsimon
Copy link
Contributor
@jfsimon jfsimon commented Mar 27, 2013

This definitely fixes the bytes convertion method.

@lazyhammer thanks for your support & indulgence
@fabpot sorry for the running gag
@vicb I opened a ticket: https://bugs.php.net/bug.php?id=64530

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #7481

@vicb
< 8000 div class="timeline-comment-actions flex-shrink-0 d-flex flex-items-center">
Copy link
Contributor
vicb commented Mar 27, 2013

To be really perfect, I would write

return (intval($match[2], $bases[$match[1]]) * (1 << $shifts[$match[3]]));

as

return intval($match[2], $bases[$match[1]]) << $shifts[$match[3]];

@vicb
Copy link
Contributor
vicb commented Mar 27, 2013

and btw, great you've opened a ticket on php.net.

@jfsimon
Copy link
Contributor Author
jfsimon commented Mar 27, 2013

@vicb I just amended my commit which is now perfect, thanks!

fabpot added a commit that referenced this pull request Mar 27, 2013
This PR was merged into the master branch.

Discussion
----------

Fixes bytes convertion method, last episode

This definitely fixes the bytes convertion method.

@lazyhammer thanks for your support & indulgence
@fabpot sorry for the running gag
@vicb I opened a ticket: https://bugs.php.net/bug.php?id=64530

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #7481

Commits
-------

233b945 fixed bytes convertion method, again
@fabpot fabpot merged commit 233b945 into symfony:master Mar 27, 2013
@lazyhammer
Copy link
Contributor

Oh, great. I'm late again :(
I've just noticed one undocumented feature of intval(). If you pass 0 as base, it will guess the base! So we can simplify this to:

$shifts = array('K' => 10, 'k' => 10, 'M' => 20, 'm' => 20, 'G' => 30, 'g' => 30);
$result = intval($iniMax, 0);
if (isset($shifts[substr($iniMax, -1)])) {
    $result <<= $shifts[substr($iniMax, -1)];
}

@jfsimon
Copy link
Contributor Author
jfsimon commented Mar 27, 2013

@lazyhammer PHP is really full of secrets ;)

@vicb
Copy link
Contributor
vicb commented Mar 27, 2013

@lazyhammer did you get that by looking at the source code or is this documented somewhere ? - If this is not documented, it would be a great addition to http://www.php.net/docs.php

@lazyhammer
Copy link
Contributor

@vicb There are too many comments on intval's doc, so for me it was easier to read the source code :)

@vicb
Copy link
Contributor
vicb commented Mar 27, 2013

@lazyhammer may be you can create a ticket on php.net for "(, 0) -> autodetection" to be added to the documentation (ie the main section of intval doc, not the comments) ?

@pborreli
Copy link
Contributor

If you need more info for documenting :

PHP intval function uses C function strtol which accept 0 as base parameter http://www.cplusplus.com/reference/cstdlib/strtol/

If the value of base is zero, the syntax expected is similar to that of integer constants, which is formed by a succession of:

  • An optional sign character (+ or -)
  • An optional prefix indicating octal or hexadecimal base ("0" or "0x"/"0X" respectively)
  • A sequence of decimal digits (if no base prefix was specified) or either octal or hexadecimal digits if a specific prefix is present

Numerical base (radix) that determines the valid characters and their interpretation.
If this is 0, the base used is determined by the format in the sequence (see above).

@vicb
Copy link
Contributor
vicb commented Mar 27, 2013

I think a PHP documentation (vs C) is needed on php.net (vs gh/Symfony). Thanks for the details anyway.

@lazyhammer
Copy link
Contributor

@pborreli I know, but thanks anyway :)
@vicb I submitted a patch: https://edit.php.net/?patch=en/reference/var/functions/intval.xml&project=PHP

@vicb
Copy link
Contributor
vicb commented Mar 27, 2013

👍

@stof
Copy link
Member
stof commented Mar 27, 2013

@lazyhammer The link you gave is not the link to the patch but the link to the page to submit a new patch

@vicb
Copy link
Contributor
vicb commented Mar 27, 2013

@stof I can see the patch

On 03/27/2013 01:50 PM, Christophe Coevoet wrote:

@lazyhammer https://github.com/lazyhammer The link you gave is not
the link to the patch but the link to the page to submit a new patch


Reply to this email directly or view it on GitHub
#7489 (comment).

@lazyhammer
Copy link
Contributor

@stof I can't find another link.
The strange thing is that the preview shows the old version, at least for me. But you can see it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants
0