-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Yaml] Add support for dumping null
as an empty value by using the Yaml::DUMP_NULL_AS_EMPTY
flag
#58243
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
Conversation
The name of the constant makes me think that we stills should improve the parser to try to support all possible inline notations. Do you have a list of examples that would currently not be parsed properly? |
@xabbuh by taking into account the cases we don't want to support as listed by @stof in #58232 (comment), I would say that the only unsupported feature we may want to add support to is empty values in sequences, i.e. |
see #58279 |
6764111
to
0e0246b
Compare
null
as an empty string by using the Yaml::DUMP_NULL_AS_EMPTY_IN_EXPANDED_NOTATION
flagnull
as an empty value by using the Yaml::DUMP_NULL_AS_EMPTY
flag
I just pushed a new approach that takes into account @xabbuh's PR on supporting empty values in sequences. |
0e0246b
to
f8d019a
Compare
60c09d3
to
6d7e910
Compare
Status: Needs review |
6d7e910
to
ece1285
Compare
ece1285
to
a7d3a34
Compare
…`Yaml::DUMP_NULL_AS_EMPTY` flag
a7d3a34
to
1fc1379
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For 7.3
Thank you @alexandre-daubois. |
Another try after #58232. I went for the second solution from @stof (#58232 (comment)), namely dumping empty value as null just in expanded sequences and mappings. When the inline limit of the dumper is reached, it fallbacks on using
null
(examples in tests).I'm not 100% happy about the name of the constant,
DUMP_NULL_AS_EMPTY_IN_EXPANDED_NOTATION
. It's verbose. I'm open to suggestions!