Skip to content

Keep interior comments in empty flow collections - #412

Open
BetterAndBetterII wants to merge 1 commit into
yaml:mainfrom
BetterAndBetterII:fix/empty-flow-comment-406
Open

BetterAndBetterII wants to merge 1 commit into
yaml:mainfrom
BetterAndBetterII:fix/empty-flow-comment-406

Conversation

@BetterAndBetterII

Copy link
Copy Markdown

Empty flow collections were dropping interior comments (FootComment with empty content). Hold that comment as HeadComment so it survives emit.

Fixes #406

Copilot AI lite review requested due to automatic review settings September 2, 2026 21:39

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The emitter change is narrowly scoped, addresses the described failure mode for empty flow collections, and is backed by new regression tests covering the reported cases.

Pull request overview

Fixes emission of interior comments for otherwise-empty flow collections ({} / []) by preventing the START-event FootComment from being flushed before the opening bracket is written, keeping the comment inside the collection as intended (per #406).

Changes:

  • Reclassify START-event FootComment as HeadComment for empty flow sequences/mappings so it isn’t emitted before [/{.
  • Ensure flow sequences also flush any pending HeadComment when emitting the closing ] (covers the empty-sequence case with no items to trigger comment processing).
  • Add targeted regression coverage in both emitter.yaml and a Go unit test that validates bracket/comment ordering and re-parsability.
File summaries
File Description
internal/libyaml/emitter.go Holds interior comments for empty flow collections and emits them inside brackets; adds sequence-end head-comment flush.
internal/libyaml/testdata/emitter.yaml Adds roundtrip regression cases covering empty flow mapping/sequence comment placement scenarios from #406.
internal/libyaml/emitter_test.go Adds a focused unit test ensuring the emitted YAML keeps the comment between the opening and closing brackets and remains parseable.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread internal/libyaml/testdata/emitter.yaml Outdated
Comment thread internal/libyaml/testdata/emitter.yaml Outdated
@ingydotnet
ingydotnet force-pushed the fix/empty-flow-comment-406 branch from d3b7e5a to 4ec2d11 Compare September 2, 2026 22:00
Comment thread internal/libyaml/emitter.go
Comment thread internal/libyaml/emitter_test.go Outdated

@ccoVeille ccoVeille left a comment

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.

Sounds good. Thanks for working on this

@BetterAndBetterII
BetterAndBetterII force-pushed the fix/empty-flow-comment-406 branch from 6b209eb to 1bf0b5b Compare September 5, 2026 14:30
@BetterAndBetterII BetterAndBetterII changed the title fix(emitter): keep interior comments inside empty flow collections Keep interior comments in empty flow collections Sep 5, 2026
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.

comment in empty flow mapping are leading to invalid YAML

4 participants