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
2 changes: 2 additions & 0 deletions .claude-plugin/skills/revdiff/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ When you are launching revdiff for the user (e.g., right after a refactor or ana

**When the recent change likely created new untracked files** (new packages, new test files, new docs, new scripts that haven't been `git add`-ed yet), pass `--untracked` so those files appear in the tree. Use this in working-tree mode (no ref, no `--staged`); skip it for ref-to-ref reviews where untracked files are not part of the historical diff.

Pass `--start-at-change` only when the user explicitly asks for that cursor preference; never infer it automatically.

Run the launcher through the override-chain resolver:

```bash
Expand Down
1 change: 1 addition & 0 deletions .claude-plugin/skills/revdiff/references/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ Then uncomment and edit the values you want to change.
| `--wrap` | `REVDIFF_WRAP` | Enable line wrapping in diff view | `false` |
| `--wrap-indent` | `REVDIFF_WRAP_INDENT` | Indent wrap continuation rows by N columns so they hang under the first row's content (helps when reviewing markdown lists where unindented continuation can be misread as a new bullet) | `0` |
| `--page-overlap` | `REVDIFF_PAGE_OVERLAP` | Keep N lines from the previous screen when paging the diff | `0` |
| `--start-at-change` | `REVDIFF_START_AT_CHANGE` | Position the cursor on the first changed line | `false` |
| `--collapsed` | `REVDIFF_COLLAPSED` | Start in collapsed diff mode | `false` |
| `--compact` | `REVDIFF_COMPACT` | Start in compact diff mode (small context around changes) | `false` |
| `--compact-context` | `REVDIFF_COMPACT_CONTEXT` | Number of context lines around changes when in compact mode | `5` |
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,7 @@ Positional arguments support several forms:
| `--compact` | Start in compact diff mode (small context around changes), env: `REVDIFF_COMPACT` | `false` |
| `--compact-context` | Number of context lines around changes when in compact mode, env: `REVDIFF_COMPACT_CONTEXT` | `5` |
| `--cross-file-hunks` | Allow `[` and `]` to continue into adjacent files, env: `REVDIFF_CROSS_FILE_HUNKS` | `false` |
| `--start-at-change` | Position the cursor on the first changed line, env: `REVDIFF_START_AT_CHANGE` | `false` |
| `--line-numbers` | Show line numbers in diff gutter, env: `REVDIFF_LINE_NUMBERS` | `false` |
| `--blame` | Show blame gutter, env: `REVDIFF_BLAME` | `false` |
| `--word-diff` | Highlight intra-line word-level changes in paired add/remove lines, env: `REVDIFF_WORD_DIFF` | `false` |
Expand Down
1 change: 1 addition & 0 deletions app/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ type options struct {
Compact bool `long:"compact" ini-name:"compact" env:"REVDIFF_COMPACT" description:"start in compact diff mode (small context around changes)"`
CompactContext int `long:"compact-context" ini-name:"compact-context" env:"REVDIFF_COMPACT_CONTEXT" default:"5" description:"number of context lines around changes when in compact mode"`
CrossFileHunks bool `long:"cross-file-hunks" ini-name:"cross-file-hunks" env:"REVDIFF_CROSS_FILE_HUNKS" description:"allow [ and ] to jump across file boundaries"`
StartAtChange bool `long:"start-at-change" ini-name:"start-at-change" env:"REVDIFF_START_AT_CHANGE" description:"position the cursor on the first changed line"`
LineNumbers bool `long:"line-numbers" ini-name:"line-numbers" env:"REVDIFF_LINE_NUMBERS" description:"show line numbers in diff gutter"`
Blame bool `long:"blame" ini-name:"blame" env:"REVDIFF_BLAME" description:"show blame gutter"`
WordDiff bool `long:"word-diff" ini-name:"word-diff" env:"REVDIFF_WORD_DIFF" description:"highlight intra-line word-level changes in paired add/remove lines"`
Expand Down
26 changes: 26 additions & 0 deletions app/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ func TestParseArgs_Defaults(t *testing.T) {
assert.False(t, opts.Compact)
assert.Equal(t, 5, opts.CompactContext)
assert.False(t, opts.CrossFileHunks)
assert.False(t, opts.StartAtChange)
assert.False(t, opts.LineNumbers)
assert.False(t, opts.Blame)
assert.False(t, opts.ExitCodeOnAnnotations)
Expand Down Expand Up @@ -371,6 +372,31 @@ func TestParseArgs_CrossFileHunks(t *testing.T) {
})
}

func TestParseArgs_StartAtChange(t *testing.T) {
t.Run("flag", func(t *testing.T) {
opts, err := parseArgs(append(noConfigArgs(t), "--start-at-change"))
require.NoError(t, err)
assert.True(t, opts.StartAtChange)
})

t.Run("env", func(t *testing.T) {
t.Setenv("REVDIFF_START_AT_CHANGE", "true")
opts, err := parseArgs(noConfigArgs(t))
require.NoError(t, err)
assert.True(t, opts.StartAtChange)
})

t.Run("config file", func(t *testing.T) {
cfgDir := t.TempDir()
cfgPath := filepath.Join(cfgDir, "config")
err := os.WriteFile(cfgPath, []byte("[Application Options]\nstart-at-change = true\n"), 0o600)
require.NoError(t, err)
opts, err := parseArgs([]string{"--config", cfgPath})
require.NoError(t, err)
assert.True(t, opts.StartAtChange)
})
}

func TestParseArgs_LineNumbers(t *testing.T) {
t.Run("flag", func(t *testing.T) {
opts, err := parseArgs(append(noConfigArgs(t), "--line-numbers"))
Expand Down
1 change: 1 addition & 0 deletions app/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ func run(opts options) (int, error) {
Compact: opts.Compact,
CompactContext: opts.CompactContext,
CrossFileHunks: opts.CrossFileHunks,
StartAtChange: opts.StartAtChange,
LineNumbers: opts.LineNumbers,
ShowBlame: opts.Blame,
ShowUntracked: opts.startupUntracked(),
Expand Down
23 changes: 17 additions & 6 deletions app/ui/diffnav.go
Original file line number Diff line number Diff line change
Expand Up @@ -901,17 +901,28 @@ func (m *Model) syncTOCActiveSection() {
}
}

// positionOnFirstChange puts the cursor on the first changed line, falling back to the first visible
// line when the file carries no hunks at all (context-only sources). in collapsed mode it lands on
// the delete-only placeholder rather than skipping the hunk, since that head line stays visible.
//
// this only positions the cursor: a caller loading a new file MUST follow it with
// centerViewportOnCursor and must not drop that call as a duplicate render. moveToNextHunk scrolls
// via centerHunkInViewport, which sets the offset before rendering, so the offset clamps against the
// previously loaded file's length; on the no-hunk path nothing renders at all.
func (m *Model) positionOnFirstChange() {
m.nav.diffCursor = -1
m.moveToNextHunk()
if m.nav.diffCursor == -1 {
m.skipInitialDividers()
}
}

// applyPendingHunkJump moves the cursor to the first or last hunk after a cross-file navigation.
func (m *Model) applyPendingHunkJump() {
forward := *m.nav.pendingHunkJump
m.nav.pendingHunkJump = nil
if forward {
m.nav.diffCursor = -1
m.moveToNextHunk()
if m.nav.diffCursor != -1 {
return
}
m.skipInitialDividers()
m.positionOnFirstChange()
return
}

Expand Down
11 changes: 10 additions & 1 deletion app/ui/loaders.go
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,8 @@ func (m Model) loadSelectedIfChanged() (tea.Model, tea.Cmd) {
// overlay shows loading state (not stale data) while the re-fetch is in
// flight, then re-runs the same parallel pipeline as startup via tea.Batch.
// The selected file in the tree is restored by SelectByPath in
// handleFilesLoaded; the diff cursor resets to the top of the file. Named
// handleFilesLoaded; the diff cursor resets to the top of the file, or to its
// first change when start-at-change is enabled. Named
// triggerReload (not reload) to avoid shadowing the Model.reload field.
func (m *Model) triggerReload() tea.Cmd {
m.filesLoadSeq++
Expand Down Expand Up @@ -604,6 +605,14 @@ func (m Model) handleFileLoaded(msg fileLoadedMsg) (tea.Model, tea.Cmd) {
}
}

// sits below the three jump branches so an explicit jump target always wins, and must return
// early because the GotoTop below would undo the scroll.
if m.cfg.startAtChange {
m.positionOnFirstChange()
m.centerViewportOnCursor()
return m, blameCmd
}

m.layout.viewport.SetContent(m.renderDiff())
m.layout.viewport.GotoTop()
return m, blameCmd
Expand Down
206 changes: 206 additions & 0 deletions app/ui/loaders_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2083,3 +2083,209 @@ func TestModel_FilesReloadPreservesVisibleRowWhenFilesAboveChange(t *testing.T)
require.True(t, m.tree.SelectByVisibleRow(2))
require.Equal(t, "f.go", m.tree.SelectedFile(), "reload should preserve the row when files above disappear")
}

func TestModel_HandleFileLoaded_StartAtChange(t *testing.T) {
withChange := []diff.DiffLine{
{ChangeType: diff.ChangeDivider},
{OldNum: 40, NewNum: 40, Content: "ctx", ChangeType: diff.ChangeContext},
{OldNum: 41, NewNum: 41, Content: "ctx", ChangeType: diff.ChangeContext},
{NewNum: 42, Content: "add", ChangeType: diff.ChangeAdd},
{OldNum: 42, NewNum: 43, Content: "ctx", ChangeType: diff.ChangeContext},
}

load := func(t *testing.T, on bool, lines []diff.DiffLine, prep func(m *Model)) Model {
t.Helper()
m := testModel([]string{"a.go"}, nil)
m.file.name = "a.go"
m.cfg.startAtChange = on
if prep != nil {
prep(&m)
}
result, _ := m.handleFileLoaded(fileLoadedMsg{file: "a.go", seq: m.file.loadSeq, lines: lines})
return result.(Model)
}

t.Run("off keeps the first visible line", func(t *testing.T) {
assert.Equal(t, 1, load(t, false, withChange, nil).nav.diffCursor)
})

t.Run("on lands on the first changed line", func(t *testing.T) {
assert.Equal(t, 3, load(t, true, withChange, nil).nav.diffCursor)
})

t.Run("context-only file falls back to the first visible line", func(t *testing.T) {
contextOnly := []diff.DiffLine{
{ChangeType: diff.ChangeDivider},
{OldNum: 1, NewNum: 1, Content: "ctx", ChangeType: diff.ChangeContext},
{OldNum: 2, NewNum: 2, Content: "ctx", ChangeType: diff.ChangeContext},
}
assert.Equal(t, 1, load(t, true, contextOnly, nil).nav.diffCursor)
})

t.Run("empty diff leaves the cursor at zero", func(t *testing.T) {
assert.Equal(t, 0, load(t, true, nil, nil).nav.diffCursor)
})

t.Run("collapsed delete-only hunk keeps its placeholder", func(t *testing.T) {
deleteFirst := []diff.DiffLine{
{OldNum: 1, NewNum: 1, Content: "ctx", ChangeType: diff.ChangeContext},
{OldNum: 2, Content: "gone", ChangeType: diff.ChangeRemove},
{OldNum: 3, NewNum: 2, Content: "ctx", ChangeType: diff.ChangeContext},
{NewNum: 3, Content: "add", ChangeType: diff.ChangeAdd},
}
m := load(t, true, deleteFirst, func(m *Model) { m.modes.collapsed.enabled = true })
assert.Equal(t, 1, m.nav.diffCursor)
})

t.Run("pending hunk jump wins", func(t *testing.T) {
back := false
m := load(t, true, withChange, func(m *Model) { m.nav.pendingHunkJump = &back })
assert.Equal(t, 3, m.nav.diffCursor)
assert.Nil(t, m.nav.pendingHunkJump)
})

t.Run("compact anchor wins", func(t *testing.T) {
m := load(t, true, withChange, func(m *Model) {
m.compact.pendingAnchor = &compactAnchor{seq: m.file.loadSeq, srcLine: 41, changeType: diff.ChangeContext, hunkIdx: 0}
})
assert.Equal(t, 2, m.nav.diffCursor, "anchor restores the pre-toggle line, overriding start-at-change")
})

t.Run("reapplies on every subsequent file load", func(t *testing.T) {
m := load(t, true, withChange, nil)
require.Equal(t, 3, m.nav.diffCursor)
m.file.name = "b.go"
m.file.loadSeq++
result, _ := m.handleFileLoaded(fileLoadedMsg{file: "b.go", seq: m.file.loadSeq, lines: withChange})
assert.Equal(t, 3, result.(Model).nav.diffCursor, "switching files positions again, not once per session")
})

t.Run("annotation jump wins", func(t *testing.T) {
m := load(t, true, withChange, func(m *Model) {
m.pendingAnnotJump = &annotation.Annotation{File: "a.go", Line: 41, Type: string(diff.ChangeContext)}
})
assert.Equal(t, 2, m.nav.diffCursor, "annotation target overrides start-at-change")
assert.Nil(t, m.pendingAnnotJump)
})

// fileLoadedMsg can arrive before the first WindowSizeMsg; the change must still be on
// screen once the resize lands, not merely selected somewhere far below the fold
t.Run("change is visible when the window size arrives after the load", func(t *testing.T) {
lines := make([]diff.DiffLine, 0, 401)
for i := 1; i <= 400; i++ {
lines = append(lines, diff.DiffLine{OldNum: i, NewNum: i, Content: "ctx", ChangeType: diff.ChangeContext})
}
lines = append(lines, diff.DiffLine{NewNum: 401, Content: "add", ChangeType: diff.ChangeAdd})

m := testModel([]string{"a.go"}, nil)
m.file.name = "a.go"
m.cfg.startAtChange = true
m.ready = false
m.layout.viewport.Height = 0

result, _ := m.handleFileLoaded(fileLoadedMsg{file: "a.go", seq: m.file.loadSeq, lines: lines})
m = result.(Model)
require.Equal(t, 400, m.nav.diffCursor, "cursor lands on the change with no viewport height yet")

result, _ = m.Update(tea.WindowSizeMsg{Width: 120, Height: 40})
m = result.(Model)

cursorY, top, height := m.cursorViewportY(), m.layout.viewport.YOffset, m.layout.viewport.Height
require.Positive(t, height)
assert.GreaterOrEqual(t, cursorY, top, "change must not sit above the visible window")
assert.Less(t, cursorY, top+height, "change must not sit below the visible window")
})
}

// pins the centerViewportOnCursor call in the start-at-change branch of handleFileLoaded. it is the
// only render on the no-hunk path, and on the hunk path it is what re-applies an offset that
// centerHunkInViewport clamped against the previous file's length. deleting it as a duplicate render
// leaves the pane painting the old file.
func TestModel_StartAtChange_RendersTheLoadedFile(t *testing.T) {
short := []diff.DiffLine{{OldNum: 1, NewNum: 1, Content: "short file", ChangeType: diff.ChangeContext}}

// testModel leaves the viewport zero-sized, and a zero-height viewport renders "" — which would
// make every assertion below pass against an unrendered pane
newModel := func(t *testing.T, files []string) Model {
t.Helper()
m := testModel(files, nil)
m.cfg.startAtChange = true
m.layout.viewport.Width, m.layout.viewport.Height = 80, 20
return m
}

load := func(t *testing.T, m Model, file string, lines []diff.DiffLine) Model {
t.Helper()
m.file.name = file
m.file.loadSeq++
result, _ := m.handleFileLoaded(fileLoadedMsg{file: file, seq: m.file.loadSeq, lines: lines})
return result.(Model)
}

t.Run("context-only file replaces the previous file's content", func(t *testing.T) {
m := newModel(t, []string{"a.go", "b.md"})
m = load(t, m, "a.go", []diff.DiffLine{
{OldNum: 1, NewNum: 1, Content: "first file only", ChangeType: diff.ChangeContext},
})

m = load(t, m, "b.md", []diff.DiffLine{
{OldNum: 1, NewNum: 1, Content: "second file only", ChangeType: diff.ChangeContext},
})

painted := m.layout.viewport.View()
assert.Contains(t, painted, "second file only", "the pane must paint the file that just loaded")
assert.NotContains(t, painted, "first file only", "stale content from the previous file must be gone")
})

t.Run("offset survives a short previous file", func(t *testing.T) {
lines := make([]diff.DiffLine, 0, 401)
for i := 1; i <= 400; i++ {
lines = append(lines, diff.DiffLine{OldNum: i, NewNum: i, Content: "ctx", ChangeType: diff.ChangeContext})
}
lines = append(lines, diff.DiffLine{NewNum: 401, Content: "the change", ChangeType: diff.ChangeAdd})

m := newModel(t, []string{"a.go", "b.go"})
m = load(t, m, "a.go", short)
require.Zero(t, m.layout.viewport.YOffset, "the short file leaves the viewport at the top")

m = load(t, m, "b.go", lines)

require.Equal(t, 400, m.nav.diffCursor)
assert.Positive(t, m.layout.viewport.YOffset,
"offset must be re-applied after the new content is installed, not clamped against the short file")
assert.Contains(t, m.layout.viewport.View(), "the change", "the change must actually be on screen")
})
}

// a markdown TOC is built only for full-context files, which by definition carry no hunks, so
// start-at-change cannot move the cursor out from under the TOC's active section.
func TestModel_StartAtChange_MarkdownTOCUnaffected(t *testing.T) {
load := func(t *testing.T, lines []diff.DiffLine) Model {
t.Helper()
m := testModel([]string{"a.md"}, nil)
m.file.name = "a.md"
m.file.singleFile = true
m.cfg.startAtChange = true
result, _ := m.handleFileLoaded(fileLoadedMsg{file: "a.md", seq: m.file.loadSeq, lines: lines})
return result.(Model)
}

t.Run("full-context markdown keeps its TOC and its top cursor", func(t *testing.T) {
m := load(t, []diff.DiffLine{
{OldNum: 1, NewNum: 1, Content: "# One", ChangeType: diff.ChangeContext},
{OldNum: 2, NewNum: 2, Content: "body", ChangeType: diff.ChangeContext},
{OldNum: 3, NewNum: 3, Content: "## Two", ChangeType: diff.ChangeContext},
})
require.NotNil(t, m.file.mdTOC, "full-context markdown must still build a TOC")
assert.Equal(t, 0, m.nav.diffCursor, "no hunks means the fallback keeps the top position")
})

t.Run("changed markdown builds no TOC", func(t *testing.T) {
m := load(t, []diff.DiffLine{
{OldNum: 1, NewNum: 1, Content: "# One", ChangeType: diff.ChangeContext},
{NewNum: 2, Content: "## Two", ChangeType: diff.ChangeAdd},
})
assert.Nil(t, m.file.mdTOC, "a diff with changes is not full-context, so no TOC exists to go stale")
assert.Equal(t, 1, m.nav.diffCursor)
})
}
3 changes: 3 additions & 0 deletions app/ui/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ type modelConfigState struct {
noConfirmDiscard bool // skip confirmation prompt on discard quit
noConfirmReload bool // skip confirmation prompt on reload (R)
crossFileHunks bool // allow [ and ] to jump across file boundaries
startAtChange bool // put the cursor on the first changed line when a file loads
treeWidthRatio int // 1-10 units for file tree panel
tabSpaces string // spaces to replace tabs with
wrapIndent int // extra indent (in columns) for wrap continuation rows; 0 disables
Expand Down Expand Up @@ -747,6 +748,7 @@ type ModelConfig struct {
PageOverlap int // rows carried over from the previous screen on page up/down; 0 disables
Collapsed bool // start in collapsed diff mode
CrossFileHunks bool // allow [ and ] to jump across file boundaries
StartAtChange bool // put the cursor on the first changed line when a file loads
LineNumbers bool // show line numbers in diff gutter
ShowBlame bool // show blame gutter; requires Blamer
ShowUntracked bool // show untracked files in the tree; requires LoadUntracked
Expand Down Expand Up @@ -903,6 +905,7 @@ func NewModel(cfg ModelConfig) (Model, error) {
noConfirmDiscard: cfg.NoConfirmDiscard,
noConfirmReload: cfg.NoConfirmReload,
crossFileHunks: cfg.CrossFileHunks,
startAtChange: cfg.StartAtChange,
treeWidthRatio: cfg.TreeWidthRatio,
tabSpaces: strings.Repeat(" ", cfg.TabWidth),
wrapIndent: max(0, cfg.WrapIndent),
Expand Down
2 changes: 2 additions & 0 deletions plugins/codex/skills/revdiff/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,8 @@ When you are launching revdiff for the user (e.g., right after a refactor or ana

**When the recent change likely created new untracked files** (new packages, new test files, new docs, new scripts that haven't been `git add`-ed yet), pass `--untracked` so those files appear in the tree. Use this in working-tree mode (no ref, no `--staged`); skip it for ref-to-ref reviews where untracked files are not part of the historical diff.

Pass `--start-at-change` only when the user explicitly asks for that cursor preference; never infer it automatically.

Run the launcher script:

```bash
Expand Down
1 change: 1 addition & 0 deletions plugins/codex/skills/revdiff/references/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ Then uncomment and edit the values you want to change.
| `--wrap` | `REVDIFF_WRAP` | Enable line wrapping in diff view | `false` |
| `--wrap-indent` | `REVDIFF_WRAP_INDENT` | Indent wrap continuation rows by N columns so they hang under the first row's content (helps when reviewing markdown lists where unindented continuation can be misread as a new bullet) | `0` |
| `--page-overlap` | `REVDIFF_PAGE_OVERLAP` | Keep N lines from the previous screen when paging the diff | `0` |
| `--start-at-change` | `REVDIFF_START_AT_CHANGE` | Position the cursor on the first changed line | `false` |
| `--collapsed` | `REVDIFF_COLLAPSED` | Start in collapsed diff mode | `false` |
| `--compact` | `REVDIFF_COMPACT` | Start in compact diff mode (small context around changes) | `false` |
| `--compact-context` | `REVDIFF_COMPACT_CONTEXT` | Number of context lines around changes when in compact mode | `5` |
Expand Down
Loading