From 5cd71a50f6a5d278cee4fdf248ec22beadab184f Mon Sep 17 00:00:00 2001 From: madhermit <1425712+madhermit@users.noreply.github.com> Date: Tue, 14 Jul 2026 16:12:44 -0600 Subject: [PATCH 1/2] Auto-advance r to the next unreviewed file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Marking a file reviewed now jumps the selection to the next file that still needs eyes (wrapping past the end, skipping the All row and already-reviewed files), so a full review pass is r-r-r instead of r-j-r-j; when nothing is left it flashes 'all changes reviewed' and stays. Un-marking stays put — going back to a file to unmark it is deliberate. The unreviewed-only view keeps its existing behavior (the marked file drops out, which advances by itself). Also de-ambiguates the filter's labels: the footer hint read 'U unreviewed', which parses as un-review; it now reads 'hide reviewed', and the active-filter title says 'unreviewed only'. --- CHANGELOG.md | 9 +++++++ README.md | 4 +-- internal/tui/diff/model.go | 48 ++++++++++++++++++++++++++++----- internal/tui/diff/model_test.go | 25 +++++++++++++++++ internal/tui/splitlist.go | 20 ++++++++++++++ 5 files changed, 98 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db4e692..2c1f0d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,15 @@ pre-1.0). ### Added +### Changed + +- `r` (mark reviewed) now advances to the next unreviewed file — wrapping + past the end, flashing "all changes reviewed" when none remain — so a + review pass is `r` `r` `r` instead of `r` `j` `r` `j`. Un-marking stays + put. The `U` filter's footer hint now reads "hide reviewed" and the + active-filter title "unreviewed only", since "unreviewed" alone read + ambiguously as an action. + ### Fixed - `rift diff --watch` live-reloads the browser as the working tree changes — diff --git a/README.md b/README.md index 1869df2..e0a4b86 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,7 @@ Add `--tests` to `rift diff` or `rift log` to read a change by the _tests_ it to ### Review tracking -In `rift diff`, press `r` to mark the selected file **reviewed** (a `✓` replaces its status glyph) and `U` to show only the files you haven't reviewed yet — so you can walk a large change file by file and watch the list shrink as you tick each one off. A mark is keyed to the file's content, so it **resets the moment the file changes** (e.g. an agent edits it again) and re-applies on revert, making it easy to re-review only what's new. Marks persist per worktree (in `.git/rift/`, uncommitted); `--unreviewed` narrows `--print` / `--json` / `--name-only` to the not-yet-reviewed files. +In `rift diff`, press `r` to mark the selected file **reviewed** (a `✓` replaces its status glyph) — the selection then jumps to the next unreviewed file, so a full pass is just `r` `r` `r`; pressing `r` on a reviewed file unmarks it (and stays put). `U` narrows the list to only the files you haven't reviewed yet, and shows it as `unreviewed only` in the title. A mark is keyed to the file's content, so it **resets the moment the file changes** (e.g. an agent edits it again) and re-applies on revert, making it easy to re-review only what's new. Marks persist per worktree (in `.git/rift/`, uncommitted); `--unreviewed` narrows `--print` / `--json` / `--name-only` to the not-yet-reviewed files. ![rift diff — marking changes reviewed and filtering to what's left](docs/images/reviewed.gif) @@ -159,7 +159,7 @@ Every work screen (`diff`, `log`, `stash`, `stage`, and the tests lens) shares t | Screen | Keys | | ---------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `diff` | `s` toggle staged · `w` toggle watch · `t` tests lens · `\` diff layout · `e` engine · `o` open in `$EDITOR` · `r` mark reviewed · `U` show only unreviewed | +| `diff` | `s` toggle staged · `w` toggle watch · `t` tests lens · `\` diff layout · `e` engine · `o` open in `$EDITOR` · `r` mark reviewed (advances to the next unreviewed) · `U` hide reviewed | | `log` | `⏎` drill into the commit's files (or tests) · `\` layout · `e` engine · `c` cherry-pick · `r` revert | | `stash` | `a` apply · `p` pop · `x` drop · `\` layout · `e` engine | | `stage` | `s` stage · `u` unstage · `a` stage all · `o` open in `$EDITOR` · `e` engine (fixed layout, no `\`) | diff --git a/internal/tui/diff/model.go b/internal/tui/diff/model.go index 53fc02c..c8d7df4 100644 --- a/internal/tui/diff/model.go +++ b/internal/tui/diff/model.go @@ -136,7 +136,7 @@ func New(repo *git.Repo, engine diff.Engine, files []git.ChangedFile, staged boo if !commitDiff { // In a commit diff the files are historical: editing the working-tree copy // (`o`) or reviewing it would be misleading, so those are working-tree only. - hints = append(hints, [2]string{"o", "open"}, [2]string{"r", "review"}, [2]string{"U", "unreviewed"}) + hints = append(hints, [2]string{"o", "open"}, [2]string{"r", "review"}, [2]string{"U", "hide reviewed"}) } hints = append(hints, [2]string{"y", "yank"}, [2]string{"?", "help"}, [2]string{"q", "quit"}) @@ -247,7 +247,7 @@ func (m Model) listTitle() string { title += fmt.Sprintf(" · %d/%d reviewed", n, total) } if m.unreviewedOnly { - title += " · unreviewed" + title += " · unreviewed only" } return title } @@ -328,21 +328,57 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, cmd } -// toggleReviewed flips the selected file's reviewed mark. In the unreviewed-only -// view the file then drops out of the list, advancing to the next one. +// toggleReviewed flips the selected file's reviewed mark. Marking a file +// advances to the next unreviewed one (wrapping around), so a review pass is +// r-r-r rather than r-j-r-j; un-marking stays put — going back to a file to +// unmark it is deliberate. In the unreviewed-only view the file instead drops +// out of the list, which advances by itself. func (m Model) toggleReviewed() (tea.Model, tea.Cmd) { sel, ok := m.list.Selected() if m.marks == nil || !ok || sel.Path == "" { return m, nil } - m.marks.state.Toggle(sel.Path, m.marks.hashes[sel.Path]) + marked := m.marks.state.Toggle(sel.Path, m.marks.hashes[sel.Path]) m.list = m.list.SetListTitle(m.listTitle()) if m.unreviewedOnly { var cmd tea.Cmd m.list, cmd = m.list.SetItems(m.displayFiles()) return m, cmd } - return m, nil + if !marked { + return m, nil + } + next, ok := nextUnreviewed(m.list.VisibleItems(), sel.Path, m.marks.reviewed) + if !ok { + m.list = m.list.SetFlash("all changes reviewed") + return m, nil + } + var cmd tea.Cmd + m.list, cmd = m.list.SelectKey(next) + return m, cmd +} + +// nextUnreviewed returns the path of the first file after cur (wrapping) that +// isn't reviewed, skipping the synthetic All row; ok=false when every file is +// reviewed. +func nextUnreviewed(items []git.ChangedFile, cur string, reviewed func(string) bool) (string, bool) { + start := 0 + for i, f := range items { + if f.Path == cur { + start = i + break + } + } + for off := 1; off <= len(items); off++ { + f := items[(start+off)%len(items)] + if f.Path == "" || f.Path == cur { + continue + } + if !reviewed(f.Path) { + return f.Path, true + } + } + return "", false } // toggleUnreviewedOnly switches between showing all changed files and only the diff --git a/internal/tui/diff/model_test.go b/internal/tui/diff/model_test.go index e19fa39..7bbe569 100644 --- a/internal/tui/diff/model_test.go +++ b/internal/tui/diff/model_test.go @@ -45,3 +45,28 @@ func TestPreviewFiles(t *testing.T) { t.Errorf("all changes: got %v (should skip the empty All entry)", got) } } + +// TestNextUnreviewed covers the r auto-advance target: the next visible +// unreviewed file after the current one, wrapping, skipping the All row. +func TestNextUnreviewed(t *testing.T) { + items := []git.ChangedFile{{Path: ""}, {Path: "a"}, {Path: "b"}, {Path: "c"}} + reviewed := func(m map[string]bool) func(string) bool { + return func(p string) bool { return m[p] } + } + + if next, ok := nextUnreviewed(items, "a", reviewed(map[string]bool{"a": true})); !ok || next != "b" { + t.Errorf("advance from a: got %q ok=%v, want b", next, ok) + } + // b reviewed too: skip to c. + if next, ok := nextUnreviewed(items, "a", reviewed(map[string]bool{"a": true, "b": true})); !ok || next != "c" { + t.Errorf("skip reviewed: got %q ok=%v, want c", next, ok) + } + // Wrap around past the All row. + if next, ok := nextUnreviewed(items, "c", reviewed(map[string]bool{"b": true, "c": true})); !ok || next != "a" { + t.Errorf("wrap: got %q ok=%v, want a", next, ok) + } + // Everything reviewed: no target. + if _, ok := nextUnreviewed(items, "c", reviewed(map[string]bool{"a": true, "b": true, "c": true})); ok { + t.Error("all reviewed should report no next target") + } +} diff --git a/internal/tui/splitlist.go b/internal/tui/splitlist.go index cb83c96..c000600 100644 --- a/internal/tui/splitlist.go +++ b/internal/tui/splitlist.go @@ -382,6 +382,26 @@ func (m SplitList[T]) WithBreadcrumb(screen string) SplitList[T] { // gate list-pane-only actions on it (e.g. stash apply/pop/drop). func (m SplitList[T]) Reading() bool { return m.active == splitPreviewPane } +// SelectKey moves the selection to the item whose Match value equals key and +// loads its preview — a no-op when the key isn't in the filtered set or is +// already selected. Parents use it for programmatic advances (e.g. jumping to +// the next unreviewed file after a mark). +func (m SplitList[T]) SelectKey(key string) (SplitList[T], tea.Cmd) { + if key == "" || m.cfg.Match == nil { + return m, nil + } + for i, it := range m.filtered { + if m.cfg.Match(it) == key { + if i == m.selected { + return m, nil + } + m.selected = i + return m.requestPreview() + } + } + return m, nil +} + // SelectedKey returns the Match value of the current selection, or "" when the // list is empty. Parents capture it before a reload to restore the selection. func (m SplitList[T]) SelectedKey() string { From cdfb61d2f8290fd5a8bcf077956bf8b71a51221b Mon Sep 17 00:00:00 2001 From: madhermit <1425712+madhermit@users.noreply.github.com> Date: Tue, 14 Jul 2026 16:17:54 -0600 Subject: [PATCH 2/2] Keep the cursor on the next unreviewed file in the U view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Marking a file in the unreviewed-only view re-filters the list via SetItems, which reset the selection to the top — advancing only for a top-to-bottom pass and yanking the reviewer back to row one when they marked from the middle. Re-select the next unreviewed file after the marked one instead (the same nextUnreviewed target the default view's auto-advance uses), falling back to the top only when the list empties. --- internal/tui/diff/model.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/internal/tui/diff/model.go b/internal/tui/diff/model.go index c8d7df4..43a0f8d 100644 --- a/internal/tui/diff/model.go +++ b/internal/tui/diff/model.go @@ -184,8 +184,8 @@ func fileRow(marks *reviewMarks) func(git.ChangedFile, int, bool) string { // displayFiles is the list the SplitList shows. In the default view it's every // changed file under a synthetic "All" entry. In the unreviewed-only view it's -// just the not-yet-reviewed files with no "All" row — so as you tick files off, -// the cursor (reset to the top by the re-filter) lands on the next one to review. +// just the not-yet-reviewed files with no "All" row; toggleReviewed re-selects +// the next unreviewed file when one drops out. func (m Model) displayFiles() []git.ChangedFile { if m.unreviewedOnly && m.marks != nil { return m.marks.state.Unreviewed(m.files, m.marks.hashes) @@ -341,8 +341,12 @@ func (m Model) toggleReviewed() (tea.Model, tea.Cmd) { marked := m.marks.state.Toggle(sel.Path, m.marks.hashes[sel.Path]) m.list = m.list.SetListTitle(m.listTitle()) if m.unreviewedOnly { + // The marked file drops out of this view; keep the cursor on the next + // unreviewed file after it (not the top — marking from the middle of + // the list must not yank the reviewer back to row one). + next, _ := nextUnreviewed(m.list.VisibleItems(), sel.Path, m.marks.reviewed) var cmd tea.Cmd - m.list, cmd = m.list.SetItems(m.displayFiles()) + m.list, cmd = m.list.SetItemsSelecting(m.displayFiles(), next) return m, cmd } if !marked {