Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes handling of comments within TOML arrays by ensuring they are properly preserved during parsing and encoding. The change allows comments to be attached to array elements and formatted correctly in multiline array output.
Changes:
- Updated TOML decoder to capture and preserve comments within arrays
- Enhanced TOML encoder to write multiline array format when elements have comments
- Added a roundtrip test case to verify comment preservation in TOML arrays
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/yqlib/toml_test.go | Adds test data and test case for TOML arrays with comments to verify roundtrip behavior |
| pkg/yqlib/encoder_toml.go | Implements multiline array formatting logic that preserves element head comments |
| pkg/yqlib/decoder_toml.go | Adds comment collection and attachment to array elements during TOML parsing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Attach any pending comments to this array element | ||
| if len(pendingArrayComments) > 0 { | ||
| yamlNode.HeadComment = strings.Join(pendingArrayComments, "\n") | ||
| pendingArrayComments = make([]string, 0) |
There was a problem hiding this comment.
Use nil or [:0] instead of make([]string, 0) to reset the slice. The current approach allocates a new slice unnecessarily. Simply setting to nil or reslicing would be more efficient.
| pendingArrayComments = make([]string, 0) | |
| pendingArrayComments = pendingArrayComments[:0] |
| if strings.TrimSpace(commentLine) != "" { | ||
| if !strings.HasPrefix(strings.TrimSpace(commentLine), "#") { | ||
| commentLine = "# " + commentLine | ||
| } | ||
| if _, err := w.Write([]byte(" " + commentLine + "\n")); err != nil { |
There was a problem hiding this comment.
The variable commentLine is being modified after trimming, but the original untrimmed value is used in the write. This could lead to confusion. Consider storing the trimmed value in a separate variable or modifying commentLine before the prefix check.
| if strings.TrimSpace(commentLine) != "" { | |
| if !strings.HasPrefix(strings.TrimSpace(commentLine), "#") { | |
| commentLine = "# " + commentLine | |
| } | |
| if _, err := w.Write([]byte(" " + commentLine + "\n")); err != nil { | |
| trimmed := strings.TrimSpace(commentLine) | |
| if trimmed != "" { | |
| if !strings.HasPrefix(trimmed, "#") { | |
| trimmed = "# " + trimmed | |
| } | |
| if _, err := w.Write([]byte(" " + trimmed + "\n")); err != nil { |
No description provided.