[Yaml] Allow using _ in some numeric notations #18096
Conversation
|
It seems that binaries too should accept this, but as on 2.3 it is not handled (maybe on master though ? as the binary notation was introduced on php 5.4 |
|
Leading |
|
Indeed. I'll fix that then. |
| return '0x' === $scalar[0].$scalar[1] ? hexdec($scalar) : (float) $scalar; | ||
| case '.inf' === $scalarLower: | ||
| case '.nan' === $scalarLower: | ||
| return -log(0); | ||
| case '-.inf' === $scalarLower: | ||
| return log(0); | ||
| case preg_match('/^(-|\+)?[0-9,]+(\.[0-9]+)?$/', $scalar): |
There was a problem hiding this comment.
I don't think I saw anywhere that the , was authorized though ? I kept it, but nothing in the specs says it should be used as a separator (And as @nicolas-grekas noticed, _123_456_ should not be a valid number, so shouldn't ,123,456, be also considered invalid or a string ?
There was a problem hiding this comment.
You should check with a reference parser, e.g. http://yaml-online-parser.appspot.com/
There was a problem hiding this comment.
That tool is awesome, and yes, the , is not allowed as a separator (it renders as string when one is used in a number). I'd rather not touch it, as it would introduce a bc break.
But 123_ seems to be a valid integer too, so I need to keep that (it will simplify some regex)
2ac1ddb to
0fbc94e
Compare
|
Some thoughts:
|
|
Given that this will change the way we treat the value, I'm against putting this in a patch release, as it might break existing project. This should go in master |
|
and the dumper needs to be changed, to detect that a string |
|
I will rebase this on |
|
Closing in favor of #18486 |
This PR was merged into the 3.2-dev branch. Discussion ---------- [Yaml] Allow using _ in some numeric notations | Q | A | ------------- | --- | Branch | master | Bug fix? | no ? | New feature? | yes ? | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #18094 | License | MIT | Doc PR | ~ Allows to use the `_` to group "big" ints, as suggested in the yaml integer type specification. As discussed in #18094, we should check if it is still part of the 1.2 specification, but I don't really see why not ? I can't see anywhere anything saying it is not valid anymore... as there are links to these types in some other specs. This is #18096, but targetted on master as this is considered as a new feature. I also support the dump of such values as only strings. I think I should change how it is dumped thoug, and use the escape filter instead though (as I was misusing the data provider and it provided strange results at the time) Commits ------- e6da11c [Yaml] Allow using _ in some numeric notations
Allows to use the
_to group "big" ints, as suggested in the yaml integer type specification. As discussed in #18094, we should check if it is still part of the 1.2 specification, but I don't really see why not ? I can't see anywhere anything saying it is not valid anymore... as there are links to these types in some other specs.