From 28dcdb566ad0052c616b40efda1ad852cbb04970 Mon Sep 17 00:00:00 2001 From: Robby He <448523760@qq.com> Date: Fri, 5 Dec 2025 06:56:58 +0800 Subject: [PATCH] Fix `handle_shortcut_overlay_key` for cross-platform consistency (#7583) **Summary** - Shortcut toggle using `?` in `handle_shortcut_overlay_key` fails to trigger on some platforms (notably Windows). Current match requires `KeyCode::Char('?')` with `KeyModifiers::NONE`. Some terminals set `SHIFT` when producing `?` (since it is typically `Shift + /`), so the strict `NONE` check prevents toggling. **Impact** - On Windows consoles/terminals, pressing `?` with an empty composer often does nothing, leading to inconsistent UX compared to macOS/Linux. **Root Cause** - Crossterm/terminal backends report modifiers inconsistently across platforms. Generating `?` may include `SHIFT`. The code enforces `modifiers == NONE`, so valid `?` presses with `SHIFT` are ignored. AltGr keyboards may also surface as `ALT`. **Repro Steps** - Open the TUI, ensure the composer is empty. - Press `?`. - Expected: Shortcut overlay toggles. - Actual (Windows frequently): No toggle occurs. **Fix Options** - Option 1 (preferred): Accept `?` regardless of `SHIFT`, but reject `CONTROL` and `ALT`. - Rationale: Keeps behavior consistent across platforms with minimal code change. - Example change: - Before: matching `KeyModifiers::NONE` only. - After: allow `SHIFT`, disallow `CONTROL | ALT`. - Suggested condition: ```rust let toggles = matches!(key_event.code, KeyCode::Char('?')) && !key_event.modifiers.intersects(KeyModifiers::CONTROL | KeyModifiers::ALT) && self.is_empty(); ``` - Option 2: Platform-specific handling (Windows vs non-Windows). - Implement two variants or conditional branches using `#[cfg(target_os = "windows")]`. - On Windows, accept `?` with `SHIFT`; on other platforms, retain current behavior. - Trade-off: Higher maintenance burden and code divergence for limited benefit. --- close #5495 --- codex-rs/tui/src/bottom_pane/chat_composer.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 4eeeb4bce..4deb5125c 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -1504,14 +1504,9 @@ impl ChatComposer { return false; } - let toggles = matches!( - key_event, - KeyEvent { - code: KeyCode::Char('?'), - modifiers: KeyModifiers::NONE, - .. - } if self.is_empty() - ); + let toggles = matches!(key_event.code, KeyCode::Char('?')) + && !has_ctrl_or_alt(key_event.modifiers) + && self.is_empty(); if !toggles { return false;