[Yaml] parse mappings with a colon in a key #20841
Conversation
xabbuh
commented
Dec 9, 2016
•
| Q | A |
|---|---|
| Branch? | master |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #19874 |
| License | MIT |
| Doc PR |
|
Does it change the parsed value for previously accepted values ? If yes, we should not put this in 2.7 IMO |
| @@ -246,6 +246,8 @@ public function getTestsForParse() | |||
| array('{\'foo\': \'bar\', "bar": \'foo: bar\'}', array('foo' => 'bar', 'bar' => 'foo: bar')), | |||
| array('{\'foo\'\'\': \'bar\', "bar\"": \'foo: bar\'}', array('foo\'' => 'bar', 'bar"' => 'foo: bar')), | |||
| array('{\'foo: \': \'bar\', "bar: ": \'foo: bar\'}', array('foo: ' => 'bar', 'bar: ' => 'foo: bar')), | |||
| array('{foo:bar: "baz"}', array('foo:bar' => 'baz')), | |||
There was a problem hiding this comment.
@stof This was parsed differently before (it would have been parsed as ['foo' => 'bar: baz']).
Actually, I am really mixed about what to do here. But I could live with merging it into master to be completely safe.
There was a problem hiding this comment.
as it was accepted before, we should indeed do it in master.
changing the behavior in a 2.7 patch release is too likely to break things silently.
There was a problem hiding this comment.
Actually, in master, it should still parse as before and generate a deprecation notice, and the behavior changed in 4.0.
There was a problem hiding this comment.
Then let's postpone this PR for 4.0. The deprecation should already be covered by #19504 if I didn't overlook any edge cases.
There was a problem hiding this comment.
The current deprecation is in part wrong, values such as {foo:[bar]} or {foo:} are valid, but the deprecation is triggered ({foo:} is not supported by the parser currently so for this one, I guess it's ok).
And what about replacing its message by The behaviour of colon not followed by an indicator (e.g " ", ",", "{", "}", "[", "]") is deprecated since 3.2. It will be considered as part of the value in 4.0.?
There was a problem hiding this comment.
In fact, this is not valid YAML at all. However, we have to improve the deprecation handling (see #22829).
…abbuh) This PR was merged into the 3.3-dev branch. Discussion ---------- [Yaml] parse omitted inlined mapping values as null | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #20841 (comment) | License | MIT | Doc PR | As @GuilhemN mentioned in #20841 (comment) when using the inline YAML notation, it is currently not possible to completely omit the mapping value to have it parsed as `null` (you have to pass `~` or `null` explicitly). Commits ------- c473504 parse omitted inlined mapping values as null
|
Closing here for now. To be reopened when we start the development of 4.0. |