diff --git a/crates/tty7-core/src/daemon/pane.rs b/crates/tty7-core/src/daemon/pane.rs index 67a05d1b..ad6fc8ad 100644 --- a/crates/tty7-core/src/daemon/pane.rs +++ b/crates/tty7-core/src/daemon/pane.rs @@ -1255,7 +1255,11 @@ pub struct Restore { /// for and cannot see. Leaving the alternate screen also does the useful thing /// in the common case: the primary buffer still holds the pre-`vim` scrollback /// from earlier in the same snapshot. -fn restore_preamble(banner: Option<&str>) -> Vec { +/// +/// On Windows it ends by scrolling the restored screen out of the viewport +/// ([`SCROLL_RESTORED_AWAY`]), which is a correctness requirement rather than a +/// matter of taste — see that constant. +pub fn restore_preamble(banner: Option<&str>) -> Vec { let mut out = Vec::new(); out.extend_from_slice(b"\x1b[?1049l\x1b[?25h\x1b[?7h\x1b[0m"); if let Some(banner) = banner.map(str::trim).filter(|b| !b.is_empty()) { @@ -1265,9 +1269,40 @@ fn restore_preamble(banner: Option<&str>) -> Vec { out.extend_from_slice(banner.replace(['\r', '\n'], " ").as_bytes()); out.extend_from_slice(b" \xe2\x94\x80\xe2\x94\x80\x1b[0m\r\n"); } + if cfg!(windows) { + out.extend_from_slice(SCROLL_RESTORED_AWAY); + } out } +/// Push the restored screen into the client's scrollback and put the cursor +/// back at the top-left, so the incoming shell starts on a blank viewport. +/// +/// A pty on unix hands the terminal a stream; a ConPTY hands it a *rendering of +/// a screen buffer it owns*. That buffer starts blank with its cursor at the +/// top-left, and conhost addresses it absolutely: PSReadLine redrawing the line +/// being typed emits `ESC[6;20H`, meaning row 6 of conhost's buffer, and every +/// frame conhost paints is positioned the same way. Those row numbers are only +/// correct if the client's viewport is conhost's buffer, row for row. +/// +/// Restored output breaks exactly that. It is output conhost never produced and +/// knows nothing about, so leaving it on screen puts the shell's first prompt +/// some rows below where conhost believes it is, and the first keystroke +/// repaints the input line *over the restored text* — the prompt stops +/// responding and the old screen fills with fragments of what is being typed. +/// Nothing the client can do fixes that after the fact: the offset is not a +/// constant (the screen scrolls) and it would have to be unpicked from every +/// absolute address in the stream. +/// +/// So the restored screen goes where it can be kept without claiming a row: +/// `ESC[2J` on the primary screen scrolls the viewport into history rather than +/// erasing it, so it is a scroll away, and `ESC[H` leaves the cursor where a +/// fresh ConPTY expects to find it. +/// +/// Not done on unix, where the shell positions itself relatively and the +/// restored screen can simply stay where the user can see it. +pub const SCROLL_RESTORED_AWAY: &[u8] = b"\x1b[2J\x1b[H"; + impl DaemonPane { pub fn spawn( id: u64, @@ -3626,6 +3661,33 @@ mod tests { assert!(text.contains("this shell is new")); } + /// The ConPTY constraint, from the daemon's side. A pane whose shell runs + /// on a ConPTY must open with an empty viewport and the cursor at the + /// top-left, because that is the state conhost's own screen buffer starts + /// in and every row it names afterwards is counted from there. Restored + /// output left on screen shifts all of them, and the shell's first repaint + /// of the line being typed lands on the old text — see + /// [`SCROLL_RESTORED_AWAY`]. + #[test] + fn the_preamble_clears_the_way_for_conpty_and_only_for_conpty() { + let text = String::from_utf8(restore_preamble(Some("this shell is new"))).unwrap(); + if cfg!(windows) { + assert!( + text.ends_with("\x1b[2J\x1b[H"), + "the restored screen has to be scrolled into history and the cursor \ + homed *last*, after the banner: anything printed afterwards would \ + take back the row conhost counts from. The preamble ends {:?}", + &text[text.len().saturating_sub(16)..] + ); + } else { + assert!( + !text.contains("\x1b[2J"), + "on a real pty the shell positions itself relatively, so the screen \ + the user asked to have back stays where they can see it" + ); + } + } + #[test] fn a_banner_cannot_smuggle_extra_lines_into_the_pane() { let text = String::from_utf8(restore_preamble(Some("first\r\nsecond"))).unwrap(); diff --git a/src/terminal/remote.rs b/src/terminal/remote.rs index a58191c1..15f78582 100644 --- a/src/terminal/remote.rs +++ b/src/terminal/remote.rs @@ -2246,7 +2246,7 @@ fn win_size(size: TermSize, cell_w: u16, cell_h: u16) -> WinSize { } #[cfg(all(test, windows))] -mod windows_teardown_tests { +mod windows_tests { use super::*; fn tcp_pair() -> (std::net::TcpStream, std::net::TcpStream) { @@ -2346,6 +2346,110 @@ mod windows_teardown_tests { "the abandoned link must not tear the adopted pane down" ); } + + /// What the daemon replays into a pane restored from a stored screen, in + /// the frames and the order it sends them: the dead pane's screen, then the + /// preamble, then the new shell's own first output. + fn replay_restore_into(daemon: &mut std::net::TcpStream, old_screen: &[u8], shell: &[u8]) { + let size = WinSize { + cols: 40, + rows: 10, + cell_w: 8, + cell_h: 17, + }; + DaemonMsg::Size(size).encode(daemon).unwrap(); + DaemonMsg::Snapshot(old_screen.to_vec()) + .encode(daemon) + .unwrap(); + DaemonMsg::Size(size).encode(daemon).unwrap(); + DaemonMsg::Snapshot(crate::daemon::pane::restore_preamble(Some( + "the shell below is new", + ))) + .encode(daemon) + .unwrap(); + DaemonMsg::Output(shell.to_vec()).encode(daemon).unwrap(); + } + + fn row(term: &RemoteTerminal, line: i32) -> String { + use alacritty_terminal::grid::Dimensions as _; + use alacritty_terminal::index::{Column, Line}; + let term = term.term.lock(); + let grid = term.grid(); + (0..grid.columns()) + .map(|c| grid[Line(line)][Column(c)].c) + .collect::() + .trim_end() + .to_string() + } + + /// A restored screen must not be sitting in the viewport when the new + /// shell's ConPTY starts drawing on it. + /// + /// conhost addresses its own screen buffer absolutely — PSReadLine repaints + /// the line being typed with `ESC[6;20H` and conhost frames it the same way + /// — and that buffer starts blank with the cursor at the top-left. Restored + /// output is output conhost never produced: left on screen it shifts every + /// row conhost names, so the first keystroke repaints the input line on top + /// of the old text instead of at the prompt. The restored screen belongs in + /// scrollback, where it survives without claiming a row. + #[test] + fn a_restored_screen_leaves_the_new_shell_the_viewport_conpty_thinks_it_has() { + crate::core::config::pin_test_config_dir(); + let (client_side, mut daemon_side) = tcp_pair(); + let term = RemoteTerminal::from_stream(client_side, TermSize::new(40, 10)).unwrap(); + + // Five lines of the dead pane, then the shell painting its prompt the + // way conhost does: at row 1 of a buffer it believes is blank. + replay_restore_into( + &mut daemon_side, + b"line one\r\nline two\r\nline three\r\nline four\r\nline five\r\n", + b"\x1b[?25l\x1b[1;1HPS C:\\> \x1b[?25h", + ); + + let mut top = String::new(); + for _ in 0..400 { + top = row(&term, 0); + if top.starts_with("PS C:") { + break; + } + std::thread::sleep(std::time::Duration::from_millis(5)); + } + assert_eq!( + top, "PS C:\\>", + "the shell's prompt paints where conhost put it" + ); + + for line in 1..10 { + assert_eq!( + row(&term, line), + "", + "row {line} still holds restored output, so conhost and the client \ + disagree about which row is which: the next repaint of the input \ + line lands on the old screen instead of at the prompt" + ); + } + + // Kept, not erased: `ESC[2J` on the primary screen scrolls the viewport + // into history, so the screen the daemon restored is one scroll away. + let depth = { + use alacritty_terminal::grid::Dimensions as _; + term.term.lock().grid().history_size() as i32 + }; + let history: Vec = (-depth..0).map(|line| row(&term, line)).collect(); + for wanted in [ + "line one", + "line two", + "line three", + "line four", + "line five", + ] { + assert!( + history.iter().any(|row| row == wanted), + "{wanted:?} is not in the scrollback; the restored screen was erased \ + rather than scrolled away. History holds {history:?}" + ); + } + } } #[cfg(all(test, unix))]