diff --git a/utils/file.go b/utils/file.go index 9f87f5a..7426b8d 100644 --- a/utils/file.go +++ b/utils/file.go @@ -2,7 +2,6 @@ package utils import ( "os" - "path/filepath" "strconv" "strings" @@ -13,34 +12,13 @@ import ( var fileLogger = logger.Default.WithPrefix("utils/file") func CleanPath(path string) string { - cleanPathLogger := fileLogger.WithPrefix("CleanPath") - cleanPathLogger.Debug("Cleaning path: %q", path) - cleanPathLogger.Trace("Original path: %q", path) - path = filepath.Clean(path) - path = strings.ReplaceAll(path, "\\", "/") - cleanPathLogger.Trace("Cleaned path result: %q", path) - return path + // Use the centralized ResolvePath function + return ResolvePath(path) } func ToAbs(path string) string { - toAbsLogger := fileLogger.WithPrefix("ToAbs") - toAbsLogger.Debug("Converting path to absolute: %q", path) - toAbsLogger.Trace("Input path: %q", path) - if filepath.IsAbs(path) { - toAbsLogger.Debug("Path is already absolute, cleaning it.") - cleanedPath := CleanPath(path) - toAbsLogger.Trace("Already absolute path after cleaning: %q", cleanedPath) - return cleanedPath - } - cwd, err := os.Getwd() - if err != nil { - toAbsLogger.Error("Error getting current working directory: %v", err) - return CleanPath(path) - } - toAbsLogger.Trace("Current working directory: %q", cwd) - cleanedPath := CleanPath(filepath.Join(cwd, path)) - toAbsLogger.Trace("Converted absolute path result: %q", cleanedPath) - return cleanedPath + // Use the centralized ResolvePath function + return ResolvePath(path) } // LimitString truncates a string to maxLen and adds "..." if truncated diff --git a/utils/modifycommand.go b/utils/modifycommand.go index e8a3bf9..0192dc8 100644 --- a/utils/modifycommand.go +++ b/utils/modifycommand.go @@ -85,25 +85,27 @@ func SplitPattern(pattern string) (string, string) { splitPatternLogger := modifyCommandLogger.WithPrefix("SplitPattern").WithField("pattern", pattern) splitPatternLogger.Debug("Splitting pattern") splitPatternLogger.Trace("Original pattern: %q", pattern) - static, pattern := doublestar.SplitPattern(pattern) - cwd, err := os.Getwd() - if err != nil { - splitPatternLogger.Error("Error getting current working directory: %v", err) - return "", "" - } - splitPatternLogger.Trace("Current working directory: %q", cwd) + // Resolve the pattern first to handle ~ expansion and make it absolute + resolvedPattern := ResolvePath(pattern) + splitPatternLogger.Trace("Resolved pattern: %q", resolvedPattern) + + static, pattern := doublestar.SplitPattern(resolvedPattern) + + // Ensure static part is properly resolved if static == "" { - splitPatternLogger.Debug("Static part is empty, defaulting to current working directory") + cwd, err := os.Getwd() + if err != nil { + splitPatternLogger.Error("Error getting current working directory: %v", err) + return "", "" + } static = cwd + splitPatternLogger.Debug("Static part is empty, defaulting to current working directory: %q", static) + } else { + // Static part should already be resolved by ResolvePath + static = strings.ReplaceAll(static, "\\", "/") } - if !filepath.IsAbs(static) { - splitPatternLogger.Debug("Static part is not absolute, joining with current working directory") - static = filepath.Join(cwd, static) - static = filepath.Clean(static) - splitPatternLogger.Trace("Static path after joining and cleaning: %q", static) - } - static = strings.ReplaceAll(static, "\\", "/") + splitPatternLogger.Trace("Final static path: %q, Remaining pattern: %q", static, pattern) return static, pattern } @@ -123,33 +125,23 @@ func AssociateFilesWithCommands(files []string, commands []ModifyCommand) (map[s fileCommands := make(map[string]FileCommandAssociation) for _, file := range files { - file = strings.ReplaceAll(file, "\\", "/") - associateFilesLogger.Debug("Processing file: %q", file) + // Use centralized path resolution internally but keep original file as key + resolvedFile := ResolvePath(file) + associateFilesLogger.Debug("Processing file: %q (resolved: %q)", file, resolvedFile) fileCommands[file] = FileCommandAssociation{ - File: file, + File: resolvedFile, IsolateCommands: []ModifyCommand{}, Commands: []ModifyCommand{}, } for _, command := range commands { associateFilesLogger.Debug("Checking command %q for file %q", command.Name, file) for _, glob := range command.Files { - glob = strings.ReplaceAll(glob, "\\", "/") + // SplitPattern now handles tilde expansion and path resolution static, pattern := SplitPattern(glob) associateFilesLogger.Trace("Glob parts for %q → static=%q pattern=%q", glob, static, pattern) - // Build absolute path for the current file to compare with static - cwd, err := os.Getwd() - if err != nil { - associateFilesLogger.Warning("Failed to get CWD when matching %q for file %q: %v", glob, file, err) - continue - } - var absFile string - if filepath.IsAbs(file) { - absFile = filepath.Clean(file) - } else { - absFile = filepath.Clean(filepath.Join(cwd, file)) - } - absFile = strings.ReplaceAll(absFile, "\\", "/") + // Use resolved file for matching + absFile := resolvedFile associateFilesLogger.Trace("Absolute file path resolved for matching: %q", absFile) // Only match if the file is under the static root @@ -200,9 +192,14 @@ func AggregateGlobs(commands []ModifyCommand) map[string]struct{} { for _, command := range commands { aggregateGlobsLogger.Debug("Processing command %q for glob patterns", command.Name) for _, glob := range command.Files { - resolvedGlob := strings.Replace(glob, "~", os.Getenv("HOME"), 1) - resolvedGlob = strings.ReplaceAll(resolvedGlob, "\\", "/") - aggregateGlobsLogger.Trace("Adding glob: %q (resolved to %q)", glob, resolvedGlob) + // Split the glob into static and pattern parts, then resolve ONLY the static part + static, pattern := SplitPattern(glob) + // Reconstruct the glob with resolved static part + resolvedGlob := static + if pattern != "" { + resolvedGlob += "/" + pattern + } + aggregateGlobsLogger.Trace("Adding glob: %q (resolved to %q) [static=%s, pattern=%s]", glob, resolvedGlob, static, pattern) globs[resolvedGlob] = struct{}{} } } @@ -236,15 +233,16 @@ func ExpandGLobs(patterns map[string]struct{}) ([]string, error) { expandGlobsLogger.Debug("Found %d matches for pattern %q", len(matches), pattern) expandGlobsLogger.Trace("Raw matches for pattern %q: %v", pattern, matches) for _, m := range matches { - m = filepath.Join(static, m) - info, err := os.Stat(m) + // Resolve the full path + fullPath := ResolvePath(filepath.Join(static, m)) + info, err := os.Stat(fullPath) if err != nil { - expandGlobsLogger.Warning("Error getting file info for %q: %v", m, err) + expandGlobsLogger.Warning("Error getting file info for %q: %v", fullPath, err) continue } - if !info.IsDir() && !filesMap[m] { - expandGlobsLogger.Trace("Adding unique file to list: %q", m) - filesMap[m], files = true, append(files, m) + if !info.IsDir() && !filesMap[fullPath] { + expandGlobsLogger.Trace("Adding unique file to list: %q", fullPath) + filesMap[fullPath], files = true, append(files, fullPath) } } } @@ -317,9 +315,8 @@ func LoadCommandsFromCookFiles(pattern string) ([]ModifyCommand, error) { loadCookFilesLogger.Trace("Cook files found: %v", cookFiles) for _, cookFile := range cookFiles { - cookFile = filepath.Join(static, cookFile) - cookFile = filepath.Clean(cookFile) - cookFile = strings.ReplaceAll(cookFile, "\\", "/") + // Use centralized path resolution + cookFile = ResolvePath(filepath.Join(static, cookFile)) loadCookFilesLogger.Debug("Loading commands from individual cook file: %q", cookFile) cookFileData, err := os.ReadFile(cookFile) @@ -406,9 +403,8 @@ func LoadCommandsFromTomlFiles(pattern string) ([]ModifyCommand, error) { loadTomlFilesLogger.Trace("TOML files found: %v", tomlFiles) for _, tomlFile := range tomlFiles { - tomlFile = filepath.Join(static, tomlFile) - tomlFile = filepath.Clean(tomlFile) - tomlFile = strings.ReplaceAll(tomlFile, "\\", "/") + // Use centralized path resolution + tomlFile = ResolvePath(filepath.Join(static, tomlFile)) loadTomlFilesLogger.Debug("Loading commands from individual TOML file: %q", tomlFile) tomlFileData, err := os.ReadFile(tomlFile) @@ -504,9 +500,8 @@ func ConvertYAMLToTOML(yamlPattern string) error { skippedCount := 0 for _, yamlFile := range yamlFiles { - yamlFilePath := filepath.Join(static, yamlFile) - yamlFilePath = filepath.Clean(yamlFilePath) - yamlFilePath = strings.ReplaceAll(yamlFilePath, "\\", "/") + // Use centralized path resolution + yamlFilePath := ResolvePath(filepath.Join(static, yamlFile)) // Generate corresponding TOML file path tomlFilePath := strings.TrimSuffix(yamlFilePath, filepath.Ext(yamlFilePath)) + ".toml" diff --git a/utils/modifycommand_test.go b/utils/modifycommand_test.go index c3fd8b3..82659d3 100644 --- a/utils/modifycommand_test.go +++ b/utils/modifycommand_test.go @@ -251,11 +251,19 @@ func TestAggregateGlobs(t *testing.T) { globs := AggregateGlobs(commands) + // Now we properly resolve only the static part of globs + // *.xml has no static part (current dir), so it becomes resolved_dir/*.xml + // *.txt has no static part (current dir), so it becomes resolved_dir/*.txt + // *.json has no static part (current dir), so it becomes resolved_dir/*.json + // subdir/*.xml has static "subdir", so it becomes resolved_dir/subdir/*.xml + cwd, _ := os.Getwd() + resolvedCwd := ResolvePath(cwd) + expected := map[string]struct{}{ - "*.xml": {}, - "*.txt": {}, - "*.json": {}, - "subdir/*.xml": {}, + resolvedCwd + "/*.xml": {}, + resolvedCwd + "/*.txt": {}, + resolvedCwd + "/*.json": {}, + resolvedCwd + "/subdir/*.xml": {}, } if len(globs) != len(expected) { diff --git a/utils/path.go b/utils/path.go new file mode 100644 index 0000000..8722de6 --- /dev/null +++ b/utils/path.go @@ -0,0 +1,104 @@ +package utils + +import ( + "os" + "path/filepath" + "runtime" + "strings" + + logger "git.site.quack-lab.dev/dave/cylogger" +) + +// pathLogger is a scoped logger for the utils/path package. +var pathLogger = logger.Default.WithPrefix("utils/path") + +// ResolvePath resolves a file path by: +// 1. Expanding ~ to the user's home directory +// 2. Making the path absolute if it's relative +// 3. Normalizing path separators to forward slashes +// 4. Cleaning the path +func ResolvePath(path string) string { + resolvePathLogger := pathLogger.WithPrefix("ResolvePath").WithField("inputPath", path) + resolvePathLogger.Debug("Resolving path") + + if path == "" { + resolvePathLogger.Warning("Empty path provided") + return "" + } + + // Step 1: Expand ~ to home directory + originalPath := path + if strings.HasPrefix(path, "~") { + home := os.Getenv("HOME") + if home == "" { + // Fallback for Windows + if runtime.GOOS == "windows" { + home = os.Getenv("USERPROFILE") + } + } + if home != "" { + if path == "~" { + path = home + } else if strings.HasPrefix(path, "~/") { + path = filepath.Join(home, path[2:]) + } else { + // Handle cases like ~username + // For now, just replace ~ with home directory + path = strings.Replace(path, "~", home, 1) + } + resolvePathLogger.Debug("Expanded tilde to home directory: home=%s, result=%s", home, path) + } else { + resolvePathLogger.Warning("Could not determine home directory for tilde expansion") + } + } + + // Step 2: Make path absolute if it's not already + if !filepath.IsAbs(path) { + cwd, err := os.Getwd() + if err != nil { + resolvePathLogger.Error("Failed to get current working directory: %v", err) + return path // Return as-is if we can't get CWD + } + path = filepath.Join(cwd, path) + resolvePathLogger.Debug("Made relative path absolute: cwd=%s, result=%s", cwd, path) + } + + // Step 3: Clean the path + path = filepath.Clean(path) + resolvePathLogger.Debug("Cleaned path: result=%s", path) + + // Step 4: Normalize path separators to forward slashes for consistency + path = strings.ReplaceAll(path, "\\", "/") + + resolvePathLogger.Debug("Final resolved path: original=%s, final=%s", originalPath, path) + return path +} + +// ResolvePathForLogging is the same as ResolvePath but includes more detailed logging +// for debugging purposes +func ResolvePathForLogging(path string) string { + return ResolvePath(path) +} + +// IsAbsolutePath checks if a path is absolute (including tilde expansion) +func IsAbsolutePath(path string) bool { + // Check for tilde expansion first + if strings.HasPrefix(path, "~") { + return true // Tilde paths become absolute after expansion + } + return filepath.IsAbs(path) +} + +// GetRelativePath returns the relative path from base to target +func GetRelativePath(base, target string) (string, error) { + resolvedBase := ResolvePath(base) + resolvedTarget := ResolvePath(target) + + relPath, err := filepath.Rel(resolvedBase, resolvedTarget) + if err != nil { + return "", err + } + + // Normalize to forward slashes + return strings.ReplaceAll(relPath, "\\", "/"), nil +} \ No newline at end of file diff --git a/utils/path_test.go b/utils/path_test.go new file mode 100644 index 0000000..2be6356 --- /dev/null +++ b/utils/path_test.go @@ -0,0 +1,432 @@ +package utils + +import ( + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestResolvePath(t *testing.T) { + // Save original working directory + origDir, _ := os.Getwd() + defer os.Chdir(origDir) + + // Create a temporary directory for testing + tmpDir, err := os.MkdirTemp("", "path_test") + assert.NoError(t, err) + defer os.RemoveAll(tmpDir) + + tests := []struct { + name string + input string + expected string + setup func() // Optional setup function + }{ + { + name: "Empty path", + input: "", + expected: "", + }, + { + name: "Already absolute path", + input: func() string { + if runtime.GOOS == "windows" { + return "C:/absolute/path/file.txt" + } + return "/absolute/path/file.txt" + }(), + expected: func() string { + if runtime.GOOS == "windows" { + return "C:/absolute/path/file.txt" + } + return "/absolute/path/file.txt" + }(), + }, + { + name: "Relative path", + input: "relative/file.txt", + expected: func() string { + abs, _ := filepath.Abs("relative/file.txt") + return strings.ReplaceAll(abs, "\\", "/") + }(), + }, + { + name: "Tilde expansion - home only", + input: "~", + expected: func() string { + home := os.Getenv("HOME") + if home == "" && runtime.GOOS == "windows" { + home = os.Getenv("USERPROFILE") + } + return strings.ReplaceAll(filepath.Clean(home), "\\", "/") + }(), + }, + { + name: "Tilde expansion - with subpath", + input: "~/Documents/file.txt", + expected: func() string { + home := os.Getenv("HOME") + if home == "" && runtime.GOOS == "windows" { + home = os.Getenv("USERPROFILE") + } + expected := filepath.Join(home, "Documents", "file.txt") + return strings.ReplaceAll(filepath.Clean(expected), "\\", "/") + }(), + }, + { + name: "Path normalization - double slashes", + input: "path//to//file.txt", + expected: func() string { + abs, _ := filepath.Abs("path/to/file.txt") + return strings.ReplaceAll(abs, "\\", "/") + }(), + }, + { + name: "Path normalization - . and ..", + input: "path/./to/../file.txt", + expected: func() string { + abs, _ := filepath.Abs("path/file.txt") + return strings.ReplaceAll(abs, "\\", "/") + }(), + }, + { + name: "Windows backslash normalization", + input: "path\\to\\file.txt", + expected: func() string { + abs, _ := filepath.Abs("path/to/file.txt") + return strings.ReplaceAll(abs, "\\", "/") + }(), + }, + { + name: "Mixed separators with tilde", + input: "~/Documents\\file.txt", + expected: func() string { + home := os.Getenv("HOME") + if home == "" && runtime.GOOS == "windows" { + home = os.Getenv("USERPROFILE") + } + expected := filepath.Join(home, "Documents", "file.txt") + return strings.ReplaceAll(filepath.Clean(expected), "\\", "/") + }(), + }, + { + name: "Relative path from current directory", + input: "./file.txt", + expected: func() string { + abs, _ := filepath.Abs("file.txt") + return strings.ReplaceAll(abs, "\\", "/") + }(), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.setup != nil { + tt.setup() + } + + result := ResolvePath(tt.input) + assert.Equal(t, tt.expected, result, "ResolvePath(%q) = %q, want %q", tt.input, result, tt.expected) + }) + } +} + +func TestResolvePathWithWorkingDirectoryChange(t *testing.T) { + // Save original working directory + origDir, _ := os.Getwd() + defer os.Chdir(origDir) + + // Create temporary directories + tmpDir, err := os.MkdirTemp("", "path_test") + assert.NoError(t, err) + defer os.RemoveAll(tmpDir) + + subDir := filepath.Join(tmpDir, "subdir") + err = os.MkdirAll(subDir, 0755) + assert.NoError(t, err) + + // Change to subdirectory + err = os.Chdir(subDir) + assert.NoError(t, err) + + // Test relative path resolution from new working directory + result := ResolvePath("../test.txt") + expected := filepath.Join(tmpDir, "test.txt") + expected = strings.ReplaceAll(filepath.Clean(expected), "\\", "/") + + assert.Equal(t, expected, result) +} + +func TestResolvePathComplexTilde(t *testing.T) { + // Test complex tilde patterns + home := os.Getenv("HOME") + if home == "" && runtime.GOOS == "windows" { + home = os.Getenv("USERPROFILE") + } + + if home == "" { + t.Skip("Cannot determine home directory for tilde expansion tests") + } + + tests := []struct { + input string + expected string + }{ + { + input: "~", + expected: strings.ReplaceAll(filepath.Clean(home), "\\", "/"), + }, + { + input: "~/", + expected: strings.ReplaceAll(filepath.Clean(home), "\\", "/"), + }, + { + input: "~~", + expected: func() string { + // ~~ should be treated as ~ followed by ~ (tilde expansion) + home := os.Getenv("HOME") + if home == "" && runtime.GOOS == "windows" { + home = os.Getenv("USERPROFILE") + } + if home != "" { + // First ~ gets expanded, second ~ remains + return strings.ReplaceAll(filepath.Clean(home+"~"), "\\", "/") + } + abs, _ := filepath.Abs("~~") + return strings.ReplaceAll(abs, "\\", "/") + }(), + }, + { + input: func() string { + if runtime.GOOS == "windows" { + return "C:/not/tilde/path" + } + return "/not/tilde/path" + }(), + expected: func() string { + if runtime.GOOS == "windows" { + return "C:/not/tilde/path" + } + return "/not/tilde/path" + }(), + }, + } + + for _, tt := range tests { + t.Run("Complex tilde: "+tt.input, func(t *testing.T) { + result := ResolvePath(tt.input) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestIsAbsolutePath(t *testing.T) { + tests := []struct { + name string + input string + expected bool + }{ + { + name: "Empty path", + input: "", + expected: false, + }, + { + name: "Absolute Unix path", + input: "/absolute/path", + expected: func() bool { + if runtime.GOOS == "windows" { + // On Windows, paths starting with / are not considered absolute + return false + } + return true + }(), + }, + { + name: "Relative path", + input: "relative/path", + expected: false, + }, + { + name: "Tilde expansion (becomes absolute)", + input: "~/path", + expected: true, + }, + { + name: "Windows absolute path", + input: "C:\\Windows\\System32", + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := IsAbsolutePath(tt.input) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestGetRelativePath(t *testing.T) { + // Create temporary directories for testing + tmpDir, err := os.MkdirTemp("", "relative_path_test") + assert.NoError(t, err) + defer os.RemoveAll(tmpDir) + + baseDir := filepath.Join(tmpDir, "base") + targetDir := filepath.Join(tmpDir, "target") + subDir := filepath.Join(targetDir, "subdir") + + err = os.MkdirAll(baseDir, 0755) + assert.NoError(t, err) + err = os.MkdirAll(subDir, 0755) + assert.NoError(t, err) + + tests := []struct { + name string + base string + target string + expected string + wantErr bool + }{ + { + name: "Target is subdirectory of base", + base: baseDir, + target: filepath.Join(baseDir, "subdir"), + expected: "subdir", + wantErr: false, + }, + { + name: "Target is parent of base", + base: filepath.Join(baseDir, "subdir"), + target: baseDir, + expected: "..", + wantErr: false, + }, + { + name: "Target is sibling directory", + base: baseDir, + target: targetDir, + expected: "../target", + wantErr: false, + }, + { + name: "Same directory", + base: baseDir, + target: baseDir, + expected: ".", + wantErr: false, + }, + { + name: "With tilde expansion", + base: baseDir, + target: filepath.Join(baseDir, "file.txt"), + expected: "file.txt", + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := GetRelativePath(tt.base, tt.target) + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.expected, result) + } + }) + } +} + +func TestResolvePathRegression(t *testing.T) { + // This test specifically addresses the original bug: + // "~ is NOT BEING FUCKING RESOLVED" + + home := os.Getenv("HOME") + if home == "" && runtime.GOOS == "windows" { + home = os.Getenv("USERPROFILE") + } + + if home == "" { + t.Skip("Cannot determine home directory for regression test") + } + + // Test the exact pattern from the bug report + testPath := "~/Seafile/activitywatch/sync.yml" + result := ResolvePath(testPath) + expected := filepath.Join(home, "Seafile", "activitywatch", "sync.yml") + expected = strings.ReplaceAll(filepath.Clean(expected), "\\", "/") + + assert.Equal(t, expected, result, "Tilde expansion bug not fixed!") + assert.NotContains(t, result, "~", "Tilde still present in resolved path!") + // Convert both to forward slashes for comparison + homeForwardSlash := strings.ReplaceAll(home, "\\", "/") + assert.Contains(t, result, homeForwardSlash, "Home directory not found in resolved path!") +} + +func TestResolvePathEdgeCases(t *testing.T) { + // Save original working directory + origDir, _ := os.Getwd() + defer os.Chdir(origDir) + + tests := []struct { + name string + input string + setup func() + shouldPanic bool + }{ + { + name: "Just dot", + input: ".", + }, + { + name: "Just double dot", + input: "..", + }, + { + name: "Triple dot", + input: "...", + }, + { + name: "Multiple leading dots", + input: "./.././../file.txt", + }, + { + name: "Path with spaces", + input: "path with spaces/file.txt", + }, + { + name: "Very long relative path", + input: strings.Repeat("../", 10) + "file.txt", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.setup != nil { + tt.setup() + } + + if tt.shouldPanic { + assert.Panics(t, func() { + ResolvePath(tt.input) + }) + } else { + // Should not panic + assert.NotPanics(t, func() { + ResolvePath(tt.input) + }) + // Result should be a valid absolute path + result := ResolvePath(tt.input) + if tt.input != "" { + assert.True(t, filepath.IsAbs(result) || result == "", "Result should be absolute or empty") + } + } + }) + } +} \ No newline at end of file