Preserve YAML inline comments through encrypt/edit roundtrip#2131
Preserve YAML inline comments through encrypt/edit roundtrip#2131knowald wants to merge 3 commits intogetsops:mainfrom
Conversation
felixfontein
left a comment
There was a problem hiding this comment.
Please note that you need to sign-off your commit, otherwise this cannot be merged.
stores/yaml/store.go
Outdated
| store.addCommentsFoot(sequence.Content[len(sequence.Content)-1], headComments) | ||
| } | ||
| } | ||
| if len(inlineComments) > 0 && !beginning { |
There was a problem hiding this comment.
This opens the question what happens if len(inlineComments) > 0 && beginning.
Why not simply append inlineComments to headComments before the above if? That would be more compatible to what we have now (line comments ending up as head comments), and simplify the code.
There was a problem hiding this comment.
My original thought was to keep the two buffers separate end-to-end so the inline-vs-head distinction stayed explicit through the whole emit path - it felt symmetric with the split on the parsing side. Looking through it again i agree that it stops being meaningful at this point, folding definitely is the right call.
stores/yaml/store.go
Outdated
| } | ||
| } | ||
| // Trailing inline comments (rare, but handle gracefully as foot comments) | ||
| if len(inlineComments) > 0 { |
Add Inline flag to Comment struct, set during YAML parsing when a comment originates from a LineComment property. During write-back, inline comments are restored as LineComment on the correct node instead of being collapsed into HeadComment on the next node. Fixes getsops#2130 Signed-off-by: Kevin Nowald <kevin@nowald.dev>
Signed-off-by: Kevin Nowald <kevin@nowald.dev>
de8d425 to
2fd2adc
Compare
Signed-off-by: Kevin Nowald <kevin@nowald.dev>
Done, all commits are signed off now. Thanks for the heads up. |
Add Inline flag to Comment struct, set during YAML parsing when a comment originates from a LineComment property. During write-back, inline comments are restored as LineComment on the correct node instead of being collapsed into HeadComment on the next node.
Fixes #2130