Skip to content

fix(painter): trust the measured cursor row over a short terminal size - #1206

Open
bhouse-nexthop wants to merge 1 commit into
nushell:mainfrom
bhouse-nexthop:fix-1205-prompt-climb-short-winsize
Open

fix(painter): trust the measured cursor row over a short terminal size#1206
bhouse-nexthop wants to merge 1 commit into
nushell:mainfrom
bhouse-nexthop:fix-1205-prompt-climb-short-winsize

Conversation

@bhouse-nexthop

@bhouse-nexthop bhouse-nexthop commented Sep 6, 2026

Copy link
Copy Markdown

Summary

terminal::size() isn't always the truth. When the kernel's winsize is shorter than the window actually attached, the cursor can sit below the row reedline believes is the last one — and the prompt then climbs one row per keystroke.

A terminal only ever reports a cursor row it actually has, so a measured row is hard evidence of a floor on the screen height. This raises the believed height to include the measured row, wherever the cursor is read and the reported size may be stale.

Where the short winsize comes from:

situation winsize
serial console / IPMI SOL / agetty 0x0 → reedline assumes 80x24
expect / pexpect wrappers pty default 24x80
resize where SIGWINCH never reached the pty stale, too short

Public API: unchanged.
Observable behavior when the reported size is correct: unchanged. The cursor is always inside the screen, so the floor is already met and the adjustment is a no-op.

Fixes #1205

Before

Kernel says 10 rows, real window is 50, cursor measured at row 35. Anchor 35 is outside the believed screen, so remaining_lines() saturates to 0 and every repaint reads as "out of room":

keystroke remaining_lines() extra anchor after paint
s 0 1 34
h 0 1 33
o 0 1 32
w 0 1 31

The prompt walks up one row per character, leaving blank rows below it, and stops once it reaches row 10 — so it reads as intermittent.

After

The measured row 35 raises the believed height to 36, which puts the anchor back inside the screen:

keystroke remaining_lines() extra anchor after paint
s 1 0 35
h 1 0 35
o 1 0 35
w 1 0 35

Repro from the issue, which no longer climbs:

stty rows 10          # lie to the kernel; the real window is 50+ rows
seq 1 40              # push the cursor below row 10
cargo run --example basic

What changed

where change
measure_cursor_position (new) wraps the cursor read, raises the believed height to fit the answer
initialize_prompt_position, repaint_buffer reconcile, print_external_message read the cursor through it
repaint_buffer reads screen_height after the reconcile; grows to fit the anchor it settles on, which is not always the row it measured
handle_resize unchanged — still reads the cursor directly
W::Sink can be given a cursor position to answer with, for tests
initialize_prompt_position takes the reported size via an inner fn, since terminal::size() needs a tty

Additional notes

  • Why grow the height rather than clamp the anchor. Clamping is also stable, but it paints the prompt ~13 rows above where the cursor actually is, overwriting live output. The measured row is direct evidence, so growing keeps the prompt where the terminal really put it.
  • Why the height is read after the reconcile. The reconcile can raise it mid-paint. Reading it before splits one paint across two screens: remaining_lines describes the grown screen while large_buffer judges against the old, short one, reports a buffer taller than the screen and resets the anchor to row 0. That is the resize path (row 3 of the table above), so it is the normal case, not a corner.
  • Why handle_resize still reads the cursor directly. Its size came from the resize event, so it needs no correction, and the read can beat the terminal's own clamping of the cursor into a possibly shorter screen. This only defers the growth by one turn: handle_resize leaves the anchor Stale, so the next paint goes through the reconcile and grows there if needed.
  • Why initialize_prompt_position keeps == instead of >=. With the floor applied new_row is at most one past the height, so equality is exhaustive for that arm. The UseExistingPrompt arm skips the guard but is bounded too, since select_prompt_row only re-uses a range that contains the measured row. No anchor lands outside the screen.
  • The height is a one-way ratchet within a read_line. Nothing lowers it until the next terminal::size() — the next read_line, or a resize event. screen_height() is pub and reaches third-party Menu impls, so its doc now says it is a lower bound rather than a reading. Signatures are unchanged. Flagging it in case you'd rather bound the growth.
  • Also repairs a latent hole. initialize_prompt_position only substitutes a default for exactly (0, 0), and handle_resize(0, 0) has no fallback at all, so a believed height of 0 was reachable and made every paint take the large-buffer reset. A measured cursor row now repairs that.
  • Width is deliberately left alone. The measured column proves width >= col + 1 by the same argument, but the column is 0 at nearly every measurement point, so it proves nothing useful — and an over-wide belief mis-wraps rather than failing safe.

