Skip to content
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

Fix sequence indent #306

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Fix sequence indent #306

wants to merge 2 commits into from

Conversation

bcoca
Copy link

@bcoca bcoca commented Jan 25, 2025

1 of 2 fix for yaml/pyyaml#234

companion to yaml/pyyaml#842

bcoca added 2 commits January 24, 2025 18:58
so from
  stuff:
  -    indented_item

to
  stuff:
       - indented_item
@Jean-Daniel
Copy link

I'm not a libyaml maintainer, but wouldn't it be cleaner to rename yaml_emitter_write_indicator as yaml_emitter_write_indicator_indent or something like that, and creating a new yaml_emitter_write_indicator that would just call this new function with 0 as last argument.

It would simplify the patch as you would not have to update all call sites, but only the one that need to pass 1 as last argument.

That said, I don't think you can unconditionally change the libyaml output, as it will break all software relying on the current format.

@bcoca
Copy link
Author

bcoca commented Feb 3, 2025

@Jean-Daniel I agree that would be a smaller change, I might just do that (I just need to make sure I also do the defs/imports correctly).

As per being a format change, see the original ticket, all YAML importers should be able to deal with the change as it is already valid YAML. If any have a difference from the inputs it will be adding extra leading white space to the list item with the current format, which I believe is already incorrect for the spec.

@Jean-Daniel
Copy link

I agree this is not an issue for parser, but for people having many generated yaml in git repository, it would be a real issue.

For instance, people generating and storing Kubernetes manifests rely on format stability to be able to have significant diff when updating their manifests.

@jagibson
Copy link

That said, I don't think you can unconditionally change the libyaml output, as it will break all software relying on the current format.

I'm just a minor user, but it would be nice to have. Maybe this is something that can be adjusted with a feature-flag so it doesn't break default behavior?

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

Successfully merging this pull request may close these issues.

3 participants