Handle trailing commas with comments

For lines/sequences of blocks which appear to be a comma-delimited list,
do not consider comments when performing this check. Also, insert commas
before comments to avoid mangling list.

I kept the match/replace logic in lineGroup, as hasSuffix/trimSuffix were
there.

This approach does have a few weaknesses. It doesn't properly handle
sorting what appear to be quoted comments, e.g.:

- "a, #",
- "b, //",
- c

It also doesn't properly handle commented commas:

- a, # some comment, # more comment
- b, # another comment, # a comment

It is better at handling the desired cases, and the above counterexamples where
it fails feel more like less-common edge cases.

Lastly, it assumes "#" and "//" are always comments, and doesn't allow for
alternatives.

Handling the above would need quite a bit more work - likely parsing the lines
themselves and adding list_delimiter and/or comment_delimiter options.

Author: willbeason

goldens/trailing_commas.out

@@ -1,31 +1,31 @@
 Trailing comma except for last
   keep-sorted-test start
   1,
-  2,
-  3
+  2, 
+  3 
   keep-sorted-test end
 
 
 Trailing comma and a comment
 TODO: https://github.com/google/keep-sorted/issues/33 - Fix this
   keep-sorted-test start
   1,
-  2
-  3, # three
+  2, 
+  3  # three
   keep-sorted-test end
 
 Trailing comma except for last, last has a comment
 TODO: https://github.com/google/keep-sorted/issues/33 - Fix this
   keep-sorted-test start
   1,
-  2  # two,
-  3
+  2, # two
+  3 
   keep-sorted-test end
 
 Trailing comma and a comment, last has a comment
 TODO: https://github.com/google/keep-sorted/issues/33 - Fix this
   keep-sorted-test start
   1,
-  2  # two
-  3, # three
+  2, # two
+  3  # three
   keep-sorted-test end

keepsorted/block.go

@@ -15,6 +15,7 @@
 package keepsorted
 
 import (
+	"regexp"
 	"slices"
 	"strings"
 
@@ -384,13 +385,13 @@ func handleTrailingComma(lgs []*lineGroup) (trimTrailingComma func([]*lineGroup)
 		}
 	}
 
-	if n := len(dataGroups); n > 1 && allHaveSuffix(dataGroups[0:n-1], ",") && !dataGroups[n-1].hasSuffix(",") {
-		dataGroups[n-1].append(",")
+	if n := len(dataGroups); n > 1 && allEndInComma(dataGroups[0:n-1]) && !endInComma(dataGroups[n-1]) {
+		dataGroups[n-1].replaceSuffix(possibleCommentLineEnd, ", $2")
 
 		return func(lgs []*lineGroup) {
 			for i := len(lgs) - 1; i >= 0; i-- {
 				if len(lgs[i].lines) > 0 {
-					lgs[i].trimSuffix(",")
+					lgs[i].replaceSuffix(commaLineEnd, " $2")
 					return
 				}
 			}
@@ -400,11 +401,20 @@ func handleTrailingComma(lgs []*lineGroup) (trimTrailingComma func([]*lineGroup)
 	return func([]*lineGroup) {}
 }
 
-func allHaveSuffix(lgs []*lineGroup, s string) bool {
+func allEndInComma(lgs []*lineGroup) bool {
 	for _, lg := range lgs {
-		if !lg.hasSuffix(s) {
+		if !endInComma(lg) {
 			return false
 		}
 	}
 	return true
 }
+
+var (
+	commaLineEnd           = regexp.MustCompile("(,)(\\s*(#|//).*)?$")
+	possibleCommentLineEnd = regexp.MustCompile("( +)?((#|//).*)?$")
+)
+
+func endInComma(lg *lineGroup) bool {
+	return lg.matchesSuffix(commaLineEnd)
+}

keepsorted/line_group.go

@@ -61,7 +61,7 @@ type accessRecorder struct {
 	joinedComment bool
 }
 
-// matchesAnyRegex returns true if s matches one of the regexes.
+// matchesAnyRegex returns true if s matchesSuffix one of the regexes.
 func matchesAnyRegex(s string, regexes []*regexp.Regexp) bool {
 	for _, regex := range regexes {
 		if regex.FindStringSubmatch(s) != nil {
@@ -119,7 +119,7 @@ func groupLines(lines []string, metadata blockMetadata) []*lineGroup {
 	// Returns another boolean indicating whether the group should be ending
 	// after that line if so.
 	shouldAddToRegexDelimitedGroup := func(l string) (addToGroup bool, finishGroupAfter bool) {
-        if metadata.opts.GroupStartRegex != nil {
+		if metadata.opts.GroupStartRegex != nil {
 			// For GroupStartRegex, all non-regex-matching lines should be
 			// part of the group including prior lines.
 			return !matchesAnyRegex(l, metadata.opts.GroupStartRegex), false
@@ -408,11 +408,20 @@ func (lg *lineGroup) hasSuffix(s string) bool {
 	return len(lg.lines) > 0 && strings.HasSuffix(lg.lines[len(lg.lines)-1], s)
 }
 
+func (lg *lineGroup) matchesSuffix(re *regexp.Regexp) bool {
+	return len(lg.lines) > 0 && re.MatchString(lg.lines[len(lg.lines)-1])
+}
+
 func (lg *lineGroup) trimSuffix(s string) {
 	lg.access = accessRecorder{}
 	lg.lines[len(lg.lines)-1] = strings.TrimSuffix(lg.lines[len(lg.lines)-1], s)
 }
 
+func (lg *lineGroup) replaceSuffix(re *regexp.Regexp, replacement string) {
+	lg.access = accessRecorder{}
+	lg.lines[len(lg.lines)-1] = re.ReplaceAllString(lg.lines[len(lg.lines)-1], replacement)
+}
+
 func (lg *lineGroup) commentOnly() bool {
 	lg.access.commentOnly = true
 	return len(lg.lines) == 0

keepsorted/options.go

@@ -453,7 +453,7 @@ func (opts blockOptions) hasGroupPrefix(s string) bool {
 }
 
 // trimIgnorePrefix removes the first matching IgnorePrefixes from s, if s
-// matches one of the IgnorePrefixes.
+// matchesSuffix one of the IgnorePrefixes.
 func (opts blockOptions) trimIgnorePrefix(s string) string {
 	_, s, _ = opts.cutFirstPrefix(s, slices.Values(opts.IgnorePrefixes))
 	return s