feat(review): collapse and expand files like a GitHub diff#493
Open
narthur wants to merge 5 commits into
Open
Conversation
Long agent changesets force you to scroll past files you've already read. This adds GitHub-style per-file collapse: `x` collapses the selected file to its header, `Shift+X` collapses or expands every file, and the chevron in each file header toggles it by mouse. Collapse is a pure derivation: a collapsed file is swapped for a zero-hunk placeholder variant in buildReviewState, so the existing empty-file render, geometry, and hunk-cursor paths handle it with no new renderer flags. State is session-only and reconciled across watch reloads alongside gap expansion, so there is no on-disk persistence layer to gate by repo root or validate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
PR author is not in the allowed authors list. |
Back collapsed-file state with a fileId->true Record instead of a Set so it reconciles through the existing removeKeys stale-id pruning, deleting the parallel pruneCollapsedFileIds helper. Extract anchorFileHeaderTop so a bulk Shift+X collapse re-pins the selected file's header like the single-file toggle. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Guard the two behaviors the collapse refactor changed: bulk Shift+X re-pins the selected file's header (not just single-file x), and collapse state keyed by an old patch is pruned through removeKeys when a reload swaps the file's fetcher. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add the two coverage gaps the review flagged on the feature: Shift-X must collapse/expand every file (distinguished from single-file x by asserting the second file also collapses), and the header chevron toggles collapse while its stopPropagation keeps a chevron click from also firing the header select. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…face The collapse map is internal: the UI reads collapse state through the swapped-in zero-hunk variant (isCollapsed), never the id map. Keep it as private state and assert collapse in tests through the observable visibleFiles stream instead of widening the public ReviewController contract for tests alone. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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 agent changesets mix files you still need to read with ones you've already reviewed, and there was no way to set a file aside while scrolling the stream. This adds GitHub-style per-file collapse so you can fold away what you're done with.
This is the manual, user-driven counterpart to the prior collapse attempts, designed to sidestep the feedback they ran into:
vmark-reviewed, per-repo marker store) — reviewers flagged disk persistence written for non-repo inputs, path traversal on the hash filename, and synchronous disk I/O blocking the first render, plus a "what's the prior art forv?" question. Avoided here: collapse is session-only in-memory state (reconciled across watch reloads exactly like the existing gap-expansion state), so there is no marker store, repo-root gating, hash validation, or TTL to get wrong.mhide files) — different intent (hide vs. collapse); its one note was a click-fallthrough guard, which we mirror by stopping propagation on the chevron handler.User-visible
xcollapses/expands the selected file.Shift+Xcollapses or expands every file.▾/▸) in any file header toggles that file; clicking the rest of the header still selects/jumps as before.Collapse state is session-only and resets on relaunch (it survives watch-mode reloads).
What it does
buildReviewState(src/ui/lib/fileCollapse.ts), so the existing empty-file render (PierreDiffView), geometry (bodyHeight: 1), and hunk-cursor paths handle it with no new renderer flags or row kinds. Hunk navigation ([/]) naturally skips collapsed files because they expose no hunks.useReviewControllerowns acollapsedFileIdsset withtoggleFileCollapsed/toggleAllFilesCollapsed, reconciled in the samefilesSnapshotblock as gap expansion. Toggling re-pins the file header to the top so a height change can't scroll it out of view.WeakMapso its object identity stays stable across renders (the geometry cache keys on theDiffFileobject).Tests
src/ui/lib/fileCollapse.test.ts— variant swap empties hunks while preserving stats/identity, returns a stable object, swaps only collapsed ids, and prunes ids that left the changeset.src/ui/hooks/useReviewController.test.tsx— toggling collapses to a zero-hunk variant, skips hunk navigation, expands back, and collapse-all/expand-all round-trip.test/pty/collapse.test.ts— real PTY:xcollapses the file to the placeholder (with the▸chevron) andxagain restores the diff.AppHost.interactions,ui-components(Help dialog), andui-lib(menus) for the chevron and new entries.bun run typecheck,bun run lint, andbun run test:tty-smokeclean.🤖 Generated with Claude Code