From e744274e3f397af63c37a71933533ad4a555b76d Mon Sep 17 00:00:00 2001 From: Jakob Koschel Date: Thu, 30 Jul 2026 08:25:41 +0000 Subject: [PATCH 1/2] Add `remove_duplicates` ignore_comments and merge_comments logic Added `DuplicateResolution` enum to handle advanced deduplication modes. The new mores are `ignore_comments` to keep only the first comment for deduplicated items and `merge_comments` to merge comments from all identical items when deduplicated (identical comment lines are also deduplicated when merging). --- README.md | 5 ++++ goldens/duplicates_ignore_comments.in | 18 +++++++++++++++ goldens/duplicates_ignore_comments.out | 12 ++++++++++ goldens/duplicates_merge_comments.in | 21 +++++++++++++++++ goldens/duplicates_merge_comments.out | 16 +++++++++++++ keepsorted/block.go | 32 +++++++++++++++++++++----- keepsorted/keep_sorted_test.go | 12 +++++----- keepsorted/options.go | 27 ++++++++++++++++++++-- keepsorted/options_parser.go | 20 ++++++++++++++++ 9 files changed, 149 insertions(+), 14 deletions(-) create mode 100644 goldens/duplicates_ignore_comments.in create mode 100644 goldens/duplicates_ignore_comments.out create mode 100644 goldens/duplicates_merge_comments.in create mode 100644 goldens/duplicates_merge_comments.out diff --git a/README.md b/README.md index 063b31b..7416397 100644 --- a/README.md +++ b/README.md @@ -899,6 +899,11 @@ rotation: foo The duplicate handling can be changed with the switch `remove_duplicates`: +* `yes` (default): Deduplicates based on both code and comments. +* `no`: Leaves duplicates untouched. +* `ignore_comments`: Deduplicates based on code lines only, retaining only the comment of the very first duplicate occurrence. +* `merge_comments`: Deduplicates based on code lines only, merging unique comments from all occurrences to the single remaining entry. + ```diff +# keep-sorted start remove_duplicates=no rotation: bar diff --git a/goldens/duplicates_ignore_comments.in b/goldens/duplicates_ignore_comments.in new file mode 100644 index 0000000..2dbc0d2 --- /dev/null +++ b/goldens/duplicates_ignore_comments.in @@ -0,0 +1,18 @@ +Remove duplicates but ignore comments: +// keep-sorted-test start remove_duplicates=ignore_comments sticky_comments=yes +// First foo +foo +bar +// Second foo +foo +// Third foo +foo +// keep-sorted-test end + +Remove duplicates ignoring comments, when the first item has no comment: +// keep-sorted-test start remove_duplicates=ignore_comments sticky_comments=yes +foo +bar +// Second foo +foo +// keep-sorted-test end diff --git a/goldens/duplicates_ignore_comments.out b/goldens/duplicates_ignore_comments.out new file mode 100644 index 0000000..a074d1d --- /dev/null +++ b/goldens/duplicates_ignore_comments.out @@ -0,0 +1,12 @@ +Remove duplicates but ignore comments: +// keep-sorted-test start remove_duplicates=ignore_comments sticky_comments=yes +bar +// First foo +foo +// keep-sorted-test end + +Remove duplicates ignoring comments, when the first item has no comment: +// keep-sorted-test start remove_duplicates=ignore_comments sticky_comments=yes +bar +foo +// keep-sorted-test end diff --git a/goldens/duplicates_merge_comments.in b/goldens/duplicates_merge_comments.in new file mode 100644 index 0000000..1a94a7e --- /dev/null +++ b/goldens/duplicates_merge_comments.in @@ -0,0 +1,21 @@ +Merge duplicate comments: +// keep-sorted-test start remove_duplicates=merge_comments sticky_comments=yes +// First foo +foo +bar +// Second foo +foo +// Third foo +foo +// keep-sorted-test end + +Merge duplicate comments where some are identical: +// keep-sorted-test start remove_duplicates=merge_comments sticky_comments=yes +// Common foo flag +foo +bar +// Common foo flag +foo +// Different foo flag +foo +// keep-sorted-test end diff --git a/goldens/duplicates_merge_comments.out b/goldens/duplicates_merge_comments.out new file mode 100644 index 0000000..69db1f2 --- /dev/null +++ b/goldens/duplicates_merge_comments.out @@ -0,0 +1,16 @@ +Merge duplicate comments: +// keep-sorted-test start remove_duplicates=merge_comments sticky_comments=yes +bar +// First foo +// Second foo +// Third foo +foo +// keep-sorted-test end + +Merge duplicate comments where some are identical: +// keep-sorted-test start remove_duplicates=merge_comments sticky_comments=yes +bar +// Common foo flag +// Different foo flag +foo +// keep-sorted-test end diff --git a/keepsorted/block.go b/keepsorted/block.go index 9ff284b..ab8c6f1 100644 --- a/keepsorted/block.go +++ b/keepsorted/block.go @@ -259,15 +259,35 @@ func (b block) sorted() (sorted []string, alreadySorted bool) { } removedDuplicate := false - if b.metadata.opts.RemoveDuplicates { - seen := map[string]bool{} + if b.metadata.opts.RemoveDuplicates != DuplicateResolutionFalse { + seenStrings := map[string]bool{} + seenLines := map[string]*lineGroup{} var deduped []*lineGroup for _, lg := range groups { - if s := lg.String(); !seen[s] { - seen[s] = true - deduped = append(deduped, lg) + if b.metadata.opts.RemoveDuplicates == DuplicateResolutionTrue { + if s := lg.String(); !seenStrings[s] { + seenStrings[s] = true + deduped = append(deduped, lg) + } else { + removedDuplicate = true + } } else { - removedDuplicate = true + codeMapKey := strings.Join(lg.lines, "\n") + + if firstLg, ok := seenLines[codeMapKey]; !ok { + seenLines[codeMapKey] = lg + deduped = append(deduped, lg) + } else { + removedDuplicate = true + + if b.metadata.opts.RemoveDuplicates == DuplicateResolutionMergeComments { + for _, newComment := range lg.comment { + if !slices.Contains(firstLg.comment, newComment) { + firstLg.comment = append(firstLg.comment[:len(firstLg.comment):len(firstLg.comment)], newComment) + } + } + } + } } } groups = deduped diff --git a/keepsorted/keep_sorted_test.go b/keepsorted/keep_sorted_test.go index b90cd0b..fb397f2 100644 --- a/keepsorted/keep_sorted_test.go +++ b/keepsorted/keep_sorted_test.go @@ -838,7 +838,7 @@ func TestLineSorting(t *testing.T) { name: "AlreadySorted_ExceptForDuplicate", opts: blockOptions{ - RemoveDuplicates: true, + RemoveDuplicates: DuplicateResolutionTrue, }, in: []string{ "Bar", @@ -1020,7 +1020,7 @@ func TestLineSorting(t *testing.T) { opts: func() blockOptions { opts := blockOptions{ - RemoveDuplicates: true, + RemoveDuplicates: DuplicateResolutionTrue, StickyComments: true, } opts.setCommentMarker("//") @@ -1048,7 +1048,7 @@ func TestLineSorting(t *testing.T) { name: "RemoveDuplicates_IgnoresTraliningCommas", opts: blockOptions{ - RemoveDuplicates: true, + RemoveDuplicates: DuplicateResolutionTrue, }, in: []string{ "foo,", @@ -1065,7 +1065,7 @@ func TestLineSorting(t *testing.T) { name: "RemoveDuplicates_IgnoresTrailingCommas_RemovesCommaIfLastElement", opts: blockOptions{ - RemoveDuplicates: true, + RemoveDuplicates: DuplicateResolutionTrue, }, in: []string{ "foo,", @@ -1082,7 +1082,7 @@ func TestLineSorting(t *testing.T) { name: "RemoveDuplicates_IgnoresTrailingCommas_RemovesCommaIfOnlyElement", opts: blockOptions{ - RemoveDuplicates: true, + RemoveDuplicates: DuplicateResolutionTrue, }, in: []string{ "foo,", @@ -1097,7 +1097,7 @@ func TestLineSorting(t *testing.T) { name: "RemoveDuplicates_Keep", opts: blockOptions{ - RemoveDuplicates: false, + RemoveDuplicates: DuplicateResolutionFalse, }, in: []string{ "foo", diff --git a/keepsorted/options.go b/keepsorted/options.go index 4dfb978..cc08ac2 100644 --- a/keepsorted/options.go +++ b/keepsorted/options.go @@ -35,6 +35,16 @@ import ( // true is unmarshaled as 1, false as 0. type IntOrBool int +type DuplicateResolution int + +const ( + DuplicateResolutionFalse DuplicateResolution = iota + DuplicateResolutionTrue + DuplicateResolutionIgnoreComments + DuplicateResolutionMergeComments +) + + type ByRegexOption struct { Pattern *regexp.Regexp Template *string @@ -147,7 +157,7 @@ type blockOptions struct { // Any other positive integer specifies the number of newlines to separate the groups. NewlineSeparated IntOrBool `key:"newline_separated"` // RemoveDuplicates determines whether we drop lines that are an exact duplicate. - RemoveDuplicates bool `key:"remove_duplicates"` + RemoveDuplicates DuplicateResolution `key:"remove_duplicates"` // Syntax used to start a comment for keep-sorted annotation, e.g. "//". commentMarker string @@ -161,7 +171,7 @@ var ( StickyPrefixes: nil, // Will be populated with the comment marker of the start directive. Order: OrderAsc, CaseSensitive: true, - RemoveDuplicates: true, + RemoveDuplicates: DuplicateResolutionTrue, } fieldIndexByKey map[string]int @@ -250,6 +260,19 @@ func formatValue(val reflect.Value) (string, error) { default: return strconv.Itoa(i), nil } + case reflect.TypeFor[DuplicateResolution](): + switch val.Interface().(DuplicateResolution) { + case DuplicateResolutionFalse: + return "no", nil + case DuplicateResolutionTrue: + return "yes", nil + case DuplicateResolutionIgnoreComments: + return "ignore_comments", nil + case DuplicateResolutionMergeComments: + return "merge_comments", nil + default: + panic(fmt.Errorf("unhandled DuplicateResolution value: %v", val)) + } case reflect.TypeFor[int](): return strconv.Itoa(int(val.Int())), nil case reflect.TypeFor[[]int](): diff --git a/keepsorted/options_parser.go b/keepsorted/options_parser.go index 7848ab7..aaa8ee2 100644 --- a/keepsorted/options_parser.go +++ b/keepsorted/options_parser.go @@ -59,6 +59,9 @@ func (p *parser) popValue(typ reflect.Type) (reflect.Value, error) { case reflect.TypeFor[int](): val, err := p.popInt() return reflect.ValueOf(val), err + case reflect.TypeFor[DuplicateResolution](): + val, err := p.popDuplicateResolution() + return reflect.ValueOf(val), err case reflect.TypeFor[[]int](): val, err := p.popIntList() return reflect.ValueOf(val), err @@ -98,6 +101,23 @@ func (p *parser) popBool() (bool, error) { return b, nil } +func (p *parser) popDuplicateResolution() (DuplicateResolution, error) { + val, rest, _ := strings.Cut(p.line, " ") + p.line = rest + switch val { + case "yes", "true": + return DuplicateResolutionTrue, nil + case "no", "false": + return DuplicateResolutionFalse, nil + case "keep_first_comment", "ignore_comments": + return DuplicateResolutionIgnoreComments, nil + case "merge_comments": + return DuplicateResolutionMergeComments, nil + default: + return DuplicateResolutionFalse, fmt.Errorf("unrecognized remove_duplicates value %q", val) + } +} + func (p *parser) popInt() (int, error) { val, rest, _ := strings.Cut(p.line, " ") p.line = rest From c13f4d3712955c5110b3d00f49325858e5a3c6a3 Mon Sep 17 00:00:00 2001 From: Jakob Koschel Date: Mon, 10 Aug 2026 14:35:29 +0000 Subject: [PATCH 2/2] Address review comments on duplicate comments handling - Use slices.Clone before deduplication loop when merging comments - Rename remove_duplicates=ignore_comments to keep_first_comment everywhere - Remove duplicate new line - Add tests for remove_duplicates option parsing and resolution --- README.md | 2 +- ...ts.in => duplicates_keep_first_comment.in} | 4 +-- ....out => duplicates_keep_first_comment.out} | 4 +-- keepsorted/block.go | 3 ++- keepsorted/options.go | 7 +++-- keepsorted/options_parser.go | 4 +-- keepsorted/options_parser_test.go | 26 +++++++++++++++++++ keepsorted/options_test.go | 25 ++++++++++++++++++ 8 files changed, 63 insertions(+), 12 deletions(-) rename goldens/{duplicates_ignore_comments.in => duplicates_keep_first_comment.in} (59%) rename goldens/{duplicates_ignore_comments.out => duplicates_keep_first_comment.out} (53%) diff --git a/README.md b/README.md index 7416397..91c14f3 100644 --- a/README.md +++ b/README.md @@ -901,7 +901,7 @@ The duplicate handling can be changed with the switch `remove_duplicates`: * `yes` (default): Deduplicates based on both code and comments. * `no`: Leaves duplicates untouched. -* `ignore_comments`: Deduplicates based on code lines only, retaining only the comment of the very first duplicate occurrence. +* `keep_first_comment`: Deduplicates based on code lines only, retaining only the comment of the very first duplicate occurrence. * `merge_comments`: Deduplicates based on code lines only, merging unique comments from all occurrences to the single remaining entry. ```diff diff --git a/goldens/duplicates_ignore_comments.in b/goldens/duplicates_keep_first_comment.in similarity index 59% rename from goldens/duplicates_ignore_comments.in rename to goldens/duplicates_keep_first_comment.in index 2dbc0d2..4b0e2ea 100644 --- a/goldens/duplicates_ignore_comments.in +++ b/goldens/duplicates_keep_first_comment.in @@ -1,5 +1,5 @@ Remove duplicates but ignore comments: -// keep-sorted-test start remove_duplicates=ignore_comments sticky_comments=yes +// keep-sorted-test start remove_duplicates=keep_first_comment sticky_comments=yes // First foo foo bar @@ -10,7 +10,7 @@ foo // keep-sorted-test end Remove duplicates ignoring comments, when the first item has no comment: -// keep-sorted-test start remove_duplicates=ignore_comments sticky_comments=yes +// keep-sorted-test start remove_duplicates=keep_first_comment sticky_comments=yes foo bar // Second foo diff --git a/goldens/duplicates_ignore_comments.out b/goldens/duplicates_keep_first_comment.out similarity index 53% rename from goldens/duplicates_ignore_comments.out rename to goldens/duplicates_keep_first_comment.out index a074d1d..cee1519 100644 --- a/goldens/duplicates_ignore_comments.out +++ b/goldens/duplicates_keep_first_comment.out @@ -1,12 +1,12 @@ Remove duplicates but ignore comments: -// keep-sorted-test start remove_duplicates=ignore_comments sticky_comments=yes +// keep-sorted-test start remove_duplicates=keep_first_comment sticky_comments=yes bar // First foo foo // keep-sorted-test end Remove duplicates ignoring comments, when the first item has no comment: -// keep-sorted-test start remove_duplicates=ignore_comments sticky_comments=yes +// keep-sorted-test start remove_duplicates=keep_first_comment sticky_comments=yes bar foo // keep-sorted-test end diff --git a/keepsorted/block.go b/keepsorted/block.go index ab8c6f1..6d43fd4 100644 --- a/keepsorted/block.go +++ b/keepsorted/block.go @@ -281,9 +281,10 @@ func (b block) sorted() (sorted []string, alreadySorted bool) { removedDuplicate = true if b.metadata.opts.RemoveDuplicates == DuplicateResolutionMergeComments { + firstLg.comment = slices.Clone(firstLg.comment) for _, newComment := range lg.comment { if !slices.Contains(firstLg.comment, newComment) { - firstLg.comment = append(firstLg.comment[:len(firstLg.comment):len(firstLg.comment)], newComment) + firstLg.comment = append(firstLg.comment, newComment) } } } diff --git a/keepsorted/options.go b/keepsorted/options.go index cc08ac2..0aaa925 100644 --- a/keepsorted/options.go +++ b/keepsorted/options.go @@ -40,11 +40,10 @@ type DuplicateResolution int const ( DuplicateResolutionFalse DuplicateResolution = iota DuplicateResolutionTrue - DuplicateResolutionIgnoreComments + DuplicateResolutionKeepFirstComment DuplicateResolutionMergeComments ) - type ByRegexOption struct { Pattern *regexp.Regexp Template *string @@ -266,8 +265,8 @@ func formatValue(val reflect.Value) (string, error) { return "no", nil case DuplicateResolutionTrue: return "yes", nil - case DuplicateResolutionIgnoreComments: - return "ignore_comments", nil + case DuplicateResolutionKeepFirstComment: + return "keep_first_comment", nil case DuplicateResolutionMergeComments: return "merge_comments", nil default: diff --git a/keepsorted/options_parser.go b/keepsorted/options_parser.go index aaa8ee2..8c502b4 100644 --- a/keepsorted/options_parser.go +++ b/keepsorted/options_parser.go @@ -109,8 +109,8 @@ func (p *parser) popDuplicateResolution() (DuplicateResolution, error) { return DuplicateResolutionTrue, nil case "no", "false": return DuplicateResolutionFalse, nil - case "keep_first_comment", "ignore_comments": - return DuplicateResolutionIgnoreComments, nil + case "keep_first_comment": + return DuplicateResolutionKeepFirstComment, nil case "merge_comments": return DuplicateResolutionMergeComments, nil default: diff --git a/keepsorted/options_parser_test.go b/keepsorted/options_parser_test.go index b23e18d..b6f144d 100644 --- a/keepsorted/options_parser_test.go +++ b/keepsorted/options_parser_test.go @@ -284,6 +284,32 @@ func TestPopValue(t *testing.T) { want: IntOrBool(0), wantErr: true, }, + { + name: "DuplicateResolution_True", + input: "yes", + want: DuplicateResolutionTrue, + }, + { + name: "DuplicateResolution_False", + input: "no", + want: DuplicateResolutionFalse, + }, + { + name: "DuplicateResolution_KeepFirstComment", + input: "keep_first_comment", + want: DuplicateResolutionKeepFirstComment, + }, + { + name: "DuplicateResolution_MergeComments", + input: "merge_comments", + want: DuplicateResolutionMergeComments, + }, + { + name: "DuplicateResolution_Invalid", + input: "foo", + want: DuplicateResolutionFalse, + wantErr: true, + }, } { t.Run(tc.name, func(t *testing.T) { suffix := "trailing content..." diff --git a/keepsorted/options_test.go b/keepsorted/options_test.go index f7e2a2d..2bb8c58 100644 --- a/keepsorted/options_test.go +++ b/keepsorted/options_test.go @@ -290,6 +290,31 @@ func TestBlockOptions(t *testing.T) { GroupStartRegex: []*regexp.Regexp{regexp.MustCompile("^CREATE"), regexp.MustCompile("b")}, }, }, + { + name: "RemoveDuplicates_Yes", + in: "remove_duplicates=yes", + want: blockOptions{RemoveDuplicates: DuplicateResolutionTrue}, + }, + { + name: "RemoveDuplicates_No", + in: "remove_duplicates=no", + want: blockOptions{RemoveDuplicates: DuplicateResolutionFalse}, + }, + { + name: "RemoveDuplicates_KeepFirstComment", + in: "remove_duplicates=keep_first_comment", + want: blockOptions{RemoveDuplicates: DuplicateResolutionKeepFirstComment}, + }, + { + name: "RemoveDuplicates_MergeComments", + in: "remove_duplicates=merge_comments", + want: blockOptions{RemoveDuplicates: DuplicateResolutionMergeComments}, + }, + { + name: "RemoveDuplicates_Invalid", + in: "remove_duplicates=nah", + wantErr: `while parsing option "remove_duplicates": unrecognized remove_duplicates value "nah"`, + }, } { t.Run(tc.name, func(t *testing.T) { initZerolog(t)