fix: Format []ByRegexOption correctly. (#118)
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 a different YAML flow sequence or even invalid YAML if
the singular element has the right special characters.
e.g. Before the changes to options.go, the test was generating a
different flow sequence with 3 elements instead of 2: `by_regex=[foo,
bar, \"\\\\b(\\\\d{2})/(\\\\d{2})/(\\\\d{4})\\\\b\":
\"${3}-${1}-${2}\"]"`
We now generate strings that include mapping braces (`{` and `}`, e.g.
`by_regex=['foo, bar', {'\b(\d{2})/(\d{2})/(\d{4})\b':
'${3}-${1}-${2}'}]`). That's less than ideal, but it's still better than
generating invalid YAML. I don't see any options in the YAML library to
turn those braces off :/ https://pkg.go.dev/gopkg.in/yaml.v3diff --git a/keepsorted/options.go b/keepsorted/options.go
index 6b5c6fa..4dfb978 100644
--- a/keepsorted/options.go
+++ b/keepsorted/options.go
@@ -40,6 +40,14 @@
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
@@ -248,19 +256,18 @@
return formatIntList(val.Interface().([]int)), nil
case reflect.TypeFor[[]ByRegexOption]():
opts := val.Interface().([]ByRegexOption)
- vals := make([]string, 0, len(opts))
+ vals := make([]string, 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))
- continue
+ break
}
- 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(opts)
}
return formatList(vals)
case reflect.TypeFor[[]*regexp.Regexp]():
@@ -292,6 +299,10 @@
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)
diff --git a/keepsorted/options_test.go b/keepsorted/options_test.go
index 0ea1475..bed93a6 100644
--- a/keepsorted/options_test.go
+++ b/keepsorted/options_test.go
@@ -231,8 +231,26 @@
AllowYAMLLists: true,
ByRegex: []ByRegexOption{
{Pattern: regexp.MustCompile(`.*`)},
- {Pattern: regexp.MustCompile(`\b(\d{2})/(\d{2})/(\d{4})\b`),
- Template: &[]string{"${3}-${1}-${2}"}[0]},
+ {
+ Pattern: regexp.MustCompile(`\b(\d{2})/(\d{2})/(\d{4})\b`),
+ Template: &[]string{"${3}-${1}-${2}"}[0],
+ },
+ },
+ },
+ },
+ {
+ 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],
+ },
},
},
},
@@ -277,13 +295,13 @@
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 == "" {
@@ -291,10 +309,10 @@
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)
}
})
}
