From a85773cef0a28be68fa3c808d5d00447cb1df683 Mon Sep 17 00:00:00 2001
From: Miguel Palhas <mpalhas@gmail.com>
Date: Mon, 24 Aug 2026 14:39:48 +0100
Subject: [PATCH] fix(maestro): make shift-S resolve to the fast-forward spawn
 binding

crossterm attaches KeyModifiers::SHIFT to every uppercase char, so a real
Shift+S delivered Char { c: 'S', shift: true }. canonical() turned that
into "shift-S", which the default "S" binding never matched, so
FastForwardSpawn was unreachable from the keyboard.

canonical() now drops the shift- prefix when the char is already
uppercase, since the case already says "shifted". Regression test drives
an actual crossterm KeyEvent through canonicalise() and the keymap,
rather than asserting on the binding string, per #70.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 crates/maestro-tui/src/config.rs | 20 ++++++++++++++++++++
 crates/maestro-tui/src/input.rs  | 22 ++++++++++++++++++++--
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/crates/maestro-tui/src/config.rs b/crates/maestro-tui/src/config.rs
index 9f4eb2b..790e6bd 100644
--- a/crates/maestro-tui/src/config.rs
+++ b/crates/maestro-tui/src/config.rs
@@ -238,6 +238,26 @@ mod tests {
         assert!(s.contains("ctrl-glorb"), "diagnostic: {s}");
     }
 
+    /// Regression for #70: the previous canonical form of a real Shift+S
+    /// keypress was `"shift-S"`, which the default `"S"` binding never
+    /// matched. Drive the actual crossterm event, not the binding string,
+    /// so the dead-key bug can't come back unnoticed.
+    #[test]
+    fn real_shift_s_keypress_resolves_to_fast_forward_spawn() {
+        use crossterm::event::{Event as CtEvent, KeyCode, KeyEvent, KeyModifiers};
+
+        use crate::input::{canonicalise, Region};
+
+        let cfg = TuiConfig::load(&TuiSection::default()).unwrap();
+        let ev = CtEvent::Key(KeyEvent::new(KeyCode::Char('S'), KeyModifiers::SHIFT));
+        let key = canonicalise(&ev, |_, _| Region::Unknown).expect("key event canonicalises");
+
+        assert_eq!(
+            cfg.keymap.resolve(Mode::Normal, &key.canonical()),
+            Some(Action::FastForwardSpawn),
+        );
+    }
+
     #[test]
     fn custom_chord_timeout_is_applied() {
         let mut section = TuiSection::default();
diff --git a/crates/maestro-tui/src/input.rs b/crates/maestro-tui/src/input.rs
index 34c4d4a..137458f 100644
--- a/crates/maestro-tui/src/input.rs
+++ b/crates/maestro-tui/src/input.rs
@@ -117,7 +117,9 @@ impl InputKey {
                 if *alt {
                     out.push_str("alt-");
                 }
-                if *shift {
+                // Uppercase already implies shift; crossterm sets the flag
+                // on every such char, so skip the redundant prefix (#70).
+                if *shift && !c.is_uppercase() {
                     out.push_str("shift-");
                 }
                 out.push(*c);
@@ -580,12 +582,28 @@ mod tests {
 
     #[test]
     fn canonical_round_trips_simple_chars() {
-        for s in ["a", "ctrl-a", "ctrl-w", "alt-x", "shift-A"] {
+        for s in ["a", "ctrl-a", "ctrl-w", "alt-x"] {
             let k = InputKey::parse(s).unwrap();
             assert_eq!(k.canonical(), s, "round-trip {s:?}");
         }
     }
 
+    /// crossterm 0.28 attaches `KeyModifiers::SHIFT` to any uppercase char,
+    /// so a real Shift+S delivers `Char { c: 'S', shift: true }`. Its
+    /// canonical form must be the bare `"S"` a keymap binds, not `"shift-S"` —
+    /// the uppercase letter already says "shifted"; a redundant prefix would
+    /// make the default `S` binding unreachable from the keyboard (#70).
+    #[test]
+    fn shift_flag_is_redundant_for_an_already_uppercase_char() {
+        let k = InputKey::Char {
+            c: 'S',
+            ctrl: false,
+            alt: false,
+            shift: true,
+        };
+        assert_eq!(k.canonical(), "S");
+    }
+
     #[test]
     fn canonical_round_trips_specials() {
         for s in [