Skip to content

[Yaml] Add compact nested mapping support to Dumper #59315

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 2, 2025
Merged

[Yaml] Add compact nested mapping support to Dumper #59315

merged 1 commit into from
Jan 2, 2025

Conversation

gr8b
Copy link

@gr8b gr8b commented Dec 29, 2024 •
edited
Loading

Q A
Branch? 7.3
Bug fix? no
New feature? yes
Deprecations? no
Issues -
License MIT

Feature allow to decrease count of lines in YAML output using list hash start line also as hash dump first line.

Original output

planets:
  -
    name: Mercury
    distance: 57910000
  -
    name: Jupiter
    distance: 778500000

With Yaml::DUMP_COMPACT_NESTED_MAPPING flag enabled.

planets:
  - name: Mercury
    distance: 57910000
  - name: Jupiter
    distance: 778500000

@gr8b gr8b requested a review from xabbuh as a code owner December 29, 2024 00:08
@carsonbot carsonbot added this to the 7.3 milestone Dec 29, 2024
@carsonbot
Copy link

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has a contribution guide which I suggest you to read.

In short:

  • Always add tests
  • Keep backward compatibility (see https://symfony.com/bc).
  • Bug fixes must be submitted against the lowest maintained branch where they apply (see https://symfony.com/releases)
  • Features and deprecations must be submitted against the 7.3 branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

OskarStark
OskarStark approved these changes Dec 29, 2024
View reviewed changes
@OskarStark OskarStark changed the title [Yaml] Add compact nested mapping support to Yaml dumper [Yaml] Add compact nested mapping support to Dumper Dec 30, 2024
@OskarStark
Copy link
Contributor

CI is red, can you please check? Thanks

@gr8b gr8b force-pushed the compact-nested-mapping branch from 5f3ebbc to 6101143 Compare December 31, 2024 01:51
@gr8b
Copy link
Author

gr8b commented Dec 31, 2024

One test is still failing, link.
@OskarStark can you please clarify what src/Symfony/Component/VarDumper/Tests/Dumper/ServerDumperTest.php test is for, so I can identify what in implementation might be causing it to fail?

@fabpot fabpot force-pushed the compact-nested-mapping branch from 6101143 to f67e636 Compare January 2, 2025 11:55
@fabpot
Copy link
Member

fabpot commented Jan 2, 2025

Thank you @gr8b.

@fabpot fabpot merged commit ec3f12b into symfony:7.3 Jan 2, 2025
1 of 2 checks passed
@fabpot fabpot mentioned this pull request Jan 2, 2025
Closed
@gr8b gr8b deleted the compact-nested-mapping branch January 4, 2025 15:56
@xabbuh xabbuh mentioned this pull request Feb 2, 2025
Merged
nicolas-grekas added a commit that referenced this pull request Feb 4, 2025
@nicolas-grekas
bug #59671 [Yaml] Yaml::DUMP_COMPACT_NESTED_MAPPING does not apply wh…
dc38bb0 
…en dumping sequences (xabbuh)

This PR was squashed before being merged into the 7.3 branch.

Discussion
----------

[Yaml] Yaml::DUMP_COMPACT_NESTED_MAPPING does not apply when dumping sequences

| Q             | A
| ------------- | ---
| Branch?       | 7.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        |
| License       | MIT

following #59315, the flag should only affect how maps are dumped but shouldn't influence dumping sequences (see also symfony/symfony-docs#20510 (comment))

Commits
-------

0c124a9 [Yaml] Yaml::DUMP_COMPACT_NESTED_MAPPING does not apply when dumping sequences
@fabpot fabpot mentioned this pull request May 2, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xabbuh xabbuh xabbuh left review comments

@fabpot fabpot fabpot approved these changes

@OskarStark OskarStark OskarStark approved these changes

Assignees
No one assigned
Labels
Feature Status: Reviewed Yaml
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.
5 participants
@gr8b @carsonbot @OskarStark @fabpot @xabbuh