Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Yaml] parse omitted inlined mapping values as null#21118

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:masterfromxabbuh:inline-null-parsing
Mar 1, 2017

Conversation

@xabbuh
Copy link
Member

QA
Branch?master
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#20841 (comment)
LicenseMIT
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 asnull (you have to pass~ ornull explicitly).

sh4ka reacted with thumbs up emoji
@GuilhemN
Copy link
Contributor

GuilhemN commentedJan 1, 2017
edited
Loading

Happy new year ! Best wishes :)

About your change, why not replacing the+here by a* ?

@xabbuh
Copy link
MemberAuthor

Happy new year for you too! :)

About your change, why not replacing the + here by a * ?

That's indeed much better. I updated the code accordingly.

GuilhemN and medalibs reacted with hooray emoji

@GuilhemN
Copy link
Contributor

GuilhemN commentedJan 1, 2017
edited
Loading

Having anull key should be forbidden at the same time as it is not supported by PHP. Not sure how to do it as{: foo} and{'': foo} will be considered the same way while only the second one should be valid.
Same applies for other constants btw{false: foo} should not be valid in a PHP parser but it is equivalent to{'false': foo} currently (keys are not evaluated).

@nicolas-grekasnicolas-grekas added this to the3.3 milestoneJan 2, 2017
@fabpot
Copy link
Member

@xabbuh any comments regarding the last comment by@GuilhemN?

@xabbuh
Copy link
MemberAuthor

@GuilhemN@fabpot As far as I understand the specs the key cannot be omitted in a mapping and it will never be treated as a boolean value (correct me if I am wrong). The YAML parser athttp://yaml-online-parser.appspot.com/ seem to confirm this.

@fabpot
Copy link
Member

@xabbuh I think that's exactly the problem mentioned by@GuilhemN

You can add the following test, which is valid YAML (and works great):

'empty key is allowed' =>array('{"": "foo"}',array("" =>"foo")),

But this one should fail (but it does not):

'null key is not allowed' =>array('{: "foo"}',array(null =>"foo")),

@xabbuh
Copy link
MemberAuthor

That's indeed an issue. I opened#21643 to fix this in older maintained versions too (this one should be merged intomaster as it IMO is a new feature).

@fabpot
Copy link
Member

@xabbuh Can you rebase and add a note in the CHANGELOG?

@xabbuh
Copy link
MemberAuthor

done

@fabpot
Copy link
Member

Thank you@xabbuh.

@fabpotfabpot merged commitc473504 intosymfony:masterMar 1, 2017
fabpot added a commit that referenced this pull requestMar 1, 2017
…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
@xabbuhxabbuh deleted the inline-null-parsing branchMarch 1, 2017 15:41
nicolas-grekas added a commit that referenced this pull requestMar 2, 2017
…agi)This PR was merged into the 3.3-dev branch.Discussion----------[Yaml] Fix legacy support for omitting mapping key| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | N/A| License       | MIT| Doc PR        | N/AQuick and dirty fix for the failing legacy `InlineTest::testOmittedMappingKeyIsParsedAsColon()` test (failing since#21118).Ping@xabbuh : I'm not used to the yaml component codebase and it's probably not the most pleasant one to read. 😄 So if you think there is a cleaner way to go, please just close this one in favor of yours.Commits-------d246f2f [Yaml] Fix legacy support for omitting mapping key
@fabpotfabpot mentioned this pull requestMay 1, 2017
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

3.3

Development

Successfully merging this pull request may close these issues.

5 participants

@xabbuh@GuilhemN@fabpot@nicolas-grekas@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp