From 802bfa159e46fd9b2513eab80b9a3d78de641060 Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Thu, 9 Jul 2026 22:11:17 -0700 Subject: [PATCH] fix(list): reserve prompt rows so the table doesn't jump at exit The progressive table sits stable on screen through the loading phase, then jumps up at an unpredictable moment when the command exits and the shell prints a multi-line prompt. Emit two blank rows below the footer with the skeleton and move the cursor back over them: the prompt renders into pre-scrolled rows and the settled table stays put. Overshoot is consumed invisibly by whatever prints next; a taller prompt scrolls by just the difference. Co-Authored-By: Claude Fable 5 --- src/commands/list/CLAUDE.md | 4 +- src/commands/list/progressive_table.rs | 57 +++++++++++++++++---- tests/common/progressive_output.rs | 7 +++ tests/integration_tests/list_progressive.rs | 26 +++++++++- 4 files changed, 80 insertions(+), 14 deletions(-) diff --git a/src/commands/list/CLAUDE.md b/src/commands/list/CLAUDE.md index 9d194f808c..e453024619 100644 --- a/src/commands/list/CLAUDE.md +++ b/src/commands/list/CLAUDE.md @@ -58,4 +58,6 @@ Measures pure skeleton latency. Target: <60ms. - `collect/` — orchestrates collection, manages pre/post-skeleton phases, task definitions and execution (see `collect/mod.rs` module docstring for phase details) - `render.rs` — row formatting, skeleton rows, cell rendering - `layout.rs` — column width calculation -- `progressive_table.rs` — terminal rendering with in-place updates +- `progressive_table.rs` — terminal rendering with in-place updates; reserves + blank rows below the table so the shell prompt printed after exit renders + into pre-scrolled rows instead of jerking the settled table up diff --git a/src/commands/list/progressive_table.rs b/src/commands/list/progressive_table.rs index c8205661da..0dc618586a 100644 --- a/src/commands/list/progressive_table.rs +++ b/src/commands/list/progressive_table.rs @@ -7,6 +7,8 @@ //! - Uses our own escape-aware width calculations (StyledLine, truncate_visible) //! - Supports OSC-8 hyperlinks correctly //! - Has predictable cursor behavior based on our rendering logic +//! - Reserves blank rows below the table ([`PROMPT_RESERVE_LINES`]) so the +//! shell prompt printed after exit doesn't scroll the settled table use crossterm::{ ExecutableCommand, @@ -17,6 +19,32 @@ use std::io::{IsTerminal, Write, stdout}; use crate::display::truncate_visible; +/// Blank rows kept below the table so the shell prompt printed after exit +/// renders into pre-scrolled rows instead of scrolling the settled table. +/// +/// The table sits on screen for the whole progressive phase, so a scroll at +/// exit — the terminal making room for the shell's multi-line prompt — reads +/// as a jarring jump at an unpredictable moment. Emitting the blank rows with +/// the skeleton moves that scroll to command start, where output is expected +/// to scroll. The prompt's height is unknowable, but the failure modes are +/// asymmetric: extra reserved rows are consumed invisibly by whatever prints +/// next, while a prompt taller than `PROMPT_RESERVE_LINES + 1` rows scrolls +/// by just the difference. Two rows absorb prompts up to three rows tall +/// (fish default 1, starship default 2, tide/powerlevel10k typically 3). +const PROMPT_RESERVE_LINES: u16 = 2; + +/// Emit the prompt-reserve rows and move the cursor back up over them, +/// returning it to the resting position (the line after the last content +/// line). Callers emit this after the last content write, so the reserve +/// always sits between the table and the shell prompt. +fn write_prompt_reserve(stdout: &mut std::io::Stdout) -> std::io::Result<()> { + for _ in 0..PROMPT_RESERVE_LINES { + writeln!(stdout)?; + } + stdout.execute(MoveUp(PROMPT_RESERVE_LINES))?; + Ok(()) +} + /// Progressive table that updates rows in-place using crossterm cursor control. /// /// The table structure is: @@ -25,6 +53,11 @@ use crate::display::truncate_visible; /// - Spacer (blank line) /// - Footer (loading status / summary) /// +/// Below the footer sit [`PROMPT_RESERVE_LINES`] pre-scrolled blank rows. +/// The cursor rests on the line after the footer (the first reserved row) +/// between flushes, so all redraw math is relative to that fixed resting +/// position and the reserve stays invisible to it. +/// /// Data mutation (`update_row`, `update_footer`) is separate from rendering (`flush`). /// Call `flush()` after updates to write changes to the terminal. pub struct ProgressiveTable { @@ -79,11 +112,11 @@ impl ProgressiveTable { ) -> Self { let total_row_count = skeletons.len(); - // Limit visible rows to fit in terminal: header + rows + spacer + footer = rows + 3 - // Reserve one extra line for the cursor position after printing. + // Limit visible rows to fit in terminal: header + rows + spacer + footer + // = rows + 3, plus the prompt-reserve rows and one line for the cursor. // Only limit when we have height info — None means non-TTY or unknown. let visible_row_count = terminal_height - .map(|h| total_row_count.min(h.saturating_sub(4))) + .map(|h| total_row_count.min(h.saturating_sub(4 + PROMPT_RESERVE_LINES as usize))) .unwrap_or(total_row_count); // Build initial lines: header + visible rows + spacer + footer @@ -121,12 +154,13 @@ impl ProgressiveTable { Ok(()) } - /// Print all lines to stdout. + /// Print all lines to stdout, followed by the prompt-reserve rows. fn print_all(&self) -> std::io::Result<()> { let mut stdout = stdout(); for line in &self.lines { writeln!(stdout, "{}", line)?; } + write_prompt_reserve(&mut stdout)?; stdout.flush() } @@ -272,6 +306,7 @@ impl ProgressiveTable { "{}", truncate_visible(&final_footer, self.max_width) )?; + write_prompt_reserve(&mut stdout)?; stdout.flush() } else { // Normal: update rows in-place + footer @@ -454,14 +489,14 @@ mod tests { #[test] fn overflow_limits_visible_rows() { - // 10 rows, terminal height 8 → visible = 8 - 4 = 4 + // 10 rows, terminal height 10 → visible = 10 - 4 - PROMPT_RESERVE_LINES = 4 let skeletons: Vec = (0..10).map(|i| format!("row{i}")).collect(); let table = ProgressiveTable::new_with_height( "header".into(), skeletons, "loading".into(), 80, - Some(8), + Some(10), ); assert_eq!(table.row_count, 4); @@ -495,14 +530,14 @@ mod tests { #[test] fn overflow_boundary_exact_fit() { - // 5 rows need height 5+4=9, terminal height 9 → fits exactly, no overflow + // 5 rows need height 5+4+PROMPT_RESERVE_LINES=11 → fits exactly, no overflow let skeletons: Vec = (0..5).map(|i| format!("row{i}")).collect(); let table = ProgressiveTable::new_with_height( "header".into(), skeletons, "loading".into(), 80, - Some(9), + Some(11), ); assert_eq!(table.row_count, 5); @@ -511,14 +546,14 @@ mod tests { #[test] fn overflow_boundary_one_short() { - // 5 rows need height 5+4=9, terminal height 8 → overflow, visible = 4 + // 5 rows need height 5+4+PROMPT_RESERVE_LINES=11, height 10 → overflow, visible = 4 let skeletons: Vec = (0..5).map(|i| format!("row{i}")).collect(); let table = ProgressiveTable::new_with_height( "header".into(), skeletons, "loading".into(), 80, - Some(8), + Some(10), ); assert_eq!(table.row_count, 4); @@ -534,7 +569,7 @@ mod tests { skeletons, "loading".into(), 80, - Some(8), + Some(10), ); // Can update visible rows (0..4) diff --git a/tests/common/progressive_output.rs b/tests/common/progressive_output.rs index a0356cf257..b53d0b679a 100644 --- a/tests/common/progressive_output.rs +++ b/tests/common/progressive_output.rs @@ -192,6 +192,9 @@ pub struct ProgressiveOutput { pub exit_code: i32, /// Total execution time pub total_duration: Duration, + /// Cursor position (row, col) after all output drained — where the shell + /// prompt would print + pub final_cursor: (u16, u16), } impl ProgressiveOutput { @@ -521,11 +524,13 @@ pub fn capture_progressive_output( .unwrap_or_else(|_| panic!("Failed to wait for 'wt {}' to exit", subcommand)); let exit_code = exit_status.exit_code() as i32; let total_duration = start_time.elapsed(); + let final_cursor = parser.screen().cursor_position(); ProgressiveOutput { stages: snapshots, exit_code, total_duration, + final_cursor, } } @@ -616,6 +621,7 @@ mod tests { stages, exit_code: 0, total_duration: Duration::from_millis(150), + final_cursor: (0, 0), }; // Test samples @@ -656,6 +662,7 @@ mod tests { stages, exit_code: 0, total_duration: Duration::from_millis(150), + final_cursor: (0, 0), }; // Should verify successfully diff --git a/tests/integration_tests/list_progressive.rs b/tests/integration_tests/list_progressive.rs index c30f4f1cc3..b269a58ddc 100644 --- a/tests/integration_tests/list_progressive.rs +++ b/tests/integration_tests/list_progressive.rs @@ -96,8 +96,8 @@ fn test_list_progressive_api(mut repo: TestRepo) { #[rstest] fn test_list_progressive_overflow(mut repo: TestRepo) { // Create enough worktrees to overflow a 10-row terminal. - // With height=10: visible_rows = 10 - 4 (header + spacer + footer + cursor) = 6 - // 10 worktrees + main = 11 rows, well above the 6-row limit. + // With height=10: visible_rows = 10 - 4 (header + spacer + footer + cursor) + // - 2 (prompt reserve) = 4. 10 worktrees + main = 11 rows, well above that. for i in 1..=10 { repo.add_worktree(&format!("overflow-{:02}", i)); } @@ -132,6 +132,17 @@ fn test_list_progressive_overflow(mut repo: TestRepo) { !final_text.contains('·'), "No placeholder dots should remain after finalize.\nFinal output:\n{final_text}" ); + + // The prompt reserve leaves the cursor two rows above the bottom of the + // scrolled screen, with the footer directly above it — the shell prompt + // printed after exit renders into the reserved rows instead of scrolling + // the settled table. + assert_eq!( + output.final_cursor, + (7, 0), + "Cursor should rest 2 reserved rows above the bottom of the 10-row \ + terminal.\nFinal output:\n{final_text}" + ); } /// Tests progressive rendering with no worktrees (fast path). @@ -156,4 +167,15 @@ fn test_list_progressive_fast_command(repo: TestRepo) { output.final_output().contains("Branch"), "Should have table header" ); + + // The prompt-reserve rows are emitted below the footer with the cursor + // moved back over them: it rests directly under the last content row, so + // a reserve/MoveUp mismatch would show up as a displaced cursor. + let content_rows = output.final_output().trim_end().lines().count() as u16; + assert_eq!( + output.final_cursor, + (content_rows, 0), + "Cursor should rest directly under the footer.\nFinal output:\n{}", + output.final_output() + ); }