git.iko.soy/crush
on main help sign in
refactor(fsext): improve hierarchical ignore handling and consolidate file exclusion logic (#999)
df2e64eCarlos Alexandro Becker
internal/fsext/fileutil.go+45−48
compare with main
unifiedsplit
@@ -11,8 +11,6 @@ import (
11 "github.com/bmatcuk/doublestar/v4"
11 "github.com/bmatcuk/doublestar/v4"
12 "github.com/charlievieth/fastwalk"
12 "github.com/charlievieth/fastwalk"
13 "github.com/charmbracelet/crush/internal/home"
13 "github.com/charmbracelet/crush/internal/home"
14−
15− ignore "github.com/sabhiram/go-gitignore"
16)
14)
17
15
18type FileInfo struct {
16type FileInfo struct {
@@ -58,60 +56,22 @@ func SkipHidden(path string) bool {
58}
56}
59
57
60// FastGlobWalker provides gitignore-aware file walking with fastwalk
58// FastGlobWalker provides gitignore-aware file walking with fastwalk
59+// It uses hierarchical ignore checking like git does, checking .gitignore/.crushignore
60+// files in each directory from the root to the target path.
61type FastGlobWalker struct {
61type FastGlobWalker struct {
62− gitignore *ignore.GitIgnore
62+ directoryLister *directoryLister
63− crushignore *ignore.GitIgnore
64− rootPath string
65}
63}
66
64
67func NewFastGlobWalker(searchPath string) *FastGlobWalker {
65func NewFastGlobWalker(searchPath string) *FastGlobWalker {
68− walker := &FastGlobWalker{
66+ return &FastGlobWalker{
69− rootPath: searchPath,
67+ directoryLister: NewDirectoryLister(searchPath),
70− }
71−
72− // Load gitignore if it exists
73− gitignorePath := filepath.Join(searchPath, ".gitignore")
74− if _, err := os.Stat(gitignorePath); err == nil {
75− if gi, err := ignore.CompileIgnoreFile(gitignorePath); err == nil {
76− walker.gitignore = gi
77− }
78 }
68 }
79−
80− // Load crushignore if it exists
81− crushignorePath := filepath.Join(searchPath, ".crushignore")
82− if _, err := os.Stat(crushignorePath); err == nil {
83− if ci, err := ignore.CompileIgnoreFile(crushignorePath); err == nil {
84− walker.crushignore = ci
85− }
86− }
87−
88− return walker
89}
69}
90
70
91−// ShouldSkip checks if a path should be skipped based on gitignore, crushignore, and hidden file rules
71+// ShouldSkip checks if a path should be skipped based on hierarchical gitignore,
72+// crushignore, and hidden file rules
92func (w *FastGlobWalker) ShouldSkip(path string) bool {
73func (w *FastGlobWalker) ShouldSkip(path string) bool {
93− if SkipHidden(path) {
74+ return w.directoryLister.shouldIgnore(path, nil)
94− return true
95− }
96−
97− relPath, err := filepath.Rel(w.rootPath, path)
98− if err != nil {
99− return false
100− }
101−
102− if w.gitignore != nil {
103− if w.gitignore.MatchesPath(relPath) {
104− return true
105− }
106− }
107−
108− if w.crushignore != nil {
109− if w.crushignore.MatchesPath(relPath) {
110− return true
111− }
112− }
113−
114− return false
115}
75}
116
76
117func GlobWithDoubleStar(pattern, searchPath string, limit int) ([]string, bool, error) {
77func GlobWithDoubleStar(pattern, searchPath string, limit int) ([]string, bool, error) {
@@ -182,6 +142,43 @@ func GlobWithDoubleStar(pattern, searchPath string, limit int) ([]string, bool,
182 return results, truncated, nil
142 return results, truncated, nil
183}
143}
184
144
145+// ShouldExcludeFile checks if a file should be excluded from processing
146+// based on common patterns and ignore rules
147+func ShouldExcludeFile(rootPath, filePath string) bool {
148+ return NewDirectoryLister(rootPath).
149+ shouldIgnore(filePath, nil)
150+}
151+
152+// WalkDirectories walks a directory tree and calls the provided function for each directory,
153+// respecting hierarchical .gitignore/.crushignore files like git does.
154+func WalkDirectories(rootPath string, fn func(path string, d os.DirEntry, err error) error) error {
155+ dl := NewDirectoryLister(rootPath)
156+
157+ conf := fastwalk.Config{
158+ Follow: true,
159+ ToSlash: fastwalk.DefaultToSlash(),
160+ Sort: fastwalk.SortDirsFirst,
161+ }
162+
163+ return fastwalk.Walk(&conf, rootPath, func(path string, d os.DirEntry, err error) error {
164+ if err != nil {
165+ return fn(path, d, err)
166+ }
167+
168+ // Only process directories
169+ if !d.IsDir() {
170+ return nil
171+ }
172+
173+ // Check if directory should be ignored
174+ if dl.shouldIgnore(path, nil) {
175+ return filepath.SkipDir
176+ }
177+
178+ return fn(path, d, err)
179+ })
180+}
181+
185func PrettyPath(path string) string {
182func PrettyPath(path string) string {
186 return home.Short(path)
183 return home.Short(path)
187}
184}
internal/fsext/ignore_test.go+96
@@ -2,6 +2,7 @@ package fsext
2
2
3import (
3import (
4 "os"
4 "os"
5+ "path/filepath"
5 "testing"
6 "testing"
6
7
7 "github.com/stretchr/testify/require"
8 "github.com/stretchr/testify/require"
@@ -30,3 +31,98 @@ func TestCrushIgnore(t *testing.T) {
30 require.False(t, dl.shouldIgnore("test1.txt", nil), ".txt files should not be ignored")
31 require.False(t, dl.shouldIgnore("test1.txt", nil), ".txt files should not be ignored")
31 require.True(t, dl.shouldIgnore("test3.tmp", nil), ".tmp files should be ignored by common patterns")
32 require.True(t, dl.shouldIgnore("test3.tmp", nil), ".tmp files should be ignored by common patterns")
32}
33}
34+
35+func TestShouldExcludeFile(t *testing.T) {
36+ t.Parallel()
37+
38+ // Create a temporary directory structure for testing
39+ tempDir := t.TempDir()
40+
41+ // Create directories that should be ignored
42+ nodeModules := filepath.Join(tempDir, "node_modules")
43+ target := filepath.Join(tempDir, "target")
44+ customIgnored := filepath.Join(tempDir, "custom_ignored")
45+ normalDir := filepath.Join(tempDir, "src")
46+
47+ for _, dir := range []string{nodeModules, target, customIgnored, normalDir} {
48+ if err := os.MkdirAll(dir, 0o755); err != nil {
49+ t.Fatalf("Failed to create directory %s: %v", dir, err)
50+ }
51+ }
52+
53+ // Create .gitignore file
54+ gitignoreContent := "node_modules/\ntarget/\n"
55+ if err := os.WriteFile(filepath.Join(tempDir, ".gitignore"), []byte(gitignoreContent), 0o644); err != nil {
56+ t.Fatalf("Failed to create .gitignore: %v", err)
57+ }
58+
59+ // Create .crushignore file
60+ crushignoreContent := "custom_ignored/\n"
61+ if err := os.WriteFile(filepath.Join(tempDir, ".crushignore"), []byte(crushignoreContent), 0o644); err != nil {
62+ t.Fatalf("Failed to create .crushignore: %v", err)
63+ }
64+
65+ // Test that ignored directories are properly ignored
66+ require.True(t, ShouldExcludeFile(tempDir, nodeModules), "Expected node_modules to be ignored by .gitignore")
67+ require.True(t, ShouldExcludeFile(tempDir, target), "Expected target to be ignored by .gitignore")
68+ require.True(t, ShouldExcludeFile(tempDir, customIgnored), "Expected custom_ignored to be ignored by .crushignore")
69+
70+ // Test that normal directories are not ignored
71+ require.False(t, ShouldExcludeFile(tempDir, normalDir), "Expected src directory to not be ignored")
72+
73+ // Test that the workspace root itself is not ignored
74+ require.False(t, ShouldExcludeFile(tempDir, tempDir), "Expected workspace root to not be ignored")
75+}
76+
77+func TestShouldExcludeFileHierarchical(t *testing.T) {
78+ t.Parallel()
79+
80+ // Create a nested directory structure for testing hierarchical ignore
81+ tempDir := t.TempDir()
82+
83+ // Create nested directories
84+ subDir := filepath.Join(tempDir, "subdir")
85+ nestedNormal := filepath.Join(subDir, "normal_nested")
86+
87+ for _, dir := range []string{subDir, nestedNormal} {
88+ if err := os.MkdirAll(dir, 0o755); err != nil {
89+ t.Fatalf("Failed to create directory %s: %v", dir, err)
90+ }
91+ }
92+
93+ // Create .crushignore in subdir that ignores normal_nested
94+ subCrushignore := "normal_nested/\n"
95+ if err := os.WriteFile(filepath.Join(subDir, ".crushignore"), []byte(subCrushignore), 0o644); err != nil {
96+ t.Fatalf("Failed to create subdir .crushignore: %v", err)
97+ }
98+
99+ // Test hierarchical ignore behavior - this should work because the .crushignore is in the parent directory
100+ require.True(t, ShouldExcludeFile(tempDir, nestedNormal), "Expected normal_nested to be ignored by subdir .crushignore")
101+ require.False(t, ShouldExcludeFile(tempDir, subDir), "Expected subdir itself to not be ignored")
102+}
103+
104+func TestShouldExcludeFileCommonPatterns(t *testing.T) {
105+ t.Parallel()
106+
107+ tempDir := t.TempDir()
108+
109+ // Create directories that should be ignored by common patterns
110+ commonIgnored := []string{
111+ filepath.Join(tempDir, ".git"),
112+ filepath.Join(tempDir, "node_modules"),
113+ filepath.Join(tempDir, "__pycache__"),
114+ filepath.Join(tempDir, "target"),
115+ filepath.Join(tempDir, ".vscode"),
116+ }
117+
118+ for _, dir := range commonIgnored {
119+ if err := os.MkdirAll(dir, 0o755); err != nil {
120+ t.Fatalf("Failed to create directory %s: %v", dir, err)
121+ }
122+ }
123+
124+ // Test that common patterns are ignored even without explicit ignore files
125+ for _, dir := range commonIgnored {
126+ require.True(t, ShouldExcludeFile(tempDir, dir), "Expected %s to be ignored by common patterns", filepath.Base(dir))
127+ }
128+}
internal/fsext/ls.go+14−3
@@ -141,8 +141,16 @@ func (dl *directoryLister) shouldIgnore(path string, ignorePatterns []string) bo
141 return true
141 return true
142 }
142 }
143
143
144+ parentDir := filepath.Dir(path)
145+ ignoreParser := dl.getIgnore(parentDir)
146+ if ignoreParser.MatchesPath(relPath) {
147+ slog.Debug("ignoring dir pattern", "path", relPath, "dir", parentDir)
148+ return true
149+ }
150+
151+ // For directories, also check with trailing slash (gitignore convention)
144− if dl.getIgnore(filepath.Dir(path)).MatchesPath(relPath) {
152+ if ignoreParser.MatchesPath(relPath + "/") {
145− slog.Debug("ignoring dir pattern", "path", relPath, "dir", filepath.Dir(path))
153+ slog.Debug("ignoring dir pattern with slash", "path", relPath+"/", "dir", parentDir)
146 return true
154 return true
147 }
155 }
148
156
@@ -160,11 +168,14 @@ func (dl *directoryLister) shouldIgnore(path string, ignorePatterns []string) bo
160
168
161func (dl *directoryLister) checkParentIgnores(path string) bool {
169func (dl *directoryLister) checkParentIgnores(path string) bool {
162 parent := filepath.Dir(filepath.Dir(path))
170 parent := filepath.Dir(filepath.Dir(path))
163− for parent != dl.rootPath && parent != "." && path != "." {
171+ for parent != "." && path != "." {
164 if dl.getIgnore(parent).MatchesPath(path) {
172 if dl.getIgnore(parent).MatchesPath(path) {
165 slog.Debug("ingoring parent dir pattern", "path", path, "dir", parent)
173 slog.Debug("ingoring parent dir pattern", "path", path, "dir", parent)
166 return true
174 return true
167 }
175 }
176+ if parent == dl.rootPath {
177+ break
178+ }
168 parent = filepath.Dir(parent)
179 parent = filepath.Dir(parent)
169 }
180 }
170 return false
181 return false
internal/llm/provider/openai_test.go+1−1
@@ -75,7 +75,7 @@ func TestOpenAIClientStreamChoices(t *testing.T) {
75 },
75 },
76 }
76 }
77
77
78− ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
78+ ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
79 defer cancel()
79 defer cancel()
80
80
81 eventsChan := client.stream(ctx, messages, nil)
81 eventsChan := client.stream(ctx, messages, nil)
internal/llm/tools/grep.go+19−7
@@ -279,11 +279,13 @@ func searchWithRipgrep(ctx context.Context, pattern, path, include string) ([]gr
279 return nil, fmt.Errorf("ripgrep not found in $PATH")
279 return nil, fmt.Errorf("ripgrep not found in $PATH")
280 }
280 }
281
281
282− cmd.Args = append(
282+ // Only add ignore files if they exist
283− cmd.Args,
283+ for _, ignoreFile := range []string{".gitignore", ".crushignore"} {
284− "--ignore-file", filepath.Join(path, ".gitignore"),
284+ ignorePath := filepath.Join(path, ignoreFile)
285− "--ignore-file", filepath.Join(path, ".crushignore"),
285+ if _, err := os.Stat(ignorePath); err == nil {
286− )
286+ cmd.Args = append(cmd.Args, "--ignore-file", ignorePath)
287+ }
288+ }
287
289
288 output, err := cmd.Output()
290 output, err := cmd.Output()
289 if err != nil {
291 if err != nil {
@@ -357,14 +359,24 @@ func searchFilesWithRegex(pattern, rootPath, include string) ([]grepMatch, error
357 }
359 }
358
360
359 if info.IsDir() {
361 if info.IsDir() {
360− return nil // Skip directories
362+ // Check if directory should be skipped
363+ if walker.ShouldSkip(path) {
364+ return filepath.SkipDir
365+ }
366+ return nil // Continue into directory
361 }
367 }
362
368
363− // Use walker's shouldSkip method instead of just SkipHidden
369+ // Use walker's shouldSkip method for files
364 if walker.ShouldSkip(path) {
370 if walker.ShouldSkip(path) {
365 return nil
371 return nil
366 }
372 }
367
373
374+ // Skip hidden files (starting with a dot) to match ripgrep's default behavior
375+ base := filepath.Base(path)
376+ if base != "." && strings.HasPrefix(base, ".") {
377+ return nil
378+ }
379+
368 if includePattern != nil && !includePattern.MatchString(path) {
380 if includePattern != nil && !includePattern.MatchString(path) {
369 return nil
381 return nil
370 }
382 }
internal/llm/tools/grep_test.go+35−26
@@ -1,8 +1,6 @@
1package tools
1package tools
2
2
3import (
3import (
4− "context"
5− "encoding/json"
6 "os"
4 "os"
7 "path/filepath"
5 "path/filepath"
8 "regexp"
6 "regexp"
@@ -59,6 +57,7 @@ func TestGlobToRegexCaching(t *testing.T) {
59}
57}
60
58
61func TestGrepWithIgnoreFiles(t *testing.T) {
59func TestGrepWithIgnoreFiles(t *testing.T) {
60+ t.Parallel()
62 tempDir := t.TempDir()
61 tempDir := t.TempDir()
63
62
64 // Create test files
63 // Create test files
@@ -84,32 +83,42 @@ func TestGrepWithIgnoreFiles(t *testing.T) {
84 crushignoreContent := "node_modules/\n"
83 crushignoreContent := "node_modules/\n"
85 require.NoError(t, os.WriteFile(filepath.Join(tempDir, ".crushignore"), []byte(crushignoreContent), 0o644))
84 require.NoError(t, os.WriteFile(filepath.Join(tempDir, ".crushignore"), []byte(crushignoreContent), 0o644))
86
85
87− // Create grep tool
86+ // Test both implementations
88− grepTool := NewGrepTool(tempDir)
87+ for name, fn := range map[string]func(pattern, path, include string) ([]grepMatch, error){
88+ "regex": searchFilesWithRegex,
89+ "rg": func(pattern, path, include string) ([]grepMatch, error) {
90+ return searchWithRipgrep(t.Context(), pattern, path, include)
91+ },
92+ } {
93+ t.Run(name, func(t *testing.T) {
94+ t.Parallel()
95+
96+ if name == "rg" && getRg() == "" {
97+ t.Skip("rg is not in $PATH")
98+ }
99+
100+ matches, err := fn("hello world", tempDir, "")
101+ require.NoError(t, err)
89
102
90− // Create grep parameters
103+ // Convert matches to a set of file paths for easier testing
91− params := GrepParams{
104+ foundFiles := make(map[string]bool)
92− Pattern: "hello world",
105+ for _, match := range matches {
93− Path: tempDir,
106+ foundFiles[filepath.Base(match.path)] = true
107+ }
108+
109+ // Should find file1.txt and file2.txt
110+ require.True(t, foundFiles["file1.txt"], "Should find file1.txt")
111+ require.True(t, foundFiles["file2.txt"], "Should find file2.txt")
112+
113+ // Should NOT find ignored files
114+ require.False(t, foundFiles["file3.txt"], "Should not find file3.txt (ignored by .gitignore)")
115+ require.False(t, foundFiles["lib.js"], "Should not find lib.js (ignored by .crushignore)")
116+ require.False(t, foundFiles["secret.key"], "Should not find secret.key (ignored by .gitignore)")
117+
118+ // Should find exactly 2 matches
119+ require.Equal(t, 2, len(matches), "Should find exactly 2 matches")
120+ })
94 }
121 }
95− paramsJSON, err := json.Marshal(params)
96− require.NoError(t, err)
97−
98− // Run grep
99− call := ToolCall{Input: string(paramsJSON)}
100− response, err := grepTool.Run(context.Background(), call)
101− require.NoError(t, err)
102−
103− // Check results - should only find file1.txt and file2.txt
104− // ignored/file3.txt should be ignored by .gitignore
105− // node_modules/lib.js should be ignored by .crushignore
106− // secret.key should be ignored by .gitignore
107− result := response.Content
108− require.Contains(t, result, "file1.txt")
109− require.Contains(t, result, "file2.txt")
110− require.NotContains(t, result, "file3.txt")
111− require.NotContains(t, result, "lib.js")
112− require.NotContains(t, result, "secret.key")
113}
122}
114
123
115func TestSearchImplementations(t *testing.T) {
124func TestSearchImplementations(t *testing.T) {
internal/shell/command_block_test.go+1−2
@@ -1,7 +1,6 @@
1package shell
1package shell
2
2
3import (
3import (
4− "context"
5 "strings"
4 "strings"
6 "testing"
5 "testing"
7
6
@@ -92,7 +91,7 @@ func TestCommandBlocking(t *testing.T) {
92 BlockFuncs: tt.blockFuncs,
91 BlockFuncs: tt.blockFuncs,
93 })
92 })
94
93
95− _, _, err := shell.Exec(context.Background(), tt.command)
94+ _, _, err := shell.Exec(t.Context(), tt.command)
96
95
97 if tt.shouldBlock {
96 if tt.shouldBlock {
98 if err == nil {
97 if err == nil {
internal/shell/shell_test.go+2−2
@@ -16,7 +16,7 @@ func BenchmarkShellQuickCommands(b *testing.B) {
16 b.ReportAllocs()
16 b.ReportAllocs()
17
17
18 for b.Loop() {
18 for b.Loop() {
19− _, _, err := shell.Exec(context.Background(), "echo test")
19+ _, _, err := shell.Exec(b.Context(), "echo test")
20 exitCode := ExitCode(err)
20 exitCode := ExitCode(err)
21 if err != nil || exitCode != 0 {
21 if err != nil || exitCode != 0 {
22 b.Fatalf("Command failed: %v, exit code: %d", err, exitCode)
22 b.Fatalf("Command failed: %v, exit code: %d", err, exitCode)
@@ -100,7 +100,7 @@ func TestRunContinuity(t *testing.T) {
100
100
101func TestCrossPlatformExecution(t *testing.T) {
101func TestCrossPlatformExecution(t *testing.T) {
102 shell := NewShell(&Options{WorkingDir: "."})
102 shell := NewShell(&Options{WorkingDir: "."})
103− ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
103+ ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
104 defer cancel()
104 defer cancel()
105
105
106 // Test a simple command that should work on all platforms
106 // Test a simple command that should work on all platforms