diff --git a/CHANGELOG.md b/CHANGELOG.md index 3101f3b6..6138e457 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,24 @@ real world uses." ### Fixed +- **Kitty keyboard protocol now activates on Windows (VSCode + + Windows Terminal), so `Shift+Enter` inserts a newline instead + of submitting** (#1359, harvested from PR #1483 by + **@CrepuscularIRIS / autoghclaw**). Root cause: crossterm's + `PushKeyboardEnhancementFlags` gates the escape sequence on + `is_ansi_code_supported()`, which on Windows queries the + console mode rather than the VT capability and unconditionally + returns false — so the Kitty push (`\x1b[>1u`) was never + written, leaving xterm.js in legacy mode where `Shift+Enter` + and `Enter` both produce `\r` and are indistinguishable. + `Alt+Enter` / `Ctrl+J` were affected the same way. The fix + writes the push and pop escapes directly under `#[cfg(windows)]`, + bypassing the capability gate; terminals that don't speak the + protocol silently discard the sequences. Also extends the + pop-on-exit path to two missed call sites (the `main.rs` panic + hook and `external_editor.rs::spawn_editor_for_input`) so a + crash or `$EDITOR` invocation can no longer leave the parent + shell's keyboard state corrupted. - **Approval modal can be collapsed to a one-line banner with Tab** (harvested from PR #1455 by **@tiger-dog**). Previously the approval prompt rendered as a full-screen takeover that hid the diff --git a/crates/tui/src/main.rs b/crates/tui/src/main.rs index b8579c3d..f7104d90 100644 --- a/crates/tui/src/main.rs +++ b/crates/tui/src/main.rs @@ -593,11 +593,14 @@ async fn main() -> Result<()> { // Restore the terminal first so the panic message itself, plus the // user's shell after exit, are visible. Best-effort — we may not be // in raw / alt-screen mode if the panic happens pre-TUI. - use crossterm::event::{ - DisableBracketedPaste, DisableMouseCapture, PopKeyboardEnhancementFlags, - }; + use crossterm::event::{DisableBracketedPaste, DisableMouseCapture}; use crossterm::terminal::{LeaveAlternateScreen, disable_raw_mode}; - let _ = crossterm::execute!(std::io::stdout(), PopKeyboardEnhancementFlags); + // Use the Windows-aware helper: crossterm's PopKeyboardEnhancementFlags + // is a no-op on Windows (is_ansi_code_supported() == false), so the + // plain execute!() form would leave the terminal in Kitty-enhanced mode + // after a panic. pop_keyboard_enhancement_flags writes the pop escape + // directly on Windows (#1359). + crate::tui::ui::pop_keyboard_enhancement_flags(&mut std::io::stdout()); // Best-effort: turn off bracketed paste + mouse capture so the user's // parent shell doesn't get stuck wrapping pastes in `\e[200~…\e[201~` // or printing `\e[<…M` on every click after a TUI panic. diff --git a/crates/tui/src/tui/external_editor.rs b/crates/tui/src/tui/external_editor.rs index 10097fc8..df9272a1 100644 --- a/crates/tui/src/tui/external_editor.rs +++ b/crates/tui/src/tui/external_editor.rs @@ -15,10 +15,7 @@ use std::io::{self, Stdout, Write}; use std::process::Command; use crossterm::{ - event::{ - DisableBracketedPaste, DisableMouseCapture, EnableBracketedPaste, EnableMouseCapture, - PopKeyboardEnhancementFlags, - }, + event::{DisableBracketedPaste, DisableMouseCapture, EnableBracketedPaste, EnableMouseCapture}, execute, terminal::{EnterAlternateScreen, LeaveAlternateScreen, disable_raw_mode, enable_raw_mode}, }; @@ -133,7 +130,9 @@ pub(crate) fn spawn_editor_for_input( // #443: pop keyboard enhancement flags first so the editor // process doesn't inherit a half-configured input mode. Best- // effort — matches the shutdown / panic paths in main.rs. - let _ = execute!(terminal.backend_mut(), PopKeyboardEnhancementFlags); + // Use the Windows-aware helper: the raw crossterm execute!() is a + // no-op on Windows and would leave the editor process in Kitty mode. + super::ui::pop_keyboard_enhancement_flags(terminal.backend_mut()); let _ = disable_raw_mode(); if use_bracketed_paste { let _ = execute!(terminal.backend_mut(), DisableBracketedPaste); diff --git a/crates/tui/src/tui/ui.rs b/crates/tui/src/tui/ui.rs index a66b0a91..363abee6 100644 --- a/crates/tui/src/tui/ui.rs +++ b/crates/tui/src/tui/ui.rs @@ -13,11 +13,15 @@ use crossterm::{ self, DisableBracketedPaste, DisableFocusChange, DisableMouseCapture, EnableBracketedPaste, EnableFocusChange, EnableMouseCapture, Event, KeyCode, KeyEvent, KeyEventKind, KeyModifiers, KeyboardEnhancementFlags, MouseButton, MouseEvent, MouseEventKind, - PopKeyboardEnhancementFlags, PushKeyboardEnhancementFlags, }, execute, terminal::{EnterAlternateScreen, LeaveAlternateScreen, disable_raw_mode, enable_raw_mode}, }; +// On Windows the push/pop helpers write the escapes directly; crossterm's +// PushKeyboardEnhancementFlags / PopKeyboardEnhancementFlags commands are +// never referenced, so the imports are gated to avoid -D warnings failures. +#[cfg(not(windows))] +use crossterm::event::{PopKeyboardEnhancementFlags, PushKeyboardEnhancementFlags}; use ratatui::{ Frame, Terminal, layout::{Constraint, Direction, Layout, Rect, Size}, @@ -279,6 +283,14 @@ pub async fn run_tui(config: &Config, options: TuiOptions) -> Result<()> { // (`REPORT_EVENT_TYPES`, `REPORT_ALL_KEYS_AS_ESCAPE_CODES`) emit // release events that the existing key handlers would mis-route // as duplicate presses. + // + // On Windows, crossterm's `PushKeyboardEnhancementFlags` command always + // reports the terminal as unsupported (`is_ansi_code_supported` returns + // false), so the escape is written directly instead. VSCode's integrated + // terminal and Windows Terminal ≥1.17 honour the kitty keyboard protocol + // and will correctly disambiguate Shift+Enter from plain Enter once this + // sequence is received. Terminals that do not understand it silently + // ignore it. recover_terminal_modes(&mut stdout, use_mouse_capture, use_bracketed_paste); let color_depth = palette::ColorDepth::detect(); let palette_mode = palette::PaletteMode::detect(); @@ -478,7 +490,7 @@ pub async fn run_tui(config: &Config, options: TuiOptions) -> Result<()> { persistence_actor::persist(PersistRequest::ClearCheckpoint); persistence_actor::persist(PersistRequest::Shutdown); - let _ = execute!(terminal.backend_mut(), PopKeyboardEnhancementFlags); + pop_keyboard_enhancement_flags(terminal.backend_mut()); execute!(terminal.backend_mut(), DisableFocusChange)?; disable_raw_mode()?; if use_alt_screen { @@ -7071,7 +7083,7 @@ fn pause_terminal( // to a child process so it doesn't inherit a half-configured input // mode. Best-effort — terminals that didn't accept the flags // silently ignore the pop. Matches the shutdown and panic paths. - let _ = execute!(terminal.backend_mut(), PopKeyboardEnhancementFlags); + pop_keyboard_enhancement_flags(terminal.backend_mut()); execute!(terminal.backend_mut(), DisableFocusChange)?; disable_raw_mode()?; if use_alt_screen { @@ -7143,6 +7155,24 @@ fn reset_terminal_viewport(terminal: &mut AppTerminal, sync_output_enabled: bool } fn push_keyboard_enhancement_flags(writer: &mut W) { + // crossterm's PushKeyboardEnhancementFlags command unconditionally + // returns Unsupported on Windows (is_ansi_code_supported() == false), so + // the ANSI escape is written directly on that platform. Modern Windows + // terminals (VSCode integrated terminal, Windows Terminal ≥1.17) honour + // the kitty keyboard protocol; terminals that do not silently discard it. + #[cfg(windows)] + { + let flags = KeyboardEnhancementFlags::DISAMBIGUATE_ESCAPE_CODES.bits(); + if let Err(err) = write!(writer, "\x1b[>{}u", flags).and_then(|()| writer.flush()) { + tracing::debug!( + target: "kitty_keyboard", + ?err, + "PushKeyboardEnhancementFlags direct write failed on Windows" + ); + } + return; + } + #[cfg(not(windows))] if let Err(err) = execute!( writer, PushKeyboardEnhancementFlags(KeyboardEnhancementFlags::DISAMBIGUATE_ESCAPE_CODES) @@ -7155,6 +7185,29 @@ fn push_keyboard_enhancement_flags(writer: &mut W) { } } +pub(crate) fn pop_keyboard_enhancement_flags(writer: &mut W) { + // Mirror of push_keyboard_enhancement_flags: crossterm's + // PopKeyboardEnhancementFlags also has is_ansi_code_supported() == false + // on Windows, so write the pop escape directly to restore the terminal to + // its pre-launch keyboard mode. + // pub(crate) so the panic hook in main.rs and external_editor.rs can + // also call the Windows-aware path instead of using the raw crossterm + // execute!() macro which silently no-ops on Windows. + #[cfg(windows)] + { + if let Err(err) = write!(writer, "\x1b[<1u").and_then(|()| writer.flush()) { + tracing::debug!( + target: "kitty_keyboard", + ?err, + "PopKeyboardEnhancementFlags direct write failed on Windows" + ); + } + return; + } + #[cfg(not(windows))] + let _ = execute!(writer, PopKeyboardEnhancementFlags); +} + /// Re-establish terminal mode flags. Idempotent and best-effort: each /// underlying flag is silently discarded by terminals that don't support /// it, and a single flag's failure doesn't prevent later flags from being @@ -7167,6 +7220,13 @@ fn push_keyboard_enhancement_flags(writer: &mut W) { /// Excluded by design: raw mode and the alternate screen — those persist /// across focus events and are only re-established by `resume_terminal` /// after a suspension, which always runs a separate path. +/// +/// Note: calling this on every FocusGained event pushes one extra Kitty +/// keyboard mode level onto the terminal's stack without a preceding pop. +/// After N focus cycles the stack reaches depth N; at shutdown only one +/// level is popped. On terminals with a finite stack this is benign because +/// the terminal clears the stack on process exit. A future improvement is +/// to pop-then-push here so the stack stays at depth ≤1. fn recover_terminal_modes( writer: &mut W, use_mouse_capture: bool, diff --git a/crates/tui/src/tui/ui/tests.rs b/crates/tui/src/tui/ui/tests.rs index 628bffba..2f5fcb7f 100644 --- a/crates/tui/src/tui/ui/tests.rs +++ b/crates/tui/src/tui/ui/tests.rs @@ -129,6 +129,34 @@ fn recover_terminal_modes_runs_without_panic_on_windows() { recover_terminal_modes(&mut buf, false, false); } +// On Windows crossterm's PushKeyboardEnhancementFlags never writes bytes +// (is_ansi_code_supported() == false), so the fix writes the escape +// directly. Verify the direct path emits the expected Kitty keyboard +// protocol sequence so the Windows fix for #1359 is not accidentally reverted. +#[cfg(windows)] +#[test] +fn push_keyboard_flags_writes_kitty_push_sequence_on_windows() { + let mut buf: Vec = Vec::new(); + push_keyboard_enhancement_flags(&mut buf); + let seq = String::from_utf8_lossy(&buf); + assert!( + seq.contains("\x1b[>1u"), + "push_keyboard_enhancement_flags must write kitty push (\\x1b[>1u) on Windows (#1359); got: {seq:?}" + ); +} + +#[cfg(windows)] +#[test] +fn pop_keyboard_flags_writes_kitty_pop_sequence_on_windows() { + let mut buf: Vec = Vec::new(); + pop_keyboard_enhancement_flags(&mut buf); + let seq = String::from_utf8_lossy(&buf); + assert!( + seq.contains("\x1b[<1u"), + "pop_keyboard_enhancement_flags must write kitty pop (\\x1b[<1u) on Windows (#1359); got: {seq:?}" + ); +} + #[test] fn terminal_origin_reset_resets_scroll_region_origin_without_destructive_clear() { assert!( diff --git a/docs/KEYBINDINGS.md b/docs/KEYBINDINGS.md index f10e8054..3db301f1 100644 --- a/docs/KEYBINDINGS.md +++ b/docs/KEYBINDINGS.md @@ -99,6 +99,10 @@ When `[memory] enabled = true`, typing `# foo` and pressing `Enter` appends `foo | `y` / `Y` | Trust the workspace (Trust step) | | `n` / `N` | Skip the trust prompt | +## v0.8.29 audit notes + +- **`Shift+Enter` / `Alt+Enter` newlines now work in VSCode on Windows (#1359).** crossterm's `PushKeyboardEnhancementFlags` command unconditionally returns `Unsupported` on Windows (`is_ansi_code_supported() == false`), so the Kitty keyboard protocol escape was never written to the terminal. Without it, VSCode's xterm.js stays in legacy mode where `Shift+Enter` is indistinguishable from plain `Enter`, causing the composer to send the message instead of inserting a newline. The fix writes the push/pop escapes (`\x1b[>1u` / `\x1b[<1u`) directly on Windows, bypassing crossterm's capability gate. VSCode integrated terminal and Windows Terminal ≥1.17 both honour the Kitty keyboard protocol; terminals that do not understand the sequences silently discard them. + ## v0.8.13 audit notes - **Ctrl-S is stash, not history search.** Fixed in this revision — `Alt-R` is history search.