Fix backup logic, ensure configurations retain formatting, and sort ListConfigs output

Author: cortiz

.goreleaser.yaml

@@ -15,7 +15,7 @@ builds:
       - arm64
     main: ./cmd/sshc
 archives:
-  - format: tar.gz
+  - formats: [ tar.gz ]
     # this name template makes the OS and Arch compatible with what nm and otool expect
     name_template: >-
       {{ .ProjectName }}_
@@ -27,11 +27,11 @@ archives:
     # use zip for windows archives
     format_overrides:
       - goos: windows
-        format: zip
+        formats: [ zip ]
 checksum:
   name_template: 'checksums.txt'
 snapshot:
-  name_template: "{{ incpatch .Version }}-next"
+  version_template: "{{ incpatch .Version }}-next"
 changelog:
   sort: asc
   filters:

internal/config/manager.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"slices"
 	"strings"
 )
 
@@ -36,16 +37,22 @@ func (m *Manager) Init() error {
 	backupFile := configFile + ".backup"
 
 	if _, err := os.Stat(configFile); err == nil {
-		// 1. backup current ssh config
-		content, err := os.ReadFile(configFile)
-		if err != nil {
-			return fmt.Errorf("failed to read ssh config for backup: %w", err)
-		}
-		if err := os.WriteFile(backupFile, content, 0600); err != nil {
-			return fmt.Errorf("failed to create backup: %w", err)
+		// 1. backup current ssh config if backup doesn't exist
+		if _, err := os.Stat(backupFile); os.IsNotExist(err) {
+			content, err := os.ReadFile(configFile)
+			if err != nil {
+				return fmt.Errorf("failed to read ssh config for backup: %w", err)
+			}
+			if err := os.WriteFile(backupFile, content, 0600); err != nil {
+				return fmt.Errorf("failed to create backup: %w", err)
+			}
 		}
 
 		// 2. Create a new ssh config with the include
+		content, err := os.ReadFile(configFile)
+		if err != nil {
+			return fmt.Errorf("failed to read ssh config: %w", err)
+		}
 		// Check if Include line already exists
 		found := false
 		for line := range strings.SplitSeq(string(content), "\n") {
@@ -128,23 +135,23 @@ func (m *Manager) UpdateConfig(name string, opts ConfigOptions) error {
 		} else if strings.HasPrefix(lowerTrimmed, "hostname ") {
 			foundFields["hostname"] = true
 			if opts.Hostname != "" {
-				indent := line[:strings.Index(lowerTrimmed, "hostname")]
+				indent := line[:strings.Index(strings.ToLower(line), "hostname")]
 				newLines = append(newLines, indent+"Hostname "+opts.Hostname)
 			} else {
 				newLines = append(newLines, line)
 			}
 		} else if strings.HasPrefix(lowerTrimmed, "user ") {
 			foundFields["user"] = true
 			if opts.User != "" {
-				indent := line[:strings.Index(lowerTrimmed, "user")]
+				indent := line[:strings.Index(strings.ToLower(line), "user")]
 				newLines = append(newLines, indent+"User "+opts.User)
 			} else {
 				newLines = append(newLines, line)
 			}
 		} else if strings.HasPrefix(lowerTrimmed, "port ") {
 			foundFields["port"] = true
 			if opts.Port != 0 {
-				indent := line[:strings.Index(lowerTrimmed, "port")]
+				indent := line[:strings.Index(strings.ToLower(line), "port")]
 				newLines = append(newLines, indent+fmt.Sprintf("Port %d", opts.Port))
 			} else {
 				newLines = append(newLines, line)
@@ -157,23 +164,23 @@ func (m *Manager) UpdateConfig(name string, opts ConfigOptions) error {
 				if absPath, err := filepath.Abs(identity); err == nil {
 					identity = absPath
 				}
-				indent := line[:strings.Index(lowerTrimmed, "identityfile")]
+				indent := line[:strings.Index(strings.ToLower(line), "identityfile")]
 				newLines = append(newLines, indent+"IdentityFile "+identity)
 			} else {
 				newLines = append(newLines, line)
 			}
 		} else if strings.HasPrefix(lowerTrimmed, "forwardagent ") {
 			foundFields["forwardagent"] = true
 			if opts.ForwardAgent != "" {
-				indent := line[:strings.Index(lowerTrimmed, "forwardagent")]
+				indent := line[:strings.Index(strings.ToLower(line), "forwardagent")]
 				newLines = append(newLines, indent+"ForwardAgent "+opts.ForwardAgent)
 			} else {
 				newLines = append(newLines, line)
 			}
 		} else if strings.HasPrefix(lowerTrimmed, "proxyjump ") {
 			foundFields["proxyjump"] = true
 			if opts.ProxyJump != "" {
-				indent := line[:strings.Index(lowerTrimmed, "proxyjump")]
+				indent := line[:strings.Index(strings.ToLower(line), "proxyjump")]
 				newLines = append(newLines, indent+"ProxyJump "+opts.ProxyJump)
 			} else {
 				newLines = append(newLines, line)
@@ -258,5 +265,6 @@ func (m *Manager) ListConfigs() ([]string, error) {
 			configs = append(configs, entry.Name())
 		}
 	}
+	slices.Sort(configs)
 	return configs, nil
 }

internal/config/manager_test.go

@@ -90,6 +90,27 @@ func TestManager_Init_Backup(t *testing.T) {
 	if count != 1 {
 		t.Errorf("Include line found %d times, expected 1", count)
 	}
+
+	// Verify that backup was NOT overwritten
+	// 1. Change the config content
+	newConfigContent := "Host new\n  Hostname updated.com"
+	if err := os.WriteFile(filepath.Join(tmpDir, "config"), []byte(newConfigContent), 0600); err != nil {
+		t.Fatal(err)
+	}
+
+	// 2. Run Init again
+	if err := m.Init(); err != nil {
+		t.Fatal(err)
+	}
+
+	// 3. Backup should still have the ORIGINAL content, not the newConfigContent
+	backupContentAfter, err := os.ReadFile(filepath.Join(tmpDir, "config.backup"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(backupContentAfter) != initialContent {
+		t.Errorf("Backup was overwritten! Got: %s, Want: %s", string(backupContentAfter), initialContent)
+	}
 }
 
 func TestManager_AddRemoveConfig(t *testing.T) {
@@ -129,6 +150,37 @@ func TestManager_AddRemoveConfig(t *testing.T) {
 	}
 }
 
+func TestManager_ListConfigs_Sorted(t *testing.T) {
+	tmpDir, err := os.MkdirTemp("", "sshc-test-list")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	m := &Manager{
+		SshDir: tmpDir,
+	}
+	_ = m.Init()
+
+	// Add configs in non-alphabetical order
+	names := []string{"zebra", "apple", "banana"}
+	for _, name := range names {
+		if err := m.AddConfig(name, "Host "+name); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	configs, err := m.ListConfigs()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	expected := []string{"apple", "banana", "zebra"}
+	if !slices.Equal(configs, expected) {
+		t.Errorf("ListConfigs() not sorted. Got: %v, Want: %v", configs, expected)
+	}
+}
+
 func TestManager_UpdateConfig(t *testing.T) {
 	tmpDir, err := os.MkdirTemp("", "sshc-test-update")
 	if err != nil {
@@ -218,3 +270,44 @@ func TestManager_UpdateConfig(t *testing.T) {
 		t.Errorf("ProxyJump not added")
 	}
 }
+
+func TestManager_UpdateConfig_Indentation(t *testing.T) {
+	tmpDir, err := os.MkdirTemp("", "sshc-test-indent")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	m := &Manager{
+		SshDir: tmpDir,
+	}
+	_ = m.Init()
+
+	name := "indent-test"
+	// Use 8 spaces for indentation
+	initialContent := "Host my-host\n        Hostname old-hostname\n"
+	if err := m.AddConfig(name, initialContent); err != nil {
+		t.Fatal(err)
+	}
+
+	opts := ConfigOptions{
+		Hostname: "new-hostname",
+	}
+	if err := m.UpdateConfig(name, opts); err != nil {
+		t.Fatal(err)
+	}
+
+	content, err := os.ReadFile(m.GetConfigPath(name))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	lines := strings.Split(strings.TrimSpace(string(content)), "\n")
+	for _, line := range lines {
+		if strings.Contains(line, "Hostname") {
+			if !strings.HasPrefix(line, "        ") {
+				t.Errorf("Indentation lost. Expected 8 spaces at start of line: %q", line)
+			}
+		}
+	}
+}