Add --include prefix filter flag#103
Conversation
Extract matchesPrefix() and normalizePrefixes() from ExcludeFilter into shared prefix.go helpers. Create IncludeFilter that keeps only files matching given prefixes, mirroring ExcludeFilter's decorator pattern.
Add include []string parameter to makeGitRenderer, makeHgRenderer, and makeNoVCSRenderer. IncludeFilter wraps before ExcludeFilter so include narrows first, then exclude refines. Update setupVCSRenderer to pass opts.Include. Add tests for include wrapping, all-files+include, and functional include+exclude composition.
- Add diff.IncludeFilter to Renderer implementors list in CLAUDE.md - Update gotcha bullet with --include filtering and --stdin mutual exclusivity - Add --include example to Pi plugin SKILL.md
There was a problem hiding this comment.
Pull request overview
Adds a new --include (-I) prefix filter to complement existing --exclude (-X), wiring it through CLI parsing/config/env support and applying it as a renderer decorator (include first, then exclude).
Changes:
- Introduces shared prefix helper utilities (
normalizePrefixes,matchesPrefix) and refactorsExcludeFilterto use them. - Adds
IncludeFilterdecorator and wires--includethrough CLI parsing, validation, and renderer factories (git/hg/no-vcs). - Expands unit tests and updates user-facing docs/references to describe
--includebehavior and interactions.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| site/index.html | Updates marketing copy to mention --include alongside --exclude. |
| site/docs.html | Adds examples/options text for --include and updates mutual exclusivity notes. |
| plugins/pi/skills/revdiff/SKILL.md | Adds an --include example to pi skill docs. |
| plugins/codex/skills/revdiff/references/usage.md | Documents --include usage/examples and composition with --exclude. |
| plugins/codex/skills/revdiff/references/config.md | Documents --include config/env support. |
| app/main_test.go | Adds CLI parsing/validation tests and renderer composition tests for include+exclude. |
| app/main.go | Adds Include option/flag, validates conflicts, and wraps renderers with IncludeFilter before ExcludeFilter. |
| app/diff/prefix.go | New shared prefix normalization/matching helpers. |
| app/diff/include_test.go | New unit tests for IncludeFilter and shared prefix helpers. |
| app/diff/include.go | New IncludeFilter renderer decorator implementation. |
| app/diff/exclude_test.go | Refactors tests to use shared prefix helpers. |
| app/diff/exclude.go | Refactors ExcludeFilter to use shared prefix helpers and removes duplicated logic. |
| README.md | Documents --include option, examples, and interactions with other modes. |
| CLAUDE.md | Updates project documentation to mention IncludeFilter and new flag behavior. |
| .claude-plugin/skills/revdiff/references/usage.md | Mirrors --include usage docs in claude plugin references. |
| .claude-plugin/skills/revdiff/references/config.md | Mirrors --include config/env docs in claude plugin references. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func (f *IncludeFilter) ChangedFiles(ref string, staged bool) ([]FileEntry, error) { | ||
| entries, err := f.inner.ChangedFiles(ref, staged) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("include filter, changed files: %w", err) | ||
| } | ||
|
|
||
| filtered := make([]FileEntry, 0, len(entries)) | ||
| for _, e := range entries { | ||
| if matchesPrefix(e.Path, f.prefixes) { | ||
| filtered = append(filtered, e) | ||
| } | ||
| } | ||
| return filtered, nil |
There was a problem hiding this comment.
If all provided include prefixes normalize to empty (e.g., env REVDIFF_INCLUDE=, or values containing only whitespace), f.prefixes becomes empty and matchesPrefix will always return false, causing ChangedFiles to return an empty list (effectively hiding all files). Consider treating an empty normalized prefix list as a no-op (return entries unchanged) or returning a clear validation error when no non-empty prefixes remain. Adding a unit test for this case would prevent regressions.
umputun
left a comment
There was a problem hiding this comment.
lgtm overall, clean implementation that mirrors ExcludeFilter well. tests pass, linter clean, no races.
couple things:
-
prefix.go- I'd prefer movingmatchesPrefixandnormalizePrefixesintodiff.goinstead of a separate file. two small functions shared by both filters don't justify a new file. tests for them should move todiff_test.goaccordingly -
duplicate tests -
TestMatchesPrefixandTestNormalizePrefixesexist in bothinclude_test.goandexclude_test.go. move them todiff_test.go(with the functions) and drop both duplicates -
minor:
makeNoVCSRendereracceptsincludeparam but it's unreachable - no-VCS mode requires--only, and--include+--onlyis rejected byparseArgs. not a blocker,--excludehas the same pattern, but worth knowing
the --include / --only mutual exclusivity makes sense, they have different semantics (prefix filter vs exact/suffix file selection)
Move matchesPrefix and normalizePrefixes to the bottom of diff.go and their tests to diff_test.go. Drop duplicate tests from include_test.go and exclude_test.go. Delete prefix.go.
No-VCS mode requires --only, which is mutually exclusive with --include. --exclude on FileReader (which only returns --only files) is a no-op. Remove both filter params and simplify to direct FileReader return.
|
All issues addressed:
|
umputun
left a comment
There was a problem hiding this comment.
lgtm, all review points addressed. thx for the cleanup on makeNoVCSRenderer - removing both unreachable params is cleaner than what I suggested.
one non-blocking note from Copilot: if all include prefixes normalize to empty (e.g. REVDIFF_INCLUDE=,), matchesPrefix returns false for everything and all files silently disappear. unlikely edge case, but --exclude is harmless in the same scenario (nothing excluded = no-op) while --include is destructive (nothing included = empty list). something to keep in mind for later, not a blocker.
When all --include prefixes normalize to empty (e.g. REVDIFF_INCLUDE=,), treat as no-op instead of silently dropping all files. Also remove unreachable include/exclude params from makeNoVCSRenderer since no-VCS mode requires --only which is mutually exclusive with --include.
|
fix: handle empty normalized include prefixes as no-op |
Add
--include(-I) flag for prefix-based file inclusion, complementing the existing--exclude(-X). Both flags compose: include narrows the file set first, then exclude removes from it.Changes
Core filter (
app/diff/)matchesPrefixandnormalizePrefixeshelpers intoprefix.go, refactorExcludeFilterto use themIncludeFilterininclude.go— same decorator pattern, inverted logic (keep matching files)CLI wiring (
app/main.go)--include/-Iflag with config file (ini-name), env var (REVDIFF_INCLUDE), and comma-delimited support--include+--onlyand--stdin+--includeare errorsTests (
app/diff/include_test.go,app/main_test.go)Docs