mirror of
https://github.com/openai/codex.git
synced 2026-10-19 16:17:49 +00:00
Preserve terminal scrollback when growing the TUI viewport (#45271)
## Why Scrolling with `CSI S` can discard history rows in QTermWidget and xterm.js when the viewport grows. ## What changed Use newlines at the bottom of the history scroll region for `ScrollbackStrategy::Standard`, preserving the composer and restoring the cursor and full-screen scroll region afterward. Fall back to full-screen scrolling when only one history row exists, since `DECSTBM` requires two distinct rows. Skip scrolling when there are no history rows or no rows to scroll. ## Testing Add regression tests for composer and cursor preservation, scroll-region restoration, emitted newline sequences, single-row scrollback preservation, and no-op growth. Snapshot visible geometry for growth by one, three, and four rows. GitOrigin-RevId: 1dbfc30bb6997dcc2738ec6b2f42d35c786124de
This commit is contained in:
@@ -3,6 +3,8 @@
|
||||
use crate::custom_terminal::Terminal;
|
||||
use crate::insert_history::HistoryLineWrapPolicy;
|
||||
use crate::insert_history::InsertHistoryMode;
|
||||
use crate::insert_history::ResetScrollRegion;
|
||||
use crate::insert_history::SetScrollRegion;
|
||||
use codex_terminal_detection::TerminalInfo;
|
||||
use codex_terminal_detection::TerminalName;
|
||||
use crossterm::cursor::MoveTo;
|
||||
@@ -57,10 +59,32 @@ impl ScrollbackStrategy {
|
||||
where
|
||||
B: Backend<Error = io::Error> + Write,
|
||||
{
|
||||
if scroll_by == 0 || viewport_top == 0 {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
match self {
|
||||
Self::FullScreen => {
|
||||
Self::Standard if viewport_top > 1 => {
|
||||
// CSI S can discard departing rows in QTermWidget and xterm.js. Newlines at the
|
||||
// bottom of the history region preserve native scrollback while protecting the
|
||||
// composer. Restore the cursor without changing the renderer's cached position.
|
||||
let cursor = terminal.last_known_cursor_pos;
|
||||
let writer = terminal.backend_mut();
|
||||
queue!(
|
||||
writer,
|
||||
SetScrollRegion(1..viewport_top),
|
||||
MoveTo(/*x*/ 0, viewport_top - 1)
|
||||
)?;
|
||||
for _ in 0..scroll_by {
|
||||
queue!(writer, Print("\r\n"))?;
|
||||
}
|
||||
queue!(writer, ResetScrollRegion, MoveTo(cursor.x, cursor.y))?;
|
||||
Ok(())
|
||||
}
|
||||
Self::FullScreen | Self::Standard => {
|
||||
// Partial DEC scroll regions can discard rows instead of moving them into Windows
|
||||
// Terminal's scrollback. Clear the stale composer, then scroll the entire screen.
|
||||
// Also use this path for a single history row: DECSTBM requires two distinct rows.
|
||||
terminal.clear_after_position(Position::new(/*x*/ 0, viewport_top))?;
|
||||
let writer = terminal.backend_mut();
|
||||
queue!(
|
||||
@@ -72,7 +96,7 @@ impl ScrollbackStrategy {
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
Self::Standard | Self::Zellij => terminal
|
||||
Self::Zellij => terminal
|
||||
.backend_mut()
|
||||
.scroll_region_up(0..viewport_top, scroll_by),
|
||||
}
|
||||
|
||||
@@ -11,6 +11,8 @@ use crossterm::cursor::MoveTo;
|
||||
use crossterm::queue;
|
||||
use crossterm::style::Print;
|
||||
use pretty_assertions::assert_eq;
|
||||
use ratatui::backend::CrosstermBackend;
|
||||
use ratatui::layout::Position;
|
||||
use ratatui::layout::Rect;
|
||||
use ratatui::layout::Size;
|
||||
use ratatui::text::Line;
|
||||
@@ -167,3 +169,161 @@ fn full_screen_viewport_growth_preserves_terminal_scrollback() {
|
||||
history-row-4
|
||||
");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn standard_viewport_growth_preserves_composer_and_cursor() {
|
||||
for scroll_by in [1, 3, 4] {
|
||||
let width = 24;
|
||||
let height = 6;
|
||||
let backend = VT100Backend::with_scrollback(width, height, /*scrollback_len*/ 32);
|
||||
let mut terminal = Terminal::with_options(backend).expect("terminal with scrollback");
|
||||
let viewport_top = 4;
|
||||
terminal.set_viewport_area(Rect::new(
|
||||
/*x*/ 0,
|
||||
viewport_top,
|
||||
width,
|
||||
/*height*/ 2,
|
||||
));
|
||||
for (row, text) in [
|
||||
"history-1",
|
||||
"history-2",
|
||||
"history-3",
|
||||
"history-4",
|
||||
"composer-1",
|
||||
"composer-2",
|
||||
]
|
||||
.into_iter()
|
||||
.enumerate()
|
||||
{
|
||||
queue!(
|
||||
terminal.backend_mut(),
|
||||
MoveTo(/*x*/ 0, row as u16),
|
||||
Print(text)
|
||||
)
|
||||
.expect("seed terminal row");
|
||||
}
|
||||
let cursor = Position::new(/*x*/ 7, /*y*/ 5);
|
||||
terminal
|
||||
.set_cursor_position(cursor)
|
||||
.expect("position cursor");
|
||||
|
||||
ScrollbackStrategy::Standard
|
||||
.grow_viewport(
|
||||
&mut terminal,
|
||||
viewport_top,
|
||||
Size::new(width, height),
|
||||
scroll_by,
|
||||
)
|
||||
.expect("grow standard viewport");
|
||||
|
||||
let screen = terminal.backend().vt100().screen();
|
||||
assert_eq!(
|
||||
(screen.cursor_position(), terminal.last_known_cursor_pos),
|
||||
((cursor.y, cursor.x), cursor)
|
||||
);
|
||||
// vt100 intentionally omits scrollback for partial regions, even for newlines. Snapshot
|
||||
// visible geometry here; the wire test below and real-terminal smoke cover scrollback.
|
||||
insta::assert_snapshot!(
|
||||
format!("standard_viewport_growth_{scroll_by}"),
|
||||
screen.contents()
|
||||
);
|
||||
|
||||
// A newline at the physical bottom must scroll the full screen after the region reset.
|
||||
queue!(
|
||||
terminal.backend_mut(),
|
||||
MoveTo(/*x*/ 0, height - 1),
|
||||
Print("\r\nnext-row")
|
||||
)
|
||||
.expect("write after restoring the scroll region");
|
||||
assert_eq!(
|
||||
terminal
|
||||
.backend()
|
||||
.vt100()
|
||||
.screen()
|
||||
.rows(/*start*/ 0, width)
|
||||
.collect::<Vec<_>>()[4..],
|
||||
["composer-2", "next-row"]
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn standard_viewport_growth_uses_scrollback_preserving_newlines() {
|
||||
for scroll_by in [1, 3, 4] {
|
||||
let screen_size = Size::new(/*width*/ 24, /*height*/ 6);
|
||||
let cursor = Position::new(/*x*/ 7, /*y*/ 5);
|
||||
let backend = CrosstermBackend::new(Vec::<u8>::new());
|
||||
let mut terminal =
|
||||
Terminal::with_screen_size_and_cursor_position_for_test(backend, screen_size, cursor);
|
||||
ScrollbackStrategy::Standard
|
||||
.grow_viewport(
|
||||
&mut terminal,
|
||||
/*viewport_top*/ 4,
|
||||
screen_size,
|
||||
scroll_by,
|
||||
)
|
||||
.expect("grow standard viewport");
|
||||
assert_eq!(
|
||||
terminal.backend().writer(),
|
||||
format!(
|
||||
"\x1b[1;4r\x1b[4;1H{}\x1b[r\x1b[6;8H",
|
||||
"\r\n".repeat(usize::from(scroll_by))
|
||||
)
|
||||
.as_bytes()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn standard_viewport_growth_with_one_history_row_preserves_scrollback() {
|
||||
let width = 24;
|
||||
let height = 3;
|
||||
let backend = VT100Backend::with_scrollback(width, height, /*scrollback_len*/ 32);
|
||||
let mut terminal = Terminal::with_options(backend).expect("terminal with scrollback");
|
||||
queue!(
|
||||
terminal.backend_mut(),
|
||||
Print("history\r\nstale-composer\r\nstale-footer")
|
||||
)
|
||||
.expect("seed terminal");
|
||||
ScrollbackStrategy::Standard
|
||||
.grow_viewport(
|
||||
&mut terminal,
|
||||
/*viewport_top*/ 1,
|
||||
Size::new(width, height),
|
||||
/*scroll_by*/ 1,
|
||||
)
|
||||
.expect("grow past the single history row");
|
||||
let screen = terminal.backend().vt100().screen();
|
||||
let mut history = screen.clone();
|
||||
history.set_scrollback(/*rows*/ usize::MAX);
|
||||
insta::assert_snapshot!(format!("SCROLLBACK:\n{}\nVISIBLE:\n{}", history.contents(), screen.contents()), @r"
|
||||
SCROLLBACK:
|
||||
history
|
||||
VISIBLE:
|
||||
");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn standard_viewport_growth_without_rows_is_cursor_neutral() {
|
||||
for (viewport_top, scroll_by) in [(4, 0), (0, 1)] {
|
||||
let backend = VT100Backend::new(/*width*/ 24, /*height*/ 6);
|
||||
let mut terminal = Terminal::with_options(backend).expect("terminal");
|
||||
let cursor = Position::new(/*x*/ 7, /*y*/ 5);
|
||||
terminal
|
||||
.set_cursor_position(cursor)
|
||||
.expect("position cursor");
|
||||
let before = terminal.backend().vt100().screen().state_formatted();
|
||||
ScrollbackStrategy::Standard
|
||||
.grow_viewport(
|
||||
&mut terminal,
|
||||
viewport_top,
|
||||
Size::new(/*width*/ 24, /*height*/ 6),
|
||||
scroll_by,
|
||||
)
|
||||
.expect("no-op viewport growth");
|
||||
assert_eq!(
|
||||
terminal.backend().vt100().screen().state_formatted(),
|
||||
before
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
+10
@@ -0,0 +1,10 @@
|
||||
---
|
||||
source: tui/src/tui/scrollback_tests.rs
|
||||
expression: screen.contents()
|
||||
---
|
||||
history-2
|
||||
history-3
|
||||
history-4
|
||||
|
||||
composer-1
|
||||
composer-2
|
||||
+10
@@ -0,0 +1,10 @@
|
||||
---
|
||||
source: tui/src/tui/scrollback_tests.rs
|
||||
expression: screen.contents()
|
||||
---
|
||||
history-4
|
||||
|
||||
|
||||
|
||||
composer-1
|
||||
composer-2
|
||||
+10
@@ -0,0 +1,10 @@
|
||||
---
|
||||
source: tui/src/tui/scrollback_tests.rs
|
||||
expression: screen.contents()
|
||||
---
|
||||
|
||||
|
||||
|
||||
|
||||
composer-1
|
||||
composer-2
|
||||
Reference in New Issue
Block a user