diff --git a/src/internal/path/path.go b/src/internal/path/path.go index 6a5dc74..2eb04c8 100644 --- a/src/internal/path/path.go +++ b/src/internal/path/path.go @@ -6,6 +6,8 @@ import ( "path/filepath" "runtime" "strings" + + "github.com/CodingWithCalvin/dtvem.cli/src/internal/constants" ) // IsInPath checks if a directory is in the system PATH @@ -14,7 +16,7 @@ func IsInPath(dir string) bool { // Get the path separator for this OS separator := ":" - if runtime.GOOS == "windows" { + if runtime.GOOS == constants.OSWindows { separator = ";" } @@ -34,6 +36,79 @@ func IsInPath(dir string) bool { return false } +// IsDtvemShimsPath reports whether path looks like a dtvem shims directory. +// It matches the standard installation patterns: +// - /dtvem/shims (e.g., ~/.local/share/dtvem/shims under XDG_DATA_HOME) +// - /.dtvem/shims (the default Windows/macOS layout, leading dot) +// +// Comparison is case-insensitive on Windows. Custom DTVEM_ROOT layouts whose +// final two components don't match these patterns are not detected. +func IsDtvemShimsPath(path string) bool { + if path == "" { + return false + } + + cleaned := filepath.Clean(path) + leaf := filepath.Base(cleaned) + parent := filepath.Base(filepath.Dir(cleaned)) + + leafEq := func(a, b string) bool { return a == b } + if runtime.GOOS == constants.OSWindows { + leafEq = strings.EqualFold + } + + if !leafEq(leaf, "shims") { + return false + } + return leafEq(parent, "dtvem") || leafEq(parent, ".dtvem") +} + +// FindStaleShimsEntries scans pathEntries for entries that look like dtvem +// shims directories but do not match currentShimsDir. The returned slice +// preserves the order of appearance in pathEntries and has the original +// (un-cleaned) entry strings, so callers can match them against registry +// or config-file content. +// +// Comparison against currentShimsDir is case-insensitive on Windows. +func FindStaleShimsEntries(pathEntries []string, currentShimsDir string) []string { + if currentShimsDir == "" { + return nil + } + currentClean := filepath.Clean(currentShimsDir) + + var stale []string + for _, entry := range pathEntries { + trimmed := strings.TrimSpace(entry) + if trimmed == "" { + continue + } + if !IsDtvemShimsPath(trimmed) { + continue + } + entryClean := filepath.Clean(trimmed) + if runtime.GOOS == constants.OSWindows { + if strings.EqualFold(entryClean, currentClean) { + continue + } + } else { + if entryClean == currentClean { + continue + } + } + stale = append(stale, entry) + } + return stale +} + +// SplitPath splits the PATH environment variable using the OS-appropriate separator. +func SplitPath(pathEnv string) []string { + separator := ":" + if runtime.GOOS == constants.OSWindows { + separator = ";" + } + return strings.Split(pathEnv, separator) +} + // ShimsDir returns the path to the shims directory // This replicates the root directory logic from config package to avoid circular dependencies. // Must stay in sync with config.getRootDir(). @@ -103,7 +178,7 @@ func LookPathExcludingShims(execName string) string { // On Windows, it tries .exe, .cmd, .bat extensions. // On Unix, it checks if the file exists and has execute permission. func findExecutableInDir(dir, execName string) string { - if runtime.GOOS == "windows" { + if runtime.GOOS == constants.OSWindows { // Windows: try .exe, .cmd, .bat extensions for _, ext := range []string{".exe", ".cmd", ".bat"} { candidate := filepath.Join(dir, execName+ext) diff --git a/src/internal/path/path_test.go b/src/internal/path/path_test.go index 2a27ca1..31e0b35 100644 --- a/src/internal/path/path_test.go +++ b/src/internal/path/path_test.go @@ -372,6 +372,167 @@ func TestLookPathExcludingShims_SkipsShimsDir(t *testing.T) { }) } +func TestIsDtvemShimsPath(t *testing.T) { + // Platform-specific path separator handling: filepath.Join produces + // backslashes on Windows and forward slashes on Unix, which matches what + // real PATH entries look like on each platform. + tests := []struct { + name string + path string + want bool + }{ + { + name: "leading-dot dtvem under home", + path: filepath.Join("C:", "Users", "testuser", ".dtvem", "shims"), + want: true, + }, + { + name: "no-dot dtvem under XDG data home", + path: filepath.Join("C:", "Users", "testuser", ".local", "share", "dtvem", "shims"), + want: true, + }, + { + name: "unix style leading-dot", + path: "/home/testuser/.dtvem/shims", + want: true, + }, + { + name: "unix style XDG", + path: "/home/testuser/.local/share/dtvem/shims", + want: true, + }, + { + name: "trailing slash is normalized", + path: "/home/testuser/.dtvem/shims/", + want: true, + }, + { + name: "shims under non-dtvem parent does not match", + path: "/home/testuser/something/shims", + want: false, + }, + { + name: "dtvem dir without shims leaf does not match", + path: "/home/testuser/.dtvem/bin", + want: false, + }, + { + name: "empty string", + path: "", + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := IsDtvemShimsPath(tt.path) + if got != tt.want { + t.Errorf("IsDtvemShimsPath(%q) = %v, want %v", tt.path, got, tt.want) + } + }) + } +} + +func TestIsDtvemShimsPath_WindowsCaseInsensitive(t *testing.T) { + if runtime.GOOS != constants.OSWindows { + t.Skip("Windows-only: case-insensitive path matching") + } + + cases := []string{ + `C:\Users\testuser\.DTVEM\Shims`, + `C:\Users\testuser\.local\share\DTVEM\SHIMS`, + `C:\Users\testuser\.Dtvem\shims`, + } + for _, p := range cases { + if !IsDtvemShimsPath(p) { + t.Errorf("IsDtvemShimsPath(%q) = false, want true (Windows case-insensitive)", p) + } + } +} + +func TestFindStaleShimsEntries(t *testing.T) { + // Build paths that look right on the current platform so the + // case-insensitive comparison logic exercises real separators. + currentXDG := filepath.Join("C:", "Users", "testuser", ".local", "share", "dtvem", "shims") + staleHome := filepath.Join("C:", "Users", "testuser", ".dtvem", "shims") + unrelated := filepath.Join("C:", "Windows", "System32") + + tests := []struct { + name string + entries []string + current string + want []string + }{ + { + name: "stale leading-dot entry alongside current XDG", + entries: []string{currentXDG, unrelated, staleHome}, + current: currentXDG, + want: []string{staleHome}, + }, + { + name: "no stale entries when only current is present", + entries: []string{currentXDG, unrelated}, + current: currentXDG, + want: nil, + }, + { + name: "current dir is the leading-dot variant", + entries: []string{staleHome, currentXDG, unrelated}, + current: staleHome, + want: []string{currentXDG}, + }, + { + name: "empty entries are skipped", + entries: []string{"", staleHome, " "}, + current: currentXDG, + want: []string{staleHome}, + }, + { + name: "preserves original entry strings (not cleaned)", + entries: []string{staleHome + string(filepath.Separator), unrelated}, + current: currentXDG, + want: []string{staleHome + string(filepath.Separator)}, + }, + { + name: "empty current shimsDir returns nil", + entries: []string{staleHome}, + current: "", + want: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := FindStaleShimsEntries(tt.entries, tt.current) + if len(got) != len(tt.want) { + t.Fatalf("FindStaleShimsEntries() = %v, want %v", got, tt.want) + } + for i := range got { + if got[i] != tt.want[i] { + t.Errorf("FindStaleShimsEntries()[%d] = %q, want %q", i, got[i], tt.want[i]) + } + } + }) + } +} + +func TestFindStaleShimsEntries_WindowsCaseInsensitive(t *testing.T) { + if runtime.GOOS != constants.OSWindows { + t.Skip("Windows-only: case-insensitive comparison") + } + + current := `C:\Users\testuser\.local\share\dtvem\shims` + // Same logical path as `current` but with mixed casing — should NOT be + // flagged stale. + sameAsCurrentDifferentCase := `C:\Users\TESTUSER\.LOCAL\share\dtvem\SHIMS` + stale := `C:\Users\testuser\.dtvem\shims` + + got := FindStaleShimsEntries([]string{sameAsCurrentDifferentCase, stale}, current) + if len(got) != 1 || got[0] != stale { + t.Errorf("FindStaleShimsEntries() = %v, want exactly [%q]", got, stale) + } +} + func TestFindExecutableInDir(t *testing.T) { tempDir := t.TempDir() diff --git a/src/internal/path/path_unix.go b/src/internal/path/path_unix.go index 1aa6c7a..eb45666 100644 --- a/src/internal/path/path_unix.go +++ b/src/internal/path/path_unix.go @@ -68,6 +68,12 @@ func AddToPath(shimsDir string, skipConfirmation bool, userInstall bool) error { return fmt.Errorf("could not determine config file for shell %s", shell) } + // Warn about any stale dtvem shims directories in PATH (e.g. left over + // after switching XDG_DATA_HOME or upgrading from a pre-XDG install). + // We don't auto-rewrite shell config files on Unix because users often + // customize them heavily; surface the entries with manual cleanup steps. + warnAboutStaleShimsEntries(shimsDir, configFile) + // Check if the directory is already in PATH if IsInPath(shimsDir) { ui.Info("%s is already in your PATH", shimsDir) @@ -134,6 +140,25 @@ func AddToPath(shimsDir string, skipConfirmation bool, userInstall bool) error { return nil } +// warnAboutStaleShimsEntries scans the current PATH for dtvem shims directories +// that don't match shimsDir and prints manual cleanup instructions for each. +// We don't auto-rewrite shell config files on Unix to avoid clobbering user edits. +func warnAboutStaleShimsEntries(shimsDir, configFile string) { + stale := FindStaleShimsEntries(SplitPath(os.Getenv("PATH")), shimsDir) + if len(stale) == 0 { + return + } + + ui.Warning("Found stale dtvem shims entries in your PATH:") + for _, s := range stale { + ui.Info(" %s", s) + } + ui.Info("These were likely left over from a prior install or before XDG_DATA_HOME was set.") + ui.Info("Edit %s and remove the export lines that reference the stale paths above.", ui.Highlight(configFile)) + ui.Info("After editing, restart your terminal or run: source %s", configFile) + ui.Info("") +} + // containsPathModification checks if the config file already has dtvem PATH modification func containsPathModification(configFile, shimsDir string) bool { f, err := os.Open(configFile) diff --git a/src/internal/path/path_windows.go b/src/internal/path/path_windows.go index 4e27657..454ac65 100644 --- a/src/internal/path/path_windows.go +++ b/src/internal/path/path_windows.go @@ -129,8 +129,16 @@ func checkSystemPath(shimsDir string) (bool, string, error) { } } + // Even if shimsDir is in the right place, stale dtvem shims entries + // from prior installs (e.g. ~/.dtvem/shims left behind after switching + // to XDG_DATA_HOME) need to be cleaned up. + hasStale := len(FindStaleShimsEntries(paths, shimsDir)) > 0 + if foundAt == 0 { - return false, "", nil // Already at beginning + if hasStale { + return true, pathActionMove, nil + } + return false, "", nil // Already at beginning, nothing stale } else if foundAt > 0 { return true, pathActionMove, nil // Exists but not at beginning } @@ -167,8 +175,15 @@ func checkUserPath(shimsDir string) (bool, string, error) { } } + // Even if shimsDir is in the right place, stale dtvem shims entries + // from prior installs need to be cleaned up. + hasStale := len(FindStaleShimsEntries(paths, shimsDir)) > 0 + if foundAt == 0 { - return false, "", nil // Already at beginning + if hasStale { + return true, pathActionMove, nil + } + return false, "", nil // Already at beginning, nothing stale } else if foundAt > 0 { return true, pathActionMove, nil // Exists but not at beginning } @@ -261,6 +276,7 @@ func modifySystemPath(shimsDir, action string) error { // Parse and filter current PATH entries paths := strings.Split(currentPath, ";") var filteredPaths []string + var removedStale []string for _, p := range paths { trimmed := strings.TrimSpace(p) @@ -271,6 +287,11 @@ func modifySystemPath(shimsDir, action string) error { if strings.EqualFold(trimmed, shimsDir) { continue } + // Skip stale dtvem shims entries from prior installs + if IsDtvemShimsPath(trimmed) { + removedStale = append(removedStale, trimmed) + continue + } filteredPaths = append(filteredPaths, trimmed) } @@ -289,6 +310,10 @@ func modifySystemPath(shimsDir, action string) error { // Broadcast WM_SETTINGCHANGE to notify running processes broadcastSettingChange() + for _, stale := range removedStale { + ui.Success("Removed stale dtvem shims entry from System PATH: %s", stale) + } + if action == pathActionMove { ui.Success("Moved %s to the beginning of your System PATH", shimsDir) } else { @@ -314,6 +339,7 @@ func modifyUserPath(shimsDir, action string) error { // Parse and filter current PATH entries var filteredPaths []string + var removedStale []string if currentPath != "" { paths := strings.Split(currentPath, ";") for _, p := range paths { @@ -325,6 +351,11 @@ func modifyUserPath(shimsDir, action string) error { if strings.EqualFold(trimmed, shimsDir) { continue } + // Skip stale dtvem shims entries from prior installs + if IsDtvemShimsPath(trimmed) { + removedStale = append(removedStale, trimmed) + continue + } filteredPaths = append(filteredPaths, trimmed) } } @@ -344,6 +375,10 @@ func modifyUserPath(shimsDir, action string) error { // Broadcast WM_SETTINGCHANGE to notify running processes broadcastSettingChange() + for _, stale := range removedStale { + ui.Success("Removed stale dtvem shims entry from User PATH: %s", stale) + } + if action == pathActionMove { ui.Success("Moved %s to the beginning of your User PATH", shimsDir) } else {