Skip to content

Preserve YAML inline comments through encrypt/edit roundtrip#2131

Open
knowald wants to merge 3 commits intogetsops:mainfrom
knowald:fix/yaml-inline-comment-roundtrip
Open

Preserve YAML inline comments through encrypt/edit roundtrip#2131
knowald wants to merge 3 commits intogetsops:mainfrom
knowald:fix/yaml-inline-comment-roundtrip

Conversation

@knowald
Copy link
Copy Markdown

@knowald knowald commented Apr 1, 2026

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

@knowald knowald marked this pull request as draft April 1, 2026 09:22
@felixfontein felixfontein changed the title fix: preserve YAML inline comments through encrypt/edit roundtrip Preserve YAML inline comments through encrypt/edit roundtrip Apr 6, 2026
@knowald knowald marked this pull request as ready for review April 7, 2026 15:19
Copy link
Copy Markdown
Contributor

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please note that you need to sign-off your commit, otherwise this cannot be merged.

store.addCommentsFoot(sequence.Content[len(sequence.Content)-1], headComments)
}
}
if len(inlineComments) > 0 && !beginning {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
}
// Trailing inline comments (rare, but handle gracefully as foot comments)
if len(inlineComments) > 0 {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks.

knowald added 2 commits April 7, 2026 17:42
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>
@knowald knowald force-pushed the fix/yaml-inline-comment-roundtrip branch from de8d425 to 2fd2adc Compare April 7, 2026 15:42
@knowald
Copy link
Copy Markdown
Author

knowald commented Apr 7, 2026

Please note that you need to sign-off your commit, otherwise this cannot be merged.

Done, all commits are signed off now. Thanks for the heads up.

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.

YAML store converts inline comments to preceding-line comments on encrypt/edit roundtrip

2 participants