Skip to content

Don't fail read_line when the cursor-position query times out at prompt init - #1202

Merged
kronberger-droid merged 2 commits into
nushell:mainfrom
Blfrg:fix/tolerate-cursor-query-timeout-at-prompt-init
Sep 10, 2026
Merged

kronberger-droid merged 2 commits into
nushell:mainfrom
Blfrg:fix/tolerate-cursor-query-timeout-at-prompt-init

Conversation

@Blfrg

@Blfrg Blfrg commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

initialize_prompt_position asks the terminal for the cursor position before painting a prompt. crossterm waits a fixed 2s for the reply and then fails; a terminal busy repainting, a multiplexer briefly holding the reply, or a slow remote link is enough to trip it. That's a transient condition, not a broken terminal — but this was the only cursor_position() call site in the painter that still propagated the error, so it aborted the whole read_line(), and a host typically treats that as fatal (reubeno/brush#1329: one late reply exits the interactive shell).

No public API change; observable behavior changes only in the failure case described.

Before

read_line() returns Err("The cursor position could not be read within a normal duration") if the terminal doesn't answer the pre-prompt DSR query within 2s. Nothing was consumed from the user's input; the host just loses the editor.

After

On no answer: print \r\n so the prompt at least starts at column 0 on a row of its own, set the row to the bottom of the screen as Stale so the next paint's drift check asks again (and itself tolerates no answer), and carry on.

The substitute is always the bottom row, never the last-known one. The drift check only re-anchors when the cursor turns out to be above the cached row, so a guess is only repairable if it errs high. The newline just printed has already moved the cursor past the last-known row in the common REPL case, and clear_from_anchor erases down from the anchor, so reusing it would wipe the output above the prompt with no way to recover. No row is greater than the bottom, so it is the only guess that always lands on the repairable side.

The query is split out into anchor_prompt so it can be unit-tested without a tty; the test writers already return an error from cursor_position(), which is exactly the case handled. Two tests added.

Additional notes

Verified against a real host: brush at main (which has no retry of its own) built against 0.49.0 + this change — a pty harness that withholds the reply to exactly one query no longer ends the session, where stock 0.49.0 does. Ported cleanly to main; the only difference is self.stdout.cursor_position() vs cursor::position().

cargo test --lib on main: 1662 pass, 1 pre-existing failure unrelated to this change (crossterm_defaults_were_the_bright_palette_entries, fails identically on clean main with rustc 1.98).

Complementary, not a substitute, for a host-side retry — reubeno/brush#1331 adds one there.

…pt init

`initialize_prompt_position` asks the terminal where the cursor is
before painting a prompt. crossterm waits a fixed 2s for the reply and
then fails; a terminal busy repainting, a multiplexer briefly holding
the reply, or a slow remote link is enough to trip it. That is a
transient condition, not a broken terminal -- but this was the only
query site that still propagated the error, so it aborted the whole
`read_line`, and a host application typically treats that as fatal
(reubeno/brush#1329: one late reply exits the interactive shell).

Every other `cursor_position()` call in the painter already degrades to
its last-known row. Do the same here: print a newline so the prompt at
least starts at column 0 on a row of its own, keep the row as `Stale`
so the next paint's drift check asks again (and itself tolerates no
answer), and carry on. With no row known at all, assume the bottom of
the screen: the drift check only re-anchors when the cursor turns out
to be *above* the cached row, so a guess that errs high self-heals on
the next answered query, whereas row 0 would never be corrected.

The query is split out of `initialize_prompt_position` into
`anchor_prompt` so it can be unit-tested without a tty; the test
writers already return an error from `cursor_position()`, which is
exactly the case being handled.

Verified against brush (nushell/reedline 0.49.0 + this change, host
with no retry of its own): a pty harness that withholds the reply to
one query no longer ends the session.
Comment thread src/painting/painter.rs Outdated
Comment on lines +524 to +527
let row = match self.prompt_start_row {
PromptStartRow::Verified(row) | PromptStartRow::Stale(row) => row,
PromptStartRow::Unverified => self.screen_height().saturating_sub(1),
};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let row = match self.prompt_start_row {
PromptStartRow::Verified(row) | PromptStartRow::Stale(row) => row,
PromptStartRow::Unverified => self.screen_height().saturating_sub(1),
};
self.prompt_start_row =
PromptStartRow::Stale(self.screen_height().saturating_sub(1));
return Ok(());

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think reusing the old row is wrong here.
We are essentially substituting the row measurement, so what matters is which side of the
truth the substitute can land on.
The old row can land on either side, and in the common REPL case it lands below the true
cursor, which the drift check cannot repair since it only re-anchors upward.
The anchor is what clear_from_anchor erases down from, so that paint wipes the output
sitting above the prompt.
The bottom of the screen can only land on the repairable side, since no row is greater.
Your own reasoning in the comment above covers this, it just wants to apply to this branch
too: self.screen_height().saturating_sub(1) unconditionally.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, and the mismatch is that I only applied the "guess must be able to self-heal" invariant to the Unverified arm. Reusing the last-known row breaks it in the other two: anchor_prompt has already printed \r\n by the time the fallback runs, so the true cursor is below the cached row in the common REPL case, and that undershoot is the one direction the drift check can't fix. Bottom of the screen dominates in all three arms, not just Unverified.

Applied as 56cb1c2: unconditional Stale(self.screen_height().saturating_sub(1)), comment rewritten around that argument, and the test that asserted the old row was reused now asserts Stale(9) (renamed to ..._assumes_bottom_over_last_known_row). cargo test --lib still 1662 pass with only the pre-existing palette failure; clippy clean.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed empirically against a real host. Backported both variants onto 0.49.0 under brush main (which has its own retry now, so the harness withholds every query) and put a VT100 emulator in the loop so the drift check gets the true cursor row rather than a canned 1;1R:

  • echo BEFORE_42, withhold the pre-prompt query, answer the drift check truthfully, next prompt. Last-known row (as first submitted): BEFORE_42 and its command line are erased from screen and scrollback — clear_from_anchor ran from the stale row. Bottom row (56cb1c2): both preserved (scrolled, not wiped).
  • With no query ever answered, read_line keeps returning and the shell stays interactive in both variants; 2s per unanswered query as expected.

…unanswered

Reusing the last-known row was wrong: `anchor_prompt` has already printed
a newline by then, so in the common REPL case the true cursor sits below
the cached row. The drift check only re-anchors upward, so an
undershooting guess is never repaired, and `clear_from_anchor` then wipes
the output above the prompt. The bottom of the screen is the only guess
that always lands on the repairable side.

Review feedback on nushell#1202.
@kronberger-droid
kronberger-droid merged commit 9711eac into nushell:main Sep 10, 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.

2 participants