feat: navigate annotations across files with }/{ keys#162
Merged
Conversation
Adds }/{ keybindings to jump to next/previous annotation across files
in a single flat list (alphabetical files, file-level first per file,
then ascending lines — matches the @ popup order). Always crosses file
boundaries; silent no-op at the first/last annotation.
Reuses the existing jumpToAnnotationTarget machinery (collapsed-hunk
auto-expansion + cross-file pendingAnnotJump handoff) by exposing it
through tryJumpToAnnotationTarget which returns a jumped-bool. The
walker retries on ok=false so non-jumpable targets (path filtered out
of the tree, line missing from the loaded diff in compact mode) cannot
trap the user on a hidden annotation. Loop is bounded by len(flat).
The flat list is rebuilt every keypress so additions and deletions
take effect without explicit invalidation. ChangeDivider rows collapse
to a synthetic line=-1 cursor key so file-level annotations stay
reachable from a leading divider.
Docs synced: README.md, site/docs.html, docs/ARCHITECTURE.md, both
.claude-plugin and plugins/codex reference usage.md copies.
- [minor] app/ui/annotnav.go:25 — handleAnnotNav was O(N²) (len(flat) outer retries × pickAdjacentAnnotation O(N) inner). Refactored to compute the starting flat-list index once via startingFlatIndex, then walk indexes directly with ±1 step. Each candidate is examined at most once → O(N) worst case even when every annotation is non-jumpable. - [minor] app/ui/annotnav.go:81 — currentAnnotKey mapped ALL ChangeDivider rows to line=-1, which is correct for leading dividers but causes middle/trailing dividers (reachable via mouse-click) to short-circuit forward } back to the file-level/first same-file annotation. Fixed by walking back to the nearest preceding non-divider line via dividerAnnotKey and inheriting its position; only leading dividers (no prior non-divider line) fall back to line=-1. Tests added for middle/trailing/consecutive-divider cases at both unit and integration level. The pickAdjacentAnnotation table tests now run through a test-only wrapper around startingFlatIndex so the picking algorithm documentation is preserved.
positionOnAnnotation set diffCursor to the annotated diff line index but
left m.annot.cursorOnAnnotation=false. The cursor render path checks both:
the cursor highlight only moves to the annotation comment sub-row when
cursorOnAnnotation is true. As a result, both @ popup selection and the
new }/{ navigation visually placed the cursor on the diff line above the
comment, one row off the actual annotation.
Set cursorOnAnnotation=true after positioning, matching the invariant that
j/k navigation already maintains when stepping onto an annotated line.
File-level annotations (Line=0) are unaffected — diffCursor=-1 already
represents the annotation row directly, so the flag stays false there.
Skips the flag when the line is collapsed-hidden or a delete-only
placeholder, where the annotation comment row does not render.
There was a problem hiding this comment.
Pull request overview
Adds global annotation-to-annotation navigation so reviewers can step through many inline annotations across multiple files in a single TUI session, complementing the existing @ annotation list popup.
Changes:
- Introduces
{/}keybindings (and keymap actions) to jump to previous/next annotation across files, with silent no-op at boundaries. - Implements cross-file annotation walker logic that skips non-jumpable targets (filtered paths, compact-mode missing lines) and reuses existing jump machinery.
- Fixes cursor placement when jumping to an annotation so the cursor lands on the annotation comment sub-row (when visible), and updates docs/tests accordingly.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| app/ui/annotnav.go | New cross-file annotation navigation handler and flat-list index navigation logic. |
| app/ui/annotnav_test.go | Comprehensive unit/integration tests for {/} behavior, jumpability skipping, divider behavior, and cursor placement. |
| app/ui/annotlist.go | Adds tryJumpToAnnotationTarget (returns jumped bool) and updates jump/cursor positioning behavior. |
| app/ui/model.go | Dispatches new keymap actions to annotation navigation handler. |
| app/keymap/keymap.go | Adds next_annotation / prev_annotation actions, default bindings {/}, and help descriptions. |
| app/keymap/keymap_test.go | Extends default binding coverage to include {/}. |
| README.md | Documents {/} and new actions in the keybinding/action lists. |
| site/docs.html | Updates site docs with {/} and action list entries. |
| docs/ARCHITECTURE.md | Documents new annotnav component and updated annotlist responsibilities. |
| .claude-plugin/skills/revdiff/references/usage.md | Updates plugin usage docs with {/}. |
| plugins/codex/skills/revdiff/references/usage.md | Updates plugin usage docs with {/}. |
| app/annotations_load.go | Removes trailing whitespace at EOF. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+54
to
+56
| // true when the cursor is exactly on an existing annotation row (file, | ||
| // line, AND type all match). In that case navigation steps by index in the | ||
| // flat list. Otherwise navigation uses an insertion-point fallback. |
The onAnnot field is set whenever the cursor's diff position matches an
annotation in the store, regardless of whether the cursor visually sits on
the diff line or on the annotation comment sub-row — both visual positions
point to the same annotation conceptually. The previous wording ("exactly
on an existing annotation row") suggested a stricter visual-row check that
the implementation does not perform.
Addresses Copilot review on PR #162.
Deploying revdiff with
|
| Latest commit: |
46a2222
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://082038dd.revdiff.pages.dev |
| Branch Preview URL: | https://feature-annotation-navigatio.revdiff.pages.dev |
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.
since #156 added
--annotationsto preload review feedback as inline annotations, a single session can carry many annotations spread across many files. The@popup is fine for picking one specific item, but stepping through them in order needed dedicated keys.adds
}and{to navigate to next/previous annotation across files. Always crosses file boundaries (no opt-in flag, unlike the existing--cross-file-hunksfor[/]). Silent no-op at the first/last annotation.the walker reuses
jumpToAnnotationTarget(collapsed-hunk auto-expansion, cross-file pendingAnnotJump handoff, TOC sync) via a newtryJumpToAnnotationTargetvariant that returns whether the jump happened. On failure the walker retries the next candidate, so non-jumpable targets (path filtered out of the tree, line missing from the loaded diff in compact mode) cannot trap the user on a hidden annotation. Walk is index-based and bounded bylen(flat)for O(N) worst case.flat-list ordering matches the
@popup: alphabetical files, file-level (Line=0) first per file, then ascending lines within a file. Rebuilt every keypress so additions and deletions take effect immediately. ChangeDivider rows inherit the position of the nearest preceding non-divider line so middle/trailing dividers don't short-circuit forward navigation back to the first same-file annotation.also fixes a visual bug in
positionOnAnnotation: jumping via@popup or}/{now puts the cursor ON the annotation comment row, same asj/kwhen stepping onto an annotated line. Previously the cursor sat on the diff line above the comment.docs synced: README.md, site/docs.html, docs/ARCHITECTURE.md, both
.claude-pluginandplugins/codexreference usage.md copies.