fix: Print options correctly when their lists or sets need to be YAML…

… lists. (#48)

Right now, options that need their lists or sets to be YAML lists aren't
printing a roundtrippable string.

keepsorted/options.go

@@ -26,6 +26,8 @@ import (
 	"strconv"
 	"strings"
 	"unicode"
+
+	yaml "gopkg.in/yaml.v3"
 )
 
 type BlockOptions struct {
@@ -180,21 +182,50 @@ func parseBlockOptions(commentMarker, options string, defaults blockOptions) (_
 	return ret, warns
 }
 
-func formatValue(val reflect.Value) string {
+func formatValue(val reflect.Value) (string, error) {
 	switch val.Type() {
 	case reflect.TypeFor[bool]():
-		return boolString[val.Bool()]
+		return boolString[val.Bool()], nil
 	case reflect.TypeFor[[]string]():
-		return strings.Join(val.Interface().([]string), ",")
+		return formatList(val.Interface().([]string))
 	case reflect.TypeFor[map[string]bool]():
-		return strings.Join(slices.Sorted(maps.Keys(val.Interface().(map[string]bool))), ",")
+		return formatList(slices.Sorted(maps.Keys(val.Interface().(map[string]bool))))
 	case reflect.TypeFor[int]():
-		return strconv.Itoa(int(val.Int()))
+		return strconv.Itoa(int(val.Int())), nil
 	}
 
 	panic(fmt.Errorf("unsupported blockOptions type: %v", val.Type()))
 }
 
+func formatList(vals []string) (string, error) {
+	var specialChars bool
+	if len(vals) > 0 && strings.HasPrefix(vals[0], "[") {
+		specialChars = true
+	} else {
+		for _, val := range vals {
+			if strings.ContainsAny(val, ", ") {
+				specialChars = true
+				break
+			}
+		}
+	}
+
+	if !specialChars {
+		return strings.Join(vals, ","), nil
+	}
+
+	node := new(yaml.Node)
+	if err := node.Encode(vals); err != nil {
+		return "", fmt.Errorf("while converting list to YAML: %w", err)
+	}
+	node.Style |= yaml.FlowStyle
+	out, err := yaml.Marshal(node)
+	if err != nil {
+		return "", fmt.Errorf("while formatting YAML: %w", err)
+	}
+	return strings.TrimSpace(string(out)), nil
+}
+
 func guessCommentMarker(startLine string) string {
 	startLine = strings.TrimSpace(startLine)
 	if strings.HasPrefix(startLine, "//") {
@@ -241,13 +272,23 @@ func validate(opts *blockOptions) (warnings []error) {
 func (opts blockOptions) String() string {
 	var s []string
 	val := reflect.ValueOf(opts)
+	var errs []error
 	for _, key := range slices.Sorted(maps.Keys(fieldIndexByKey)) {
 		field := val.Type().Field(fieldIndexByKey[key])
 		fieldVal := val.FieldByIndex(field.Index)
 		if fieldVal.IsZero() {
 			continue
 		}
-		s = append(s, fmt.Sprintf("%s=%s", key, formatValue(fieldVal)))
+		val, err := formatValue(fieldVal)
+		if err != nil {
+			errs = append(errs, err)
+		} else {
+			s = append(s, fmt.Sprintf("%s=%s", key, val))
+		}
+	}
+
+	if err := errors.Join(errs...); err != nil {
+		panic(err)
 	}
 
 	return strings.Join(s, " ")

keepsorted/options_parser.go

@@ -9,7 +9,7 @@ import (
 	"strings"
 	"unicode/utf8"
 
-	"gopkg.in/yaml.v3"
+	yaml "gopkg.in/yaml.v3"
 )
 
 var (

keepsorted/options_test.go

@@ -182,7 +182,18 @@ func TestBlockOptions(t *testing.T) {
 				t.Errorf("parseBlockOptions(%q, %q) mismatch (-want +got):\n%s", tc.commentMarker, tc.in, diff)
 			}
 
-			_ = got.String() // Make sure this doesn't panic.
+			if tc.wantErr == "" {
+				t.Run("StringRoundtrip", func(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)
+					}
+					if diff := cmp.Diff(got, got2, cmp.AllowUnexported(blockOptions{})); diff != "" {
+						t.Errorf("parseBlockOptions(%q, %q) mismatch (-want +got):\n%s", tc.commentMarker, s, diff)
+					}
+				})
+			}
 		})
 	}
 }