fix(unique): ignore comments when comparing array elements#2766
Open
maximilize wants to merge 1 commit into
Open
fix(unique): ignore comments when comparing array elements#2766maximilize wants to merge 1 commit into
maximilize wants to merge 1 commit into
Conversation
The unique operator computed the dedup key for non-scalar elements by encoding them to YAML, which includes their head/line/foot comments. A comment sitting between two otherwise equal objects attaches to one of them, changing its encoding, so the duplicates were wrongly kept. Encode a comment-free copy of the node for the key so uniqueness depends on the data only. Fixes mikefarah#2491
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
yq 'unique'keeps duplicate objects when a comment sits between two equalitems:
yq 'unique' data1.ymlreturns both items instead of one. Scalars dedupecorrectly; the problem only affects non-scalar elements, and only when a
comment lands between the equal items.
Root cause
getUniqueKeyValue(pkg/yqlib/operator_unique.go) builds the dedup key fornon-scalar elements by encoding them to YAML:
The encoded form includes the node's head/line/foot comments. The
# Commentattaches to one of the two equal nodes, so their encodings differ and they are
treated as distinct. Scalars use
.Valuedirectly, which is why they wereunaffected.
Fix
Encode a comment-free deep copy of the node for the key, so uniqueness depends
on the data only.
Test
Added a scenario to
operator_unique_test.gofor the comment-between-equal-itemscase. It fails before the change (both items kept) and passes after (deduped to
one).
go test ./pkg/yqlib -run TestUniqueOperatorScenarios, gofmt,go vet,golangci-lint and
scripts/spelling.share clean for the change.Fixes #2491