diff --git a/Cargo.lock b/Cargo.lock index 6549e03e..4675498e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -236,7 +236,7 @@ dependencies = [ [[package]] name = "alacritty_terminal" version = "0.26.1-dev" -source = "git+https://github.com/l0ng-ai/alacritty?rev=1276f128fbaa8832cbc66210675cbe8aeb570499#1276f128fbaa8832cbc66210675cbe8aeb570499" +source = "git+https://github.com/l0ng-ai/alacritty?rev=fe249ca805b4813940fbaee3974357b7e73df30a#fe249ca805b4813940fbaee3974357b7e73df30a" dependencies = [ "base64 0.22.1", "bitflags 2.13.1", diff --git a/Cargo.toml b/Cargo.toml index 2405f9d6..6df6e316 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -333,8 +333,9 @@ gpui_platform = { git = "https://github.com/zed-industries/zed", rev = "1d217ee3 # (`tty7 capture --plain`). The daemon parses nothing; it is a byte pipe, and its # PTY is driven by `portable-pty`. # -# The `tty7` branch is Zed's `fcf32fe` (the rev Zed pins) plus two commits, both -# still missing from alacritty master as of 852e971: +# The `tty7` branch is Zed's `fcf32fe` (the rev Zed pins) plus four commits. +# Three are fixes still missing from alacritty master as of 852e971 and one +# (`Config::conpty_resize`, #415) is a tty7-only opt-in: # # 1. `push_keyboard_mode` capped its stack by removing from `title_stack` instead # of `keyboard_mode_stack` — a copy-paste slip from `push_title` that compiles @@ -348,10 +349,22 @@ gpui_platform = { git = "https://github.com/zed-industries/zed", rev = "1d217ee3 # column instead of two and shifts the rest of the line left by one. The fork # re-scores the sequence with `UnicodeWidthStr` and widens the cell (issue # #203). +# 3. `Config::conpty_resize` resizes the primary grid the way conhost re-anchors +# its own buffer, so a ConPTY pane's absolute cursor addresses stay right +# after a resize (#415). +# 4. `Term::reset_keyboard_modes` empties both screens' kitty keyboard stacks +# and clears the flags in effect. No escape sequence can: `CSI = 0 u` only +# rewrites the flags in effect, so a push a killed program left on either +# screen's stack came back on the next pop or screen switch. The reader calls +# it when the shell's prompt returns (#1155). Additive; nothing upstream +# calls it. # -# Each patch has a guard test in `src/terminal/remote.rs`; drop this fork once -# both land upstream. -alacritty_terminal = { git = "https://github.com/l0ng-ai/alacritty", rev = "1276f128fbaa8832cbc66210675cbe8aeb570499" } +# Patches 1, 2 and 4 each have a guard test in `src/terminal/remote.rs` +# (`deep_keyboard_mode_pushes_leave_the_reader_alive`, +# `emoji_presentation_sequences_reserve_two_columns`, +# `a_prompt_forgets_kitty_flags_a_killed_program_left_on_either_screen`); drop +# this fork once they land upstream and #415 has another home. +alacritty_terminal = { git = "https://github.com/l0ng-ai/alacritty", rev = "fe249ca805b4813940fbaee3974357b7e73df30a" } anyhow = "1" log = "0.4" diff --git a/crates/tty7-core/src/core/term_modes.rs b/crates/tty7-core/src/core/term_modes.rs index 6078aa19..ba62e196 100644 --- a/crates/tty7-core/src/core/term_modes.rs +++ b/crates/tty7-core/src/core/term_modes.rs @@ -125,6 +125,15 @@ impl KeyboardStacks { self.reload(); } + /// Both stacks emptied and no flags in effect, on whichever screen is up + /// — what `Term::reset_keyboard_modes` leaves in the client's emulator. + fn clear(&mut self) { + *self = Self { + on_alt: self.on_alt, + ..Self::default() + }; + } + /// The bytes that rebuild these stacks on a fresh emulator, around `dec` /// (mode sets that may include the switch to the alternate screen). /// @@ -406,9 +415,15 @@ impl TerminalModes { /// terminal again, so whatever program switched 2031 on is gone — possibly /// killed before it could send `?2031l`. A report after that would be /// typed into the shell's command line. + /// + /// The same goes for the kitty keyboard stacks, on both screens: the + /// client clears them at the prompt (`Term::reset_keyboard_modes`), so a + /// replay prelude built from flags a dead program pushed before the mark + /// would hand them straight back to the next program (#1155). fn osc_end(&mut self) { if matches!(self.params.as_slice(), b"133;A" | b"133;B" | b"133;D") { self.on.retain(|m| *m != COLOR_SCHEME_UPDATES); + self.keyboard.clear(); } self.params.clear(); self.state = State::Text; @@ -699,6 +714,38 @@ mod tests { ); } + /// #1155: a prompt mark forgets what the program before it pushed, on + /// both screens, but not what is pushed after it. + #[test] + fn a_shell_prompt_clears_both_kitty_keyboard_stacks() { + for mark in [ + &b"\x1b]133;A\x07"[..], + b"\x1b]133;B\x1b\\", + b"\x1b]133;D;0\x07", + ] { + let mut bytes = b"\x1b[>1u\x1b[?1049h\x1b[>7u".to_vec(); + bytes.extend_from_slice(mark); + let k = keyboard(&bytes); + assert_eq!( + k, + KeyboardStacks { + on_alt: true, + ..KeyboardStacks::default() + }, + "the screen stays where it was; only the flags go" + ); + bytes.extend_from_slice(b"\x1b[>1u"); + let k = keyboard(&bytes); + assert_eq!((k.alternate.as_slice(), k.active), (&[1][..], 1)); + } + assert_eq!(keyboard(b"\x1b[>1u\x1b]133;C\x07").active, 1); + + // A replay prelude cut past the mark has nothing left to restore. + let mut head = TerminalModes::new(); + head.feed(b"\x1b[>7u\x1b]133;A\x07"); + assert!(head.restore_bytes().is_none()); + } + #[test] fn bracketed_paste_reads_as_the_shell_last_left_it() { // zle switches it on to read a line and off again before running it, diff --git a/crates/tty7-core/src/daemon/pane.rs b/crates/tty7-core/src/daemon/pane.rs index d03d2b67..25cdc1f6 100644 --- a/crates/tty7-core/src/daemon/pane.rs +++ b/crates/tty7-core/src/daemon/pane.rs @@ -1744,8 +1744,11 @@ pub fn restore_preamble(banner: Option<&str>) -> Vec { /// - `2004` is bracketed paste: a paste into a shell that does not know the /// protocol arrives with `ESC[200~` typed around it. /// - `1` is DECCKM, which sends the arrow keys as `ESC O A` instead of `ESC[A`. -/// - `CSI = 0 ; 1 u` sets the kitty keyboard flags to none, the same reset -/// `stale_mode_resets` in the client uses. +/// - `CSI = 0 ; 1 u` sets the kitty keyboard flags in effect to none. It +/// leaves their stacks alone — no sequence clears those — so a pop could +/// still bring a dead program's flags back; the new shell's first prompt +/// mark is what clears the stacks (`Term::reset_keyboard_modes` in the +/// client, `TerminalModes` here). /// /// Deliberately absent: `1007` (alternate scroll), which this emulator has *on* /// by default, so clearing it would walk away from the default rather than back diff --git a/src/terminal/remote.rs b/src/terminal/remote.rs index 07390333..a80721f2 100644 --- a/src/terminal/remote.rs +++ b/src/terminal/remote.rs @@ -1523,13 +1523,27 @@ impl RemoteTerminal { // comes back with the prompt's cursor. let mut cuts: Vec<(usize, ReaderCut)> = Vec::new(); command_cuts(&mut command_tok, &bytes, &mut cuts); + // Each prompt mark in it also clears the kitty + // keyboard stacks, at the mark itself. Live, + // that happens on the `Prompt` report that + // follows it; a replay carries one report for + // the whole ring, so flags a dead program left + // before an earlier prompt would otherwise come + // back to whatever runs in the pane now (#1155). + // The daemon's `TerminalModes` clears them at + // the same marks, so the prelude ahead of the + // ring agrees. let fed = feed_grid(&term, &mut processor, &bytes, cuts, &quit, |term, cut| { match cut { ReaderCut::Command(mark) => { + if mark == CommandMark::Finished { + term.reset_keyboard_modes(); + } command_cursor.apply(term, mark) } ReaderCut::Prompt(mark) => { + term.reset_keyboard_modes(); prompt_break.apply(term, mark) } ReaderCut::Parked(_) => {} @@ -1724,7 +1738,20 @@ impl RemoteTerminal { let resets = stale_mode_resets(*term.mode()); if !resets.is_empty() { processor.advance(&mut *term, &resets); - drop(term); + } + // Nothing in front of the shell could have + // negotiated kitty keyboard flags, so they + // all go — both screens' stacks, not just + // the flags in effect. `CSI = 0 u` cannot + // do that: a program killed on the + // alternate screen left its push on that + // screen's stack, and the next program to + // enter it got the flags back and its + // Escape as `CSI 27 u` — which a ConPTY + // reader sees as no key at all (#1155). + term.reset_keyboard_modes(); + drop(term); + if !resets.is_empty() { proxy.send_event(AlacEvent::Wakeup); } } @@ -2822,9 +2849,8 @@ fn stale_mode_resets(mode: TermMode) -> Vec { if mode.contains(TermMode::FOCUS_IN_OUT) { seq.extend_from_slice(b"\x1b[?1004l"); } - if mode.intersects(TermMode::KITTY_KEYBOARD_PROTOCOL) || mode.contains(TermMode::ALT_SCREEN) { - seq.extend_from_slice(b"\x1b[=0;1u"); - } + // The kitty keyboard flags are not reset here: no sequence clears their + // stacks, so the caller does it through `Term::reset_keyboard_modes`. seq } @@ -4411,6 +4437,160 @@ mod replay_tests { view stopped receiving output" ); } + + /// What a physical Escape is written to the pane as, by the encoder a + /// local ConPTY pane uses (#1155 is a ConPTY-only symptom), once the + /// reader has applied everything up to `barrier`. Also returns the reply + /// a `CSI ? u` sent after the barrier gets. + fn escape_after(frames: Vec, barrier: &str) -> (Vec, Option) { + use crate::terminal::input::{KeyFlags, keystroke_to_bytes}; + crate::core::config::pin_test_config_dir(); + let (client_side, mut daemon) = socket_pair(); + let term = RemoteTerminal::from_stream(client_side, TermSize::new(80, 24)).unwrap(); + for frame in frames { + frame.encode(&mut daemon).unwrap(); + } + DaemonMsg::Output(format!("{barrier}\x1b[?u").into_bytes()) + .encode(&mut daemon) + .unwrap(); + let text = settled(&term, &[barrier]); + assert!(text.contains(barrier), "the barrier never arrived:\n{text}"); + let mut reply = None; + for _ in 0..200 { + while let Ok(event) = term.events.try_recv() { + if let AlacEvent::PtyWrite(text) = event { + reply = Some(text); + } + } + if reply.is_some() { + break; + } + std::thread::sleep(std::time::Duration::from_millis(5)); + } + let mode = *term.term.lock().mode(); + let flags = KeyFlags::from_mode_with_local_conpty(&mode, true); + let escape = keystroke_to_bytes(&gpui::Keystroke::parse("escape").unwrap(), flags); + drop(daemon); + (escape.unwrap_or_default(), reply) + } + + fn at_prompt() -> DaemonMsg { + DaemonMsg::Prompt { + active: true, + at_prompt: true, + last_exit: Some(130), + } + } + + /// #1155: a program pushed kitty keyboard flags on the main screen and + /// again on the alternate one, and was killed there. Once the prompt is + /// back, the next program to enter the alternate screen must not find the + /// dead one's flags on its stack. + #[test] + fn a_prompt_forgets_kitty_flags_a_killed_program_left_on_either_screen() { + let (escape, reply) = escape_after( + vec![ + DaemonMsg::Output(b"\x1b[>1u\x1b[?1049h\x1b[>7u".to_vec()), + at_prompt(), + DaemonMsg::Output(b"\x1b[?1049h".to_vec()), + ], + "NEXT-PROGRAM", + ); + assert_eq!(escape, b"\x1b", "Escape must reach the next program bare"); + assert_eq!( + reply.as_deref(), + Some("\x1b[?0u"), + "a flags query must agree with the flags in effect" + ); + } + + /// #1155: a program that left the alternate screen *before* popping + /// popped the main screen's stack, so its push stayed on the alternate + /// one while no flags were in effect at the prompt. + #[test] + fn a_prompt_forgets_a_kitty_push_stranded_on_the_inactive_screen() { + let (escape, _) = escape_after( + vec![ + DaemonMsg::Output(b"\x1b[?1049h\x1b[>7u\x1b[?1049l\x1b[7u".to_vec()), + at_prompt(), + DaemonMsg::Output(b"\x1b[>1u\x1b[7u".to_vec()), + at_prompt(), + DaemonMsg::Output(b"\x1b[?1049h\x1b[>1u".to_vec()), + ], + "NEXT-PROGRAM", + ); + assert_eq!(escape, b"\x1b[27u"); + assert_eq!(reply.as_deref(), Some("\x1b[?1u")); + } + + /// The same on a re-attach. The replay carries one prompt report for the + /// whole ring, and here it says a program owns the pane — so the prompt + /// the shell drew between the dead program and this one is only in the + /// ring's bytes, and has to clear the flags where it stands. + #[test] + fn a_replay_forgets_kitty_flags_from_before_an_earlier_prompt() { + let (escape, _) = escape_after( + vec![ + DaemonMsg::Size(ws(80, 24)), + DaemonMsg::Snapshot(b"\x1b[>1u\x1b[?1049h\x1b[>7u".to_vec()), + DaemonMsg::Snapshot( + b"\x1b]133;D;130\x07\x1b]133;A\x07$ \x1b]133;B\x07codex\r\n\x1b]133;C\x07" + .to_vec(), + ), + DaemonMsg::Snapshot(b"\x1b[?1049h".to_vec()), + DaemonMsg::Prompt { + active: true, + at_prompt: false, + last_exit: Some(130), + }, + ], + "NEXT-PROGRAM", + ); + assert_eq!(escape, b"\x1b"); + + // What the program after the prompt pushes is replayed as it was. + let (escape, _) = escape_after( + vec![ + DaemonMsg::Size(ws(80, 24)), + DaemonMsg::Snapshot(b"\x1b[>7u\x1b]133;A\x07\x1b]133;C\x07\x1b[>1u".to_vec()), + DaemonMsg::Prompt { + active: true, + at_prompt: false, + last_exit: None, + }, + ], + "NEXT-PROGRAM", + ); + assert_eq!(escape, b"\x1b[27u"); + } } #[cfg(all(test, windows))] @@ -5799,14 +5979,18 @@ mod tests { assert!(text.contains("\x1b[?25h")); assert!(text.contains("\x1b[?1002l")); assert!(text.contains("\x1b[?1006l")); - assert!(text.ends_with("\x1b[=0;1u")); + assert!( + !text.contains("u"), + "kitty flags are the caller's: {text:?}" + ); let kitty = TermMode::SHOW_CURSOR | TermMode::DISAMBIGUATE_ESC_CODES; - assert_eq!(stale_mode_resets(kitty), b"\x1b[=0;1u"); + assert!(stale_mode_resets(kitty).is_empty()); } #[test] fn prompt_report_scrubs_stale_tui_modes() { + crate::core::config::pin_test_config_dir(); let (client_side, mut daemon_side) = UnixStream::pair().unwrap(); let term = RemoteTerminal::from_stream(client_side, TermSize::new(80, 24)).unwrap();