fix(ui): match --only patterns with ./ prefix and absolute entries#128
fix(ui): match --only patterns with ./ prefix and absolute entries#128
Conversation
filterOnly only resolved patterns against workDir when the pattern was absolute, so `./CLAUDE.md` silently dropped matching files in both git mode (repo-relative entries) and no-VCS mode (absolute entries from FileReader). Normalize both entry and pattern to workDir-relative form before comparing, mirroring FallbackRenderer.pathMatches. Related to #127
There was a problem hiding this comment.
Pull request overview
Fixes -F/--only path matching in the UI loader so patterns with ./ prefixes and absolute FileReader entries match consistently by normalizing both entries and patterns to workDir-relative paths (mirroring the fallback renderer logic). This addresses Issue #127 where revdiff -F ./CLAUDE.md could yield an empty file list.
Changes:
- Refactors
Model.filterOnlyto use a new helper (entryMatchesOnly) that normalizes entries/patterns relative toworkDirbefore matching. - Adds
workDirRelhelper to safely convert absolute/relative inputs intoworkDir-relative paths. - Adds test cases covering
./-prefixed patterns, absolute entries, and escaping..patterns.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| app/ui/loaders.go | Adds workDir-relative normalization for --only matching via new helper methods. |
| app/ui/loaders_test.go | Adds regression tests for ./-prefixed --only patterns and absolute-entry matching. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var filtered []diff.FileEntry | ||
| for _, e := range entries { | ||
| for _, pattern := range m.cfg.only { | ||
| if e.Path == pattern || strings.HasSuffix(e.Path, "/"+pattern) { | ||
| if m.entryMatchesOnly(e.Path, pattern) { | ||
| filtered = append(filtered, e) | ||
| break | ||
| } | ||
| // resolve absolute pattern relative to workDir for matching against repo-relative files | ||
| if m.cfg.workDir != "" && filepath.IsAbs(pattern) { | ||
| rel, err := filepath.Rel(m.cfg.workDir, pattern) | ||
| if err == nil && !strings.HasPrefix(rel, "..") && (e.Path == rel || strings.HasSuffix(e.Path, "/"+rel)) { | ||
| filtered = append(filtered, e) | ||
| break | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
filterOnly calls entryMatchesOnly for every (entry, pattern) pair, and entryMatchesOnly recomputes workDirRel(entry) each time even though entry is constant across the inner loop. Consider precomputing the normalized entry once per entry (and possibly normalizing patterns once upfront) to avoid repeated filepath.Rel work on large file lists.
| // both entries and patterns are also normalized to workDir-relative form for matching, | ||
| // so "./CLAUDE.md" matches "CLAUDE.md" and an absolute entry from FileReader | ||
| // ("/repo/CLAUDE.md") matches a relative pattern ("CLAUDE.md"). |
There was a problem hiding this comment.
The filterOnly doc comment says entries/patterns are normalized to workDir-relative form for matching, but the implementation only does that when cfg.workDir is non-empty (otherwise it falls back to exact/suffix only). Consider clarifying this in the comment so behavior is accurate when workDir is unset.
| // both entries and patterns are also normalized to workDir-relative form for matching, | |
| // so "./CLAUDE.md" matches "CLAUDE.md" and an absolute entry from FileReader | |
| // ("/repo/CLAUDE.md") matches a relative pattern ("CLAUDE.md"). | |
| // when workDir is set, entries and patterns are also normalized to workDir-relative | |
| // form for matching, so "./CLAUDE.md" matches "CLAUDE.md" and an absolute entry | |
| // from FileReader ("/repo/CLAUDE.md") matches a relative pattern ("CLAUDE.md"). | |
| // when workDir is unset, matching falls back to exact-path or suffix checks only. |
| path = filepath.Join(m.cfg.workDir, path) | ||
| } | ||
| rel, err := filepath.Rel(m.cfg.workDir, path) | ||
| if err != nil || strings.HasPrefix(rel, "..") { |
There was a problem hiding this comment.
workDirRel uses strings.HasPrefix(rel, "..") to detect paths outside workDir. This incorrectly treats valid in-repo paths like "..foo" (a filename starting with "..") as escaping. Prefer filepath.IsLocal(rel) (or rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator))) to accurately detect traversal outside the workDir.
| if err != nil || strings.HasPrefix(rel, "..") { | |
| if err != nil || !filepath.IsLocal(rel) { |
workDir-relative normalization only applies when cfg.workDir is set; when empty, matching falls back to exact/suffix only. Make that explicit in the comment.
Deploying revdiff with
|
| Latest commit: |
aacfc68
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://654845bf.revdiff.pages.dev |
| Branch Preview URL: | https://fix-only-dot-prefix.revdiff.pages.dev |
-F/--onlydropped matching files when the pattern started with./.filterOnlyonly resolved againstworkDirfor absolute patterns, so:CLAUDE.mdvs pattern./CLAUDE.md- no matchFileReaderreturns absolute paths, entry/repo/CLAUDE.mdvs pattern./CLAUDE.md- no matchNormalize both entry and pattern to
workDir-relative form before comparing, mirroringFallbackRenderer.pathMatches. Preserves existing exact/suffix fast path.Fixes #127