feat: Add .stashignore support for gitignore-style scan exclusions #6485
Conversation
|
Why not extend or interface with it from the existing ignores menu? |
Different mental models:
The And finally, glob syntax (as opposed to regex) is a familiar convention: We also wouldn't want to try and "sync" the |
WithoutPants
left a comment
WithoutPants
left a comment
There was a problem hiding this comment.
Static code review only
pkg/file/stashignore.go
Outdated
| // Always accept .stashignore files themselves so they can be read. | ||
| if filepath.Base(path) == stashIgnoreFilename { | ||
| return true | ||
| } |
There was a problem hiding this comment.
Is this true? I don't think we actually need to accept the files themselves.
There was a problem hiding this comment.
Good catch, we load directly from the FS, not via our scanner, so this exception isn't needed. Removed.
| patterns, err := f.loadIgnoreFile(stashIgnorePath) | ||
| if err != nil || patterns == nil { | ||
| // Cache negative result (file doesn't exist or has no patterns). | ||
| f.cache.Store(dir, &ignoreEntry{patterns: nil, dir: dir}) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Shouldn't we log something if the file fails to be loaded?
There was a problem hiding this comment.
Now logging unexpected file system errors e.g. couldn't open file. "Expected" errors like "file doesn't exist" are ignored.
| // collectIgnoreEntries gathers all ignore entries from library root to the given directory. | ||
| // It walks up the directory tree from dir to libraryRoot and returns entries in order | ||
| // from root to most specific. | ||
| func (f *StashIgnoreFilter) collectIgnoreEntries(dir string, libraryRoot string) []*ignoreEntry { |
There was a problem hiding this comment.
Having to do this for every scanned file is likely to cause some non-trivial overhead. We'd perhaps get some benefit from caching the ignoreEntry list for a given directory path. It needn't be exhaustive - a LRU cache would probably do and would save calculating everything, it could be used to shortcut the calculation for sub-directories as well.
There was a problem hiding this comment.
Great point, I've implemented both optimisations:
- LRU cache for collected entries keyed by dir + libraryRoot. This means files in the same directory will avoid re-walking the tree.
- If parent's entries are cached, we just check if the child dir has a
.stashignoreand extend (if it has one), avoiding walking the tree.
All existing tests pass, but I didn't add tests specifically for the optimisations. Is that ok? if not, how might I approach those tests?
3 participants
Adds support for
.stashignorefiles that allow users to exclude files and directories from scanning using gitignore-style patterns. Closes #1139.Some LLM usage as Go is not my preferred language.
Changes
StashIgnoreFilterinpkg/file/stashignore.gothat checks for.stashignorefiles during scansscanFilteralongside existing exclusion logic. The scanner doesn't appear to expose plugin hooks for file filtering..stashignorefiles (patterns cascade from library root to subdirectories)Tasks.mdUsage
Place a
.stashignorefile in any directory within your library:Test Plan