fix(config): ordering of list option values not preserved (#805)

Originally, options with a slice value were treated as sets instead of
lists. This was because options like `default-ssh-keys` do not care
about the order of keys. However, the order for example is important for
the default sort column options, so it needs to be preserved. This PR
fixes this.

internal/cmd/config/add.go

@@ -3,7 +3,6 @@ package config
 import (
 	"fmt"
 	"os"
-	"slices"
 
 	"github.com/spf13/cobra"
 
@@ -52,8 +51,7 @@ func runAdd(s state.State, cmd *cobra.Command, args []string) error {
 	case []string:
 		before := util.AnyToStringSlice(val)
 		newVal := append(before, values...)
-		slices.Sort(newVal)
-		newVal = slices.Compact(newVal)
+		newVal = util.RemoveDuplicates(newVal)
 		val = newVal
 		added = util.ToAnySlice(util.SliceDiff[[]string](newVal, before))
 	default:

internal/cmd/config/add_test.go

@@ -156,11 +156,11 @@ active_context = "test_context"
 			args:   []string{"--global", "array-option", "c", "b", "c", "a", "a"},
 			config: testConfig,
 			expErr: "Warning: some values were already present or duplicate\n",
-			expOut: `Added '[a b c]' to 'array-option' globally
+			expOut: `Added '[c b a]' to 'array-option' globally
 active_context = "test_context"
 
 [preferences]
-  array_option = ["a", "b", "c"]
+  array_option = ["c", "b", "a"]
   debug = true
   poll_interval = "1.234s"
 

internal/cmd/util/util.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"os"
 	"reflect"
-	"slices"
 	"sort"
 	"strings"
 	"text/template"
@@ -237,24 +236,38 @@ func FilterNil[T any](values []T) []T {
 	return filtered
 }
 
-// SliceDiff returns the difference between the two passed slices. The returned slice contains all elements that are present in a but not in b.
-// Note that it does not preserve order.
+// SliceDiff returns the difference between the two passed slices. The returned slice contains all elements that
+// are present in a but not in b. The order of a is preserved.
 func SliceDiff[S ~[]E, E cmp.Ordered](a, b []E) []E {
-	m := make(map[E]struct{})
-	for _, x := range a {
-		m[x] = struct{}{}
-	}
-	for _, x := range b {
-		delete(m, x)
-	}
 	var diff S
-	for x := range m {
-		diff = append(diff, x)
+	seen := make(map[E]struct{})
+	for _, v := range b {
+		seen[v] = struct{}{}
+	}
+	for _, v := range a {
+		if _, ok := seen[v]; ok {
+			continue
+		}
+		diff = append(diff, v)
 	}
-	slices.Sort(diff)
 	return diff
 }
 
+// RemoveDuplicates removes duplicates from the passed slice while preserving the order of the elements.
+// The first occurrence of an element is kept, all following occurrences are removed.
+func RemoveDuplicates[S ~[]E, E cmp.Ordered](values S) S {
+	var unique S
+	seen := make(map[E]struct{})
+	for _, v := range values {
+		if _, ok := seen[v]; ok {
+			continue
+		}
+		seen[v] = struct{}{}
+		unique = append(unique, v)
+	}
+	return unique
+}
+
 func AnyToAnySlice(a any) []any {
 	val := reflect.ValueOf(a)
 	if val.Kind() != reflect.Slice {

internal/cmd/util/util_test.go

@@ -413,3 +413,12 @@ func TestToStringSliceDelimited(t *testing.T) {
 	assert.Equal(t, []string{"a", "b", "c"}, util.ToStringSliceDelimited("a,b,c"))
 	assert.Equal(t, []string{"0", "1", "2"}, util.ToStringSliceDelimited([]int{0, 1, 2}))
 }
+
+func TestRemoveDuplicates(t *testing.T) {
+	assert.Equal(t, []string{"a", "b", "c"}, util.RemoveDuplicates([]string{"a", "b", "c"}))
+	assert.Equal(t, []string{"a", "b", "c"}, util.RemoveDuplicates([]string{"a", "b", "c", "a", "b", "c"}))
+	assert.Equal(t, []string{"a", "b", "c"}, util.RemoveDuplicates([]string{"a", "b", "c", "c", "b", "a"}))
+	assert.Equal(t, []string{"c", "b", "a"}, util.RemoveDuplicates([]string{"c", "b", "a", "a", "b", "c"}))
+	assert.Equal(t, []string{"a"}, util.RemoveDuplicates([]string{"a", "a", "a", "a", "a"}))
+	assert.Equal(t, []int{1, 2, 3, 4, 5}, util.RemoveDuplicates([]int{1, 2, 1, 1, 3, 2, 1, 4, 3, 2, 5, 4, 3, 2, 1}))
+}

internal/state/config/options.go

@@ -2,7 +2,6 @@ package config
 
 import (
 	"fmt"
-	"slices"
 	"strings"
 	"time"
 
@@ -288,12 +287,8 @@ func (o *Option[T]) Parse(values []string) (any, error) {
 		if err != nil {
 			return nil, fmt.Errorf("invalid duration value: %s", value)
 		}
-
 	case []string:
-		newVal := values[:]
-		slices.Sort(newVal)
-		newVal = slices.Compact(newVal)
-		val = newVal
+		val = util.RemoveDuplicates(values)
 	default:
 		return nil, fmt.Errorf("unsupported type %T", t)
 	}