fix(sidebar): Move to Group is a submenu listing every sidebar group (#1063)

* fix(sidebar): Move to Group is a submenu listing every sidebar group

The tab menu's Move to Group listed only pinned groups, laid out flat on
the grounds that there are never many. A sidebar that auto-groups by repo
holds dozens of groups, none of which could be picked. It is now a submenu
with every group the sidebar draws, in sidebar order, the tab's current one
checked, then Ungrouped (back to auto grouping) and New Group. Picking an
auto group pins it, as its header's pin does, and keeps the tab there.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(sidebar): Move to Group review fixes

- Ungrouped row replaced by Remove from Group, enabled only for a tab kept
  in a group; move_targets is pure (no locale lookup) over GroupKey.
- move_tab_to: an auto group is pinned with the tab in one edit
  (pin_auto_group_with), gpui-tested.
- A separator splits pinned from auto groups; targets are taken before the
  submenu borrows cx.
- No CHANGELOG hunk; the description carries it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(sidebar): Move to Group comment and pin_auto_group_with tidy

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(sidebar): Move to Group's labels line up with the tab menu

One left-checked row makes gpui_component reserve a check column on every
row of that menu, so the whole submenu sat an icon's width right of the tab
menu it hangs off. Putting the check on the right keeps the labels at the
parent's inset.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(sidebar): pinning a repo whose folder is kept joins the kept 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. Move to Group lists both; picking the auto one (or its
header's pin) pushed a second folder group on the same path, two
identical headers splitting the folder's tabs by list order. Join the
kept group instead, as pin_folder already does.

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com>

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",

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 => "グループを固定",

src/ui/i18n/mod.rs

@@ -1326,6 +1326,7 @@ l10n_keys! {
     SidebarUngroupedGroup,
     SidebarMoveToGroup,
     SidebarNewGroup,
+    SidebarRemoveFromGroup,
     SidebarNewGroupName,
     SidebarRenameGroup,
     SidebarPinGroup,

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 => "固定分组",

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<Self>) {
+        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>) {
+        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<usize>, cx: &mut Context<Self>) {
         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<GroupKey>], 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<GroupKey>],
+    groups: &WorkspaceGroups,
+    index: usize,
+) -> Vec<MoveTarget> {
+    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<GroupKey>],
     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<bool> { 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.

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<Self>,
-        window: &Window,
-        cx: &App,
+        window: &mut Window,
+        cx: &mut Context<PopupMenu>,
     ) -> 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::<Config>().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::<Config>().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 {