Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .claude-plugin/skills/revdiff/references/usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ Use `--stdin` to review arbitrary piped or redirected text as one synthetic file
| `a` or `Enter` (diff pane) | Annotate current diff line |
| `A` | Add file-level annotation (stored at top of diff) |
| `@` | Toggle annotation list popup (navigate and jump to any annotation) |
| `}` / `{` | Jump to next/previous annotation (always crosses file boundaries; silent no-op at the first/last annotation) |
| `d` | Delete annotation under cursor |
| `Ctrl+E` (during annotation input) | Open `$EDITOR` for multi-line annotation |
| `Esc` | Cancel annotation input |
Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,7 @@ In the Claude Code and Codex plugins, you can also tell the agent to use a past
| `a` or `Enter` (diff pane) | Annotate current diff line |
| `A` | Add file-level annotation (stored at top of diff) |
| `@` | Toggle annotation list popup (navigate and jump to any annotation) |
| `}` / `{` | Jump to next/previous annotation (always crosses file boundaries; silent no-op at the first/last annotation) |
| `d` | Delete annotation under cursor |
| `Ctrl+E` (during annotation input) | Open `$EDITOR` for multi-line annotation |
| `Esc` | Cancel annotation input |
Expand Down Expand Up @@ -755,7 +756,7 @@ When the leader is pressed, the status bar shows `Pending: ctrl+w, esc to cancel

**Search:** `search`

**Annotations:** `confirm` (annotate line / select file), `annotate_file`, `delete_annotation`, `annot_list`
**Annotations:** `confirm` (annotate line / select file), `annotate_file`, `delete_annotation`, `annot_list`, `next_annotation`, `prev_annotation`

**View:** `toggle_collapsed`, `toggle_compact`, `toggle_wrap`, `toggle_tree`, `toggle_line_numbers`, `toggle_blame`, `toggle_word_diff`, `toggle_hunk`, `toggle_untracked`, `mark_reviewed`, `theme_select`, `filter`, `info`, `reload`

Expand Down
1 change: 0 additions & 1 deletion app/annotations_load.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,4 +245,3 @@ func buildLineSet(lines []diff.DiffLine) map[lineKey]struct{} {
}
return out
}

7 changes: 7 additions & 0 deletions app/keymap/keymap.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ const (
ActionAnnotateFile Action = "annotate_file"
ActionDeleteAnnotation Action = "delete_annotation"
ActionAnnotList Action = "annot_list"
ActionNextAnnotation Action = "next_annotation"
ActionPrevAnnotation Action = "prev_annotation"
ActionToggleCollapsed Action = "toggle_collapsed"
ActionToggleCompact Action = "toggle_compact"
ActionToggleWrap Action = "toggle_wrap"
Expand Down Expand Up @@ -77,6 +79,7 @@ var validActions = map[Action]bool{
ActionTogglePane: true, ActionFocusTree: true, ActionFocusDiff: true,
ActionSearch: true,
ActionConfirm: true, ActionAnnotateFile: true, ActionDeleteAnnotation: true, ActionAnnotList: true,
ActionNextAnnotation: true, ActionPrevAnnotation: true,
ActionToggleCollapsed: true, ActionToggleCompact: true, ActionToggleWrap: true, ActionToggleTree: true,
ActionToggleLineNums: true, ActionToggleBlame: true, ActionToggleWordDiff: true, ActionToggleHunk: true,
ActionMarkReviewed: true, ActionFilter: true, ActionToggleUntracked: true,
Expand Down Expand Up @@ -204,6 +207,8 @@ func defaultDescriptions() []HelpEntry {
{ActionAnnotateFile, "annotate file", "Annotations"},
{ActionDeleteAnnotation, "delete annotation", "Annotations"},
{ActionAnnotList, "annotation list", "Annotations"},
{ActionNextAnnotation, "next annotation (across files)", "Annotations"},
{ActionPrevAnnotation, "previous annotation (across files)", "Annotations"},

// view toggles
{ActionToggleCollapsed, "toggle collapsed view", "View"},
Expand Down Expand Up @@ -258,6 +263,8 @@ func defaultBindings() map[string]Action {
"A": ActionAnnotateFile,
"d": ActionDeleteAnnotation,
"@": ActionAnnotList,
"}": ActionNextAnnotation,
"{": ActionPrevAnnotation,
"v": ActionToggleCollapsed,
"C": ActionToggleCompact,
"w": ActionToggleWrap,
Expand Down
1 change: 1 addition & 0 deletions app/keymap/keymap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ func TestDefault_allExpectedBindings(t *testing.T) {
{"/", ActionSearch},
{"a", ActionConfirm}, {"enter", ActionConfirm},
{"A", ActionAnnotateFile}, {"d", ActionDeleteAnnotation}, {"@", ActionAnnotList},
{"}", ActionNextAnnotation}, {"{", ActionPrevAnnotation},
{"v", ActionToggleCollapsed}, {"C", ActionToggleCompact}, {"w", ActionToggleWrap}, {"t", ActionToggleTree},
{"L", ActionToggleLineNums}, {"B", ActionToggleBlame}, {"W", ActionToggleWordDiff},
{".", ActionToggleHunk}, {" ", ActionMarkReviewed}, {"f", ActionFilter},
Expand Down
57 changes: 45 additions & 12 deletions app/ui/annotlist.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@ import (
)

// buildAnnotListItems builds a flat list of all annotations across all files.
// items are ordered by file name then line number, as returned by the store.
// Items are ordered by file name then line number, as returned by the store
// (alphabetical via Store.Files, line-ascending via Store.Get with file-level
// Line=0 first within each file). This combined ordering is load-bearing: both
// the @ popup and the }/{ walker in annotnav.go iterate this exact sequence,
// so changes to Store.Files / Store.Get ordering must keep the two consumers
// in sync.
func (m *Model) buildAnnotListItems() []annotation.Annotation {
files := m.store.Files()
items := make([]annotation.Annotation, 0, m.store.Count())
Expand All @@ -31,38 +36,66 @@ func (m Model) buildAnnotListSpec() overlay.AnnotListSpec {
return overlay.AnnotListSpec{Items: items}
}

// jumpToAnnotationTarget jumps to an annotation target returned by the overlay manager.
// jumpToAnnotationTarget jumps to an annotation target returned by the overlay
// manager. Existing entry point used by the @ popup and the mouse handler;
// preserves the original silent-fail-on-unreachable behavior.
func (m Model) jumpToAnnotationTarget(target *overlay.AnnotationTarget) (tea.Model, tea.Cmd) {
model, cmd, _ := m.tryJumpToAnnotationTarget(target)
return model, cmd
}

// tryJumpToAnnotationTarget attempts to jump and reports whether the jump
// was actually issued. ok=false is returned when the target cannot be reached:
// cross-file path is not present in the file tree (filtered, hidden by
// --include/--exclude, or untracked-with-toggle-off), or same-file
// non-file-level line is not in the loaded diff (e.g. compact mode shrank
// context away). Used by the }/{ navigator to skip non-jumpable targets and
// keep walking instead of getting trapped on a target that silently no-ops.
// Single-file mode always rejects cross-file targets — there is nowhere to go.
func (m Model) tryJumpToAnnotationTarget(target *overlay.AnnotationTarget) (tea.Model, tea.Cmd, bool) {
if target == nil {
return m, nil
return m, nil, false
}
a := annotation.Annotation{File: target.File, Line: target.Line, Type: target.ChangeType}

if a.File == m.file.name {
if a.Line != 0 && m.findDiffLineIndex(a.Line, a.Type) < 0 {
return m, nil, false
}
m.positionOnAnnotation(a)
return m, nil
return m, nil, true
}

m.pendingAnnotJump = &a
if !m.file.singleFile {
if !m.tree.SelectByPath(a.File) {
m.pendingAnnotJump = nil
return m, nil
}
if m.file.singleFile {
return m, nil, false
}
if !m.tree.SelectByPath(a.File) {
return m, nil, false
}
return m.loadSelectedIfChanged()
m.pendingAnnotJump = &a
model, cmd := m.loadSelectedIfChanged()
return model, cmd, true
}

// positionOnAnnotation moves the cursor to the given annotation's line, re-renders, and centers the viewport.
// in collapsed mode, expands the hunk containing the target line so removed lines are visible.
// In collapsed mode, expands the hunk containing the target line so removed lines are visible.
// For line-level annotations the cursor lands on the annotation comment sub-row (cursorOnAnnotation=true),
// matching what `j`/`k` navigation produces when stepping onto an annotated line. File-level annotations
// (Line=0) use diffCursor=-1 which already represents the annotation row directly. Without this flag the
// cursor would land on the diff line above the comment, leaving navigation visually one row off the target.
func (m *Model) positionOnAnnotation(a annotation.Annotation) {
m.annot.cursorOnAnnotation = false
if a.Line == 0 {
m.nav.diffCursor = -1
} else {
idx := m.findDiffLineIndex(a.Line, a.Type)
if idx >= 0 {
m.nav.diffCursor = idx
m.ensureHunkExpanded(idx)
hunks := m.findHunks()
if !m.isCollapsedHidden(idx, hunks) && !m.isDeleteOnlyPlaceholder(idx, hunks) {
m.annot.cursorOnAnnotation = true
}
}
}
m.layout.focus = paneDiff
Expand Down
178 changes: 178 additions & 0 deletions app/ui/annotnav.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
package ui

import (
tea "github.com/charmbracelet/bubbletea"

"github.com/umputun/revdiff/app/annotation"
"github.com/umputun/revdiff/app/diff"
"github.com/umputun/revdiff/app/ui/overlay"
)

// handleAnnotNav jumps to the next or previous annotation in the flat
// cross-file list (alphabetical files, file-level first per file, then
// ascending lines — same order as the @ popup). At the boundaries the
// action is a silent no-op. Cross-file handoff goes through the existing
// tryJumpToAnnotationTarget machinery, so collapsed-hunk expansion, TOC
// sync, and pendingAnnotJump-based file load all come for free.
//
// When a target is in the store but not currently displayable (cross-file
// path filtered out of the tree, or same-file line missing from the loaded
// diff in compact mode), the walker advances past it and tries the next
// candidate in the same direction. Without that walk-past the cursor would
// not move on a silent-fail target and every subsequent }/{ press would
// recompute the same hidden target, trapping the user.
//
// The walk is index-based: starting position is computed once via
// startingFlatIndex, then the loop steps by ±1 per attempt. Each candidate
// is examined at most once, so the worst case is O(N) over flat — even
// when every annotation is non-jumpable.
func (m Model) handleAnnotNav(forward bool) (tea.Model, tea.Cmd) {
flat := m.buildAnnotListItems()
if len(flat) == 0 {
return m, nil
}
cur := m.currentAnnotKey()
step := 1
if !forward {
step = -1
}
for idx := startingFlatIndex(flat, cur, forward); idx >= 0 && idx < len(flat); idx += step {
target := flat[idx]
nextModel, cmd, jumped := m.tryJumpToAnnotationTarget(&overlay.AnnotationTarget{
File: target.File,
ChangeType: target.Type,
Line: target.Line,
})
if jumped {
return nextModel, cmd
}
}
return m, nil
}

// cursorAnnotKey is the cursor's position in annotation space. onAnnot is
// true when the cursor's diff position matches an annotation in the store
// (file, line, AND type), regardless of whether the cursor visually sits
// on the diff line or on the annotation comment sub-row — both point to
// the same annotation. In that case navigation steps by index in the flat
// list; otherwise it uses an insertion-point fallback.
type cursorAnnotKey struct {
file string
line int
typ string
onAnnot bool
}

// currentAnnotKey returns the cursor's annotation-space key.
//
// - File-level annotation row (diffCursor == -1) maps to (file, 0, "")
// which matches the storage form of file-level annotations.
// - Out-of-range cursor collapses to line=-1 so forward navigation treats
// any same-file annotation as strictly after.
// - ChangeDivider rows (which carry OldNum=NewNum=0) inherit the position
// of the nearest preceding non-divider line in the same file. This keeps
// middle/trailing dividers (reachable via mouse-click in the diff) from
// short-circuiting forward navigation back to the file-level annotation.
// A leading divider with no prior non-divider line falls back to line=-1
// so the file-level annotation remains reachable from the top.
func (m Model) currentAnnotKey() cursorAnnotKey {
file := m.file.name
if m.nav.diffCursor == -1 {
return cursorAnnotKey{file: file, line: 0, typ: "", onAnnot: m.hasFileAnnotation()}
}
if m.nav.diffCursor < 0 || m.nav.diffCursor >= len(m.file.lines) {
return cursorAnnotKey{file: file, line: -1, typ: "", onAnnot: false}
}
dl := m.file.lines[m.nav.diffCursor]
if dl.ChangeType == diff.ChangeDivider {
return m.dividerAnnotKey(file)
}
line := m.diffLineNum(dl)
typ := string(dl.ChangeType)
return cursorAnnotKey{file: file, line: line, typ: typ, onAnnot: m.store.Has(file, line, typ)}
}

// dividerAnnotKey returns the cursor's annotation-space key when the cursor
// sits on a ChangeDivider row. Walks back to the nearest preceding
// non-divider line and uses its line number, so forward navigation from a
// middle/trailing divider reaches the next annotation strictly after the
// divider's logical position rather than re-entering at the file-level
// annotation. A leading divider (no prior non-divider line) falls back to
// line=-1 so the file-level annotation stays reachable.
func (m Model) dividerAnnotKey(file string) cursorAnnotKey {
for i := m.nav.diffCursor - 1; i >= 0; i-- {
prev := m.file.lines[i]
if prev.ChangeType == diff.ChangeDivider {
continue
}
return cursorAnnotKey{file: file, line: m.diffLineNum(prev), typ: string(prev.ChangeType), onAnnot: false}
}
return cursorAnnotKey{file: file, line: -1, typ: "", onAnnot: false}
}

// startingFlatIndex returns the index in flat from which the walker should
// begin attempting jumps. Forward: first index strictly after the cursor,
// or one past an exact-match index. Backward: mirror. Returns an
// out-of-range index (-1 or len(flat)) when the cursor is at the
// corresponding boundary — the loop exits immediately in that case.
func startingFlatIndex(flat []annotation.Annotation, cur cursorAnnotKey, forward bool) int {
if idx, ok := exactAnnotIndex(flat, cur); ok {
if forward {
return idx + 1
}
return idx - 1
}
insIdx := annotInsertionPoint(flat, cur)
if forward {
return insIdx
}
return insIdx - 1
}

// exactAnnotIndex returns the flat-list index of an annotation that exactly
// matches the cursor (file, line, type). When cur.onAnnot is false it
// short-circuits without scanning. ok=false means "use insertion-point
// fallback instead."
func exactAnnotIndex(flat []annotation.Annotation, cur cursorAnnotKey) (int, bool) {
if !cur.onAnnot {
return 0, false
}
for i, a := range flat {
if a.File == cur.file && a.Line == cur.line && a.Type == cur.typ {
return i, true
}
}
return 0, false
}

// annotInsertionPoint returns the index where the cursor would be inserted
// in the flat list under (file, line) ordering — i.e. the index of the
// first annotation strictly after the cursor, or len(flat) if all entries
// are at or before the cursor.
func annotInsertionPoint(flat []annotation.Annotation, cur cursorAnnotKey) int {
for i, a := range flat {
if compareAnnotPos(a.File, a.Line, cur.file, cur.line) > 0 {
return i
}
}
return len(flat)
}

// compareAnnotPos compares two (file, line) annotation positions using the
// same ordering as the flat annotation list: alphabetical by file, then
// ascending by line within a file. Returns -1, 0, or 1.
func compareAnnotPos(aFile string, aLine int, bFile string, bLine int) int {
if aFile != bFile {
if aFile < bFile {
return -1
}
return 1
}
if aLine < bLine {
return -1
}
if aLine > bLine {
return 1
}
return 0
}
Loading