feat: add support for exclusion patterns with '!' prefix#2106
feat: add support for exclusion patterns with '!' prefix#2106jordikroon wants to merge 2 commits into
Conversation
|
I wonder if it would make more sense to stick to the bash/zsh syntax and do exclusions via |
|
In my honest opinion the (ext)glob syntax is the most superior but the drawback is could also be very complicated. Curious to what @dunglas thinks, as he is the initial proposer of this feature. |
|
IMO it's hard to make a pattern matching implementation that's very fast. If speed isn't critical, you can probably just do something similar to boilerplate (haven't tried): // check for the following syntax: /path/!(vendor,storage)/
func matchWithExclusions(pattern string, fileName string) bool {
inclusionAndExclusions := expandExclusions(pattern)
inclusion := inclusionAndExclusions[0]
exclusions := inclusionAndExclusions[1:]
for _, exclusion := range exclusions {
if matchPattern(exclusion, fileName) {
return false
}
}
return matchPattern(inclusion, fileName)
}
// !(dir1|dir2)/path -> []string{"*/path", "dir1/path", "dir2/path"}
func expandExclusions(s string) []string {
before, rest, found := strings.Cut(s, "!(")
if !found {
return []string{s}
}
inside, after, found := strings.Cut(rest, ")")
if !found {
return []string{s} // no closing ')'
}
out := expandExclusions(before+"*"+after)
for _, subPattern := range strings.Split(inside, "|") {
out = append(out, expandExclusions(before+subPattern+after)...)
}
return out
} |
|
Very silly idea... but couldn't we just offer a regex mode instead? |
|
I don't think a regex mode would be achievable without a BC break. Unless we add an extra config option |
|
We could arbitrarily decide on a pattern like |
|
Yeah we could also just allow something like this: watch "regex:^/app/(?!storage/|vendor/)[^/]+(?:/.*)?\.php$"Just would be nice to have a common enough syntax, so long-term the pattern matching could also be moved to the watcher dependency itself. |
|
I like the idea. I will wait a couple of days to give others the opportunity to give their comments then I will implement if nobody objects. |
|
Why not use a library? https://github.com/gobwas/glob? |
|
I'm not opposed to it either, but I think the biggest issue with the current implementation of the globs plus the proposed exclusion pattern is standardization. With regexes everyone know their syntax (bar a few dialect specific details). I don't know how fast or slow regexes are in Go, though. After profiling our C++ regexes once I nearly had a heart attack. |
|
I would also prefer switching to a popular library like https://github.com/gobwas/glob. Supporting regex could be nice, but it's a bit late, I would prefer having only one way to do the thing. |
|
See also gobwas/glob#67 |
|
The library doesn't seem that widespread to me and saw its last commit a decade ago.
It will only ever get later - I'm typically very happy with using existing libraries, but if they aren't actively maintained and don't support what we need, it's better to switch to a standard approach now, rather than later. |
|
There is https://github.com/bmatcuk/doublestar, that looks popular and maintained. |
|
Looks good, but doesn't support exclusions. :/ |
|
e-dant/watcher#103 |
Add exclusion pattern support to allow users to exclude specific paths from triggering reloads. Patterns prefixed with '!' are now parsed as exclusions and stored separately. The globalWatcher checks if events match any exclusion patterns before processing them, preventing excluded paths from triggering reloads while still allowing them to be matched for filtering purposes. - Add isExclude field to track exclusion patterns - Parse '!' prefix during pattern initialization - Rename allowReload to matchesEvent for clarity on matching logic - Extract exclusion check into separate allowReload wrapper - Add isExcludedEvent method to globalWatcher for event filtering - Filter excluded events before debouncing in event listener
Add test case for worker watch configuration with exclude patterns. Verifies that exclude patterns (prefixed with !) are correctly parsed and stored alongside include patterns in the watch configuration.
There was a problem hiding this comment.
Pull request overview
Adds support for “negative” watch patterns (prefixed with !) so users can exclude paths (e.g. vendor/, .git/) from triggering hot reloads, addressing #2095 by filtering out excluded filesystem events before reload debouncing and callback execution.
Changes:
- Parse
!-prefixed patterns as exclusions (pattern.isExclude) and separate matching logic (matchesEvent) from reload-triggering logic (allowReload). - Track exclusions per
PatternGroupand filter excluded events inglobalWatcher.listenForFileEvents()viaisExcludedEvent(). - Add unit tests covering exclusion matching and Caddyfile config acceptance for
watch !...patterns.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/watcher/watcher.go | Adds exclusion storage and filters excluded events before debouncing/reload processing. |
| internal/watcher/pattern.go | Parses ! exclusions and separates “match” vs “trigger reload” logic. |
| internal/watcher/pattern_test.go | Adds tests for exclusion matching (including AssociatedPathName). |
| caddy/config_test.go | Adds config test ensuring watch !... patterns are accepted and preserved. |
Comments suppressed due to low confidence (1)
internal/watcher/watcher.go:157
- Exclude patterns are still started as OS watchers in startWatching(), even though pattern.allowReload() always returns false for isExclude patterns and they never emit events. This adds unnecessary watcher sessions/file descriptors and can worsen performance (e.g., vendor/), without affecting exclusion filtering (which happens in globalWatcher.isExcludedEvent). Skip startSession for exclude patterns.
for _, w := range g.watchers {
w.events = g.events
w.startSession()
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func (g *globalWatcher) stopWatching() { | ||
| close(g.stop) | ||
| for _, w := range g.watchers { | ||
| w.stop() | ||
| } |
| // detect exclusion before resolving to absolute | ||
| raw := strings.TrimSpace(p.value) | ||
| if strings.HasPrefix(raw, "!") { | ||
| p.isExclude = true | ||
| raw = strings.TrimSpace(strings.TrimPrefix(raw, "!")) | ||
| } else { | ||
| p.isExclude = false | ||
| } |
Resolve #2095
Add exclusion pattern support to allow users to exclude specific paths from triggering reloads. Patterns prefixed with '!' are now parsed as exclusions and stored separately. The globalWatcher checks if events match any exclusion patterns before processing them, preventing excluded paths from triggering reloads while still allowing them to be matched for filtering purposes.