Skip to content

fix(painter): don't re-use a bottom-flush prompt anchor after a scroll #1133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
kronberger-droid wants to merge 2 commits into
base: main
Choose a base branch
from
+102 −35
Draft

fix(painter): don't re-use a bottom-flush prompt anchor after a scroll #1133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
39ce024
fix(painter): don't re-use a bottom-flush prompt anchor after a scroll
kronberger-droid Jul 24, 2026
956680b
fix(painter): re-anchor the prompt to the cursor after an external sc…
kronberger-droid Jul 24, 2026
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
24 changes: 24 additions & 0 deletions src/engine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,12 @@ pub struct Reedline {
history_cursor_on_excluded: bool,
input_mode: InputMode,

// Set when a menu event that re-queries the completer (Activate/Edit) is
// dispatched. The host completer can shell out and scroll the terminal, so
// the next paint re-verifies the prompt anchor. Consumed in `repaint`.
// See #1130.
completer_may_have_scrolled: bool,

// State of the painter after a `ReedlineEvent::ExecuteHostCommand` was requested, used after
// execution to decide if we can re-use the previous prompt or paint a new one.
suspended_state: Option<PainterSuspendedState>,
Expand Down Expand Up @@ -303,6 +309,7 @@ impl Reedline {
history_excluded_item: None,
history_cursor_on_excluded: false,
input_mode: InputMode::Regular,
completer_may_have_scrolled: false,
suspended_state: None,
last_render_snapshot: None,
painter,
Expand Down Expand Up @@ -1270,6 +1277,7 @@ impl Reedline {
if self.active_menu().is_none() {
if let Some(menu) = self.menus.iter_mut().find(|menu| menu.name() == name) {
menu.menu_event(MenuEvent::Activate(self.quick_completions));
self.completer_may_have_scrolled = true;

if self.quick_completions && menu.can_quick_complete() {
menu.update_values(
Expand Down Expand Up @@ -1355,13 +1363,17 @@ impl Reedline {
})
}
ReedlineEvent::MenuPageNext => {
// A page load re-queries the completer (list menu).
self.completer_may_have_scrolled |= self.active_menu().is_some();
self.active_menu()
.map_or(Ok(EventStatus::Inapplicable), |menu| {
menu.menu_event(MenuEvent::NextPage);
Ok(EventStatus::Handled)
})
}
ReedlineEvent::MenuPagePrevious => {
// A page load re-queries the completer (list menu).
self.completer_may_have_scrolled |= self.active_menu().is_some();
self.active_menu()
.map_or(Ok(EventStatus::Inapplicable), |menu| {
menu.menu_event(MenuEvent::PreviousPage);
Expand Down Expand Up @@ -1498,6 +1510,7 @@ impl Reedline {
}
_ => {
menu.menu_event(MenuEvent::Edit(self.quick_completions));
self.completer_may_have_scrolled = true;
menu.update_values(
&mut self.editor,
self.completer.as_mut(),
Expand Down Expand Up @@ -1525,6 +1538,7 @@ impl Reedline {
menu.menu_event(MenuEvent::Deactivate);
} else {
menu.menu_event(MenuEvent::Edit(self.quick_completions));
self.completer_may_have_scrolled = true;
}
}
Ok(EventStatus::Handled)
Expand Down Expand Up @@ -2318,6 +2332,16 @@ impl Reedline {
}
}

// A menu Activate/Edit re-queried the host completer, which owns the tty
// and may shell out to a program that scrolls the terminal (nushell's
// external completer running `fzf --height`). Mark the anchor stale so
// the paint below re-verifies it instead of trusting a row the content
// may have scrolled past. Navigation and idle repaints keep the cached
// anchor. See #1130.
if std::mem::take(&mut self.completer_may_have_scrolled) {
self.painter.invalidate_prompt_start_row();
}

let menu = self.menus.iter().find(|menu| menu.is_active());

self.painter.repaint_buffer(
Expand Down
113 changes: 78 additions & 35 deletions src/painting/painter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,17 +193,30 @@ enum PromptRowSelector {
fn select_prompt_row(
suspended_state: Option<&PainterSuspendedState>,
(column, row): (u16, u16), // NOTE: Positions are 0 based here
screen_height: u16,
) -> PromptRowSelector {
if let Some(painter_state) = suspended_state {
// The painter was suspended, try to re-use the last prompt position to avoid
// unnecessarily making new prompts.
if painter_state.previous_prompt_rows_range.contains(&row) {
//
// Re-use is only sound when the previous prompt did not sit flush against
// the bottom of the screen. When it did, a suspended program that scrolls
// the terminal (e.g. an fzf keybinding with little room left) leaves the
// cursor pinned on the bottom row, still inside the stored range, which
// is indistinguishable from an in-place return. Re-using there re-anchors
// the prompt over the scrolled-up output. Fall through to a fresh prompt
// in that ambiguous case. See nushell/reedline#1130.
let last_row = screen_height.saturating_sub(1);
if *painter_state.previous_prompt_rows_range.end() < last_row
&& painter_state.previous_prompt_rows_range.contains(&row)
{
// Cursor is still in the range of the previous prompt, re-use it.
let start_row = *painter_state.previous_prompt_rows_range.start();
return PromptRowSelector::UseExistingPrompt { start_row };
} else {
// There was some output or cursor is outside of the range of previous prompt make a
// fresh new prompt.
// There was some output, the cursor is outside of the range of the
// previous prompt, or the prompt was flush at the bottom and may have
// scrolled, so make a fresh new prompt.
}
}

Expand Down Expand Up @@ -471,7 +484,8 @@ impl Painter {
size
}
};
let prompt_selector = select_prompt_row(suspended_state, cursor::position()?);
let prompt_selector =
select_prompt_row(suspended_state, cursor::position()?, self.screen_height());
let new_row = match prompt_selector {
PromptRowSelector::UseExistingPrompt { start_row } => start_row,
PromptRowSelector::MakeNewPrompt { new_row } => {
Expand Down Expand Up @@ -548,37 +562,27 @@ impl Painter {
self.just_resized = false;
}

// Lines and distance parameters
let remaining_lines = self.remaining_lines();
let required_lines = lines.required_lines(screen_width, false, menu);

// Marking the painter state as larger buffer to avoid animations
self.large_buffer = required_lines >= screen_height;

// True if the prompt has scrolled above the cached
// `prompt_start_row` and the caller must re-anchor at row 0.
// When not verified, query the terminal; promote to verified if
// the query confirms no drift, so later paints can skip it.
// Reconcile a stale anchor: something yielded the tty (a resize, or an
// external completer running e.g. `fzf --height`) and may have scrolled
// our content since the last paint.
let should_reset_anchor = match self.prompt_start_row {
PromptStartRow::Verified(_) => false,
PromptStartRow::Stale(row) => match cursor::position() {
// The `+1` handles the case where the previous output
// ended without a newline, leaving the cursor on the
// same row as the next prompt.
Ok(position) => {
let drifted = position.1 + 1 < row;
if !drifted {
self.prompt_start_row.mark_verified(row);
PromptStartRow::Stale(row) => {
// Cursor above the cached row => content scrolled up while the
// tty was yielded. Re-anchor to the cursor (ground truth) rather
// than homing to row 0, which would yank the prompt to the top.
// The `+1` allows for output that left the cursor on the prompt
// row. See nushell/reedline#1130.
match cursor::position() {
Ok(position) if position.1 + 1 < row => {
self.prompt_start_row.mark_verified(position.1);
}
drifted
Ok(_) | Err(_) => self.prompt_start_row.mark_verified(row),
}
Err(_) => false,
},
// `initialize_prompt_position` runs before any
// `repaint_buffer`, so this branch is unreachable in normal
// flow. Panic loudly in debug builds; in release, force a
// re-anchor since the alternative is drawing over screen
// content at row 0 with no scroll.
false
}
// Unreachable in normal flow (initialize_prompt_position runs
// first); in release, reset rather than draw over row 0 content.
PromptStartRow::Unverified => {
debug_assert!(
false,
Expand All @@ -588,6 +592,14 @@ impl Painter {
}
};

// Distance parameters, computed after reconciling so they reflect the
// re-anchored row.
let remaining_lines = self.remaining_lines();
let required_lines = lines.required_lines(screen_width, false, menu);

// Marking the painter state as larger buffer to avoid animations
self.large_buffer = required_lines >= screen_height;

// Moving the start position of the cursor based on the size of the required lines
if self.large_buffer || should_reset_anchor {
for _ in 0..screen_height.saturating_sub(lines_before_cursor) {
Expand Down Expand Up @@ -1335,15 +1347,15 @@ mod tests {
#[test]
fn test_select_new_prompt_with_no_state_no_output() {
assert_eq!(
select_prompt_row(None, (0, 12)),
select_prompt_row(None, (0, 12), 24),
PromptRowSelector::MakeNewPrompt { new_row: 12 }
);
}

#[test]
fn test_select_new_prompt_with_no_state_but_output() {
assert_eq!(
select_prompt_row(None, (3, 12)),
select_prompt_row(None, (3, 12), 24),
PromptRowSelector::MakeNewPrompt { new_row: 13 }
);
}
Expand All @@ -1353,16 +1365,47 @@ mod tests {
let state = PainterSuspendedState {
previous_prompt_rows_range: 11..=13,
};
// Prompt sits well above the bottom of a 24-row screen, so re-use holds.
assert_eq!(
select_prompt_row(Some(&state), (0, 12)),
select_prompt_row(Some(&state), (0, 12), 24),
PromptRowSelector::UseExistingPrompt { start_row: 11 }
);
assert_eq!(
select_prompt_row(Some(&state), (3, 12)),
select_prompt_row(Some(&state), (3, 12), 24),
PromptRowSelector::UseExistingPrompt { start_row: 11 }
);
}

// Regression test for nushell/reedline#1130.
//
// A multi-line prompt flush against the bottom of the screen is suspended
// for an fzf-style keybinding. The program scrolls the terminal and returns
// with the cursor pinned on the bottom row, still inside the stored range.
// Re-using the old anchor would redraw the prompt over the scrolled-up
// output, so the ambiguous bottom case must make a fresh prompt instead.
#[test]
fn test_select_prompt_row_does_not_reuse_when_flush_at_bottom() {
// Screen height 8 (rows 0..=7); prompt occupied rows 5..=7 and the
// range reaches the last row (7).
let state = PainterSuspendedState {
previous_prompt_rows_range: 5..=7,
};
assert_eq!(
select_prompt_row(Some(&state), (0, 7), 8),
PromptRowSelector::MakeNewPrompt { new_row: 7 }
);

// `state_before_suspension` can even push `final_row` past the last
// visible row when the prompt is at the very bottom; still no re-use.
let overshoot = PainterSuspendedState {
previous_prompt_rows_range: 5..=8,
};
assert_eq!(
select_prompt_row(Some(&overshoot), (0, 7), 8),
PromptRowSelector::MakeNewPrompt { new_row: 7 }
);
}

fn base_snapshot() -> RenderSnapshot {
RenderSnapshot {
screen_width: 20,
Expand Down
Loading