Skip to content

[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

Merged
merged 1 commit into from
Dec 29, 2024

Conversation

alexandre-daubois
Copy link
Member

@alexandre-daubois alexandre-daubois commented Sep 12, 2024

Q A
Branch? 7.3
Bug fix? no
New feature? yes
Deprecations? no
Issues Fix #58155
License MIT

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!

@xabbuh
Copy link
Member

xabbuh commented Sep 12, 2024

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?

@alexandre-daubois
Copy link
Member Author

@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. [foo, , bar]. Currently, the second value is ignored, where the spec says it should be handled as null.

@xabbuh
Copy link
Member

xabbuh commented Sep 16, 2024

see #58279

@alexandre-daubois alexandre-daubois changed the title [Yaml] Add support for dumping null as an empty string by using the Yaml::DUMP_NULL_AS_EMPTY_IN_EXPANDED_NOTATION flag [Yaml] Add support for dumping null as an empty value by using the Yaml::DUMP_NULL_AS_EMPTY flag Sep 17, 2024
@alexandre-daubois
Copy link
Member Author

I just pushed a new approach that takes into account @xabbuh's PR on supporting empty values in sequences.

@alexandre-daubois alexandre-daubois force-pushed the null-as-empty branch 3 times, most recently from 60c09d3 to 6d7e910 Compare September 18, 2024 11:49
@alexandre-daubois
Copy link
Member Author

Status: Needs review

@OskarStark OskarStark requested a review from stof September 19, 2024 05:59
@javiereguiluz javiereguiluz added the ❄️ Feature Freeze Important Pull Requests to finish before the next Symfony "feature freeze" label Oct 3, 2024
@fabpot fabpot modified the milestones: 7.2, 7.3 Nov 20, 2024
Copy link
Contributor

@OskarStark OskarStark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For 7.3

@OskarStark OskarStark removed the ❄️ Feature Freeze Important Pull Requests to finish before the next Symfony "feature freeze" label Dec 29, 2024
@xabbuh
Copy link
Member

xabbuh commented Dec 29, 2024

Thank you @alexandre-daubois.

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

Successfully merging this pull request may close these issues.

[Yaml] Dumping with no value (like Docker Compose volumes section) isn't supported
7 participants