editor: Add scroll_beyond_last_line, cursor_surrounding_lines option#2410
Merged
Conversation
Two related but distinct knobs are exposed on `InputState`. Both
default to `None` to preserve the historical behavior — strict
addition, no breaking change.
In code-editor mode the editor reserves empty rows below the last
line of content (the "scroll past last line" affordance) and keeps
the cursor at least a few lines clear of the viewport's top/bottom
edges (the "cursor surrounding lines" auto-scroll padding). These
are two separate concepts that happen to share the same default
constant (`BOTTOM_MARGIN_ROWS = 3`):
- "Scroll past last line" answers "how far below the last line can
the user manually scroll?" — about the *scrollable region*'s size.
- "Cursor surrounding lines" answers "where in the viewport does the
cursor stay as it moves?" — about the *cursor*'s relative position
during typing / arrow navigation.
VSCode and JetBrains IDEs (WebStorm, IntelliJ) both expose the pair
as separate settings (`editor.scrollBeyondLastLine` +
`editor.cursorSurroundingLines` in VSCode). Different surfaces want
different defaults — full-pane editors typically want a small
trailing band so the bottom half of the pane doesn't read as
"missing content"; embedded editors usually want zero. This PR
gives a host the same dial without reaching into upstream.
```rust
// On InputState — both default to `None`.
pub fn scroll_past_last_line_rows(self, rows: Option<usize>) -> Self;
pub fn set_scroll_past_last_line_rows(&mut self, rows: Option<usize>, …);
pub fn cursor_surrounding_lines(self, lines: Option<usize>) -> Self;
pub fn set_cursor_surrounding_lines(&mut self, lines: Option<usize>, …);
```
`None` (default) preserves the prior behavior in both cases:
- `scroll_past_last_line_rows = None` → roughly half the viewport,
floored at `BOTTOM_MARGIN_ROWS * line_height`.
- `cursor_surrounding_lines = None` → `BOTTOM_MARGIN_ROWS` lines
on normal viewports, falls back to one line on small viewports
(`< BOTTOM_MARGIN_ROWS × 8` rows tall).
`Some(n)` overrides each with exactly `n` line-heights.
The two settings compose. Example: a JetBrains-style "Enter at
end-of-buffer keeps a single trailing empty row visible" surface
wants `scroll_past_last_line_rows = Some(1)` and
`cursor_surrounding_lines = Some(1)` — together they let the cursor
sit one row from the bottom edge with one empty row below it as
content grows.
`scroll_size.height` previously summed `empty_bottom_height` and
`ghost_lines_height`, producing an unreachable band of empty space
below the cursor-traversable area when scrolled to `max_scroll`
(visible as an unscrollable black void in code-editor surfaces).
Both terms describe extra height past the last real content row;
they should not stack. The PR uses `max` instead of `sum` so the
scrollable region is sized to whichever extension is actually in
play, leaving no unreachable empty pixels at scroll-bottom.
`cursor_surrounding_padding` is now clamped against half the visible
region. An aggressive override on a small viewport (e.g. `Some(20)`
on a 10-line viewport) used to send the bottom-edge auto-scroll
threshold below the top-edge threshold, with the per-frame
scroll-into-view computation losing a stable fixed point. With
saturation, the contract is honored when it fits and gracefully
degrades when the host asks for the impossible. Doc-comment on
`InputState::cursor_surrounding_lines` documents the saturation
explicitly so misconfiguration recovers instead of looping.
A third related defect surfaced once the API above was exercised
with the JB-style combo (`scroll_past_last_line_rows = Some(1)` +
`cursor_surrounding_lines = Some(1)`): pressing `Down` at
end-of-buffer produced one frame of visible scroll overshoot
followed by a snap-back to the clamped position on the next frame.
Held `Down` reproduced 2-3 jitter cycles before settling.
Two scroll-into-view computations existed and disagreed on the edge
clearance:
- `state.rs::scroll_to` hardcoded `edge_height = 3 * line_height`
whenever `direction.is_some() && code_editor`, ignoring the new
`cursor_surrounding_lines` override, and wrote the resulting
target into `deferred_scroll_offset` capped only against
`min(px(0.))` — the safe-range clamp lived elsewhere
(`update_scroll_offset`, run after paint).
- `element.rs::layout_cursor` honored the override via the
`cursor_surrounding_padding` helper, demanding only
`1 × line_height` clearance under `Some(1)`.
With the empty band trimmed to one line, state's 3-line demand
landed below `safe_y_min`. Element's `layout_cursor` overrides its
local computation with `state.deferred_scroll_offset`, so paint ran
at the unclamped position. The post-paint clamp in
`update_scroll_offset` pulled `scroll_handle` back into range. The
next render painted at the clamped value — visible as a one-line
jump-then-snap-back per `Down` keystroke.
Two changes that together eliminate both the demand and the
exposure:
1. State and element now agree on the edge clearance.
`cursor_surrounding_padding` is widened to `pub(super)` and
`scroll_to` calls it with the same arguments `layout_cursor`
uses. With `Some(1)` both sides demand 1 line clear; with the
default `None` both sides demand `BOTTOM_MARGIN_ROWS` (the
prior behavior). Non-code-editor / no-direction paths keep the
historical 1-line edge guard for source-compat.
2. Defense-in-depth at the producer. `scroll_to` now clamps
`scroll_offset.y` against `safe_y_min = (-scroll_size.height +
input_bounds.size.height).min(px(0.))` before storing it in
`deferred_scroll_offset` — the same range
`update_scroll_offset` enforces on persist. Even if a future
caller routes around `scroll_to` and writes
`deferred_scroll_offset` directly with an over-aggressive
target, paint and persist agree on the value.
`test_scroll_to_eob_does_not_overshoot_safe_range` exercises the
JB-style `Some(1) + Some(1)` configuration on a 50-line buffer,
moves the cursor to EOB with `MoveDirection::Down`, and asserts
`deferred_scroll_offset.y >= safe_y_min` — the producer-side
contract that paint and persist agree on the same value. The test
fails on the prior code (deferred lands roughly two line-heights
below `safe_y_min`) and passes after the fix. Placed in
`input::state::tests` alongside the existing
`test_highlighting_preserved_after_fold` runtime test (the pure
helpers covered by `test_cursor_surrounding_padding_*` in
`element.rs` remain unchanged).
Two free helpers extracted from `TextElement::prepaint` /
`layout_cursor` so the math is unit-testable:
```rust
fn empty_bottom_height(
is_code_editor: bool,
override_rows: Option<usize>,
viewport_height: Pixels,
line_height: Pixels,
) -> Pixels { … }
pub(super) fn cursor_surrounding_padding(
is_auto_grow: bool,
override_lines: Option<usize>,
visible_lines: usize,
line_height: Pixels,
) -> Pixels { … }
```
`cursor_surrounding_padding` is widened to `pub(super)` so
`state.rs::scroll_to` can call it with the same arguments
`element.rs::layout_cursor` uses (Bug C fix).
Pure-function unit tests cover the regimes:
- `empty_bottom_height`: outside code-editor (always 0), code-editor
+ `None` (half-viewport floored at `BOTTOM_MARGIN_ROWS *
line_height`), code-editor + `Some(n)` (exactly `n × line_height`,
no floor).
- `cursor_surrounding_padding`: auto-grow (always one line, no
override), non-auto-grow + `None` (small-viewport fallback +
large-viewport `BOTTOM_MARGIN_ROWS`), non-auto-grow + `Some(n)`
(exactly `n × line_height` when it fits), saturation against
half-viewport when it doesn't.
A runtime regression test
(`test_scroll_to_eob_does_not_overshoot_safe_range`) covers Bug C
end-to-end through `InputState::move_to` → `scroll_to`.
Has no effect outside `InputMode::CodeEditor` for the trailing-rows
knob; cursor-surrounding-lines applies to all multi-line modes.
🤖 AI assistance: this PR was drafted with AI help. Field, setters,
and helper extraction follow existing patterns (`soft_wrap`,
`set_soft_wrap`); test layout mirrors the existing pure-function
tests in `input/element.rs::tests` (`test_runs_for_range`,
`test_placeholder_line_runs`, etc.). No rendering-code performance
impact — each helper is called once per layout, identical work to
the inline expressions they replace.
glani
force-pushed
the
heretic/scroll-beyond-last-line
branch
from
732c230 to
0510d34
Compare
huacnlee
changed the title
Member
|
Thank you. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
In code-editor mode,
InputStatehardcodes two scroll behaviors behind asingle shared constant (
BOTTOM_MARGIN_ROWS):user can scroll (the empty band at the bottom of the editor).
of the viewport's top and bottom edges while moving.
These are two distinct concepts that happen to share one default, and
neither can be tuned without patching the crate. Different editors want
different values: a full-pane editor usually wants a small trailing band
so the bottom of the pane doesn't read as missing content, while a
compact or embedded editor usually wants none.
Solution
Expose both as separate, optional settings on
InputState, the same wayVSCode (
editor.scrollBeyondLastLine,editor.cursorSurroundingLines)and JetBrains IDEs split them:
Both default to
None, which preserves the current behavior exactly: whenunset, the paint path falls back to the existing
BOTTOM_MARGIN_ROWSconstant. This is a strict addition — no change to existing behavior.
Tests
Added coverage for both knobs:
Nonereproduces the current layout, andexplicit values change the trailing band and cursor padding as expected.