Skip to content

feat(ui): hide files already marked reviewed with H - #596

Merged
agavra merged 5 commits into
agavra:mainfrom
patrickjmccauley:feat/filter-out-reviewed-files
Aug 18, 2026
Merged

agavra merged 5 commits into
agavra:mainfrom
patrickjmccauley:feat/filter-out-reviewed-files

Conversation

@patrickjmccauley

@patrickjmccauley patrickjmccauley commented Aug 12, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

This is a bit like the Github native review screen's filter allowing users to hide already-viewed files from the UI/file explorer sidebar.

Screenshot 2026-08-12 at 1 19 27 PM

On a large review, files already marked reviewed with r keep their row in the file tree and their header row in the diff. The body collapses, but the rows stay — so the list never shrinks and the remaining work is buried in it. This adds a way to see only what is left.

What this adds

Surface Effect
H (file tree) Toggle files already marked reviewed
:set noreviewed / :set reviewed / :set reviewed! Hide / show / toggle from any pane
:reviewed Bare alias for the toggle, like :wrap
show_reviewed = false Start a session with them hidden (defaults to true)

H rather than h, because h/l pan the tree horizontally — that is how long nested paths are read in a narrow panel, and taking h would have left panning one-way. H was unbound everywhere. It is file-tree only, so the diff pane leaves it alone; :set reviewed! is the way to toggle from there.

The commands mirror :set commits / :set nocommits / :set commits! rather than inventing a shape, since that is the same kind of toggle — which files appear in the tree and the diff.

While hiding, r becomes a burn-down loop: marking the file you are reading moves you to the next unreviewed file, wrapping at the end, and parks at the overview when nothing is left.

Implementation

file_passes_filter() was already the single predicate consulted by build_visible_items, rebuild_annotations, file_render_height/effective_file_height, hunk_positions, and both diff renderers, so the tree, the diff pane, {/} file navigation and [/] hunk navigation all follow from one gate.

The one structural change is splitting that predicate in two, because "which files are displayed" and "how many files am I reviewing" stop being the same question:

  • file_matches_patterns() — i/e only. The review population: what the tree title's reviewed/total fraction counts, and what / search walks.
  • file_passes_filter() — that plus the reviewed toggle. What is actually on screen.

Without the split, the title collapses to 0/9 exactly when progress matters most. It stays 3/12, and the tree's bottom border carries a reviewed hidden cue beside the existing i:/e: chips.

Behaviour worth a second opinion

  • The header's +/- totals drop reviewed files' lines while hiding. diff_stat() is gated by file_passes_filter, so this is automatic, and it matches what i/e already do. It reads as "diff left to review", but it does mean the header and the tree title deliberately count different populations.
  • A file whose hunks are individually R-marked is not hidden. Only the file-level r flag hides. Partly to avoid surprise, partly because is_hunk_reviewed hashes every hunk in the file and has an explicit guard against speculative calls.
  • A hidden reviewed file cannot be un-reviewed, since r can no longer reach it. H / :set reviewed is the only way back, so the status messages name it.
  • Comments on hidden files are untouched and still export — same contract as i/e: a filter is a view, not an edit. The flag survives :e and resets on restart unless configured.

A bug caught while implementing

main.rs applies config after App::build has already built the annotations. total_lines() recomputes from the filter but line_annotations — what the diff renderer and the cursor read — does not, so setting the flag directly left them disagreeing (7 annotation rows against 6 real ones) and show_reviewed = false rendered a misaligned diff. Hence init_show_reviewed(), which re-derives, rather than a bare field assignment. Guarded by should_leave_no_stale_annotations_when_the_startup_default_hides_reviewed_files.

FileTreeFilter also loses its derived Default — show_reviewed defaults to true, and a derived bool default would have silently booted with reviewed files hidden.

Tests

38 tests in the filter suite, covering: hidden from the tree, from the diff render height, and from hunk-position alignment; the progress fraction holding over the whole population and still respecting i/e; / search skipping hidden files; burn-down advance, wrap, and exhaustion; hunk-reviewed files staying visible; and the startup-default rebuild.

The tree's r and H are covered through the real handle_file_list_action path rather than only via App methods, so key routing is part of the guard. Three new render tests drive the actual ui::render against a TestBackend and assert the on-screen footer cue and title. Plus keybinding tests (H maps in the tree and not in the diff; h still scrolls), command-parse tests for all four forms, and config-parse tests.

Docs

README.md, src/ui/help_popup.rs, the file-tree hint in src/ui/status_bar.rs, docs/KEYBINDINGS.md (key table, : command table, and a Filtering subsection), docs/CONFIG.md (Options row + full example), and the FileTreeFilter section of AGENTS.md.

AGENTS.md's doc-update table had no row for docs/KEYBINDINGS.md or src/ui/status_bar.rs, despite both tracking every keybinding — added, since I only found them by grepping.

@patrickjmccauley
patrickjmccauley marked this pull request as ready for review August 12, 2026 18:50

@agavra agavra left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a great addition @patrickjmccauley thank you! left one comment inline

Comment thread src/input/keybindings.rs Outdated
(KeyCode::Char('I'), _) => Action::FileTreeClearInclude,
(KeyCode::Char('E'), _) => Action::FileTreeClearExclude,
(KeyCode::Char('/'), _) => Action::FileTreeSearch,
(KeyCode::Char('H'), _) => Action::ToggleShowReviewed,

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this feature but I think we should keep close to vim keybindings, this would reserve H for moving the cursor to the first character in a line (I guess that doesn't really make sense since we can't edit, but I still prefer not introducing new single-stroke values).

I'd prefer keeping just a <leader> (:) command for now.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that makes total sense - apologies, I'm still ramping up on the vim keybinds. will refactor to the leader command, then re-request

@agavra

agavra commented Aug 18, 2026

Copy link
Copy Markdown
Owner

failing test is unrelated, i'll merge and fix that one on main

@agavra
agavra merged commit f31a8b5 into agavra:main Aug 18, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants