Move canonicalize logic to dedicated function

pkg/mod/index.go

@@ -4,7 +4,7 @@ import (
 	"bufio"
 	"io"
 	"path/filepath"
-	"sort"
+	"slices"
 	"strings"
 
 	"github.com/rogpeppe/go-internal/lockedfile"
@@ -19,15 +19,42 @@ type kloneFile struct {
 	Targets map[string]KloneFolder `yaml:"targets"`
 }
 
+func (f *kloneFile) canonicalize() {
+	newModTargets := make(map[string]KloneFolder, len(f.Targets))
+	for target, srcs := range f.Targets {
+		// Deduplicate sources based on cleaned relative path
+		uniqueSrcs := make(map[string]KloneItem, len(srcs))
+		for _, src := range srcs {
+			src.FolderName = cleanRelativePath(src.FolderName)
+			uniqueSrcs[src.FolderName] = src
+		}
+
+		// Rebuild array of sources, now without duplicates
+		srcs := make(KloneFolder, 0, len(uniqueSrcs))
+		for _, src := range uniqueSrcs {
+			srcs = append(srcs, src)
+		}
+
+		// Sort sources by folder name
+		slices.SortFunc(srcs, func(a, b KloneItem) int {
+			return a.Compare(b)
+		})
+
+		newModTargets[cleanRelativePath(target)] = srcs
+	}
+
+	f.Targets = newModTargets
+}
+
 type KloneFolder []KloneItem
 
 type KloneItem struct {
 	FolderName  string `yaml:"folder_name"`
 	KloneSource `yaml:",inline"`
 }
 
-func (i KloneItem) Less(other KloneItem) bool {
-	return i.FolderName < other.FolderName
+func (i KloneItem) Compare(other KloneItem) int {
+	return strings.Compare(i.FolderName, other.FolderName)
 }
 
 type KloneSource struct {
@@ -54,11 +81,17 @@ func (w WorkDir) editKloneFile(fn func(*kloneFile) error) error {
 		return err
 	}
 
+	// canonicalize index
+	index.canonicalize()
+
 	// update index
 	if err := fn(&index); err != nil {
 		return err
 	}
 
+	// canonicalize index
+	index.canonicalize()
+
 	topComments := ""
 
 	{
@@ -138,24 +171,20 @@ func (w WorkDir) readKloneFile() (*kloneFile, error) {
 		return nil, err
 	}
 
+	// canonicalize index
+	index.canonicalize()
+
 	return &index, nil
 }
 
 func (w WorkDir) Init() error {
 	return w.editKloneFile(func(kf *kloneFile) error {
-		if kf.Targets == nil {
-			kf.Targets = make(map[string]KloneFolder)
-		}
 		return nil
 	})
 }
 
 func (w WorkDir) AddTarget(target string, folderName string, dep KloneSource) error {
 	return w.editKloneFile(func(kf *kloneFile) error {
-		if kf.Targets == nil {
-			kf.Targets = make(map[string]KloneFolder)
-		}
-
 		for _, src := range kf.Targets[target] {
 			if src.FolderName == folderName {
 				src.KloneSource = dep
@@ -181,44 +210,21 @@ func (w WorkDir) FetchTargets(
 	fetchFn func(target string, srcs KloneFolder) error,
 ) error {
 	return w.editKloneFile(func(kf *kloneFile) error {
-		// clean target paths (only keep the last entry if they are identical)
-		newModTargets := make(map[string]KloneFolder, len(kf.Targets))
 		for target, srcs := range kf.Targets {
-			cleanedTarget := cleanRelativePath(target)
-
-			newSrcs := make(map[string]KloneItem, len(srcs))
-			for _, src := range srcs {
-				src.FolderName = cleanRelativePath(src.FolderName)
-
-				newSrcs[src.FolderName] = src
-			}
-
-			newSrcsArray := make(KloneFolder, 0, len(newSrcs))
-			for _, src := range newSrcs {
-				newSrcsArray = append(newSrcsArray, src)
-			}
-
-			sort.Slice(newSrcsArray, func(i, j int) bool {
-				return newSrcsArray[i].Less(newSrcsArray[j])
-			})
-
-			for i, src := range newSrcsArray {
+			for i, src := range srcs {
 				if err := cleanFn(target, src.FolderName, &src.KloneSource); err != nil {
 					return err
 				}
-
-				newSrcsArray[i] = src
+				srcs[i] = src
 			}
 
-			if err := fetchFn(target, newSrcsArray); err != nil {
+			if err := fetchFn(target, srcs); err != nil {
 				return err
 			}
 
-			newModTargets[cleanedTarget] = newSrcsArray
+			kf.Targets[target] = srcs
 		}
 
-		kf.Targets = newModTargets
-
 		return nil
 	})
 }

pkg/mod/index_test.go

@@ -3,7 +3,7 @@ package mod
 import (
 	"os"
 	"path"
-	"sort"
+	"slices"
 	"testing"
 )
 
@@ -16,8 +16,8 @@ func TestKloneItemSorting(t *testing.T) {
 	}
 
 	// Sort the items
-	sort.Slice(items, func(i, j int) bool {
-		return items[i].Less(items[j])
+	slices.SortFunc(items, func(a, b KloneItem) int {
+		return a.Compare(b)
 	})
 
 	// Verify the sorting order
@@ -61,6 +61,82 @@ targets:
       repo_ref: main
       repo_hash: abc123
       repo_path: path/to/repo1
+`,
+			expectErr: false,
+		},
+		{
+			name: "Sort targets (level 1)",
+			initial: `# Test comment1
+# Test comment2
+targets:
+  target2:
+    - folder_name: Folder A
+      repo_url: https://github.com/repo1
+      repo_ref: main
+      repo_hash: abc123
+      repo_path: path/to/repo1
+  target1:
+    - folder_name: Folder A
+      repo_url: https://github.com/repo1
+      repo_ref: main
+      repo_hash: abc123
+      repo_path: path/to/repo1
+`,
+			modifyFn: func(kf *kloneFile) error {
+				return nil
+			},
+			expected: `# Test comment1
+# Test comment2
+targets:
+  target1:
+    - folder_name: Folder A
+      repo_url: https://github.com/repo1
+      repo_ref: main
+      repo_hash: abc123
+      repo_path: path/to/repo1
+  target2:
+    - folder_name: Folder A
+      repo_url: https://github.com/repo1
+      repo_ref: main
+      repo_hash: abc123
+      repo_path: path/to/repo1
+`,
+			expectErr: false,
+		},
+		{
+			name: "Sort targets (level 2)",
+			initial: `# Test comment1
+# Test comment2
+targets:
+  target1:
+    - folder_name: Folder B
+      repo_url: https://github.com/repo1
+      repo_ref: main
+      repo_hash: abc123
+      repo_path: path/to/repo1
+    - folder_name: Folder A
+      repo_url: https://github.com/repo1
+      repo_ref: main
+      repo_hash: abc123
+      repo_path: path/to/repo1
+`,
+			modifyFn: func(kf *kloneFile) error {
+				return nil
+			},
+			expected: `# Test comment1
+# Test comment2
+targets:
+  target1:
+    - folder_name: Folder A
+      repo_url: https://github.com/repo1
+      repo_ref: main
+      repo_hash: abc123
+      repo_path: path/to/repo1
+    - folder_name: Folder B
+      repo_url: https://github.com/repo1
+      repo_ref: main
+      repo_hash: abc123
+      repo_path: path/to/repo1
 `,
 			expectErr: false,
 		},