diff --git a/.claude-plugin/skills/revdiff/references/config.md b/.claude-plugin/skills/revdiff/references/config.md index b4f41817..6f0c8f79 100644 --- a/.claude-plugin/skills/revdiff/references/config.md +++ b/.claude-plugin/skills/revdiff/references/config.md @@ -26,6 +26,7 @@ Then uncomment and edit the values you want to change. | `--no-status-bar` | `REVDIFF_NO_STATUS_BAR` | Hide the status bar | `false` | | `--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` | | `--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` | diff --git a/README.md b/README.md index e16d9de1..75dfec65 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,7 @@ Built for a specific use case: reviewing code changes, plans, and documents with - Intra-line word-diff: highlights the specific changed words within paired add/remove lines using a brighter background overlay, off by default — enable with `--word-diff` or toggle with `W` - Collapsed diff mode: shows final text with change markers, toggle with `v` - Word wrap mode: wraps long lines at viewport boundary with `↪` continuation markers, toggle with `w`; optional `--wrap-indent N` for hanging-indent continuations (handy for markdown lists) +- Page scroll overlap: `--page-overlap N` carries the bottom N lines of the screen to the top of the next one on PgUp/PgDn, so the seam between screens keeps context; the carryover is approximate on wrapped or annotated lines, which occupy several rows but are a single cursor stop - Horizontal scroll overflow indicators: truncated diff lines show `«` / `»` markers at the edges to signal hidden content off-screen - Vertical scrollbar thumb: a thicker `┃` segment on pane right borders indicates the visible portion of long diffs, file trees, and markdown TOCs; thumb size and position track scroll progress automatically - Line numbers: side-by-side old/new line number gutter for diffs, single column for full-context files, toggle with `L` @@ -373,6 +374,7 @@ Positional arguments support several forms: | `--no-status-bar` | Hide the status bar, env: `REVDIFF_NO_STATUS_BAR` | `false` | | `--wrap` | Enable line wrapping in diff view, env: `REVDIFF_WRAP` | `false` | | `--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), env: `REVDIFF_WRAP_INDENT` | `0` | +| `--page-overlap` | Keep N lines from the previous screen when paging the diff, env: `REVDIFF_PAGE_OVERLAP` | `0` | | `--collapsed` | Start in collapsed diff mode, env: `REVDIFF_COLLAPSED` | `false` | | `--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` | diff --git a/app/config.go b/app/config.go index 54a0cb30..9b2c5f8f 100644 --- a/app/config.go +++ b/app/config.go @@ -29,6 +29,7 @@ type options struct { NoTree bool `long:"no-tree" ini-name:"no-tree" env:"REVDIFF_NO_TREE" description:"hide the file tree pane"` Wrap bool `long:"wrap" ini-name:"wrap" env:"REVDIFF_WRAP" description:"enable line wrapping in diff view"` WrapIndent int `long:"wrap-indent" ini-name:"wrap-indent" env:"REVDIFF_WRAP_INDENT" default:"0" description:"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)"` + PageOverlap int `long:"page-overlap" ini-name:"page-overlap" env:"REVDIFF_PAGE_OVERLAP" default:"0" description:"keep N lines from the previous screen when paging the diff"` Collapsed bool `long:"collapsed" ini-name:"collapsed" env:"REVDIFF_COLLAPSED" description:"start in collapsed diff mode"` 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"` diff --git a/app/config_test.go b/app/config_test.go index 2d355ce0..ff39b5d5 100644 --- a/app/config_test.go +++ b/app/config_test.go @@ -149,6 +149,47 @@ func TestParseArgs_NoTree(t *testing.T) { }) } +func TestParseArgs_PageOverlap(t *testing.T) { + t.Run("default is zero", func(t *testing.T) { + opts, err := parseArgs(noConfigArgs(t)) + require.NoError(t, err) + assert.Equal(t, 0, opts.PageOverlap) + }) + + t.Run("flag", func(t *testing.T) { + opts, err := parseArgs(append(noConfigArgs(t), "--page-overlap", "2")) + require.NoError(t, err) + assert.Equal(t, 2, opts.PageOverlap) + }) + + t.Run("env", func(t *testing.T) { + t.Setenv("REVDIFF_PAGE_OVERLAP", "3") + opts, err := parseArgs(noConfigArgs(t)) + require.NoError(t, err) + assert.Equal(t, 3, opts.PageOverlap) + }) + + t.Run("config file", func(t *testing.T) { + cfgDir := t.TempDir() + cfgPath := filepath.Join(cfgDir, "config") + err := os.WriteFile(cfgPath, []byte("[Application Options]\npage-overlap = 4\n"), 0o600) + require.NoError(t, err) + opts, err := parseArgs([]string{"--config", cfgPath}) + require.NoError(t, err) + assert.Equal(t, 4, opts.PageOverlap) + }) + + t.Run("flag overrides config file", func(t *testing.T) { + cfgDir := t.TempDir() + cfgPath := filepath.Join(cfgDir, "config") + err := os.WriteFile(cfgPath, []byte("[Application Options]\npage-overlap = 4\n"), 0o600) + require.NoError(t, err) + opts, err := parseArgs([]string{"--config", cfgPath, "--page-overlap", "1"}) + require.NoError(t, err) + assert.Equal(t, 1, opts.PageOverlap) + }) +} + func TestParseArgs_Wrap(t *testing.T) { t.Run("flag", func(t *testing.T) { opts, err := parseArgs(append(noConfigArgs(t), "--wrap")) diff --git a/app/main.go b/app/main.go index d58abbbc..007979f3 100644 --- a/app/main.go +++ b/app/main.go @@ -214,6 +214,7 @@ func run(opts options) (int, error) { NoTree: opts.NoTree, Wrap: opts.Wrap, WrapIndent: opts.WrapIndent, + PageOverlap: opts.PageOverlap, Collapsed: opts.Collapsed, Compact: opts.Compact, CompactContext: opts.CompactContext, diff --git a/app/ui/diffnav.go b/app/ui/diffnav.go index d061cd7e..fc308064 100644 --- a/app/ui/diffnav.go +++ b/app/ui/diffnav.go @@ -114,14 +114,25 @@ func (m *Model) moveDiffCursorUpWithHunks(hunks []int) { // keeps the cursor's relative screen position stable by scrolling both // cursor and viewport by the same amount. func (m *Model) moveDiffCursorPageDown() { - m.moveDiffCursorDownBy(m.layout.viewport.Height) + m.moveDiffCursorDownBy(m.pageRows()) } // moveDiffCursorPageUp moves the diff cursor up by one visual page. // keeps the cursor's relative screen position stable by scrolling both // cursor and viewport by the same amount. func (m *Model) moveDiffCursorPageUp() { - m.moveDiffCursorUpBy(m.layout.viewport.Height) + m.moveDiffCursorUpBy(m.pageRows()) +} + +// pageRows returns how far a full-page motion advances, one screen less the +// configured overlap. the overlap is approximate rather than exact, and deviates in both +// directions: the walk stops on cursor positions and one position can span several rendered +// rows (a wrapped line, an annotation block), so a tall line at the page edge carries over +// more than requested when the walk rolls back off it, and less than requested - down to +// rows skipped unseen - when worthRollingBack accepts it whole. +// half-page motions do not subtract it - they already retain half a screen. +func (m Model) pageRows() int { + return max(1, m.layout.viewport.Height-m.modes.pageOverlap) } // moveDiffCursorHalfPageDown moves the diff cursor down by half a visual page. diff --git a/app/ui/diffnav_test.go b/app/ui/diffnav_test.go index b53ee44c..9b34a6f4 100644 --- a/app/ui/diffnav_test.go +++ b/app/ui/diffnav_test.go @@ -510,6 +510,70 @@ func TestModel_PgDownPgUpPreservesRelativeCursorPosition(t *testing.T) { }) } +func TestModel_PageOverlapCarriesRowsAcrossPages(t *testing.T) { + lines := make([]diff.DiffLine, 200) + for i := range lines { + lines[i] = diff.DiffLine{NewNum: i + 1, Content: "line", ChangeType: diff.ChangeAdd} + } + + newModel := func(overlap int) Model { + m := testModel([]string{"a.go"}, map[string][]diff.DiffLine{"a.go": lines}) + m.modes.pageOverlap = overlap + result, _ := m.Update(tea.WindowSizeMsg{Width: 120, Height: 24}) + model := result.(Model) + result, _ = model.Update(fileLoadedMsg{file: "a.go", lines: lines}) + model = result.(Model) + model.layout.focus = paneDiff + return model + } + + t.Run("zero overlap advances a full page", func(t *testing.T) { + model := newModel(0) + pageHeight := model.layout.viewport.Height + result, _ := model.Update(tea.KeyMsg{Type: tea.KeyPgDown}) + model = result.(Model) + assert.Equal(t, pageHeight, model.layout.viewport.YOffset) + }) + + t.Run("overlap keeps N rows on screen", func(t *testing.T) { + model := newModel(2) + pageHeight := model.layout.viewport.Height + result, _ := model.Update(tea.KeyMsg{Type: tea.KeyPgDown}) + model = result.(Model) + assert.Equal(t, pageHeight-2, model.layout.viewport.YOffset, + "the last 2 rows of the previous screen must be the first 2 of the new one") + }) + + t.Run("overlap applies to pgup as well", func(t *testing.T) { + model := newModel(2) + pageHeight := model.layout.viewport.Height + for range 2 { + result, _ := model.Update(tea.KeyMsg{Type: tea.KeyPgDown}) + model = result.(Model) + } + downOffset := model.layout.viewport.YOffset + result, _ := model.Update(tea.KeyMsg{Type: tea.KeyPgUp}) + model = result.(Model) + assert.Equal(t, downOffset-(pageHeight-2), model.layout.viewport.YOffset) + }) + + t.Run("half page motions ignore the overlap", func(t *testing.T) { + model := newModel(2) + pageHeight := model.layout.viewport.Height + result, _ := model.Update(tea.KeyMsg{Type: tea.KeyCtrlD}) + model = result.(Model) + assert.Equal(t, pageHeight/2, model.layout.viewport.YOffset, + "ctrl+d already retains half a screen, so the overlap must not shrink it further") + }) + + t.Run("overlap wider than the pane still advances", func(t *testing.T) { + model := newModel(1000) + result, _ := model.Update(tea.KeyMsg{Type: tea.KeyPgDown}) + model = result.(Model) + assert.Positive(t, model.layout.viewport.YOffset, "paging must never stall on an oversized overlap") + }) +} + // a line taller than the remaining page budget must not make paging scroll past unseen rows: // the walk used to step onto it and then shift the viewport by the cursor's whole visual delta, // skipping the tail of an annotation that was never rendered. diff --git a/app/ui/model.go b/app/ui/model.go index 6195eb40..d168c7aa 100644 --- a/app/ui/model.go +++ b/app/ui/model.go @@ -356,6 +356,7 @@ type modeState struct { showUntracked bool // true when untracked files are shown in tree compact bool // true when diffs are fetched with small context around changes compactContext int // number of context lines around changes when compact is enabled + pageOverlap int // rows carried over from the previous screen on page up/down; 0 disables vimMotion bool // true when the --vim-motion preset is active (gates the vim-motion interceptor in handleKey) } @@ -743,6 +744,7 @@ type ModelConfig struct { NoConfirmReload bool // skip confirmation prompt when dropping annotations on reload Wrap bool // enable line wrapping WrapIndent int // extra indent (cols) for wrap continuation rows; 0 disables + 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 LineNumbers bool // show line numbers in diff gutter @@ -921,6 +923,7 @@ func NewModel(cfg ModelConfig) (Model, error) { showUntracked: cfg.ShowUntracked && cfg.LoadUntracked != nil, compact: cfg.Compact && cfg.CompactApplicable, compactContext: cfg.CompactContext, + pageOverlap: max(0, cfg.PageOverlap), vimMotion: cfg.VimMotion, }, commits: commitsState{ diff --git a/app/ui/model_test.go b/app/ui/model_test.go index 9b27fc63..37d3f8ee 100644 --- a/app/ui/model_test.go +++ b/app/ui/model_test.go @@ -1738,6 +1738,29 @@ func TestModel_AnnotatingNoOpMessageDoesNotRerenderDiff(t *testing.T) { "blink left the input value untouched, so the diff must not be re-rendered") } +func TestNewModel_PageOverlap(t *testing.T) { + renderer := &mocks.RendererMock{ + ChangedFilesFunc: func(string, bool) ([]diff.FileEntry, error) { return nil, nil }, + FileDiffFunc: func(diff.FileDiffRequest) ([]diff.DiffLine, error) { return nil, nil }, + } + newModel := func(overlap int) Model { + return testNewModel(t, renderer, annotation.NewStore(), noopHighlighter(), + ModelConfig{PageOverlap: overlap, TreeWidthRatio: 3}) + } + + t.Run("default is no overlap", func(t *testing.T) { + assert.Equal(t, 0, newModel(0).modes.pageOverlap) + }) + + t.Run("configured value reaches mode state", func(t *testing.T) { + assert.Equal(t, 2, newModel(2).modes.pageOverlap) + }) + + t.Run("negative clamps to zero", func(t *testing.T) { + assert.Equal(t, 0, newModel(-5).modes.pageOverlap) + }) +} + func TestNewModel_NoTree(t *testing.T) { renderer := &mocks.RendererMock{ ChangedFilesFunc: func(string, bool) ([]diff.FileEntry, error) { return nil, nil }, diff --git a/plugins/codex/skills/revdiff/references/config.md b/plugins/codex/skills/revdiff/references/config.md index 2787b988..a9779fd8 100644 --- a/plugins/codex/skills/revdiff/references/config.md +++ b/plugins/codex/skills/revdiff/references/config.md @@ -26,6 +26,7 @@ Then uncomment and edit the values you want to change. | `--no-status-bar` | `REVDIFF_NO_STATUS_BAR` | Hide the status bar | `false` | | `--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` | | `--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` | diff --git a/plugins/pi/skills/revdiff/SKILL.md b/plugins/pi/skills/revdiff/SKILL.md index bf377ea5..3ca6c504 100644 --- a/plugins/pi/skills/revdiff/SKILL.md +++ b/plugins/pi/skills/revdiff/SKILL.md @@ -35,6 +35,7 @@ Tool examples: - `args: "--only README.md"`: review one standalone file - `args: "--all-files --exclude vendor"`: review all tracked files except vendor - `args: "--no-tree"`: review with the file tree pane hidden +- `args: "--page-overlap=2"`: keep 2 lines from the previous screen when paging - `args: "--description='why this refactor matters' main"`: include review context in the info popup - `args: "--description-file=/tmp/revdiff-desc.md main"`: include longer markdown review context - `args: "--annotations=/tmp/revdiff-review.md main"`: preload in-session review notes @@ -77,6 +78,7 @@ When annotations arrive from `/revdiff` or `revdiff_review`: /revdiff --all-files --exclude vendor /revdiff --only README.md /revdiff --no-tree +/revdiff --page-overlap=2 /revdiff HEAD~3 --description="why this refactor matters" /revdiff HEAD~3 --description-file=/tmp/revdiff-desc.md /revdiff main --annotations=/tmp/revdiff-review.md diff --git a/site/docs.html b/site/docs.html index a9e16414..011489f9 100644 --- a/site/docs.html +++ b/site/docs.html @@ -401,6 +401,7 @@
--no-status-barfalse--wrapfalse--wrap-indent0--page-overlap0--collapsedfalse--compactfalse--compact-context5