diff --git a/src/terminal/input.rs b/src/terminal/input.rs index 03ec4344..86ac5aeb 100644 --- a/src/terminal/input.rs +++ b/src/terminal/input.rs @@ -6,7 +6,7 @@ use super::view::TerminalView; use crate::core::config::Config; /// Everything about the terminal's current state that changes how a keystroke -/// is encoded: the kitty protocol flags, plus DECCKM (application cursor keys). +/// is encoded: keyboard modes and the PTY that receives the bytes. #[derive(Clone, Copy, Default)] pub(super) struct KeyFlags { disambiguate: bool, @@ -15,6 +15,7 @@ pub(super) struct KeyFlags { /// DECCKM. ncurses apps turn this on via `smkx` and then only recognise the /// SS3 form of the arrow keys, because that is what `kcuu1` & co. spell. app_cursor: bool, + local_conpty: bool, } impl KeyFlags { @@ -24,6 +25,14 @@ impl KeyFlags { report_all_keys: mode.contains(TermMode::REPORT_ALL_KEYS_AS_ESC), report_text: mode.contains(TermMode::REPORT_ASSOCIATED_TEXT), app_cursor: mode.contains(TermMode::APP_CURSOR), + local_conpty: false, + } + } + + pub(super) fn from_mode_with_local_conpty(mode: &TermMode, local_conpty: bool) -> Self { + Self { + local_conpty, + ..Self::from_mode(mode) } } @@ -31,6 +40,17 @@ impl KeyFlags { self.disambiguate || self.report_all_keys } + pub(super) fn legacy_newline_bytes(self) -> &'static [u8] { + if self.local_conpty { + // ConPTY decodes bare LF as Ctrl+Enter. Explicit Ctrl+J events + // preserve the key for native readers and still produce LF for + // VT readers such as ssh and WSL. + b"\x1b[74;36;10;1;8;1_\x1b[74;36;10;0;8;1_" + } else { + b"\n" + } + } + pub(super) fn app_cursor(self) -> bool { self.app_cursor } @@ -374,6 +394,9 @@ fn legacy_keystroke_to_bytes(ks: &gpui::Keystroke, flags: KeyFlags) -> Option KeyFlags { @@ -558,6 +582,7 @@ mod tests { report_all_keys: true, report_text: true, app_cursor: false, + local_conpty: false, } } @@ -567,6 +592,7 @@ mod tests { report_all_keys: false, report_text: false, app_cursor: false, + local_conpty: false, } } @@ -582,6 +608,54 @@ mod tests { } } + #[test] + fn legacy_newline_preserves_ctrl_j_for_native_console_readers() { + let ctrl_j = Keystroke::parse("ctrl-j").unwrap(); + for (local_conpty, expected) in [ + (false, b"\n".as_slice()), + (true, b"\x1b[74;36;10;1;8;1_\x1b[74;36;10;0;8;1_".as_slice()), + ] { + let flags = KeyFlags::from_mode_with_local_conpty(&TermMode::empty(), local_conpty); + assert_eq!(flags.legacy_newline_bytes(), expected); + assert_eq!( + keystroke_to_bytes(&ctrl_j, flags).as_deref(), + Some(expected) + ); + for (chord, expected) in [ + ("enter", b"\r".as_slice()), + ("ctrl-c", b"\x03".as_slice()), + ("alt-enter", b"\x1b\r".as_slice()), + ("ctrl-alt-j", b"\x1b\n".as_slice()), + ] { + assert_eq!( + keystroke_to_bytes(&Keystroke::parse(chord).unwrap(), flags).as_deref(), + Some(expected), + "{chord}, local_conpty={local_conpty}" + ); + } + } + } + + #[test] + fn kitty_newline_chords_take_precedence_over_conpty_encoding() { + for mode in [ + TermMode::DISAMBIGUATE_ESC_CODES, + TermMode::REPORT_ALL_KEYS_AS_ESC, + ] { + let flags = KeyFlags::from_mode_with_local_conpty(&mode, true); + for (chord, expected) in [ + ("ctrl-j", b"\x1b[106;5u".as_slice()), + ("shift-enter", b"\x1b[13;2u".as_slice()), + ] { + assert_eq!( + keystroke_to_bytes(&Keystroke::parse(chord).unwrap(), flags).as_deref(), + Some(expected), + "{chord}" + ); + } + } + } + #[test] fn plain_text_defers_to_the_ime_unless_kitty_wants_every_key() { let plain = Modifiers::default(); @@ -934,6 +1008,7 @@ mod tests { report_all_keys: true, report_text: true, app_cursor: false, + local_conpty: false, }; for flags in [kitty(), full] { assert_eq!( @@ -1128,6 +1203,7 @@ mod tests { report_all_keys: false, report_text: false, app_cursor: false, + local_conpty: false, } } @@ -1209,6 +1285,7 @@ mod tests { report_all_keys: true, report_text: false, app_cursor: false, + local_conpty: false, }; let none = Modifiers::default(); assert_eq!( @@ -1237,6 +1314,7 @@ mod tests { report_all_keys: true, report_text: false, app_cursor: false, + local_conpty: false, }; assert_eq!(tab_bytes(false, full), b"\x1b[9u".to_vec()); } @@ -1316,6 +1394,7 @@ mod tests { report_all_keys: true, report_text: true, app_cursor: false, + local_conpty: false, }; for key in ["f1", "f2", "f4", "f5", "f7", "f10", "f11", "f12"] { for mods in [none, shift, ctrl] { @@ -1352,6 +1431,7 @@ mod tests { report_all_keys: true, report_text: true, app_cursor: false, + local_conpty: false, }; let none = Modifiers::default(); assert_eq!( @@ -1367,6 +1447,7 @@ mod tests { report_all_keys: true, report_text: true, app_cursor: false, + local_conpty: false, }; let none = Modifiers::default(); assert_eq!( diff --git a/src/terminal/remote.rs b/src/terminal/remote.rs index ec225a46..8862bbda 100644 --- a/src/terminal/remote.rs +++ b/src/terminal/remote.rs @@ -110,7 +110,7 @@ struct ReaderSignals { /// the reader puts back the cursor a repaint parked. Decided per pane from /// its [`PtySource`], and shared rather than copied because the reader can /// learn better mid-stream — see the `RemoteContext` arm. - repair_cursor: Arc, + local_conpty: Arc, } /// What kind of pty is at the far end of a pane's link, which is what decides @@ -146,20 +146,6 @@ impl PtySource { _ => PtySource::Raw, } } - - /// Whether the cursor a repaint parked has to be put back — see - /// [`crate::terminal::parked_cursor`]. - /// - /// Only conhost parks one. On a raw pty the application owns the cursor and - /// is free to end a repaint on the text it just wrote and then echo the - /// next keystroke straight after it, with no positioning of its own: vim - /// opens its command line that way, and putting the cursor back on the cell - /// the repaint hid it on drops the `wq!` typed next onto the row being - /// edited (#430, and #774 for the Windows client that reached a Linux host - /// and was repaired anyway). - fn repairs_parked_cursor(self) -> bool { - self == PtySource::LocalConpty - } } #[derive(Clone, Debug, PartialEq)] @@ -613,12 +599,12 @@ pub struct RemoteTerminal { /// flag under the term lock before every grid mutation, so once it is set /// the abandoned thread can only exit, never write. reader_quit: Arc, - /// Whether this pane's pty is a ConPTY, and so whether the reader repairs + /// Whether this pane's pty is a ConPTY, for input encoding and repairing /// the cursor a repaint parks. Held here so a relink hands the same answer /// to the reader it starts: a pane's pty does not change kind when the link /// to it is rebuilt, and the route a relink carries cannot tell a /// native-SSH pane from a local shell. - repair_cursor: Arc, + local_conpty: Arc, } /// The workspace id a spawn carries, so the pane's shell gets `$TTY7_WS` and a @@ -944,7 +930,7 @@ impl RemoteTerminal { // rebuilt from `route`: the pty on the far side is the same pty // it was before the link dropped, and only this value still // remembers what a `RemoteContext` taught the old reader. - repair_cursor: self.repair_cursor.clone(), + local_conpty: self.local_conpty.clone(), }, ); self.reader_thread = Some(reader); @@ -1038,7 +1024,7 @@ impl RemoteTerminal { let clipboard_write_busy = Arc::new(AtomicBool::new(false)); let reader_quit = Arc::new(AtomicBool::new(false)); - let repair_cursor = Arc::new(AtomicBool::new(pty.repairs_parked_cursor())); + let local_conpty = Arc::new(AtomicBool::new(pty == PtySource::LocalConpty)); let reader_thread = Self::spawn_reader( term.clone(), proxy.clone(), @@ -1062,7 +1048,7 @@ impl RemoteTerminal { images: images.clone(), clipboard_writes: clipboard_writes.clone(), clipboard_write_busy: clipboard_write_busy.clone(), - repair_cursor: repair_cursor.clone(), + local_conpty: local_conpty.clone(), }, ); @@ -1102,7 +1088,7 @@ impl RemoteTerminal { proxy, reader_thread: Some(reader_thread), reader_quit, - repair_cursor, + local_conpty, }) } @@ -1173,7 +1159,7 @@ impl RemoteTerminal { images, clipboard_writes, clipboard_write_busy, - repair_cursor, + local_conpty, } = signals; let mut awaiting_replay = awaiting_replay; crate::core::threads::promote_to_user_interactive(); @@ -1240,7 +1226,7 @@ impl RemoteTerminal { // emulator to the cut, act on the state that // sequence left behind, carry on. let mut cuts: Vec<(usize, CursorCut)> = Vec::new(); - if repair_cursor.load(Ordering::Relaxed) { + if local_conpty.load(Ordering::Relaxed) { cursor_scan.feed(&out_batch, |off, c| cuts.push((off, c))); } { @@ -1561,7 +1547,7 @@ impl RemoteTerminal { .as_ref() .is_some_and(|c| c.kind == RemoteKind::NativeSsh) { - repair_cursor.store(false, Ordering::Relaxed); + local_conpty.store(false, Ordering::Relaxed); } if let Ok(mut guard) = remote.lock() { *guard = ctx; @@ -1692,6 +1678,10 @@ impl RemoteTerminal { self.child_exited.load(Ordering::SeqCst) } + pub(super) fn is_local_conpty(&self) -> bool { + self.local_conpty.load(Ordering::Relaxed) + } + /// Queues a keystroke — or a paste, or a mouse report — for the link. /// /// Callers are gpui event handlers on the UI thread, so this returns @@ -4084,6 +4074,7 @@ mod parked_cursor_tests { let (client_side, daemon_side) = socket_pair(); let term = RemoteTerminal::from_stream_with(client_side, size, Vec::new(), pty) .expect("a terminal over a socket pair"); + assert_eq!(term.is_local_conpty(), pty == PtySource::LocalConpty); (term, daemon_side) } @@ -4127,8 +4118,6 @@ mod parked_cursor_tests { PtySource::Raw }, ); - assert!(PtySource::LocalConpty.repairs_parked_cursor()); - assert!(!PtySource::Raw.repairs_parked_cursor()); } #[test] @@ -4280,6 +4269,7 @@ mod parked_cursor_tests { term.remote_context().is_some(), "the reader never applied the context" ); + assert!(!term.is_local_conpty()); DaemonMsg::Output(b"\x1b[6;4H".to_vec()) .encode(&mut daemon_side) diff --git a/src/terminal/view.rs b/src/terminal/view.rs index 1c82402e..d669ad1e 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -2965,7 +2965,10 @@ impl TerminalView { } pub(super) fn key_flags(&self) -> super::input::KeyFlags { - super::input::KeyFlags::from_mode(self.terminal.term.lock().mode()) + super::input::KeyFlags::from_mode_with_local_conpty( + self.terminal.term.lock().mode(), + self.terminal.is_local_conpty(), + ) } fn tab_bytes(&self, shift: bool) -> Vec { @@ -4497,7 +4500,7 @@ impl TerminalView { { cx.propagate(); } else { - self.send_shortcut_bytes(b"\n", "enter", cx); + self.send_shortcut_bytes(self.key_flags().legacy_newline_bytes(), "enter", cx); } } @@ -9894,9 +9897,22 @@ mod gpui_tests { use super::*; use crate::daemon::protocol::{ClientMsg, DaemonMsg}; use crate::daemon::transport::Stream; + use crate::terminal::remote::PtySource; use gpui::{Entity, TestAppContext, point}; fn harness(cx: &mut TestAppContext) -> (gpui::WindowHandle, Stream) { + let pty = if cfg!(windows) { + PtySource::LocalConpty + } else { + PtySource::Raw + }; + harness_on(cx, pty) + } + + fn harness_on( + cx: &mut TestAppContext, + pty: PtySource, + ) -> (gpui::WindowHandle, Stream) { // Building a view reads the config. Whether that hit the real user // directory used to come down to which test happened to pin the // scratch dir first. @@ -9908,8 +9924,13 @@ mod gpui_tests { cx.set_global(Config::default()); }); let window = cx.add_window(|window, cx| { - let terminal = RemoteTerminal::from_stream(client_side, TermSize::new(80, 24)) - .expect("socketpair-backed terminal"); + let terminal = RemoteTerminal::from_stream_with( + client_side, + TermSize::new(80, 24), + Vec::new(), + pty, + ) + .expect("socketpair-backed terminal"); TerminalView::with_terminal(terminal, 1, window, cx) }); (window, daemon_side) @@ -12516,7 +12537,7 @@ mod gpui_tests { #[gpui::test] fn shift_enter_reaches_a_foreground_tui_with_kitty_encoding(cx: &mut TestAppContext) { crate::core::config::pin_test_config_dir(); - let (window, mut daemon) = harness(cx); + let (window, mut daemon) = harness_on(cx, PtySource::LocalConpty); cx.update(|cx| crate::ui::keymap::init(cx)); DaemonMsg::Output(b"\x1b[>1u".to_vec()) .encode(&mut daemon) @@ -12546,12 +12567,18 @@ mod gpui_tests { next_input_until_timeout(&mut daemon), Some(b"\x1b[13;2u".to_vec()) ); + + vcx.simulate_keystrokes("ctrl-j"); + assert_eq!( + next_input_until_timeout(&mut daemon), + Some(b"\x1b[106;5u".to_vec()) + ); } #[gpui::test] fn shift_enter_reaches_a_foreground_tui_as_lf_without_kitty(cx: &mut TestAppContext) { crate::core::config::pin_test_config_dir(); - let (window, mut daemon) = harness(cx); + let (window, mut daemon) = harness_on(cx, PtySource::Raw); cx.update(|cx| crate::ui::keymap::init(cx)); window .update(cx, |view, window, cx| { @@ -12565,6 +12592,32 @@ mod gpui_tests { vcx.simulate_keystrokes("shift-enter"); assert_eq!(next_input_until_timeout(&mut daemon), Some(b"\n".to_vec())); + + vcx.simulate_keystrokes("ctrl-j"); + assert_eq!(next_input_until_timeout(&mut daemon), Some(b"\n".to_vec())); + } + + #[gpui::test] + fn newline_chords_reach_conpty_as_ctrl_j_without_kitty(cx: &mut TestAppContext) { + let (window, mut daemon) = harness_on(cx, PtySource::LocalConpty); + cx.update(|cx| crate::ui::keymap::init(cx)); + window + .update(cx, |view, window, cx| { + assert!(!view.input_active()); + window.activate_window(); + view.focus_handle.focus(window, cx); + }) + .unwrap(); + + let mut vcx = gpui::VisualTestContext::from_window(window.into(), cx); + for chord in ["shift-enter", "ctrl-j"] { + vcx.simulate_keystrokes(chord); + assert_eq!( + next_input_until_timeout(&mut daemon), + Some(b"\x1b[74;36;10;1;8;1_\x1b[74;36;10;0;8;1_".to_vec()), + "{chord} must preserve Ctrl+J for native console readers" + ); + } } #[gpui::test]