diff --git a/docs/customization/keybindings.mdx b/docs/customization/keybindings.mdx index 96f45502..a31fcb83 100644 --- a/docs/customization/keybindings.mdx +++ b/docs/customization/keybindings.mdx @@ -45,7 +45,7 @@ the panel tabs. They are all in the command palette, and all bindable here. ```json { "keybindings": { - "NextTab": "cmd-shift-]", + "NextTab": "ctrl-alt-]", "SplitRight": ["cmd-d"], "ResizePaneLeft": "ctrl-alt-left", "ToggleFullscreen": "" @@ -57,7 +57,7 @@ An action's value takes one of two shapes: | Value | Means | |---|---| -| `"cmd-shift-]"` | **Add** this shortcut. The default keeps working — ⌃ ⇥ still switches tabs | +| `"ctrl-alt-]"` | **Add** this shortcut. The default keeps working — ⌃ ⇥ still opens the tab switcher | | `["cmd-d"]` | **Replace**: exactly these shortcuts, nothing else. List several to have several | | `""` or `[]` | **Unbind** the action | diff --git a/docs/reference/keyboard-shortcuts.mdx b/docs/reference/keyboard-shortcuts.mdx index a89becae..2ac9f009 100644 --- a/docs/reference/keyboard-shortcuts.mdx +++ b/docs/reference/keyboard-shortcuts.mdx @@ -13,7 +13,8 @@ it shows what *your* copy is bound to. This is the shipped default. | New Tab | ⌘ T | Ctrl ⇧ T | | Close Pane / Tab | ⌘ W | Ctrl ⇧ W | | Reopen Closed Tab | ⌘ ⇧ T | Alt ⇧ T | -| Next Tab · Previous Tab | ⌃ ⇥ · ⌃ ⇧ ⇥ | same | +| Next Tab · Previous Tab | ⌘ ⇧ ] · ⌘ ⇧ [ | Ctrl PgDn · Ctrl PgUp | +| Recent Tab Switcher (hold to browse, release to pick) | ⌃ ⇥ · ⌃ ⇧ ⇥ | same | | Go to Tab 1–9 | ⌘ 1…⌘ 9 | Alt 1…Alt 9 | | New Workspace | ⌘ ⇧ N | Ctrl ⇧ N | | Switch Workspace | ⌘ ⇧ O | Ctrl ⇧ O | diff --git a/src/core/actions.rs b/src/core/actions.rs index c9e3a504..302497ea 100644 --- a/src/core/actions.rs +++ b/src/core/actions.rs @@ -52,6 +52,8 @@ actions!( SwapPanePrev, NextTab, PrevTab, + SelectNextTab, + SelectPrevTab, ActivateTab1, ActivateTab2, ActivateTab3, diff --git a/src/ui/app.rs b/src/ui/app.rs index 3c77ebe9..6a85497c 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -4660,17 +4660,14 @@ impl Tty7App { mru_order(&stamps, self.active) } + /// The neighbouring tab in the order the strip shows them, wrapping at + /// the ends — no switcher, no MRU (#867). "Shows" matters with the + /// sidebar grouping tabs: the next row is not always the next index. fn cycle_tab(&mut self, forward: bool, window: &mut Window, cx: &mut Context) { - let n = self.tabs.len(); - if n < 2 { - return; + let order = self.visual_tab_order(cx); + if let Some(next) = step_in_order(&order, self.active, forward) { + self.activate(next, window, cx); } - let next = if forward { - (self.active + 1) % n - } else { - (self.active + n - 1) % n - }; - self.activate(next, window, cx); } pub(crate) fn activate(&mut self, index: usize, window: &mut Window, cx: &mut Context) { @@ -5545,8 +5542,8 @@ impl Tty7App { ResizePaneDown => self.resize_pane(Dir::Down, window, cx), SwapPaneNext => self.swap_pane(true, window, cx), SwapPanePrev => self.swap_pane(false, window, cx), - NextTab => self.cycle_tab(true, window, cx), - PrevTab => self.cycle_tab(false, window, cx), + SelectNextTab => self.cycle_tab(true, window, cx), + SelectPrevTab => self.cycle_tab(false, window, cx), ToggleMaximizePane => self.toggle_maximize(window, cx), ToggleFullscreen => self.toggle_fullscreen(window, cx), ToggleTabSidebar => self.toggle_tab_sidebar(cx), @@ -8556,6 +8553,12 @@ impl Render for Tty7App { .on_action( cx.listener(|this, _: &PrevTab, window, cx| this.tab_switch(false, window, cx)), ) + .on_action(cx.listener(|this, _: &SelectNextTab, window, cx| { + this.cycle_tab(true, window, cx) + })) + .on_action(cx.listener(|this, _: &SelectPrevTab, window, cx| { + this.cycle_tab(false, window, cx) + })) .on_action(cx.listener(|this, _: &ActivateTab1, window, cx| { this.activate_visual(0, window, cx) })) @@ -8806,6 +8809,24 @@ impl Render for Tty7App { /// A zero stamp means the tab was never activated, and those keep strip order /// at the back. `active` leads regardless — its own stamp only lands on the /// next frame. +/// The tab after (or before) `active` in `order`, wrapping round. `None` +/// when there is nowhere else to go; a tab missing from `order` starts from +/// its end, so the step still lands on a tab the user can see. +fn step_in_order(order: &[usize], active: usize, forward: bool) -> Option { + let n = order.len(); + if n < 2 { + return None; + } + let pos = order.iter().position(|&i| i == active); + let next = match (pos, forward) { + (Some(p), true) => (p + 1) % n, + (Some(p), false) => (p + n - 1) % n, + (None, true) => 0, + (None, false) => n - 1, + }; + Some(order[next]).filter(|&i| i != active) +} + fn mru_order(stamps: &[u64], active: usize) -> Vec { let mut order: Vec = (0..stamps.len()).collect(); order.sort_by_key(|&i| (stamps[i] == 0, std::cmp::Reverse(stamps[i]), i)); @@ -9880,7 +9901,7 @@ mod tests { TabAgentSession, clear_window_override_values, close_prompt, document_column_px, join_shell_args, leaf_shares_the_window_daemon, mru_order, pane_free_for, parse_ssh_connect_input, parse_ssh_option_words, rename_outcome, side_panel_max, - split_shell_args, strip_band, wd_path_saveable, + split_shell_args, step_in_order, strip_band, wd_path_saveable, }; use gpui::{Edges, point, px, size}; @@ -10222,6 +10243,42 @@ mod tests { assert!(mru_order(&[], 0).is_empty()); } + #[test] + fn stepping_through_tabs_follows_the_strip_and_wraps() { + let order = [0, 1, 2]; + assert_eq!(step_in_order(&order, 0, true), Some(1)); + assert_eq!(step_in_order(&order, 2, true), Some(0)); + assert_eq!(step_in_order(&order, 0, false), Some(2)); + assert_eq!(step_in_order(&order, 1, false), Some(0)); + // Pressing it again keeps going — this is not the MRU switcher, which + // bounces between the last two tabs (#867). + let mut at = 0; + let seen: Vec = (0..4) + .map(|_| { + at = step_in_order(&order, at, true).unwrap(); + at + }) + .collect(); + assert_eq!(seen, vec![1, 2, 0, 1]); + } + + #[test] + fn stepping_follows_the_grouped_sidebar_order_not_the_index() { + // Sidebar groups reorder the rows: index 2 is shown second. + let order = [0, 2, 1, 3]; + assert_eq!(step_in_order(&order, 0, true), Some(2)); + assert_eq!(step_in_order(&order, 2, true), Some(1)); + assert_eq!(step_in_order(&order, 3, true), Some(0)); + assert_eq!(step_in_order(&order, 0, false), Some(3)); + } + + #[test] + fn stepping_has_nowhere_to_go_with_one_tab() { + assert_eq!(step_in_order(&[], 0, true), None); + assert_eq!(step_in_order(&[0], 0, true), None); + assert_eq!(step_in_order(&[0], 0, false), None); + } + #[test] fn restore_only_attaches_panes_the_workspace_owns_or_nobody_claims() { let ours = crate::core::session::WorkspaceId::new(); diff --git a/src/ui/i18n/en.rs b/src/ui/i18n/en.rs index c6de1a62..2ddb940d 100644 --- a/src/ui/i18n/en.rs +++ b/src/ui/i18n/en.rs @@ -1482,6 +1482,8 @@ pub fn translate_en(key: L10nKey) -> &'static str { L10nKey::CmdSwapPanePrevious => "Swap Pane Previous", L10nKey::CmdNextTab => "Next Tab", L10nKey::CmdPreviousTab => "Previous Tab", + L10nKey::CmdRecentTabSwitcher => "Recent Tab Switcher", + L10nKey::CmdRecentTabSwitcherReverse => "Recent Tab Switcher (Reverse)", L10nKey::CmdCopyWorkingDirectory => "Copy Working Directory", L10nKey::CmdCopySessionId => "Copy Session ID", L10nKey::CmdCopySessionIdSubtitle => "the coding agent's own session id", diff --git a/src/ui/i18n/ja.rs b/src/ui/i18n/ja.rs index 7b7eb124..1555f99b 100644 --- a/src/ui/i18n/ja.rs +++ b/src/ui/i18n/ja.rs @@ -1546,6 +1546,8 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> { L10nKey::CmdSwapPanePrevious => "前のペインと入れ替え", L10nKey::CmdNextTab => "次のタブ", L10nKey::CmdPreviousTab => "前のタブ", + L10nKey::CmdRecentTabSwitcher => "最近のタブを切り替える", + L10nKey::CmdRecentTabSwitcherReverse => "最近のタブを切り替える(逆順)", L10nKey::CmdCopyWorkingDirectory => "作業ディレクトリをコピー", L10nKey::CmdCopySessionId => "セッション ID をコピー", L10nKey::CmdCopySessionIdSubtitle => "コーディングエージェント自身のセッション ID", diff --git a/src/ui/i18n/mod.rs b/src/ui/i18n/mod.rs index f3d6b4c7..8d3a75e3 100644 --- a/src/ui/i18n/mod.rs +++ b/src/ui/i18n/mod.rs @@ -1198,6 +1198,8 @@ l10n_keys! { CmdSwapPanePrevious, CmdNextTab, CmdPreviousTab, + CmdRecentTabSwitcher, + CmdRecentTabSwitcherReverse, CmdCopyWorkingDirectory, CmdCopySessionId, CmdCopySessionIdSubtitle, diff --git a/src/ui/i18n/zh.rs b/src/ui/i18n/zh.rs index fe153ff8..8bd12199 100644 --- a/src/ui/i18n/zh.rs +++ b/src/ui/i18n/zh.rs @@ -1391,6 +1391,8 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> { L10nKey::CmdSwapPanePrevious => "与上一窗格交换", L10nKey::CmdNextTab => "下一标签页", L10nKey::CmdPreviousTab => "上一标签页", + L10nKey::CmdRecentTabSwitcher => "最近标签页切换器", + L10nKey::CmdRecentTabSwitcherReverse => "最近标签页切换器(反向)", L10nKey::CmdCopyWorkingDirectory => "复制工作目录", L10nKey::CmdCopySessionId => "复制会话 ID", L10nKey::CmdCopySessionIdSubtitle => "编码 agent 自身的会话 ID", diff --git a/src/ui/keymap.rs b/src/ui/keymap.rs index f7b77e86..8125af35 100644 --- a/src/ui/keymap.rs +++ b/src/ui/keymap.rs @@ -370,8 +370,22 @@ pub(crate) fn default_bindings() -> Vec<(&'static str, &'static str)> { ("ResizePaneDown", ""), ("SwapPaneNext", ""), ("SwapPanePrev", ""), + // Ctrl+Tab opens the most-recently-used switcher (a quick tap lands + // on the tab you were just in); these step to the neighbouring tab + // in strip order with no popup (#867). ⌘⇧] / ⌘⇧[ is iTerm2's and + // Safari's chord. Off macOS Ctrl+Shift+] is FocusNextPane, so they + // take Ctrl+PgDn / Ctrl+PgUp, the chord GNOME Terminal, xfce4-terminal + // and every browser use for the same thing. ("NextTab", "ctrl-tab"), ("PrevTab", "ctrl-shift-tab"), + ( + "SelectNextTab", + per_platform("secondary-shift-]", "ctrl-pagedown"), + ), + ( + "SelectPrevTab", + per_platform("secondary-shift-[", "ctrl-pageup"), + ), ("ActivateTab1", per_platform("secondary-1", "alt-1")), ("ActivateTab2", per_platform("secondary-2", "alt-2")), ("ActivateTab3", per_platform("secondary-3", "alt-3")), @@ -654,8 +668,20 @@ fn authored_entry(action: &str) -> Option<(CommandGroup, String)> { CommandGroup::TabsPanes, t(L10nKey::CmdSwapPanePrevious).to_string(), ), - "NextTab" => (CommandGroup::TabsPanes, t(L10nKey::CmdNextTab).to_string()), + // `NextTab` / `PrevTab` keep their names — `Config::keybindings` is + // keyed by them — but what they do is open the MRU switcher, so that + // is what the page calls them. "Next Tab" belongs to the action that + // actually goes to the next tab, the same one the palette runs. + "NextTab" => ( + CommandGroup::TabsPanes, + t(L10nKey::CmdRecentTabSwitcher).to_string(), + ), "PrevTab" => ( + CommandGroup::TabsPanes, + t(L10nKey::CmdRecentTabSwitcherReverse).to_string(), + ), + "SelectNextTab" => (CommandGroup::TabsPanes, t(L10nKey::CmdNextTab).to_string()), + "SelectPrevTab" => ( CommandGroup::TabsPanes, t(L10nKey::CmdPreviousTab).to_string(), ), @@ -1018,8 +1044,11 @@ fn tmux_preset(prefix: &str) -> Vec<(String, String)> { ("ToggleMaximizePane", p("z")), ("FocusNextPane", p("o")), ("FocusPrevPane", p(";")), - ("NextTab", p("n")), - ("PrevTab", p("p")), + // tmux's `next-window` / `previous-window`: straight to the + // neighbour, no chooser. The MRU switcher it used to open never + // committed here — the modifier it waits on is not held (#867). + ("SelectNextTab", p("n")), + ("SelectPrevTab", p("p")), ("ActivateTab1", p("1")), ("ActivateTab2", p("2")), ("ActivateTab3", p("3")), @@ -1340,6 +1369,8 @@ fn make_binding(action: &str, keystroke: &str) -> Option { "SwapPanePrev" => KeyBinding::new(keystroke, SwapPanePrev, None), "NextTab" => KeyBinding::new(keystroke, NextTab, None), "PrevTab" => KeyBinding::new(keystroke, PrevTab, None), + "SelectNextTab" => KeyBinding::new(keystroke, SelectNextTab, None), + "SelectPrevTab" => KeyBinding::new(keystroke, SelectPrevTab, None), "ActivateTab1" => KeyBinding::new(keystroke, ActivateTab1, None), "ActivateTab2" => KeyBinding::new(keystroke, ActivateTab2, None), "ActivateTab3" => KeyBinding::new(keystroke, ActivateTab3, None), @@ -2519,17 +2550,22 @@ mod gpui_tests { running_on_json( cx, r#"{"keybinding_preset": "tmux", - "keybindings": {"NextTab": "ctrl-alt-]", "SplitRight": ["ctrl-alt-d"]}}"#, + "keybindings": {"SelectNextTab": "ctrl-alt-]", "SplitRight": ["ctrl-alt-d"]}}"#, ); // The preset is a scheme, and it still replaces the default chord. - assert!(fired(cx, "ctrl-tab").is_empty()); + assert!(fired(cx, per_platform("secondary-shift-]", "ctrl-pagedown")).is_empty()); // A chord added on top of it is added to the preset's chord. assert_eq!( fired(cx, "ctrl-b n").first(), - Some(&NextTab::name_for_type()) + Some(&SelectNextTab::name_for_type()) ); assert_eq!( fired(cx, "ctrl-alt-]").first(), + Some(&SelectNextTab::name_for_type()) + ); + // The MRU switcher the preset no longer takes over keeps Ctrl+Tab. + assert_eq!( + fired(cx, "ctrl-tab").first(), Some(&NextTab::name_for_type()) ); // A list replaces the preset's chord the way it replaces a default. diff --git a/src/ui/palette.rs b/src/ui/palette.rs index 838f0cd0..2493b034 100644 --- a/src/ui/palette.rs +++ b/src/ui/palette.rs @@ -48,8 +48,8 @@ pub enum CommandKind { ResizePaneDown, SwapPaneNext, SwapPanePrev, - NextTab, - PrevTab, + SelectNextTab, + SelectPrevTab, ToggleMaximizePane, ToggleFullscreen, ToggleTabSidebar, @@ -154,8 +154,8 @@ impl CommandKind { ResizePaneDown => "resize-pane-down", SwapPaneNext => "swap-pane-next", SwapPanePrev => "swap-pane-prev", - NextTab => "next-tab", - PrevTab => "prev-tab", + SelectNextTab => "next-tab", + SelectPrevTab => "prev-tab", ToggleMaximizePane => "zoom-pane", ToggleFullscreen => "full-screen", ToggleTabSidebar => "tab-bar-position", @@ -264,8 +264,10 @@ impl CommandKind { ResizePaneDown => "ResizePaneDown", SwapPaneNext => "SwapPaneNext", SwapPanePrev => "SwapPanePrev", - NextTab => "NextTab", - PrevTab => "PrevTab", + // What the palette runs is the plain next/previous step, not the + // MRU switcher `NextTab` opens, so its chord hint is that one's. + SelectNextTab => "SelectNextTab", + SelectPrevTab => "SelectPrevTab", ToggleMaximizePane => "ToggleMaximizePane", ToggleFullscreen => "ToggleFullscreen", ToggleTabSidebar => "ToggleTabSidebar", @@ -458,8 +460,8 @@ impl Command { Command::localized(L10nKey::CmdResizePaneDown, ResizePaneDown), Command::localized(L10nKey::CmdSwapPaneNext, SwapPaneNext), Command::localized(L10nKey::CmdSwapPanePrevious, SwapPanePrev), - Command::localized(L10nKey::CmdNextTab, NextTab), - Command::localized(L10nKey::CmdPreviousTab, PrevTab), + Command::localized(L10nKey::CmdNextTab, SelectNextTab), + Command::localized(L10nKey::CmdPreviousTab, SelectPrevTab), Command::localized(L10nKey::CmdCopyWorkingDirectory, CopyWorkingDirectory), Command::localized(L10nKey::CmdCopySessionId, CopyAgentSessionId) .with_subtitle(t(L10nKey::CmdCopySessionIdSubtitle)), diff --git a/src/ui/tab_sidebar.rs b/src/ui/tab_sidebar.rs index 98ac9174..c582cd9e 100644 --- a/src/ui/tab_sidebar.rs +++ b/src/ui/tab_sidebar.rs @@ -1902,7 +1902,7 @@ impl Tty7App { .collect() } - fn visual_tab_order(&self, cx: &gpui::App) -> Vec { + pub(crate) fn visual_tab_order(&self, cx: &gpui::App) -> Vec { if cx.global::().tab_bar_position != crate::core::config::TabBarPosition::Left { return (0..self.tabs.len()).collect(); }