fix(ui): raise the tab-number badges on the tab chord's modifier (#1128)

The badges were revealed on a bare secondary modifier — ⌘ on macOS, Ctrl
everywhere else — whatever `ActivateTab1`…`ActivateTab9` were bound to. Off
macOS the nine ship on Alt, so holding Alt raised nothing while Ctrl, which
switches no tab there, lit up every number; and a tab row rebound onto Alt, the
case reported, still had to be held with Ctrl to see the hint.

The hold is read from `ActivateTab1` now, so it follows the binding and is
matched exactly. An unbound action, a chord with no modifier, a key that is not
the digit the badge draws, and the tmux preset's `prefix 1` raise no badges at
all rather than promising numbers no key answers.

Co-authored-by: Du Leilei <duleilei@sailyond.com>

CHANGELOG.md

@@ -548,6 +548,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **The tab-number badges follow the chord the tabs are actually on.** Holding
+  the modifier that switches tabs draws the numbers 1…9 on the tab strip and
+  the sidebar, but the modifier was assumed: the platform's secondary, ⌘ on
+  macOS and Ctrl everywhere else, whatever `ActivateTab1`…`ActivateTab9` were
+  bound to. Off macOS — where the nine ship on Alt — holding Alt raised
+  nothing while Ctrl, which switches no tab there, lit every number up, and a
+  tab row rebound onto Alt still had to be held with Ctrl to see the hint; on
+  macOS a rebind onto Alt failed the same way the other way round. The hold is
+  now read from `ActivateTab1`, so it moves with the binding (Alt, ⌘, or the
+  Ctrl+Alt a config asked for) and is held exactly: an action with no chord, a
+  chord with no modifier, a key that is not the number the badge draws, or the
+  tmux preset's `prefix 1` raise no badges at all, rather than promising
+  numbers no key answers.
+
 - **Quick launch found only the agents in the GUI's own bare `PATH`.** The app
   starts with the launchd environment (`/usr/bin:/bin:/usr/sbin:/sbin`) and
   fills it in by running the login shell, but only as a *login* shell — which

src/ui/hints.rs

