From b257ef998545a310b42b73a58a1930c4ad7f655b Mon Sep 17 00:00:00 2001 From: Jeffrey Faer Date: Mon, 9 Mar 2026 15:15:53 -0600 Subject: [PATCH 1/3] fix: Format []ByRegexOption correctly. If there's a mix of mappings and singular elements, the mappings will (maybe) be quoted correctly but the singular elements won't be, which can result in a YAML flow sequence that's not valid YAML. --- keepsorted/options.go | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/keepsorted/options.go b/keepsorted/options.go index e36d20d..65ef013 100644 --- a/keepsorted/options.go +++ b/keepsorted/options.go @@ -238,21 +238,25 @@ func formatValue(val reflect.Value) (string, error) { return strconv.Itoa(int(val.Int())), nil case reflect.TypeFor[[]ByRegexOption](): opts := val.Interface().([]ByRegexOption) - vals := make([]string, 0, len(opts)) + vals := make([]any, len(opts)) seenTemplate := false - for _, opt := range opts { + for i, opt := range opts { if opt.Template != nil { seenTemplate = true - vals = append(vals, fmt.Sprintf(`%q: %q`, opt.Pattern.String(), *opt.Template)) + vals[i] = map[string]string{opt.Pattern.String(): *opt.Template} continue } - vals = append(vals, opt.Pattern.String()) + vals[i] = opt.Pattern.String() } if seenTemplate { // always presented as a yaml sequence to preserve any `k:v` items - return fmt.Sprintf("[%s]", strings.Join(vals, ", ")), nil + return formatYAMLList(vals) } - return formatList(vals) + s := make([]string, len(vals)) + for i, val := range vals { + s[i] = val.(string) + } + return formatList(s) case reflect.TypeFor[[]*regexp.Regexp](): regexps := val.Interface().([]*regexp.Regexp) vals := make([]string, len(regexps)) @@ -282,6 +286,10 @@ func formatList(vals []string) (string, error) { return strings.Join(vals, ","), nil } + return formatYAMLList(vals) +} + +func formatYAMLList[T any](vals []T) (string, error) { node := new(yaml.Node) if err := node.Encode(vals); err != nil { return "", fmt.Errorf("while converting list to YAML: %w", err) From a45c7d39afa9cb2174314f330b5b75510b11b13f Mon Sep 17 00:00:00 2001 From: Jeffrey Faer Date: Mon, 9 Mar 2026 15:19:55 -0600 Subject: [PATCH 2/3] add a test --- keepsorted/options_test.go | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/keepsorted/options_test.go b/keepsorted/options_test.go index dccfd75..bdcfa56 100644 --- a/keepsorted/options_test.go +++ b/keepsorted/options_test.go @@ -212,6 +212,20 @@ func TestBlockOptions(t *testing.T) { }, }, }, + { + name: "RegexWithTemplateAndSingletonWithSpecialChars", + in: `by_regex=['foo, bar', '\b(\d{2})/(\d{2})/(\d{4})\b': '${3}-${1}-${2}']`, + defaultOptions: blockOptions{AllowYAMLLists: true}, + + want: blockOptions{ + AllowYAMLLists: true, + ByRegex: []ByRegexOption{ + {Pattern: regexp.MustCompile(`foo, bar`)}, + {Pattern: regexp.MustCompile(`\b(\d{2})/(\d{2})/(\d{4})\b`), + Template: &[]string{"${3}-${1}-${2}"}[0]}, + }, + }, + }, { name: "OrderAsc", in: "order=asc", @@ -253,13 +267,13 @@ func TestBlockOptions(t *testing.T) { got, warns := parseBlockOptions(tc.commentMarker, tc.in, tc.defaultOptions) if err := errors.Join(warns...); err != nil { if tc.wantErr == "" { - t.Errorf("parseBlockOptions(%q, %q) = %v", tc.commentMarker, tc.in, err) + t.Errorf("parseBlockOptions(%#v, %#v) = %v", tc.commentMarker, tc.in, err) } else if !strings.Contains(err.Error(), tc.wantErr) { - t.Errorf("parseBlockOptions(%q, %q) = %v, expected to contain %q", tc.commentMarker, tc.in, err, tc.wantErr) + t.Errorf("parseBlockOptions(%#v, %#v) = %v, expected to contain %q", tc.commentMarker, tc.in, err, tc.wantErr) } } if diff := cmp.Diff(tc.want, got, cmp.AllowUnexported(blockOptions{}), cmpRegexp); diff != "" { - t.Errorf("parseBlockOptions(%q, %q) mismatch (-want +got):\n%s", tc.commentMarker, tc.in, diff) + t.Errorf("parseBlockOptions(%#v, %#v) mismatch (-want +got):\n%s", tc.commentMarker, tc.in, diff) } if tc.wantErr == "" { @@ -267,10 +281,10 @@ func TestBlockOptions(t *testing.T) { s := got.String() got2, warns := parseBlockOptions(tc.commentMarker, s, tc.defaultOptions) if err := errors.Join(warns...); err != nil { - t.Errorf("parseBlockOptions(%q, %q) = %v", tc.commentMarker, s, err) + t.Errorf("parseBlockOptions(%#v, %#v) = %v", tc.commentMarker, s, err) } if diff := cmp.Diff(got, got2, cmp.AllowUnexported(blockOptions{}), cmpRegexp); diff != "" { - t.Errorf("parseBlockOptions(%q, %q) mismatch (-want +got):\n%s", tc.commentMarker, s, diff) + t.Errorf("parseBlockOptions(%#v, %#v) mismatch (-want +got):\n%s", tc.commentMarker, s, diff) } }) } From 5a2e20df1664c600862ee6e7f1ef0740d9ea4d5c Mon Sep 17 00:00:00 2001 From: Jeffrey Faer Date: Mon, 9 Mar 2026 15:53:22 -0600 Subject: [PATCH 3/3] one last tweak --- keepsorted/options.go | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/keepsorted/options.go b/keepsorted/options.go index 65ef013..dc0567a 100644 --- a/keepsorted/options.go +++ b/keepsorted/options.go @@ -40,6 +40,14 @@ type ByRegexOption struct { Template *string } +func (opt ByRegexOption) MarshalYAML() (any, error) { + if opt.Template == nil { + return opt.Pattern.String(), nil + } + + return map[string]string{opt.Pattern.String(): *opt.Template}, nil +} + // SortOrder defines whether we sort in ascending or descending order. type SortOrder string @@ -238,25 +246,20 @@ func formatValue(val reflect.Value) (string, error) { return strconv.Itoa(int(val.Int())), nil case reflect.TypeFor[[]ByRegexOption](): opts := val.Interface().([]ByRegexOption) - vals := make([]any, len(opts)) + vals := make([]string, len(opts)) seenTemplate := false for i, opt := range opts { if opt.Template != nil { seenTemplate = true - vals[i] = map[string]string{opt.Pattern.String(): *opt.Template} - continue + break } vals[i] = opt.Pattern.String() } if seenTemplate { // always presented as a yaml sequence to preserve any `k:v` items - return formatYAMLList(vals) + return formatYAMLList(opts) } - s := make([]string, len(vals)) - for i, val := range vals { - s[i] = val.(string) - } - return formatList(s) + return formatList(vals) case reflect.TypeFor[[]*regexp.Regexp](): regexps := val.Interface().([]*regexp.Regexp) vals := make([]string, len(regexps))