From df8a381aab128cdd27eb230d4b6b43fe3f188203 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 25 Apr 2026 18:46:57 -0700 Subject: [PATCH] tui: reset keyboard reporting on exit --- codex-rs/tui/src/lib.rs | 4 +- codex-rs/tui/src/tui.rs | 241 ++++----------------- codex-rs/tui/src/tui/keyboard_modes.rs | 280 +++++++++++++++++++++++++ 3 files changed, 327 insertions(+), 198 deletions(-) create mode 100644 codex-rs/tui/src/tui/keyboard_modes.rs diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 2dbe06707769..65f6b6a0b171 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -1576,7 +1576,7 @@ pub(crate) async fn resolve_cwd_for_resume_or_fork( reason = "TUI should no longer be displayed, so we can write to stderr." )] fn restore() { - if let Err(err) = tui::restore() { + if let Err(err) = tui::restore_after_exit() { eprintln!( "failed to restore terminal. Run `reset` or restart your terminal to recover: {err}" ); @@ -1595,7 +1595,7 @@ impl TerminalRestoreGuard { #[cfg_attr(debug_assertions, allow(dead_code))] fn restore(&mut self) -> color_eyre::Result<()> { if self.active { - crate::tui::restore()?; + crate::tui::restore_after_exit()?; self.active = false; } Ok(()) diff --git a/codex-rs/tui/src/tui.rs b/codex-rs/tui/src/tui.rs index 291a8ca63cb7..8bd8b93bfe48 100644 --- a/codex-rs/tui/src/tui.rs +++ b/codex-rs/tui/src/tui.rs @@ -19,9 +19,6 @@ use crossterm::event::DisableFocusChange; use crossterm::event::EnableBracketedPaste; use crossterm::event::EnableFocusChange; use crossterm::event::KeyEvent; -use crossterm::event::KeyboardEnhancementFlags; -use crossterm::event::PopKeyboardEnhancementFlags; -use crossterm::event::PushKeyboardEnhancementFlags; use crossterm::terminal::EnterAlternateScreen; use crossterm::terminal::LeaveAlternateScreen; use crossterm::terminal::supports_keyboard_enhancement; @@ -54,117 +51,14 @@ mod frame_rate_limiter; mod frame_requester; #[cfg(unix)] mod job_control; +mod keyboard_modes; /// Target frame interval for UI redraw scheduling. pub(crate) const TARGET_FRAME_INTERVAL: Duration = frame_rate_limiter::MIN_FRAME_INTERVAL; -const DISABLE_KEYBOARD_ENHANCEMENT_ENV_VAR: &str = "CODEX_TUI_DISABLE_KEYBOARD_ENHANCEMENT"; /// A type alias for the terminal type used in this application pub type Terminal = CustomTerminal>; -fn keyboard_enhancement_disabled() -> bool { - let disable_env = std::env::var(DISABLE_KEYBOARD_ENHANCEMENT_ENV_VAR).ok(); - let is_wsl = running_in_wsl(); - let is_vscode_terminal = is_wsl && running_in_vscode_terminal(); - keyboard_enhancement_disabled_for(disable_env.as_deref(), is_wsl, is_vscode_terminal) -} - -fn keyboard_enhancement_disabled_for( - disable_env: Option<&str>, - is_wsl: bool, - is_vscode_terminal: bool, -) -> bool { - if let Some(disabled) = parse_bool_env(disable_env) { - return disabled; - } - - // VS Code running a WSL shell can hide TERM_PROGRAM from the Linux process - // environment, so `running_in_vscode_terminal` also probes the Windows-side - // environment through WSL interop. - is_wsl && is_vscode_terminal -} - -fn parse_bool_env(value: Option<&str>) -> Option { - match value.map(str::trim) { - Some("1") => Some(true), - Some(value) if value.eq_ignore_ascii_case("true") => Some(true), - Some(value) if value.eq_ignore_ascii_case("yes") => Some(true), - Some("0") => Some(false), - Some(value) if value.eq_ignore_ascii_case("false") => Some(false), - Some(value) if value.eq_ignore_ascii_case("no") => Some(false), - _ => None, - } -} - -fn running_in_wsl() -> bool { - #[cfg(target_os = "linux")] - { - crate::clipboard_paste::is_probably_wsl() - } - - #[cfg(not(target_os = "linux"))] - { - false - } -} - -fn running_in_vscode_terminal() -> bool { - vscode_terminal_detected( - std::env::var("TERM_PROGRAM").ok().as_deref(), - windows_term_program().as_deref(), - ) -} - -fn vscode_terminal_detected( - linux_term_program: Option<&str>, - windows_term_program: Option<&str>, -) -> bool { - term_program_is_vscode(linux_term_program) || term_program_is_vscode(windows_term_program) -} - -fn term_program_is_vscode(value: Option<&str>) -> bool { - value.is_some_and(|value| value.eq_ignore_ascii_case("vscode")) -} - -fn windows_term_program() -> Option { - #[cfg(target_os = "linux")] - { - static WINDOWS_TERM_PROGRAM: std::sync::OnceLock> = - std::sync::OnceLock::new(); - WINDOWS_TERM_PROGRAM - .get_or_init(read_windows_term_program) - .clone() - } - - #[cfg(not(target_os = "linux"))] - { - None - } -} - -#[cfg(target_os = "linux")] -fn read_windows_term_program() -> Option { - let output = std::process::Command::new("cmd.exe") - .args(["/d", "/s", "/c", "set TERM_PROGRAM"]) - .stdin(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .output() - .ok()?; - - if !output.status.success() { - return None; - } - - String::from_utf8_lossy(&output.stdout) - .lines() - .find_map(|line| { - line.trim_end_matches('\r') - .strip_prefix("TERM_PROGRAM=") - .map(str::to_string) - }) - .filter(|value| !value.trim().is_empty()) -} - fn should_emit_notification(condition: NotificationCondition, terminal_focused: bool) -> bool { match condition { NotificationCondition::Unfocused => !terminal_focused, @@ -174,10 +68,7 @@ fn should_emit_notification(condition: NotificationCondition, terminal_focused: #[cfg(test)] mod tests { - use super::keyboard_enhancement_disabled_for; - use super::parse_bool_env; use super::should_emit_notification; - use super::vscode_terminal_detected; use codex_config::types::NotificationCondition; #[test] @@ -203,68 +94,6 @@ mod tests { /*terminal_focused*/ false )); } - - #[test] - fn keyboard_enhancement_env_flag_parses_common_values() { - assert_eq!(parse_bool_env(Some("1")), Some(true)); - assert_eq!(parse_bool_env(Some("true")), Some(true)); - assert_eq!(parse_bool_env(Some("YES")), Some(true)); - assert_eq!(parse_bool_env(Some("0")), Some(false)); - assert_eq!(parse_bool_env(Some("false")), Some(false)); - assert_eq!(parse_bool_env(Some("NO")), Some(false)); - assert_eq!(parse_bool_env(Some("unexpected")), None); - assert_eq!(parse_bool_env(/*value*/ None), None); - } - - #[test] - fn keyboard_enhancement_auto_disables_for_vscode_in_wsl() { - assert!(keyboard_enhancement_disabled_for( - /*disable_env*/ None, /*is_wsl*/ true, /*is_vscode_terminal*/ true - )); - } - - #[test] - fn keyboard_enhancement_auto_disable_requires_wsl_and_vscode() { - assert!(!keyboard_enhancement_disabled_for( - /*disable_env*/ None, /*is_wsl*/ true, /*is_vscode_terminal*/ false - )); - assert!(!keyboard_enhancement_disabled_for( - /*disable_env*/ None, /*is_wsl*/ false, /*is_vscode_terminal*/ true - )); - } - - #[test] - fn keyboard_enhancement_env_flag_overrides_auto_detection() { - assert!(!keyboard_enhancement_disabled_for( - Some("0"), - /*is_wsl*/ true, - /*is_vscode_terminal*/ true - )); - assert!(keyboard_enhancement_disabled_for( - Some("1"), - /*is_wsl*/ false, - /*is_vscode_terminal*/ false - )); - } - - #[test] - fn vscode_terminal_detection_uses_linux_and_windows_term_program() { - assert!(vscode_terminal_detected( - Some("vscode"), - /*windows_term_program*/ None - )); - assert!(vscode_terminal_detected( - /*linux_term_program*/ None, - Some("vscode") - )); - assert!(!vscode_terminal_detected( - /*linux_term_program*/ None, - Some("WindowsTerminal") - )); - assert!(!vscode_terminal_detected( - /*linux_term_program*/ None, /*windows_term_program*/ None - )); - } } pub fn set_modes() -> Result<()> { @@ -277,16 +106,7 @@ pub fn set_modes() -> Result<()> { // Some terminals (notably legacy Windows consoles) do not support // keyboard enhancement flags. Attempt to enable them, but continue // gracefully if unsupported. - if !keyboard_enhancement_disabled() { - let _ = execute!( - stdout(), - PushKeyboardEnhancementFlags( - KeyboardEnhancementFlags::DISAMBIGUATE_ESCAPE_CODES - | KeyboardEnhancementFlags::REPORT_EVENT_TYPES - | KeyboardEnhancementFlags::REPORT_ALTERNATE_KEYS - ) - ); - } + keyboard_modes::enable_keyboard_enhancement(); let _ = execute!(stdout(), EnableFocusChange); Ok(()) @@ -334,29 +154,58 @@ impl Command for DisableAlternateScroll { } } -fn restore_common(should_disable_raw_mode: bool) -> Result<()> { - // Pop may fail on platforms that didn't support the push; ignore errors. - let _ = execute!(stdout(), PopKeyboardEnhancementFlags); - execute!(stdout(), DisableBracketedPaste)?; +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum RawModeRestore { + Disable, + Keep, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum KeyboardRestore { + PopStack, + ResetAfterExit, +} + +fn restore_common( + raw_mode_restore: RawModeRestore, + keyboard_restore: KeyboardRestore, +) -> Result<()> { + match keyboard_restore { + KeyboardRestore::PopStack => keyboard_modes::restore_keyboard_enhancement_stack(), + KeyboardRestore::ResetAfterExit => keyboard_modes::reset_keyboard_reporting_after_exit(), + } + + let mut first_error = execute!(stdout(), DisableBracketedPaste).err(); let _ = execute!(stdout(), DisableFocusChange); - if should_disable_raw_mode { - disable_raw_mode()?; + if matches!(raw_mode_restore, RawModeRestore::Disable) + && let Err(err) = disable_raw_mode() + { + first_error.get_or_insert(err); } let _ = execute!(stdout(), crossterm::cursor::Show); - Ok(()) + match first_error { + Some(err) => Err(err), + None => Ok(()), + } } /// Restore the terminal to its original state. /// Inverse of `set_modes`. pub fn restore() -> Result<()> { - let should_disable_raw_mode = true; - restore_common(should_disable_raw_mode) + restore_common(RawModeRestore::Disable, KeyboardRestore::PopStack) +} + +/// Restore the terminal after Codex is exiting. +/// +/// Uses a stronger keyboard reset than [`restore`] so the parent shell recovers even if a +/// terminal missed the stack pop that normally pairs with [`set_modes`]. +pub fn restore_after_exit() -> Result<()> { + restore_common(RawModeRestore::Disable, KeyboardRestore::ResetAfterExit) } /// Restore the terminal to its original state, but keep raw mode enabled. pub fn restore_keep_raw() -> Result<()> { - let should_disable_raw_mode = false; - restore_common(should_disable_raw_mode) + restore_common(RawModeRestore::Keep, KeyboardRestore::PopStack) } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -436,7 +285,7 @@ pub fn init() -> Result { fn set_panic_hook() { let hook = panic::take_hook(); panic::set_hook(Box::new(move |panic_info| { - let _ = restore(); // ignore any errors as we are already failing + let _ = restore_after_exit(); // ignore any errors as we are already failing hook(panic_info); })); } @@ -476,8 +325,8 @@ impl Tui { // Detect keyboard enhancement support before any EventStream is created so the // crossterm poller can acquire its lock without contention. - let enhanced_keys_supported = - !keyboard_enhancement_disabled() && supports_keyboard_enhancement().unwrap_or(false); + let enhanced_keys_supported = !keyboard_modes::keyboard_enhancement_disabled() + && supports_keyboard_enhancement().unwrap_or(false); // Cache this to avoid contention with the event reader. supports_color::on_cached(supports_color::Stream::Stdout); let _ = crate::terminal_palette::default_colors(); diff --git a/codex-rs/tui/src/tui/keyboard_modes.rs b/codex-rs/tui/src/tui/keyboard_modes.rs new file mode 100644 index 000000000000..1dcc6725b1f7 --- /dev/null +++ b/codex-rs/tui/src/tui/keyboard_modes.rs @@ -0,0 +1,280 @@ +//! Terminal keyboard enhancement setup and teardown helpers. +//! +//! The TUI uses crossterm's keyboard enhancement stack while it owns the terminal, but +//! process exit gets a stronger reset so the parent shell does not inherit enhanced key +//! reporting if a terminal misses the normal stack pop. + +use std::fmt; +use std::io::stdout; + +use crossterm::Command; +use crossterm::event::KeyboardEnhancementFlags; +use crossterm::event::PopKeyboardEnhancementFlags; +use crossterm::event::PushKeyboardEnhancementFlags; +use ratatui::crossterm::execute; + +const DISABLE_KEYBOARD_ENHANCEMENT_ENV_VAR: &str = "CODEX_TUI_DISABLE_KEYBOARD_ENHANCEMENT"; + +pub(super) fn keyboard_enhancement_disabled() -> bool { + let disable_env = std::env::var(DISABLE_KEYBOARD_ENHANCEMENT_ENV_VAR).ok(); + let is_wsl = running_in_wsl(); + let is_vscode_terminal = is_wsl && running_in_vscode_terminal(); + keyboard_enhancement_disabled_for(disable_env.as_deref(), is_wsl, is_vscode_terminal) +} + +fn keyboard_enhancement_disabled_for( + disable_env: Option<&str>, + is_wsl: bool, + is_vscode_terminal: bool, +) -> bool { + if let Some(disabled) = parse_bool_env(disable_env) { + return disabled; + } + + // VS Code running a WSL shell can hide TERM_PROGRAM from the Linux process + // environment, so `running_in_vscode_terminal` also probes the Windows-side + // environment through WSL interop. + is_wsl && is_vscode_terminal +} + +fn parse_bool_env(value: Option<&str>) -> Option { + match value.map(str::trim) { + Some("1") => Some(true), + Some(value) if value.eq_ignore_ascii_case("true") => Some(true), + Some(value) if value.eq_ignore_ascii_case("yes") => Some(true), + Some("0") => Some(false), + Some(value) if value.eq_ignore_ascii_case("false") => Some(false), + Some(value) if value.eq_ignore_ascii_case("no") => Some(false), + _ => None, + } +} + +fn running_in_wsl() -> bool { + #[cfg(target_os = "linux")] + { + crate::clipboard_paste::is_probably_wsl() + } + + #[cfg(not(target_os = "linux"))] + { + false + } +} + +fn running_in_vscode_terminal() -> bool { + vscode_terminal_detected( + std::env::var("TERM_PROGRAM").ok().as_deref(), + windows_term_program().as_deref(), + ) +} + +fn vscode_terminal_detected( + linux_term_program: Option<&str>, + windows_term_program: Option<&str>, +) -> bool { + term_program_is_vscode(linux_term_program) || term_program_is_vscode(windows_term_program) +} + +fn term_program_is_vscode(value: Option<&str>) -> bool { + value.is_some_and(|value| value.eq_ignore_ascii_case("vscode")) +} + +fn windows_term_program() -> Option { + #[cfg(target_os = "linux")] + { + static WINDOWS_TERM_PROGRAM: std::sync::OnceLock> = + std::sync::OnceLock::new(); + WINDOWS_TERM_PROGRAM + .get_or_init(read_windows_term_program) + .clone() + } + + #[cfg(not(target_os = "linux"))] + { + None + } +} + +#[cfg(target_os = "linux")] +fn read_windows_term_program() -> Option { + let output = std::process::Command::new("cmd.exe") + .args(["/d", "/s", "/c", "set TERM_PROGRAM"]) + .stdin(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .output() + .ok()?; + + if !output.status.success() { + return None; + } + + String::from_utf8_lossy(&output.stdout) + .lines() + .find_map(|line| { + line.trim_end_matches('\r') + .strip_prefix("TERM_PROGRAM=") + .map(str::to_string) + }) + .filter(|value| !value.trim().is_empty()) +} + +pub(super) fn enable_keyboard_enhancement() { + if keyboard_enhancement_disabled() { + return; + } + + let _ = execute!( + stdout(), + PushKeyboardEnhancementFlags( + KeyboardEnhancementFlags::DISAMBIGUATE_ESCAPE_CODES + | KeyboardEnhancementFlags::REPORT_EVENT_TYPES + | KeyboardEnhancementFlags::REPORT_ALTERNATE_KEYS + ) + ); +} + +pub(super) fn restore_keyboard_enhancement_stack() { + let _ = execute!(stdout(), PopKeyboardEnhancementFlags); +} + +pub(super) fn reset_keyboard_reporting_after_exit() { + let _ = execute!( + stdout(), + PopKeyboardEnhancementFlags, + ResetKeyboardEnhancementFlags, + DisableModifyOtherKeys + ); +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct ResetKeyboardEnhancementFlags; + +impl Command for ResetKeyboardEnhancementFlags { + fn write_ansi(&self, f: &mut impl fmt::Write) -> fmt::Result { + f.write_str("\x1b[ std::io::Result<()> { + Err(std::io::Error::new( + std::io::ErrorKind::Unsupported, + "keyboard enhancement reset is not implemented for the legacy Windows API", + )) + } + + #[cfg(windows)] + fn is_ansi_code_supported(&self) -> bool { + false + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct DisableModifyOtherKeys; + +impl Command for DisableModifyOtherKeys { + fn write_ansi(&self, f: &mut impl fmt::Write) -> fmt::Result { + f.write_str("\x1b[>4;0m") + } + + #[cfg(windows)] + fn execute_winapi(&self) -> std::io::Result<()> { + Err(std::io::Error::new( + std::io::ErrorKind::Unsupported, + "modifyOtherKeys reset is not implemented for the legacy Windows API", + )) + } + + #[cfg(windows)] + fn is_ansi_code_supported(&self) -> bool { + false + } +} + +#[cfg(test)] +mod tests { + use super::DisableModifyOtherKeys; + use super::ResetKeyboardEnhancementFlags; + use super::keyboard_enhancement_disabled_for; + use super::parse_bool_env; + use super::vscode_terminal_detected; + use crossterm::Command; + use pretty_assertions::assert_eq; + + fn ansi_for(command: impl Command) -> String { + let mut out = String::new(); + command.write_ansi(&mut out).unwrap(); + out + } + + #[test] + fn keyboard_enhancement_env_flag_parses_common_values() { + assert_eq!(parse_bool_env(Some("1")), Some(true)); + assert_eq!(parse_bool_env(Some("true")), Some(true)); + assert_eq!(parse_bool_env(Some("YES")), Some(true)); + assert_eq!(parse_bool_env(Some("0")), Some(false)); + assert_eq!(parse_bool_env(Some("false")), Some(false)); + assert_eq!(parse_bool_env(Some("NO")), Some(false)); + assert_eq!(parse_bool_env(Some("unexpected")), None); + assert_eq!(parse_bool_env(/*value*/ None), None); + } + + #[test] + fn keyboard_enhancement_auto_disables_for_vscode_in_wsl() { + assert!(keyboard_enhancement_disabled_for( + /*disable_env*/ None, /*is_wsl*/ true, /*is_vscode_terminal*/ true + )); + } + + #[test] + fn keyboard_enhancement_auto_disable_requires_wsl_and_vscode() { + assert!(!keyboard_enhancement_disabled_for( + /*disable_env*/ None, /*is_wsl*/ true, /*is_vscode_terminal*/ false + )); + assert!(!keyboard_enhancement_disabled_for( + /*disable_env*/ None, /*is_wsl*/ false, /*is_vscode_terminal*/ true + )); + } + + #[test] + fn keyboard_enhancement_env_flag_overrides_auto_detection() { + assert!(!keyboard_enhancement_disabled_for( + Some("0"), + /*is_wsl*/ true, + /*is_vscode_terminal*/ true + )); + assert!(keyboard_enhancement_disabled_for( + Some("1"), + /*is_wsl*/ false, + /*is_vscode_terminal*/ false + )); + } + + #[test] + fn vscode_terminal_detection_uses_linux_and_windows_term_program() { + assert!(vscode_terminal_detected( + Some("vscode"), + /*windows_term_program*/ None + )); + assert!(vscode_terminal_detected( + /*linux_term_program*/ None, + Some("vscode") + )); + assert!(!vscode_terminal_detected( + /*linux_term_program*/ None, + Some("WindowsTerminal") + )); + assert!(!vscode_terminal_detected( + /*linux_term_program*/ None, /*windows_term_program*/ None + )); + } + + #[test] + fn reset_keyboard_enhancement_flags_clears_all_pushed_levels() { + assert_eq!(ansi_for(ResetKeyboardEnhancementFlags), "\x1b[4;0m"); + } +}