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

Array-indent not maintained between parseDocument/toString #283

Open
grantila opened this issue Jun 14, 2021 · 2 comments
Open

Array-indent not maintained between parseDocument/toString #283

grantila opened this issue Jun 14, 2021 · 2 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@grantila
Copy link

grantila commented Jun 14, 2021

Describe the bug

First of all, thanks for an awesome yaml parser/stringifyer! I just wrote this this weekend: yaml-diff-patch using this library to allow modifying a yaml using JSON patches.

I'm not sure if this is considered a bug, but I'd very much hope so. The library would be most useful if it maintains as much as possible from the source yaml when parsing the document into AST/CST so that stringifying it produces the exact same result. This means that minimal (or no) modifications to the AST would produce a minimal (or no) diff when text diffing the original yaml with the modified. In other words, only what you change, would be part of the final diff.

To Reproduce

The following:

a:
- b: c

when parseDocument -> toString produces:

a:
  - b: c

Expected behaviour

I totally understand how this looks better. I also totally understand that a document with both these indentations in different places is pretty ugly, and being able to "reformat" it is great. However, being able to maintain all blocks' indentations individually as-is makes more sense when trying to make a minimal diff that only touches what it needs.

I noticed (in 1.10.2) that quite a few things get re-organized, long lines get chopped into multiple shorter, long one-line arrays get split into line-by-line arrays etc. I haven't tried that in 2.0 yet, so you can ignore that for now.

Versions (please complete the following information):

  • Environment: Node.js 14.15.1
  • yaml: 1.10.2 and 2.0.0-6
@grantila grantila added the bug Something isn't working label Jun 14, 2021
@grantila
Copy link
Author

I noticed (in 1.10.2) that quite a few things get re-organized, long lines get chopped into multiple shorter, long one-line arrays get split into line-by-line arrays etc. I haven't tried that in 2.0 yet, so you can ignore that for now.

Just tested in 2.0.0-6, long lines of text get chopped down (line broken), and so does long one-line arrays.

@eemeli
Copy link
Owner

eemeli commented Jun 15, 2021

I think you're reporting/conflating two different issues here. I'll address just the sequence indent here, and hope you'll file a separate issue if there's something else to consider as well.

I understand and agree with your desire to keep an input file as close as possible to its original form. At least for now, the library doesn't do that with respect to indentation. The ToString Options do include indent and indentSeq for controlling the output, but their default values are not determined from any parsed document.

While working on the parser update, I did look into this at one point, but didn't come up with an easy answer for how to detect and store the indent styles. As this isn't a priority for me just now, a PR on this would be very welcome.

@eemeli eemeli added enhancement New feature or request help wanted Extra attention is needed and removed bug Something isn't working labels Jun 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants