feat: Make skip_lines option allow skipping at end (#122)

This adds feature discussed in #80 

skip_lines now interprets negative values as lines to skip at the end of
a sorted region.

skip_lines now accepts up to two values. If two values are specified,
the first must be lines skipped at the start and the second lines
skipped at the end. Internally this is represented as an int slice.

Add ability for parse options to handle int slices.

Modify golden examples to maintain intended behavior.

Update documentation with example of skipping lines both at the start
and end.

---------

Co-authored-by: Jeffrey Faer <jeffrey.faer@gmail.com>

Author: willbeason

README.md

@@ -396,8 +396,11 @@ treated as sticky. These prefixes cannot contain space characters.
 #### Skipping lines
 
 In some cases, it may not be possible to have the start directive on the line
-immediately before the sorted region. In this case, `skip_lines` can be used to
-indicate how many lines are to be skipped before the sorted region.
+immediately before the sorted region or end immediately after. In these cases,
+`skip_lines` can be used to indicate how many lines are to be skipped before
+and/or after the sorted region. The `skip_lines` option interprets positive
+values as lines to skip at the start of the sorted region and negative values
+as lines to skip at the end.
 
 For instance, this can be used with a Markdown table, to prevent the headers
 and the dashed line after the headers from being sorted:
@@ -435,6 +438,43 @@ Alpha   | Foo
 </tr>
 </table>
 
+This can be used to keep the footers of a table from being sorted as well:
+
+<table>
+<tr>
+<td>
+
+```md
+
+Item  | Cost
+----- | -----
+Lemon | $1.50
+Pear  | $2.10
+Apple | $1.09
+----- | -----
+Total | $4.69
+
+```
+
+</td>
+<td>
+
+```diff
++<!-- keep-sorted start skip_lines=2,-2 -->
+ Item  | Cost
+ ----- | -----
+ Apple | $1.09
+ Lemon | $1.50
+ Pear  | $2.10
+ ----- | -----
+ Total | $4.69
++<!-- keep-sorted end -->
+```
+
+</td>
+</tr>
+</table>
+
 ### Sorting options
 
 Sorting options tell keep-sorted how the logical lines in your keep-sorted

goldens/simple.err

@@ -1,4 +1,4 @@
-WRN skip_lines has invalid value: -1 line=113
+WRN skip_lines has conflicting values (should one of these be positive, to skip lines at the start of the block instead?): -1,-1 line=113
 WRN unrecognized option "foo" line=113
 WRN while parsing option "ignore_prefixes": content appears to be an unterminated YAML list: "[abc, foo" line=120
 exit status 1

goldens/simple.in

@@ -110,7 +110,7 @@ B
 // keep-sorted-test end
 
 Invalid option
-  keep-sorted-test start group=yes skip_lines=-1 foo=bar
+  keep-sorted-test start group=yes skip_lines=-1,-1 foo=bar
   2
   1
   3

goldens/simple.out

@@ -111,7 +111,7 @@ C
 // keep-sorted-test end
 
 Invalid option
-  keep-sorted-test start group=yes skip_lines=-1 foo=bar
+  keep-sorted-test start group=yes skip_lines=-1,-1 foo=bar
   1
   2
   3

keepsorted/block.go

@@ -113,7 +113,8 @@ func (f *Fixer) newBlocks(filename string, lines []string, offset int, include f
 				warnings = append(warnings, finding(filename, start.index+offset, start.index+offset, warn.Error()))
 			}
 
-			start.index += opts.SkipLines
+			start.index += opts.startOffset()
+			endIndex += opts.endOffset()
 			if start.index >= endIndex {
 				continue
 			}

keepsorted/keep_sorted_test.go

@@ -222,7 +222,7 @@ func TestFindings(t *testing.T) {
 			want: []*Finding{finding(filename, 3, 5, errorUnordered, automaticReplacement(3, 5, "1\n2\n3\n"))},
 		},
 		{
-			name: "SkipLines",
+			name: "SkipLinesStart",
 
 			in: `
 // keep-sorted-test start skip_lines=2
@@ -235,6 +235,48 @@ func TestFindings(t *testing.T) {
 
 			want: []*Finding{finding(filename, 5, 7, errorUnordered, automaticReplacement(5, 7, "1\n2\n3\n"))},
 		},
+		{
+			name: "SkipLinesEnd",
+
+			in: `
+// keep-sorted-test start skip_lines=-2
+5
+4
+3
+2
+1
+// keep-sorted-test end`,
+
+			want: []*Finding{finding(filename, 3, 5, errorUnordered, automaticReplacement(3, 5, "3\n4\n5\n"))},
+		},
+		{
+			name: "SkipLinesStartAndEnd",
+
+			in: `
+// keep-sorted-test start skip_lines=1,-1
+5
+4
+3
+2
+1
+// keep-sorted-test end`,
+
+			want: []*Finding{finding(filename, 4, 6, errorUnordered, automaticReplacement(4, 6, "2\n3\n4\n"))},
+		},
+		{
+			name: "SkipLinesEndAndStart",
+
+			in: `
+// keep-sorted-test start skip_lines=-1,1
+5
+4
+3
+2
+1
+// keep-sorted-test end`,
+
+			want: []*Finding{finding(filename, 4, 6, errorUnordered, automaticReplacement(4, 6, "2\n3\n4\n"))},
+		},
 		{
 			name: "MismatchedStart",
 

keepsorted/options.go

@@ -75,6 +75,7 @@ func (opts BlockOptions) String() string {
 //   - []string:         key=a,b,c,d
 //   - map[string]bool:  key=a,b,c,d
 //   - int:              key=123
+//   - []int:            key=1,-1
 //   - ByRegexOptions    key=a,b,c,d, key=[yaml_list]
 type blockOptions struct {
 	// AllowYAMLLists determines whether list.set valued options are allowed to be specified by YAML.
@@ -85,7 +86,7 @@ type blockOptions struct {
 	///////////////////////////
 
 	// SkipLines is the number of lines to ignore before sorting.
-	SkipLines int `key:"skip_lines"`
+	SkipLines []int `key:"skip_lines"`
 	// Group determines whether we group lines together based on increasing indentation.
 	Group bool
 	// GroupPrefixes tells us about other types of lines that should be added to a group.
@@ -243,6 +244,8 @@ func formatValue(val reflect.Value) (string, error) {
 		}
 	case reflect.TypeFor[int]():
 		return strconv.Itoa(int(val.Int())), nil
+	case reflect.TypeFor[[]int]():
+		return formatIntList(val.Interface().([]int)), nil
 	case reflect.TypeFor[[]ByRegexOption]():
 		opts := val.Interface().([]ByRegexOption)
 		vals := make([]string, 0, len(opts))
@@ -301,6 +304,14 @@ func formatList(vals []string) (string, error) {
 	return strings.TrimSpace(string(out)), nil
 }
 
+func formatIntList(intVals []int) string {
+	vals := make([]string, 0, len(intVals))
+	for _, v := range intVals {
+		vals = append(vals, strconv.Itoa(v))
+	}
+	return strings.Join(vals, ",")
+}
+
 func guessCommentMarker(startLine string) string {
 	startLine = strings.TrimSpace(startLine)
 	for _, marker := range []string{"//", "#", "/*", "--", ";", "<!--"} {
@@ -323,9 +334,22 @@ func (opts *blockOptions) setCommentMarker(marker string) {
 
 func validate(opts *blockOptions) (warnings []error) {
 	var warns []error
-	if opts.SkipLines < 0 {
-		warns = append(warns, fmt.Errorf("skip_lines has invalid value: %v", opts.SkipLines))
-		opts.SkipLines = 0
+	if len(opts.SkipLines) > 2 {
+		warns = append(warns, fmt.Errorf("skip_lines accepts at most two values: %v", formatIntList(opts.SkipLines)))
+		opts.SkipLines = nil
+	} else if len(opts.SkipLines) == 2 {
+		if cmp.Compare(opts.SkipLines[0], 0) == cmp.Compare(opts.SkipLines[1], 0) {
+			// Both are the same sign. It's okay for both to be 0.
+			if opts.SkipLines[0] < 0 {
+				// Both are negative.
+				warns = append(warns, fmt.Errorf("skip_lines has conflicting values (should one of these be positive, to skip lines at the start of the block instead?): %v", formatIntList(opts.SkipLines)))
+				opts.SkipLines = nil
+			} else if opts.SkipLines[0] > 0 {
+				// Both are positive.
+				warns = append(warns, fmt.Errorf("skip_lines has conflicting values (should one of these be negative, to skip lines at the end of the block instead?): %v", formatIntList(opts.SkipLines)))
+				opts.SkipLines = nil
+			}
+		}
 	}
 
 	if opts.NewlineSeparated < 0 {
@@ -531,6 +555,28 @@ func (opts blockOptions) maybeParseNumeric(s string) numericTokens {
 	return t
 }
 
+func (opts blockOptions) startOffset() int {
+	if len(opts.SkipLines) == 2 {
+		return max(opts.SkipLines[0], opts.SkipLines[1])
+	} else if len(opts.SkipLines) == 1 {
+		if opts.SkipLines[0] > 0 {
+			return opts.SkipLines[0]
+		}
+	}
+	return 0
+}
+
+func (opts blockOptions) endOffset() int {
+	if len(opts.SkipLines) == 2 {
+		return min(opts.SkipLines[0], opts.SkipLines[1])
+	} else if len(opts.SkipLines) == 1 {
+		if opts.SkipLines[0] < 0 {
+			return opts.SkipLines[0]
+		}
+	}
+	return 0
+}
+
 // numericTokens is the result of parsing all numeric tokens out of a string.
 //
 // e.g. a string like "Foo_123" becomes

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[[]int]():
+		val, err := p.popIntList()
+		return reflect.ValueOf(val), err
 	case reflect.TypeFor[SortOrder]():
 		val, err := p.popSortOrder()
 		return reflect.ValueOf(val), err
@@ -189,6 +192,10 @@ func (p *parser) popList() ([]string, error) {
 	return popListValue(p, func(s string) (string, error) { return s, nil })
 }
 
+func (p *parser) popIntList() ([]int, error) {
+	return popListValue(p, strconv.Atoi)
+}
+
 func (p *parser) popByRegexOption() ([]ByRegexOption, error) {
 	return popListValue(p, func(s string) (ByRegexOption, error) {
 		pat, err := regexp.Compile(s)

keepsorted/options_parser_test.go

@@ -52,6 +52,24 @@ func TestPopValue(t *testing.T) {
 			want:    int(0),
 			wantErr: true,
 		},
+		{
+			name: "IntList_Empty",
+
+			input: "",
+			want:  []int{},
+		},
+		{
+			name: "IntList_SingleElement",
+
+			input: "1",
+			want:  []int{1},
+		},
+		{
+			name: "IntList_MultipleElements_WithRepeats",
+
+			input: "1,-2,1",
+			want:  []int{1, -2, 1},
+		},
 		{
 			name: "List_Empty",
 

keepsorted/options_test.go

@@ -70,10 +70,22 @@ func TestBlockOptions(t *testing.T) {
 			want: blockOptions{Group: true},
 		},
 		{
-			name: "SkipLines",
+			name: "SkipLinesStart",
 			in:   "skip_lines=10",
 
-			want: blockOptions{SkipLines: 10},
+			want: blockOptions{SkipLines: []int{10}},
+		},
+		{
+			name: "SkipLinesEnd",
+			in:   "skip_lines=-3",
+
+			want: blockOptions{SkipLines: []int{-3}},
+		},
+		{
+			name: "SkipLinesStartAndEnd",
+			in:   "skip_lines=2,-1",
+
+			want: blockOptions{SkipLines: []int{2, -1}},
 		},
 		{
 			name: "NewlineSeparated_Bool",
@@ -94,10 +106,22 @@ func TestBlockOptions(t *testing.T) {
 			wantErr: "newline_separated has invalid value: -1",
 		},
 		{
-			name: "ErrorSkipLinesIsNegative",
-			in:   "skip_lines=-1",
+			name: "ErrorSkipBothNegative",
+			in:   "skip_lines=-1,-1",
+
+			wantErr: "skip_lines has conflicting values (should one of these be positive, to skip lines at the start of the block instead?): -1,-1",
+		},
+		{
+			name: "ErrorSkipBothPositive",
+			in:   "skip_lines=1,1",
+
+			wantErr: "skip_lines has conflicting values (should one of these be negative, to skip lines at the end of the block instead?): 1,1",
+		},
+		{
+			name: "ErrorSkipLinesTooMany",
+			in:   "skip_lines=1,-1,2",
 
-			wantErr: "skip_lines has invalid value: -1",
+			wantErr: "skip_lines accepts at most two values: 1,-1,2",
 		},
 		{
 			name: "ItemList",