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
5 changes: 4 additions & 1 deletion src/app/navigation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,10 @@ impl App {
self.ensure_cursor_visible();
// Cap scroll change to cursor movement to prevent multi-line jumps
// when the view is catching up from a non-steady-state position.
let cursor_moved = self.diff_state.cursor_line - prev_cursor;
// `saturating_sub`: a reload can shrink the diff and clamp
// `cursor_line` (line 49) below `prev_cursor`, which would
// otherwise underflow this usize subtraction.
let cursor_moved = self.diff_state.cursor_line.saturating_sub(prev_cursor);
if self.diff_state.scroll_offset > prev_scroll + cursor_moved {
self.diff_state.scroll_offset = prev_scroll + cursor_moved;
}
Expand Down
10 changes: 10 additions & 0 deletions src/app/pr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,12 @@ impl App {
} else if let Some(anchor) = &request.anchor {
self.restore_pr_cursor_to_anchor(anchor);
}
// A reload can shrink the diff; a stale cursor left past the new end
// (same-head branch, or a restored overview line captured from the
// taller old diff) makes the next `cursor_down` clamp upward and
// panic. Clamp into the current bounds.
self.diff_state.cursor_line = self.diff_state.cursor_line.min(self.max_cursor_line());
self.ensure_cursor_visible();
Ok(())
}

Expand Down Expand Up @@ -672,6 +678,10 @@ impl App {
self.rebuild_annotations();
}

// Same-head reload keeps the old cursor; clamp it into the (possibly
// shorter) new diff so a following `cursor_down` can't underflow.
self.diff_state.cursor_line = self.diff_state.cursor_line.min(self.max_cursor_line());

Ok(head_changed)
}

Expand Down
24 changes: 24 additions & 0 deletions src/app/tests/scroll_behavior_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -725,3 +725,27 @@ fn move_cursor_to_annotation_wrap_on_scrolls_target_fully_into_view() {
"already-visible target should not change scroll_offset"
);
}

#[test]
fn cursor_down_with_stale_cursor_past_end_does_not_underflow() {
// Regression: a PR reload can shrink the diff and leave `cursor_line`
// parked past the new `max_cursor_line`. The next `cursor_down` clamps
// the cursor *up* to the new max (below `prev_cursor`), which used to
// underflow the `cursor_line - prev_cursor` usize subtraction and panic
// (`attempt to subtract with overflow` at navigation.rs).
let mut app = build_scroll_app(10, 20, 0);
// Overview (multi-file) path exercises the `cursor_line - prev_cursor`
// subtraction; the single-file walk branch clamps separately.
app.is_single_file_view = false;
let max = app.max_cursor_line();

// Simulate the stale post-reload state: cursor beyond the new end.
app.diff_state.cursor_line = max + 50;

app.cursor_down(1); // must not panic

assert_eq!(
app.diff_state.cursor_line, max,
"cursor should be clamped to the current max line"
);
}
Loading