feat(ui): mark files to hide them from the review stream#277
Open
aldevv wants to merge 1 commit intomodem-dev:mainfrom
Open
feat(ui): mark files to hide them from the review stream#277aldevv wants to merge 1 commit intomodem-dev:mainfrom
aldevv wants to merge 1 commit intomodem-dev:mainfrom
Conversation
Adds m/M keybindings to mark/unmark files. Marked files disappear from the diff stream and stay in the sidebar dimmed and crossed out, with a "<n> hidden" footer hint. Visibility derivation moves into buildReviewState so subsequent filter and search features can layer on.
Contributor
Greptile SummaryThis PR adds a "mark to hide" feature: pressing
Confidence Score: 4/5Safe to merge; the core mark/unmark flow, visibility derivation, and re-selection logic are all correct and well-tested. The implementation is solid end-to-end with thorough unit and PTY test coverage. The only non-blocking concern is that src/ui/components/panes/SidebarPane.tsx — the Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["User presses m"] --> B["toggleSelectedFileMark()"]
B --> C["toggleMarkedFile(fileId) in useReviewController"]
C --> D["setMarkedFileIds — new Set with/without fileId"]
D --> E["useMemo: buildReviewState re-runs"]
E --> F["allFiles.filter(!markedFileIds.has) → unmarkedFiles"]
F --> G["filterReviewFiles(unmarkedFiles, query) → visibleFiles"]
E --> H["filterReviewFiles(allFiles, query) → sidebarFiles"]
H --> I["buildSidebarEntries(sidebarFiles, markedFileIds) → sidebarEntries"]
G --> K["resolveSelectedFile → selectedFile"]
K --> L{selectedFile in visibleFiles?}
L -- No --> M["reconcileSelectedFile useEffect → reselectFirstVisibleFile"]
L -- Yes --> N["Selection unchanged"]
E --> O["hiddenByMarkCount = allFiles.length - unmarkedFiles.length"]
O --> P["SidebarPane footer: N hidden"]
I --> Q["FileListItem: STRIKETHROUGH + muted colour if marked"]
Q --> R{User clicks marked row}
R -- onUnmarkFile provided --> S["onUnmarkFile(id) → file restored"]
R -- onUnmarkFile absent --> T["onSelectFile(id) — navigates to hidden file"]
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
src/ui/components/panes/SidebarPane.tsx:75-83
**Silent fallthrough to `onSelectFile` for marked rows**
When `entry.marked` is true but `onUnmarkFile` is absent, the click falls through to `onSelectFile(entry.id)`, which calls `jumpToFile` on a file that has been intentionally removed from the review stream. The resulting `selectFile` call sets `selectedFileId` to the hidden file, and `resolveSelectedFile` then surfaces it via the `allFiles` fallback — displaying a file the user just asked to hide. A no-op guard when `onUnmarkFile` is absent is safer than silently treating the click as a normal selection.
Reviews (1): Last reviewed commit: "feat(ui): mark files to hide them from t..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+75
to
+83
| onSelect={() => { | ||
| // Clicking a marked row should bring the file back rather than re-select a | ||
| // hidden file in the diff stream. | ||
| if (entry.marked && onUnmarkFile) { | ||
| onUnmarkFile(entry.id); | ||
| return; | ||
| } | ||
| onSelectFile(entry.id); | ||
| }} |
Contributor
There was a problem hiding this comment.
onSelectFile for marked rows
When entry.marked is true but onUnmarkFile is absent, the click falls through to onSelectFile(entry.id), which calls jumpToFile on a file that has been intentionally removed from the review stream. The resulting selectFile call sets selectedFileId to the hidden file, and resolveSelectedFile then surfaces it via the allFiles fallback — displaying a file the user just asked to hide. A no-op guard when onUnmarkFile is absent is safer than silently treating the click as a normal selection.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ui/components/panes/SidebarPane.tsx
Line: 75-83
Comment:
**Silent fallthrough to `onSelectFile` for marked rows**
When `entry.marked` is true but `onUnmarkFile` is absent, the click falls through to `onSelectFile(entry.id)`, which calls `jumpToFile` on a file that has been intentionally removed from the review stream. The resulting `selectFile` call sets `selectedFileId` to the hidden file, and `resolveSelectedFile` then surfaces it via the `allFiles` fallback — displaying a file the user just asked to hide. A no-op guard when `onUnmarkFile` is absent is safer than silently treating the click as a normal selection.
How can I resolve this? If you propose a fix, please make it concise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Long changesets mix real changes with noise (lockfiles, snapshots, vendored fixtures), and once you've reviewed a file there was no way to set it aside. You scroll past it every time and lose track of what you've already looked at.
User-visible
mmarks the focused file;M(shift+m) clears every active mark. Marked files disappear from the diff stream but stay in the sidebar dimmed and crossed out, with a<n> hiddenfooter hint reminding you how many are tucked away. Clicking a marked sidebar row unmarks it instead of trying to jump to a file that isn't there.The legacy hunk-metadata toggle moved off
m(nowH) so the mark binding lives in the most reachable spot.What it does
useReviewControllerowns themarkedFileIdsset and exposestoggleMarkedFile,clearMarkedFiles,hiddenByMarkCount, and anunmarkedFileslist alongside the existingallFiles/visibleFiles.buildReviewState, so the review stream, sidebar entries, and stats all read from one source. Subsequent filter and search features can layer on top ofunmarkedFileswithout re-deriving visibility ad hoc.buildSidebarEntriescarries amarkedflag per entry.FileListItemreads it to dim the icon/name and applySTRIKETHROUGH, so marked rows stay visible but obviously inert.SidebarPanerenders a 1-row footer (<n> hidden) when any file is marked. Clicking a marked row routes through a newonUnmarkFileprop instead ofonSelectFile.appMenusgainMark file (m)/Clear marks (Shift+M)entries; the menu hook wires the keyboard shortcuts to the focused file.Tests
bun test src/ui/lib/reviewState.test.ts— marked ids drop files fromunmarkedFiles/visibleFiles, surface inhiddenByMarkCount, and the sidebar entries keep the marked file withmarked: true.bun test src/ui/lib/files.test.ts—buildSidebarEntriesflags marked rows and tolerates unknown ids.bun test src/ui/hooks/useReviewController.test.tsx—toggleMarkedFileround-trips,clearMarkedFilesis a no-op when empty, and the controller'shiddenByMarkCount/unmarkedFilestrack the set.bun test test/pty/marked-files-integration.test.ts— new PTY test:mremoves the focused file from the stream, the sidebar footer reports1 hidden, andMrestores it.bun test src/ui/AppHost.interactions.test.tsx,src/ui/components/ui-components.test.tsx,src/ui/lib/ui-lib.test.ts— updated for the newHbinding, sidebar footer rendering, and menu wiring.bun run typecheckandbun run lintclean.