yamlfix
yamlfix copied to clipboard
lyz-code
Preserve comments for a file with only comments
Description
When a file has only comments, the comments are removed
Steps to reproduce
Run fix_code over:
---
# Keep comments!
Current behavior
The file is transformed to:
---
...
Desired behavior
The file is unchanged
Additional context
@rsnodgrass created the unit test in this PR which was closed due to inactivity, if you want to implement this, you can follow in that branch
This is a big issue, comments were deleted out of dozens of YAML files that explained the reasoning behind various YAML entries. This was not caught for quite some time until someone opened up a reformatted yaml file a month ago later and could not understand why documentation was missing.
Comments within yaml should be preserved, as they are often as important as the actual data in the yaml.
Comments are kept when the file is not empty, for example, the next snippet is left unchanged by yamlfix:
---
# this is a comment
a: 1
I understand your case, but I feel it's a corner case, as you're using a yaml file for documenting instead of holding data. I'd either suggest you to summit a PR with the fix or add a a: 1 at the end of every file.
The ruyaml maintainer made a comment on StackOverflow how this could be fixed: https://stackoverflow.com/a/65001415 Does not look that hard to implement, but I'm wondering, why he chose to not add this to the upstream / ruyaml project itself.
I'm closing this as won't fix as I don't feel this feature has a big impact. If someone want's to implement it I'm fine with reviewing it and reopening this issue.
This is an interesting bug because sometimes 'yamlfix' gets added to GitHub workflows that are shared broadly. The "owner" of the YAML files may never even be able to make a decision whether or when yamlfix gets applied...and the workflow owner has no idea whether or not all YAML files that may be processed by their change will have comments.
This is a "destructive" bug IMO since it can destroy important information in YAML files, even if those aren't yaml fields.
EVEN further, the very title in GitHub of 'yamlfix' is: "A simple opinionated yaml formatter that KEEPS YOUR COMMENTS!"
Someone adding yamlfix to a workflow or running it across sets of files would assume that the yaml files output would be syntactically identical AND preserve all comments, giving them no pause to run the tool. In reality, this is not the case.
In six months you've been the only user that has shown this need. As a maintainer I feel it's a corner case that don't reflect the general need of the program.
That being said the fix is not difficult. If you're willing to do a pull request I'm more than open to review it. Else, this will remain closed.
I'm sorry
I think this issue is a special case of #123
If you look at the shebang'ed ansible file, there are actually two yaml documents in the file - a comment only document (containing only the shebang) and the rest of the ansible yaml file, separated through the document start indicator ---.
The current implementation is only handling the case for shebang, but I could see a general implementation that even would not rely on string manipulation an also not need to rely on parsing the buffer. We could add a top-level node with a uuid before parsing, run it through everything, and then just remove the line with the uuid-node -> that would prevent ruyaml from removing the comments, it would be more general as it could handle all comment-only files and multi-document yaml files, the only special case for shebang would be to prevent it from adding the document-start indicator to the first yaml document.
@lyz-code I can implement this as well, but only if you want to have it in the codebase / want to maintain such a feature.
As an additional data point - we would also like to keep comments, even if it's an empty yaml document. Our use case is similar to others - entries are added as a way to document how to override some defaults.
I believe the user creating this one had some similar expectations: https://github.com/lyz-code/yamlfix/issues/255
@lyz-code Could you please reopen this one? I understand not wanting to spend time on it, but seeing that it's requested by a few people it may attract a PR contribution. 🙂
Tthe comments can be as valuable...or in some cases more valuable than the actual entries.
Sure @kbakk , I will only accept a PR to solve this issue as long as it doesn't increase too much the complexity of maintenance
This breaks Ansible playbooks, for example, can't use # jinja2:lstrip_blocks: true any idea how to preserve the commenters before the --- ?
@lyz-code https://github.com/lyz-code/yamlfix/pull/263 hope this is OK, it doesn't cover maintaining all comments, but prevents breaking ansible templates.