Scope — what this does not fix

The grown height is a floor (measured row + 1), not the real height, so remaining_lines() is 1 rather than the real 15 in the example above. A single-row entry is then stable, as the table shows, but anything taller still takes the scroll branch once:

entry anchors over 4 paints
show 35, 35, 35, 35
two lines 34, 34, 34, 34

So a multi-row entry or an open menu still jumps up once and then holds, instead of climbing every keystroke. Closing that needs the real height, which nothing reports. Related: menus size themselves in engine.rs before repaint_buffer runs, so on the frame the height grows they use the pre-growth value — one frame, and in the safe direction (menu smaller than the screen can hold).

Tests

  • Case matrix over the height floor, including both no-op cases, a never-initialized height of 0, and u16::MAX saturation.
  • One test per call site, driving the real measuring path rather than adjusting the height by hand.
  • Paint-level tests for the climb, an accurate winsize that must not be inflated, the mid-paint consistency of the grown height, a cursor below the cached anchor, and the anchor-vs-measured-row case.

Every production change is independently pinned — reverting any one of these five fails at least one test:

reverted tests failing
initialize_prompt_position call site 2
repaint_buffer reconcile call site 1
print_external_message call site 1
grow-to-fit-the-anchor in the reconcile 1
reading screen_height after the reconcile 1

cargo fmt --all -- --check, cargo clippy --locked --all-targets --all-features, and cargo test --all --all-features (1714 passing) are all clean, at --all-features, default, and --no-default-features.

@bhouse-nexthop
bhouse-nexthop force-pushed the fix-1205-prompt-climb-short-winsize branch 2 times, most recently from 778eb7f to 4e23aba Compare September 6, 2026 15:01
When the kernel's winsize is shorter than the window actually attached,
the cursor can sit below the row reedline believes is the last one. The
anchor was stored outside the believed screen, so `remaining_lines()`
saturated to 0, every repaint read as "out of room" and scrolled, and the
prompt walked up one row per keystroke until it reached the believed
bottom.

A terminal only ever reports a cursor row it actually has, so a measured
row is hard evidence of a floor on the screen height. Raise the believed
height to include it wherever the cursor is measured and the reported size
may be stale. When the reported size is right this is a no-op, since the
cursor is always inside the screen.

The growth can land mid-paint, so `repaint_buffer` now reads the height
after the stale-anchor reconcile rather than before it. Reading it earlier
split a paint between two screens: `remaining_lines` described the grown
one while `large_buffer` judged against the old, short one and reset the
anchor to row 0. The reconcile also grows to fit the anchor it settles on,
which is not always the row it measured.

Test writers can now be given a cursor position to answer with, and the
size `initialize_prompt_position` works from is passed in, so the tests
drive the real measuring paths rather than adjusting the height by hand.

Signed-off-by: Brad House <bhouse@nexthop.ai>
@bhouse-nexthop
bhouse-nexthop force-pushed the fix-1205-prompt-climb-short-winsize branch from 4e23aba to 2306c6f Compare September 6, 2026 15:17
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.

Prompt climbs one row per keystroke when the cursor row is below the reported terminal height

1 participant