Skip to content

RT-298: scroll through a diff line taller than the pane row by row - #459

Merged
m4ttheweric merged 2 commits into
mainfrom
rt-298-tall-line-scroll
Sep 25, 2026
Merged

m4ttheweric merged 2 commits into
mainfrom
rt-298-tall-line-scroll

Conversation

@m4ttheweric

@m4ttheweric m4ttheweric commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

RT-298. glitter's diff pane soft-wraps, but a changed line that wrapped to more rows than the pane (minified JS, one-line JSON) pinned its first row on top. Its middle and end could not be reached.

What changed

  • moveDiffCursor takes one step at a time: on a cursor line taller than the pane, a step scrolls it one row until its far end is on screen, then the cursor moves to the next line
  • a tall line entered from below opens at its bottom, so reading upward never jumps
  • the wheel goes through the same steps, so a tick spends its rows inside the line first and carries the rest onto the next lines
  • the offset into a tall cursor line (diffRowOff) belongs to the line it was set on: a stage keeps the rows being read, a resize keeps the place within the line, and a different line landing under the cursor (a discard, the same path in another commit) opens at its first row
  • tallCursorSpan reads the shared diffRowIndex and the last pane height, so the index stays the one row map for render, viewport and diffHit
  • staging, clicks and space/s/d still act on the whole line

Verification

  • Go tests: stepping down row by row then on to the next line, entering from below at the bottom, a wheel tick spilling past the line's end, a render keeping a mid-line top; the old first-row-pinned test rewritten for the new behaviour. go test ./... and go vet ./... clean
  • review (Opus, standing in for rate-limited CodeRabbit): an absolute diffTop carried a mid-line position onto a different tall line and lost the place on resize; fixed and pinned by tests that fail on the absolute version
  • live in glitter (130x30, widened to 160 mid-line keeping the 5-row offset, a 90-function one-line app.min.js): down steps one row at a time through the line and then onto const b/tail; up from below opens the line at END-OF-LONG-LINE; two wheel ticks move 6 rows; toggling the stage mid-line keeps the view

🤖 Generated with Claude Code

A changed line that wrapped to more rows than the diff pane pinned its
first row on top, so its middle and end were unreachable. A cursor step
on such a line now scrolls it one row until its far end is on screen,
then moves to the next line; a line entered from below opens at its
bottom. The wheel takes the same steps. The viewport clamps diffTop into
the tall line's range, so a click or a stage keeps the rows being read.
Staging and clicks still act on the whole line.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Note

Reviews paused

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@m4ttheweric

Copy link
Copy Markdown
Collaborator Author

@coderabbitai pause

@coderabbitai

coderabbitai Bot commented Sep 25, 2026

Copy link
Copy Markdown
✅ Action performed

Reviews paused.

Review of #459: an absolute diffTop carried a mid-line position onto a
different tall line that a discard shifted under the cursor, and a
resize re-wrapping the lines above jumped the reader to the line's top
or bottom. The offset into the cursor line is now held with the line it
belongs to, so a new line opens at its first row and a resize keeps the
place within the line.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@m4ttheweric
m4ttheweric merged commit 7e90abf into main Sep 25, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant