From be5387944a940fb9135daf1c0422703be2830b72 Mon Sep 17 00:00:00 2001
From: phm07 <22707808+phm07@users.noreply.github.com>
Date: Thu, 4 Jul 2024 18:28:35 +0200
Subject: [PATCH] fix: ordering of slice option values not preserved

---
 internal/cmd/config/add.go       |  4 +---
 internal/cmd/config/add_test.go  |  4 ++--
 internal/cmd/util/util.go        | 39 +++++++++++++++++++++-----------
 internal/cmd/util/util_test.go   |  9 ++++++++
 internal/state/config/options.go |  7 +-----
 5 files changed, 39 insertions(+), 24 deletions(-)

diff --git a/internal/cmd/config/add.go b/internal/cmd/config/add.go
index d7c9b2b0..98051899 100644
--- a/internal/cmd/config/add.go
+++ b/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:
diff --git a/internal/cmd/config/add_test.go b/internal/cmd/config/add_test.go
index 2357aa85..8dfece70 100644
--- a/internal/cmd/config/add_test.go
+++ b/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"
 
diff --git a/internal/cmd/util/util.go b/internal/cmd/util/util.go
index 45b692e6..2572fcbb 100644
--- a/internal/cmd/util/util.go
+++ b/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 {
diff --git a/internal/cmd/util/util_test.go b/internal/cmd/util/util_test.go
index 45c192e0..792289bb 100644
--- a/internal/cmd/util/util_test.go
+++ b/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}))
+}
diff --git a/internal/state/config/options.go b/internal/state/config/options.go
index 38c8e8e7..a52cc68f 100644
--- a/internal/state/config/options.go
+++ b/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)
 	}