@@ -19,15 +19,15 @@ impl Tty7App {
         self.switcher_hold_changed(m, window, cx);
         self.set_link_modifier(m.secondary(), cx);
 
-        let extra_platform = if cfg!(target_os = "macos") {
-            m.control
-        } else {
-            m.platform
-        };
-        let bare_secondary = m.secondary() && !m.alt && !m.shift && !extra_platform;
+        // The numbers drawn on the tabs are `ActivateTab1`…`ActivateTab9`, so
+        // the hold that raises them is the one those chords are on — Alt off
+        // macOS, ⌘ on it, or wherever a rebind moved them. The platform's
+        // secondary modifier is not that answer: it lit the badges on Ctrl off
+        // macOS, where the nine are on Alt, and left the real gesture dark.
+        let hold = crate::ui::keymap::tab_badge_hold(cx);
 
         self.mod_hint_gen = self.mod_hint_gen.wrapping_add(1);
-        if !bare_secondary {
+        if hold.as_ref() != Some(m) {
             self.dismiss_mod_hint(cx);
             return;
         }
@@ -105,6 +105,15 @@ mod gpui_tests {
             .expect("the app window stays open")
     }
 
+    /// The hold the badges answer to, read from the same place they do rather
+    /// than assumed — the shipped one is ⌘ on macOS and Alt off it.
+    fn hold_modifiers(window: &WindowHandle<Tty7App>, cx: &mut TestAppContext) -> Modifiers {
+        window
+            .update(cx, |_, _, cx| crate::ui::keymap::tab_badge_hold(cx))
+            .expect("the app window stays open")
+            .expect("the shipped tab chords are on a modifier")
+    }
+
     fn wait_for_badges(window: &WindowHandle<Tty7App>, cx: &mut TestAppContext) {
         let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5);
         loop {
@@ -114,7 +123,7 @@ mod gpui_tests {
             }
             assert!(
                 std::time::Instant::now() < deadline,
-                "badges never appeared within 5s of the bare-secondary hold"
+                "badges never appeared within 5s of the held tab chord"
             );
             std::thread::sleep(std::time::Duration::from_millis(10));
         }
@@ -123,7 +132,8 @@ mod gpui_tests {
     #[gpui::test]
     fn deactivation_dismisses_visible_badges(cx: &mut TestAppContext) {
         let (window, mut vcx) = harness(cx);
-        vcx.simulate_modifiers_change(Modifiers::secondary_key());
+        let hold = hold_modifiers(&window, cx);
+        vcx.simulate_modifiers_change(hold);
         wait_for_badges(&window, cx);
 
         vcx.deactivate_window();
@@ -137,7 +147,8 @@ mod gpui_tests {
     #[gpui::test]
     fn deactivation_cancels_a_pending_reveal(cx: &mut TestAppContext) {
         let (window, mut vcx) = harness(cx);
-        vcx.simulate_modifiers_change(Modifiers::secondary_key());
+        let hold = hold_modifiers(&window, cx);
+        vcx.simulate_modifiers_change(hold);
         vcx.deactivate_window();
 
         std::thread::sleep(std::time::Duration::from_millis(400));
@@ -148,4 +159,34 @@ mod gpui_tests {
             "a reveal scheduled before deactivation must not fire after it"
         );
     }
+
+    /// The regression: the badges answered to the platform's secondary
+    /// modifier rather than to the chord the numbers are drawn for. On Windows
+    /// and Linux, where the nine ship on Alt, Ctrl was the only key that lit
+    /// them — and a tab row rebound onto Alt, the case reported, raised nothing
+    /// however long it was held.
+    #[gpui::test]
+    fn the_bound_modifier_raises_the_badges_and_another_one_does_not(cx: &mut TestAppContext) {
+        let (window, mut vcx) = harness(cx);
+        vcx.update(|_, cx| {
+            // Exactly Alt+1…9, as the Settings page writes a rebind: a list,
+            // so the shipped chord is gone rather than sitting beside it.
+            cx.global_mut::<Config>().keybindings = serde_json::from_value(serde_json::json!({
+                "ActivateTab1": ["alt-1"],
+            }))
+            .expect("the binding loads");
+        });
+
+        vcx.simulate_modifiers_change(Modifiers::control());
+        std::thread::sleep(std::time::Duration::from_millis(400));
+        cx.background_executor.run_until_parked();
+        assert!(
+            !badges_shown(&window, cx),
+            "Ctrl is not the chord the tabs are numbered for here any more"
+        );
+
+        vcx.simulate_modifiers_change(Modifiers::none());
+        vcx.simulate_modifiers_change(Modifiers::alt());
+        wait_for_badges(&window, cx);
+    }
 }

src/ui/keymap.rs

@@ -1,4 +1,4 @@
-use gpui::{App, Global, KeyBinding, Keystroke};
+use gpui::{App, Global, KeyBinding, Keystroke, Modifiers};
 
 use crate::core::actions::*;
 use crate::core::config::{Config, KeybindingOverride};
@@ -1415,6 +1415,34 @@ pub(crate) fn effective_key(action: &str, cx: &App) -> Option<String> {
         .filter(|k| !k.is_empty())
 }
 
+/// The modifier hold that raises the tab-number badges — the one the tab
+/// activation chords are really on.
+///
+/// It used to be assumed: a bare secondary, ⌘ on macOS and Ctrl everywhere
+/// else. That is one platform right and one wrong. Off macOS the nine ship on
+/// Alt, so holding Alt raised nothing while Ctrl, which activates no tab there
+/// at all, lit up every number; and any rebind — `"ActivateTab1": "alt-1"` on
+/// macOS is the same complaint the other way round — moved the chords out from
+/// under a hint that stayed where it was. Read from `ActivateTab1`, the chord
+/// the first badge is drawn for, and matched against the held modifiers
+/// exactly, so a wrong modifier, or Shift added mid-hold, takes them down.
+///
+/// `None` when there is nothing to advertise: an unbound or retired
+/// `ActivateTab1`, a chord with no modifier (a bare `1` is not a hold), a key
+/// that is not the digit the badge would draw (`alt-q` does activate tab 1, but
+/// holding Alt would then be promising a number pad no key answers), or the
+/// tmux preset's two-chord `prefix 1` — a sequence has no hold to watch, and
+/// raising the badges on the bare prefix would point at half a chord.
+pub(crate) fn tab_badge_hold(cx: &App) -> Option<Modifiers> {
+    let spec = effective_key("ActivateTab1", cx)?;
+    let mut chords = spec.split_whitespace();
+    let (chord, None) = (chords.next()?, chords.next()) else {
+        return None;
+    };
+    let ks = Keystroke::parse(chord).ok()?;
+    (ks.key == "1" && ks.modifiers.modified()).then_some(ks.modifiers)
+}
+
 pub(crate) fn spec_from_keystroke(ks: &Keystroke) -> Option<String> {
     if matches!(
         ks.key.as_str(),
@@ -3076,6 +3104,42 @@ mod gpui_tests {
         });
     }
 
+    /// The hold the tab-number badges answer to is the one the tab chords are
+    /// on, not the platform's secondary: off macOS the nine ship on Alt, where
+    /// a badge raised by Ctrl hints at nothing that switches a tab.
+    #[gpui::test]
+    fn the_badge_hold_follows_the_tab_chord(cx: &mut TestAppContext) {
+        cx.update(|cx| {
+            running_on_json(cx, "{}");
+            let shipped = Keystroke::parse(per_platform("secondary-1", "alt-1"))
+                .expect("the shipped chord parses");
+            assert_eq!(
+                tab_badge_hold(cx),
+                Some(shipped.modifiers),
+                "the shipped hold is the shipped chord's modifier"
+            );
+
+            for (json, expected) in [
+                (
+                    r#"{"keybindings": {"ActivateTab1": ["alt-1"]}}"#,
+                    Some(Modifiers::alt()),
+                ),
+                // Nothing to advertise: no chord at all, a chord with no
+                // modifier to hold, a key that is not the digit the badge
+                // draws, and the tmux preset's two-chord `prefix 1`.
+                (r#"{"keybindings": {"ActivateTab1": []}}"#, None),
+                (r#"{"keybindings": {"ActivateTab1": ["1"]}}"#, None),
+                (r#"{"keybindings": {"ActivateTab1": ["alt-q"]}}"#, None),
+                (r#"{"keybinding_preset": "tmux"}"#, None),
+            ] {
+                cx.set_global(Config(
+                    serde_json::from_str(json).expect("the config parses"),
+                ));
+                assert_eq!(tab_badge_hold(cx), expected, "{json}");
+            }
+        });
+    }
+
     #[gpui::test]
     fn a_chord_added_under_the_tmux_preset_joins_the_preset_chord(cx: &mut TestAppContext) {
         use gpui::Action as _;