diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c95ade3..de14c04c 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/ui/hints.rs b/src/ui/hints.rs index c6d670aa..29ceb1a7 100644 --- a/src/ui/hints.rs +++ b/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, 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, 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::().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); + } } diff --git a/src/ui/keymap.rs b/src/ui/keymap.rs index ef89dbf6..08882fe8 100644 --- a/src/ui/keymap.rs +++ b/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 { .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 { + 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 { 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 _;