diff --git a/src/ui/i18n/en.rs b/src/ui/i18n/en.rs index 4bbb4812..fb41959b 100644 --- a/src/ui/i18n/en.rs +++ b/src/ui/i18n/en.rs @@ -2212,6 +2212,7 @@ pub fn translate_en(key: L10nKey) -> &'static str { L10nKey::SidebarUngroupedGroup => "Ungrouped", L10nKey::SidebarMoveToGroup => "Move to Group", L10nKey::SidebarNewGroup => "New Group…", + L10nKey::SidebarRemoveFromGroup => "Remove from Group", L10nKey::SidebarNewGroupName => "New Group", L10nKey::SidebarRenameGroup => "Rename Group", L10nKey::SidebarPinGroup => "Pin Group", diff --git a/src/ui/i18n/ja.rs b/src/ui/i18n/ja.rs index 088e9193..8fa911e8 100644 --- a/src/ui/i18n/ja.rs +++ b/src/ui/i18n/ja.rs @@ -2273,6 +2273,7 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> { L10nKey::SidebarUngroupedGroup => "未分類", L10nKey::SidebarMoveToGroup => "グループへ移動", L10nKey::SidebarNewGroup => "新規グループ…", + L10nKey::SidebarRemoveFromGroup => "グループから外す", L10nKey::SidebarNewGroupName => "新規グループ", L10nKey::SidebarRenameGroup => "グループ名を変更", L10nKey::SidebarPinGroup => "グループを固定", diff --git a/src/ui/i18n/mod.rs b/src/ui/i18n/mod.rs index 60f711b9..a2fe969b 100644 --- a/src/ui/i18n/mod.rs +++ b/src/ui/i18n/mod.rs @@ -1326,6 +1326,7 @@ l10n_keys! { SidebarUngroupedGroup, SidebarMoveToGroup, SidebarNewGroup, + SidebarRemoveFromGroup, SidebarNewGroupName, SidebarRenameGroup, SidebarPinGroup, diff --git a/src/ui/i18n/zh.rs b/src/ui/i18n/zh.rs index 49dc9082..373c3c2a 100644 --- a/src/ui/i18n/zh.rs +++ b/src/ui/i18n/zh.rs @@ -2061,6 +2061,7 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> { L10nKey::SidebarUngroupedGroup => "未分组", L10nKey::SidebarMoveToGroup => "移到分组", L10nKey::SidebarNewGroup => "新建分组…", + L10nKey::SidebarRemoveFromGroup => "移出分组", L10nKey::SidebarNewGroupName => "新建分组", L10nKey::SidebarRenameGroup => "重命名分组", L10nKey::SidebarPinGroup => "固定分组", diff --git a/src/ui/tab_sidebar.rs b/src/ui/tab_sidebar.rs index 71db597e..7b4cecde 100644 --- a/src/ui/tab_sidebar.rs +++ b/src/ui/tab_sidebar.rs @@ -2290,6 +2290,16 @@ impl Tty7App { cx.notify(); } + /// Put tab `index` in group `key`. An auto group's members are decided by + /// their cwds, so a tab goes into one by pinning the group, as its + /// header's pin does — the tab joins in the same edit. + pub(crate) fn move_tab_to(&mut self, index: usize, key: GroupKey, cx: &mut Context) { + match key { + GroupKey::Pinned(id) => self.set_tab_group(index, Some(id), cx), + GroupKey::Auto(auto) => self.pin_auto_group_with(auto, Some(index), cx), + } + } + /// A name no pinned group is using yet, for a group about to be made. /// /// The placeholder only has to be unique — the rename box opens on it @@ -2352,21 +2362,45 @@ impl Tty7App { /// The fold comes along: a group that was shut stays shut, rather than /// springing open in its new place. pub(crate) fn pin_auto_group(&mut self, key: AutoKey, cx: &mut Context) { + self.pin_auto_group_with(key, None, cx); + } + + /// [`pin_auto_group`](Self::pin_auto_group), taking tab `also` into the + /// new group in the same edit. + fn pin_auto_group_with(&mut self, key: AutoKey, also: Option, cx: &mut Context) { let mut group = match &key { AutoKey::Repo(root) => PinnedGroup::folder(root), AutoKey::SshHost(host) => PinnedGroup::label(host.clone()), }; group.collapsed = self.sidebar_groups.auto_collapsed.contains(&key); - let id = group.id; + // A repo whose folder is already kept — a tab dragged out of that + // group while still in the repo is drawn under the repo again — joins + // the kept group rather than making a second: two groups keeping one + // folder split its tabs between them by nothing but list order. + let kept = group.folder.as_deref().and_then(|folder| { + self.sidebar_groups + .pinned + .iter() + .find(|g| g.folder.as_deref() == Some(folder)) + .map(|g| g.id) + }); + let id = kept.unwrap_or(group.id); let wanted = Some(GroupKey::Auto(key.clone())); - for (tab, place) in self.tabs.iter().zip(self.sidebar_group_keys(cx)) { - if place == wanted { + for (i, (tab, place)) in self + .tabs + .iter() + .zip(self.sidebar_group_keys(cx)) + .enumerate() + { + if place == wanted || also == Some(i) { tab.group.set(Some(id)); } } self.edit_groups(cx, |groups| { groups.auto_collapsed.retain(|k| *k != key); - groups.pinned.push(group); + if kept.is_none() { + groups.pinned.push(group); + } }); } @@ -3077,6 +3111,35 @@ fn sidebar_sections(keys: &[Option], groups: &WorkspaceGroups) -> Vec< sections } +/// A row of the tab menu's "Move to Group". +#[derive(Debug, Clone, PartialEq)] +pub(crate) struct MoveTarget { + pub key: GroupKey, + pub name: String, + pub checked: bool, +} + +/// Every group the sidebar draws, in its order — pinned and auto alike — +/// with the one tab `index` is in checked. +pub(crate) fn move_targets( + keys: &[Option], + groups: &WorkspaceGroups, + index: usize, +) -> Vec { + let here = keys.get(index).cloned().flatten(); + sidebar_sections(keys, groups) + .into_iter() + .filter_map(|s| { + let key = s.key?; + Some(MoveTarget { + checked: here.as_ref() == Some(&key), + name: s.name.expect("a section with a key has a header"), + key, + }) + }) + .collect() +} + fn reordered_rows( keys: &[Option], groups: &WorkspaceGroups, @@ -3860,6 +3923,85 @@ mod fold_tests { }); } + /// Moving a tab into an auto group pins that group, in one edit: the + /// group's own tabs and the moved one end up in a single folder group. + #[gpui::test] + fn moving_a_tab_into_an_auto_group_pins_it_with_the_tab(cx: &mut TestAppContext) { + let (app, mut vcx, _streams) = harness_with_tabs(cx, 3); + let r = AutoKey::Repo(PathBuf::from("/w/r")); + app.update(&mut vcx, |app, cx| { + for i in 0..2 { + *app.tabs[i].auto_group.borrow_mut() = Some(r.clone()); + } + *app.tabs[2].auto_group.borrow_mut() = Some(AutoKey::Repo(PathBuf::from("/w/s"))); + cx.notify(); + }); + vcx.run_until_parked(); + + app.update(&mut vcx, |app, cx| { + app.move_tab_to(2, GroupKey::Auto(r.clone()), cx) + }); + vcx.run_until_parked(); + + app.update(&mut vcx, |app, cx| { + let [group] = app.sidebar_groups.pinned.as_slice() else { + panic!("exactly one pinned group: {:?}", app.sidebar_groups.pinned); + }; + assert_eq!(group.folder.as_deref(), Some("/w/r")); + for i in 0..3 { + assert_eq!(app.tabs[i].group.get(), Some(group.id), "tab {i}"); + } + let keys = app.sidebar_group_keys(cx); + assert!( + !keys.contains(&Some(GroupKey::Auto(r.clone()))), + "r is no longer an auto group" + ); + }); + } + + /// A tab dragged out of a folder group while still in the repo is drawn + /// under the repo's auto group, beside the folder group keeping the same + /// directory. Moving a tab into that auto group joins the kept group + /// rather than pinning the folder a second time. + #[gpui::test] + fn moving_into_an_auto_group_whose_folder_is_kept_joins_the_kept_group( + cx: &mut TestAppContext, + ) { + let (app, mut vcx, _streams) = harness_with_tabs(cx, 3); + let r = AutoKey::Repo(PathBuf::from("/w/r")); + let kept = PinnedGroup::folder(Path::new("/w/r")); + let id = kept.id; + app.update(&mut vcx, |app, cx| { + app.sidebar_groups.pinned.push(kept); + app.tabs[0].group.set(Some(id)); + for i in 0..2 { + *app.tabs[i].auto_group.borrow_mut() = Some(r.clone()); + } + *app.tabs[2].auto_group.borrow_mut() = Some(AutoKey::Repo(PathBuf::from("/w/s"))); + cx.notify(); + }); + vcx.run_until_parked(); + app.update(&mut vcx, |app, cx| { + assert!( + app.sidebar_group_keys(cx) + .contains(&Some(GroupKey::Auto(r.clone()))), + "tab 1 is drawn under the repo's auto group" + ); + app.move_tab_to(2, GroupKey::Auto(r.clone()), cx) + }); + vcx.run_until_parked(); + + app.update(&mut vcx, |app, _| { + let [group] = app.sidebar_groups.pinned.as_slice() else { + panic!("still one pinned group: {:?}", app.sidebar_groups.pinned); + }; + assert_eq!(group.id, id); + for i in 0..3 { + assert_eq!(app.tabs[i].group.get(), Some(id), "tab {i}"); + } + }); + } + /// A pinned group with no tabs is still drawn — it is kept until it is /// deleted — while an auto group with none simply is not there. #[gpui::test] @@ -4391,6 +4533,43 @@ mod tests { ); } + /// "Move to Group" offers every group the sidebar draws — auto groups + /// too, not just the pinned ones — in sidebar order. + #[test] + fn move_targets_list_every_sidebar_group_in_order() { + let mut groups = none(); + let work = PinnedGroup::label("work"); + groups.pinned = vec![work.clone()]; + let keys = vec![ + Some(g("/w/r")), + Some(GroupKey::Pinned(work.id)), + Some(host("u@h")), + None, + ]; + let shape = |index| -> Vec<(GroupKey, String, bool)> { + move_targets(&keys, &groups, index) + .into_iter() + .map(|m| (m.key, m.name, m.checked)) + .collect() + }; + assert_eq!( + shape(0), + vec![ + (GroupKey::Pinned(work.id), "work".into(), false), + (g("/w/r"), "r".into(), true), + (host("u@h"), "u@h".into(), false), + ] + ); + let checked = |index| -> Vec { shape(index).into_iter().map(|m| m.2).collect() }; + assert_eq!(checked(1), vec![true, false, false]); + assert_eq!( + checked(3), + vec![false, false, false], + "an ungrouped tab is in none" + ); + assert!(move_targets(&[None], &none(), 0).is_empty()); + } + /// With pinned groups and nothing auto-grouped below them, the rest is /// just the list — a header reading "Ungrouped" over all of it would be a /// label on nothing. diff --git a/src/ui/tab_strip.rs b/src/ui/tab_strip.rs index a86f696e..8647eade 100644 --- a/src/ui/tab_strip.rs +++ b/src/ui/tab_strip.rs @@ -8,7 +8,9 @@ use gpui_component::input::Input; use gpui_component::kbd::Kbd; use gpui_component::menu::{ContextMenuExt as _, DropdownMenu as _, PopupMenu, PopupMenuItem}; use gpui_component::tooltip::Tooltip; -use gpui_component::{ActiveTheme as _, Icon, IconName, Selectable as _, Sizable as _, h_flex}; +use gpui_component::{ + ActiveTheme as _, Icon, IconName, Selectable as _, Side, Sizable as _, h_flex, +}; use unicode_segmentation::UnicodeSegmentation as _; use crate::core::actions::{ @@ -1993,13 +1995,25 @@ impl Tty7App { index: usize, below_wording: bool, app: &gpui::WeakEntity, - window: &Window, - cx: &App, + window: &mut Window, + cx: &mut Context, ) -> PopupMenu { let Some(entity) = app.upgrade() else { return menu; }; let this = entity.read(cx); + // Taken before `this` is let go for the Move to Group submenu below. + let sidebar = + cx.global::().tab_bar_position == crate::core::config::TabBarPosition::Left; + let move_targets = sidebar.then(|| { + let keys = this.sidebar_group_keys(cx); + crate::ui::tab_sidebar::move_targets(&keys, &this.sidebar_groups, index) + }); + let in_kept_group = this + .tabs + .get(index) + .and_then(|t| t.group.get()) + .is_some_and(|g| this.sidebar_groups.contains(g)); let tab_count = this.tabs.len(); let cwd = this.tab_cwd_text(index, window, cx); let has_cwd = cwd.is_some(); @@ -2067,48 +2081,75 @@ impl Tty7App { ); } - // Where this tab sits, and where it could be put instead. + // Where this tab sits, and where it could be put instead: every group + // the sidebar draws, auto ones included, behind a submenu — a sidebar + // grouping by repo can hold dozens, far too many to lay out flat. // - // Laid out flat rather than behind a "Move to Group ▸" submenu: there - // are never many pinned groups — they are kept by hand — so a submenu - // would cost a second click to show two or three items, and - // `PopupMenu::submenu` wants a `&mut Context` this function does not - // have. The label above them says what the block is. + // An auto group's membership is its tabs' cwds, so a tab can only be + // put in one by keeping the group: picking it pins the group (as its + // header's pin does) and the tab with it. Remove from Group, offered + // only for a tab kept in a group, hands it back to auto grouping. // - // No way back to auto grouping here: that is a drag below the divider, - // the one place in the sidebar where "not kept by hand" is drawn. - // Offered only with the tabs in the sidebar for the same reason — a - // group is something the sidebar draws, and "move to group" from the - // top tab bar would name something the user cannot see. - if cx.global::().tab_bar_position == crate::core::config::TabBarPosition::Left { - let here = this - .tabs - .get(index) - .and_then(|t| t.group.get()) - .filter(|g| this.sidebar_groups.contains(*g)); - menu = menu - .separator() - .item(PopupMenuItem::label(t(L10nKey::SidebarMoveToGroup))); - for (id, name) in this.pinned_group_names() { - menu = menu.item( - PopupMenuItem::new(name) - .checked(here == Some(id)) - .on_click({ + // Offered only with the tabs in the sidebar — a group is something the + // sidebar draws, and "move to group" from the top tab bar would name + // something the user cannot see. + if let Some(targets) = move_targets { + let app = app.clone(); + menu = menu.separator().submenu( + t(L10nKey::SidebarMoveToGroup), + window, + cx, + move |sub, _window, _cx| { + // A left check makes every row of the menu reserve a + // check column, so the whole submenu sat one icon's + // width right of the tab menu beside it. On the right, + // its labels line up with the parent's. + let mut sub = sub.check_side(Side::Right); + for (i, target) in targets.iter().enumerate() { + // Pinned groups, then the auto ones, as the divider + // splits them in the sidebar. + if i > 0 && targets[i - 1].key.is_pinned() && !target.key.is_pinned() { + sub = sub.separator(); + } + let mut item = + PopupMenuItem::new(target.name.clone()).checked(target.checked); + if !target.checked { let app = app.clone(); - move |_, _window, cx| { - let _ = app - .update(cx, |this, cx| this.set_tab_group(index, Some(id), cx)); - } - }), - ); - } - menu = menu.item(PopupMenuItem::new(t(L10nKey::SidebarNewGroup)).on_click({ - let app = app.clone(); - move |_, window, cx| { - let _ = app.update(cx, |this, cx| this.new_tab_group(index, window, cx)); - } - })); + let key = target.key.clone(); + item = item.on_click(move |_, _window, cx| { + let _ = app.update(cx, |this, cx| { + this.move_tab_to(index, key.clone(), cx) + }); + }); + } + sub = sub.item(item); + } + if !targets.is_empty() { + sub = sub.separator(); + } + let remove = app.clone(); + let new = app.clone(); + sub.item( + PopupMenuItem::new(t(L10nKey::SidebarRemoveFromGroup)) + .disabled(!in_kept_group) + .on_click(move |_, _window, cx| { + let _ = remove + .update(cx, |this, cx| this.set_tab_group(index, None, cx)); + }), + ) + .item( + PopupMenuItem::new(t(L10nKey::SidebarNewGroup)).on_click( + move |_, window, cx| { + let _ = new + .update(cx, |this, cx| this.new_tab_group(index, window, cx)); + }, + ), + ) + }, + ); } + // Read again: the submenu above needed `cx` mutably. + let this = entity.read(cx); let in_repo = this.tab_is_in_repo(index, window, cx); if in_repo {