From bf8de6ac9c15efc7cb0fef5b735a45bd060ac364 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Max=20=F0=9F=91=A8=F0=9F=8F=BD=E2=80=8D=F0=9F=92=BB=20Copl?= =?UTF-8?q?an?= Date: Fri, 24 Jul 2026 16:20:21 -0700 Subject: [PATCH] streampager + curses_compat: fix Shift keys (G, @, capitals, A) broken in Ghostty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: When running under Ghostty (and cmux which uses libghostty) outside of tmux, eden/scm's internal pager and interactive commit UI did not register any Shift-modified key. Root cause: - `termwiz` `UnixTerminal::set_raw_mode()` enables `modifyOtherKeys=2` (`ESC[>4;2m`). - Ghostty's `key_encode.zig` correctly implements this: `Shift+G` is sent as `ESC[27;2;71~` (where 71 is uppercase G) and `Shift+@` as `ESC[27;2;64~`. - `termwiz` `InputParser` decodes these as `Char('G')+SHIFT` and `Char('@')+SHIFT`. Two separate code paths were affected: 1. **streampager** (Rust, used by `sl log` etc.): The keymap stores `'G'` as `(NONE, Char('G'))`. The lookup used exact modifier match, so `(SHIFT, Char('G'))` never matched — `G` was silently dropped. 2. **curses_compat** (Python, used by `sl commit --interactive` / crecord): `curses_compat.py:getch()` only handled `Modifiers.NONE` and `Modifiers.CTRL`. When `SHIFT` was set, it fell through to `return -1`, silently dropping the key so `A` (toggle all), `G`, `J`, `H`, `F` etc. did not work. Additionally, the CTRL check used `Modifiers.CTRL or Modifiers.RIGHT_CTRL` (Python `or` always evaluates to `Modifiers.CTRL`); fixed to use bitwise `|` with `&`. tmux masks this because `send-keys -l` sends plain `0x47` (`G+NONE`). Raw hex injection via tmux reproduces the bug: ``` tmux send-keys -H '1b5b32373b323b37317e' # ESC[27;2;71~ Shift+G ``` Old `sl` stays at top, new `sl` goes to bottom. Fix: Strip the SHIFT modifier and retry the lookup: 1. **streampager**: In `Screen::dispatch_key` and `Prompt::dispatch_key`, build a candidate list that includes the original key plus a SHIFT-stripped variant, then try each against the keymap/prompt match. `modifyOtherKeys=2` already sends the shifted character (uppercase), so stripping SHIFT is sufficient. 2. **curses_compat**: In `getch()`, add a `modifiers & SHIFT` branch that returns `ord(ch.upper())`. Also fix the CTRL modifier check from `or` to bitwise `|`. Test Plan: - `make oss` - builds (but requires #1365) - tmux test: ```bash tmux new -d -s t; tmux send-keys -t t './out/sl log -l 5000 --pager=always' # Check plain G goes to bottom, g to top. tmux send-keys -t t -H '1b5b32373b323b37317e' # Shift+G Ghostty modifyOtherKeys # => now goes to bottom (was stuck at top) tmux send-keys -t t -l '/' ; send -H '1b5b32373b323b36347e' # Shift+@ in prompt # => now shows Search: @ (was empty) ``` Verified old `/usr/local/bin/sl` fails, new `out/sl` passes. - `cargo test -p sapling-streampager` still 5 passed. - `python tests/run-tests.py tests/test-commit-interactive-curses.t` passes. - Manual in Ghostty.app outside tmux: `G` jumps to bottom, `/` `@` search works, capital letters searchable (previously broken). `sl commit --interactive` `A` toggles all files (previously broken). Fixes #1363 Co-authored-by: Opencode --- .../lib/third-party/streampager/src/prompt.rs | 9 ++- .../lib/third-party/streampager/src/screen.rs | 60 ++++++++++++++++++- eden/scm/sapling/curses_compat.py | 8 ++- 3 files changed, 73 insertions(+), 4 deletions(-) diff --git a/eden/scm/lib/third-party/streampager/src/prompt.rs b/eden/scm/lib/third-party/streampager/src/prompt.rs index 14493dc897b54..55ba66cc85525 100644 --- a/eden/scm/lib/third-party/streampager/src/prompt.rs +++ b/eden/scm/lib/third-party/streampager/src/prompt.rs @@ -5,7 +5,7 @@ use std::fmt::Write; use termwiz::cell::{AttributeChange, CellAttributes}; use termwiz::color::{AnsiColor, ColorAttribute}; -use termwiz::input::KeyEvent; +use termwiz::input::{KeyCode, KeyEvent}; use termwiz::surface::change::Change; use termwiz::surface::Position; use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; @@ -371,6 +371,13 @@ impl Prompt { const CTRL: Modifiers = Modifiers::CTRL; const NONE: Modifiers = Modifiers::NONE; const ALT: Modifiers = Modifiers::ALT; + // Strip SHIFT from Char keys (modifyOtherKeys=2 sends shifted char + SHIFT). + let mut key = key; + if key.modifiers.contains(Modifiers::SHIFT) { + if let KeyCode::Char(_) = key.key { + key.modifiers.remove(Modifiers::SHIFT); + } + } let value_width = width - self.prompt.width() - 4; let action = match (key.modifiers, key.key) { (NONE, Enter) | (CTRL, Char('j')) | (CTRL, Char('m')) => { diff --git a/eden/scm/lib/third-party/streampager/src/screen.rs b/eden/scm/lib/third-party/streampager/src/screen.rs index cea50925280c2..6b111e87bcc75 100644 --- a/eden/scm/lib/third-party/streampager/src/screen.rs +++ b/eden/scm/lib/third-party/streampager/src/screen.rs @@ -1241,19 +1241,40 @@ impl Screen { key: KeyEvent, event_sender: &EventSender, ) -> DisplayAction { - if let Some(binding) = self.keymap.get(key.modifiers, key.key) { + if let Some(binding) = Self::key_candidates(key) + .into_iter() + .find_map(|key| self.keymap.get(key.modifiers, key.key)) + { match binding { Binding::Action(action) => { let action = action.clone(); return self.dispatch_action(action, event_sender); } - Binding::Custom(b) => b.run(self.file.index()), + Binding::Custom(b) => { + b.run(self.file.index()); + return DisplayAction::Render; + } Binding::Unrecognized(_) => {} } } DisplayAction::Render } + fn key_candidates(key: KeyEvent) -> Vec { + use termwiz::input::{KeyCode, Modifiers}; + + let mut candidates = vec![key.clone()]; + if matches!(key.key, KeyCode::Char(_)) && key.modifiers.contains(Modifiers::SHIFT) { + let mut modifiers = key.modifiers; + modifiers.remove(Modifiers::SHIFT); + candidates.push(KeyEvent { + key: key.key, + modifiers, + }); + } + candidates + } + /// Append a digit to the repeat count. pub(crate) fn append_digit_to_repeat_count(&mut self, digit: usize) { assert!(digit < 10); @@ -1418,3 +1439,38 @@ impl Screen { self.file.set_needed_lines(needed_lines); } } + +#[cfg(test)] +mod tests { + use super::*; + use termwiz::input::{KeyCode, Modifiers}; + + #[test] + fn shifted_char_tries_exact_binding_before_unshifted_fallback() { + let key = KeyEvent { + key: KeyCode::Char('G'), + modifiers: Modifiers::SHIFT, + }; + + assert_eq!( + Screen::key_candidates(key) + .into_iter() + .map(|key| (key.modifiers, key.key)) + .collect::>(), + vec![ + (Modifiers::SHIFT, KeyCode::Char('G')), + (Modifiers::NONE, KeyCode::Char('G')), + ] + ); + } + + #[test] + fn shifted_non_char_has_no_unshifted_fallback() { + let key = KeyEvent { + key: KeyCode::UpArrow, + modifiers: Modifiers::SHIFT, + }; + + assert_eq!(Screen::key_candidates(key).len(), 1); + } +} diff --git a/eden/scm/sapling/curses_compat.py b/eden/scm/sapling/curses_compat.py index 182356cd87cd0..2f387d5a4c5e8 100644 --- a/eden/scm/sapling/curses_compat.py +++ b/eden/scm/sapling/curses_compat.py @@ -173,7 +173,13 @@ def getch(self): code = ord(ch) if modifiers == Modifiers.NONE: return code - if modifiers in [Modifiers.CTRL or Modifiers.RIGHT_CTRL]: + if modifiers & Modifiers.SHIFT: + # Ghostty with modifyOtherKeys=2 sends Shift+A as + # Char('a')+SHIFT instead of plain 'A' (0x41). + # Normalize to uppercase so crecord keybindings like + # 'A' (toggle all), 'G', 'J', 'H', 'F' work. + return ord(ch.upper()) + if modifiers & (Modifiers.CTRL | Modifiers.RIGHT_CTRL): # emulate curses behavior, ord(upper) & 0x1f return ord(ch.upper()) & 0x1F case {"Function": fn}: