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

ui/internal/views/mission/diff.go

@@ -100,18 +100,80 @@ func (m *Mission) clampDiffCursor() {
 	}
 }
 
+// moveDiffCursor takes |delta| steps. A step on a cursor line taller than
+// the pane scrolls it one row until its far end is on screen; only then
+// does the cursor move to the next line, and a line entered from below
+// opens at its bottom so reading upward never jumps.
 func (m *Mission) moveDiffCursor(delta int) {
 	n := len(m.model.Diff.Lines)
 	if n == 0 {
 		return
 	}
-	m.diffCursor += delta
-	if m.diffCursor < 0 {
-		m.diffCursor = 0
+	step := 1
+	if delta < 0 {
+		step = -1
 	}
-	if m.diffCursor >= n {
-		m.diffCursor = n - 1
+	for ; delta != 0; delta -= step {
+		if lo, hi, ok := m.tallCursorSpan(); ok {
+			if next := m.cursorRowOff(hi-lo) + step; next >= 0 && next <= hi-lo {
+				m.setCursorRowOff(next)
+				continue
+			}
+		}
+		next := min(max(m.diffCursor+step, 0), n-1)
+		if next == m.diffCursor {
+			return
+		}
+		m.diffCursor = next
+		m.setCursorRowOff(0)
+		if lo, hi, ok := m.tallCursorSpan(); ok && step < 0 {
+			m.setCursorRowOff(hi - lo)
+		}
+	}
+}
+
+// cursorRowOff is how many rows into a tall cursor line the pane opens,
+// capped at maxOff. The offset belongs to the line it was set on, so a
+// different line landing under the cursor (a discard shifting the diff, a
+// same-path file in another commit) opens at its first row, and a resize
+// keeps the reader's place within the line rather than an absolute row.
+func (m *Mission) cursorRowOff(maxOff int) int {
+	lines := m.model.Diff.Lines
+	c := min(max(m.diffCursor, 0), len(lines)-1)
+	if c < 0 || !sameDiffLine(m.diffRowOffAt, lines[c]) {
+		return 0
 	}
+	return min(max(m.diffRowOff, 0), maxOff)
+}
+
+func (m *Mission) setCursorRowOff(off int) {
+	lines := m.model.Diff.Lines
+	c := min(max(m.diffCursor, 0), len(lines)-1)
+	if c < 0 {
+		return
+	}
+	m.diffRowOff, m.diffRowOffAt = off, lines[c]
+}
+
+// sameDiffLine matches one diff line across model pushes: staging it
+// changes Selected, never its text or position.
+func sameDiffLine(a, b DiffLine) bool {
+	return a.Kind == b.Kind && a.OldNo == b.OldNo && a.NewNo == b.NewNo && a.Text == b.Text
+}
+
+// tallCursorSpan is the diffTop range [lo, hi] that reads the cursor line
+// through the pane when it wraps to more rows than the pane holds, from the
+// row index and height of the last render; ok is false for a line that fits.
+func (m *Mission) tallCursorSpan() (lo, hi int, ok bool) {
+	lines, ix, h := m.model.Diff.Lines, &m.diffRowsCache, m.diffViewH
+	if h <= 0 || len(lines) == 0 || ix.n != len(lines) || ix.key != &lines[0] {
+		return 0, 0, false
+	}
+	c := min(max(m.diffCursor, 0), len(lines)-1)
+	if ix.start[c+1]-ix.start[c] <= h {
+		return 0, 0, false
+	}
+	return ix.start[c], ix.start[c+1] - h, true
 }
 
 type diffStagePayload struct {
@@ -376,11 +438,18 @@ func (m *Mission) renderDiffLines(width, height int) string {
 	}
 	// The viewport sees the cursor line's extra rows collapsed into its
 	// first, over a pane shrunk by the same amount: the whole line then
-	// stays in view with scrolloff around it, and a line taller than the
-	// pane (a one-row virtual pane) keeps its first row on top. Rows above
-	// the cursor line are the same in both spaces, which is all top can be.
-	virtualH := max(height-extra, 1)
-	top, _ := picker.Viewport(cursorRow, m.diffTop, total-extra, virtualH, virtualH, 0)
+	// stays in view with scrolloff around it. Rows above the cursor line
+	// are the same in both spaces, which is all top can be. A line taller
+	// than the pane fills it instead, at whichever of its rows
+	// moveDiffCursor has scrolled to.
+	m.diffViewH = height
+	var top int
+	if lo, hi, ok := m.tallCursorSpan(); ok {
+		top = lo + m.cursorRowOff(hi-lo)
+	} else {
+		virtualH := max(height-extra, 1)
+		top, _ = picker.Viewport(cursorRow, m.diffTop, total-extra, virtualH, virtualH, 0)
+	}
 	m.diffTop = top
 	thumbTop, thumbH := picker.ThumbSpan(top, min(height, total), total)
 	thumbOn := lipgloss.NewStyle().Background(theme.Panel)

ui/internal/views/mission/diff_wrap_test.go

@@ -152,20 +152,133 @@ func TestCursorLineIsKeptWholeInView(t *testing.T) {
 	}
 }
 
-func TestCursorLineTallerThanThePaneKeepsItsFirstRowOnTop(t *testing.T) {
-	m := wrapFixture()
-	m.model.Diff.Lines[1].Text = strings.Repeat("word ", 400)
+// tallFixture puts the cursor on a line wrapping to far more rows than a
+// 3-row pane, rendered once so the row index and pane height are known.
+func tallFixture(t *testing.T) (m *Mission, lo, hi int) {
+	t.Helper()
+	m = wrapFixture()
+	m.model.Diff.Lines[1].Text = strings.Repeat("word ", 60)
 	m.diffCursor = 1
-	rows := strings.Split(ansi.Strip(m.renderDiffLines(30, 3)), "\n")
-	if !strings.Contains(rows[0], " 1 + word") {
+	m.renderDiffLines(tallW, tallH)
+	ix := m.diffRows(tallW - 1 - diffGutterWidth - diffMarkWidth)
+	lo, hi = ix.start[1], ix.start[2]-tallH
+	if hi-lo < 3 {
+		t.Fatalf("fixture line is not tall enough: rows [%d,%d)", ix.start[1], ix.start[2])
+	}
+	return m, lo, hi
+}
+
+const tallW, tallH = 30, 3
+
+func TestCursorLineTallerThanThePaneKeepsItsFirstRowOnTop(t *testing.T) {
+	m, lo, _ := tallFixture(t)
+	rows := strings.Split(ansi.Strip(m.renderDiffLines(tallW, tallH)), "\n")
+	if m.diffTop != lo || !strings.Contains(rows[0], " 1 + word") {
 		t.Fatalf("first row of the cursor line is not on top:\n%s", strings.Join(rows, "\n"))
 	}
+}
+
+func TestScrollingDownReadsATallLineRowByRowThenMovesOn(t *testing.T) {
+	m, lo, hi := tallFixture(t)
+	for want := lo + 1; want <= hi; want++ {
+		m.moveDiffCursor(1)
+		m.renderDiffLines(tallW, tallH)
+		if m.diffCursor != 1 || m.diffTop != want {
+			t.Fatalf("step to row %d: cursor %d top %d", want, m.diffCursor, m.diffTop)
+		}
+	}
 	m.moveDiffCursor(1)
-	rows = strings.Split(ansi.Strip(m.renderDiffLines(30, 3)), "\n")
-	// tail is the last line, so the window clamps at the end of the diff
-	// with tail on its bottom row rather than its top.
-	if len(rows) != 3 || !strings.Contains(rows[2], " 2 + tail") || strings.Contains(rows[0], " 1 + word") {
-		t.Fatalf("the window did not follow the cursor off the tall line:\n%s", strings.Join(rows, "\n"))
+	rows := strings.Split(ansi.Strip(m.renderDiffLines(tallW, tallH)), "\n")
+	if m.diffCursor != 2 || !strings.Contains(rows[len(rows)-1], " 2 + tail") {
+		t.Fatalf("past the tall line's last row the cursor should reach tail, got cursor %d:\n%s", m.diffCursor, strings.Join(rows, "\n"))
+	}
+}
+
+func TestScrollingUpIntoATallLineStartsAtItsBottom(t *testing.T) {
+	m, lo, hi := tallFixture(t)
+	m.diffCursor = 2
+	m.renderDiffLines(tallW, tallH)
+	m.moveDiffCursor(-1)
+	m.renderDiffLines(tallW, tallH)
+	if m.diffCursor != 1 || m.diffTop != hi {
+		t.Fatalf("entering from below: cursor %d top %d, want 1 and %d", m.diffCursor, m.diffTop, hi)
+	}
+	for want := hi - 1; want >= lo; want-- {
+		m.moveDiffCursor(-1)
+		m.renderDiffLines(tallW, tallH)
+		if m.diffCursor != 1 || m.diffTop != want {
+			t.Fatalf("step up to row %d: cursor %d top %d", want, m.diffCursor, m.diffTop)
+		}
+	}
+	m.moveDiffCursor(-1)
+	if m.diffCursor != 0 {
+		t.Fatalf("above the tall line's first row the cursor should reach the hunk, got %d", m.diffCursor)
+	}
+}
+
+func TestAWheelTickSpillsPastTheTallLineOntoTheNext(t *testing.T) {
+	m, lo, hi := tallFixture(t)
+	m.setCursorRowOff(hi - 2 - lo)
+	m.moveDiffCursor(2)
+	m.renderDiffLines(tallW, tallH)
+	if m.diffCursor != 1 || m.diffTop != hi {
+		t.Fatalf("two steps two rows from the end should land on the last row, got cursor %d top %d", m.diffCursor, m.diffTop)
+	}
+	m.setCursorRowOff(hi - 1 - lo)
+	m.moveDiffCursor(2)
+	if m.diffCursor != 2 {
+		t.Fatalf("one step to the last row and one onto tail, got cursor %d top %d", m.diffCursor, m.diffTop)
+	}
+}
+
+// pushLines stands in for a model push: a fresh Lines backing array, as
+// every decode makes, with the cursor line's stage state flipped.
+func pushLines(m *Mission, edit func([]DiffLine)) {
+	lines := append([]DiffLine(nil), m.model.Diff.Lines...)
+	lines[1].Selected = !lines[1].Selected
+	if edit != nil {
+		edit(lines)
+	}
+	m.model.Diff.Lines = lines
+}
+
+func TestStagingATallLineKeepsTheRowsBeingRead(t *testing.T) {
+	m, lo, _ := tallFixture(t)
+	m.moveDiffCursor(2)
+	pushLines(m, nil)
+	m.renderDiffLines(tallW, tallH)
+	if m.diffTop != lo+2 {
+		t.Fatalf("top %d after the push, want %d", m.diffTop, lo+2)
+	}
+}
+
+func TestADifferentTallLineUnderTheCursorOpensAtItsTop(t *testing.T) {
+	m, lo, _ := tallFixture(t)
+	m.moveDiffCursor(2)
+	pushLines(m, func(lines []DiffLine) { lines[1].Text = strings.Repeat("other ", 50) })
+	m.renderDiffLines(tallW, tallH)
+	if m.diffTop != lo {
+		t.Fatalf("a new line under the cursor opened at top %d, want its first row %d", m.diffTop, lo)
+	}
+}
+
+// The line above the tall one wraps too, so a resize moves where the tall
+// line starts.
+func TestAResizeKeepsThePlaceWithinATallLine(t *testing.T) {
+	m := wrapFixture()
+	m.model.Diff.Lines = []DiffLine{
+		{Kind: "context", OldNo: 1, NewNo: 1, SelIdx: -1, Text: strings.Repeat("ctx ", 20)},
+		{Kind: "add", NewNo: 2, Text: strings.Repeat("word ", 60)},
+		{Kind: "add", NewNo: 3, Text: "tail"},
+	}
+	m.diffCursor = 1
+	m.renderDiffLines(tallW, tallH)
+	m.moveDiffCursor(2)
+	const wider = tallW + 10
+	m.renderDiffLines(wider, tallH)
+	ix := m.diffRows(wider - 1 - diffGutterWidth - diffMarkWidth)
+	if got := m.diffTop - ix.start[1]; got != 2 {
+		t.Fatalf("offset into the line after widening is %d rows, want 2", got)
 	}
 }
 

ui/internal/views/mission/mission.go

@@ -70,10 +70,17 @@ type Mission struct {
 	// a line index. diffPath is the Diff.Path last seen, so a model swap
 	// that keeps the same file (a stage refreshing the hunk) preserves the
 	// cursor while one that shows a different file resets it (diff.go's
-	// clampDiffCursor).
+	// clampDiffCursor). diffViewH is the pane height of the last render,
+	// which a key or wheel step needs to know whether the cursor line is
+	// taller than the pane (tallCursorSpan); diffRowOff is how far into
+	// such a line the pane reads, owned by the line diffRowOffAt
+	// (cursorRowOff).
 	diffCursor    int
 	diffTop       int
 	diffPath      string
+	diffViewH     int
+	diffRowOff    int
+	diffRowOffAt  DiffLine
 	diffHL        diffHighlighter
 	diffRowsCache diffRowIndex