Conversation
Unmarshal from yamls with sequence merge key has been working fine for generic maps (aka. map[string]any types), but has been broken for structs. This commits fixes that, by properly handling marking a sequence as `isMerge` in keyToNodeMap. Perviously it always assumed that the entry node will be always a map, despite having logic for properly handling these.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #783 +/- ##
=======================================
Coverage 77.97% 77.97%
=======================================
Files 22 22
Lines 8108 8110 +2
=======================================
+ Hits 6322 6324 +2
Misses 1370 1370
Partials 416 416 🚀 New features to boost your workflow:
|
|
@goccy any chance on merging this? |
There was a problem hiding this comment.
Pull request overview
This pull request fixes issue #776 where unmarshaling YAML with sequence merge keys (<<: [*a, *b]) was failing for structs but working for generic maps. The root cause was that the code wasn't properly passing the merge context flag through the call chain, causing getMapNode to reject sequence nodes even when processing merge keys.
Changes:
- Added an
isMergeparameter to track when processing merge key values in the decode logic - Added test coverage for struct unmarshaling with sequence merge keys
- Added test coverage to ensure map unmarshaling continues to work correctly
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| decode.go | Refactored keyToNodeMap to accept an isMerge parameter via a new internal _keyToNodeMap function, and properly passes isMerge=true when recursively processing merge key values |
| decode_test.go | Added two test cases exercising struct and map unmarshaling with sequence merge keys (<<: [*a, *b]) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Unmarshal from yamls with sequence merge key has been working fine for generic maps (aka. map[string]any types), but has been broken for structs: see #776 for more details.
This commits fixes that, by properly handling marking a sequence as
isMergein keyToNodeMap. Previously it always assumed that the entry node will be always a map, despite having logic for properly handling these.A test case was also added, one which exercises struct unmarshaling logic.
Fixes: #776