Store multi workspace as a vec of project groups

Eric Holk , Mikayla Maki , and Max Brunsfeld created

Co-authored-by: Mikayla Maki <mikayla.c.maki@gmail.com>
Co-authored-by: Max Brunsfeld <maxbrunsfeld@gmail.com>

Change summary

crates/agent_ui/src/agent_panel.rs          |   4 
crates/sidebar/src/project_group_builder.rs |  64 ------
crates/sidebar/src/sidebar.rs               |  12 
crates/workspace/src/multi_workspace.rs     | 196 ++++++++++++++++------
crates/workspace/src/workspace.rs           |  27 ++
5 files changed, 177 insertions(+), 126 deletions(-)

Detailed changes

crates/agent_ui/src/agent_panel.rs 🔗

@@ -5540,8 +5540,8 @@ mod tests {
                 );
 
                 // Check the newest workspace's panel for the correct agent.
-                let new_workspace = multi_workspace
-                    .workspaces()
+                let workspaces = multi_workspace.workspaces();
+                let new_workspace = workspaces
                     .iter()
                     .find(|ws| ws.entity_id() != workspace.entity_id())
                     .expect("should find the new workspace");

crates/sidebar/src/project_group_builder.rs 🔗

@@ -16,38 +16,7 @@ use std::{
 
 use gpui::{App, Entity};
 use ui::SharedString;
-use workspace::{MultiWorkspace, PathList, Workspace};
-
-/// Identifies a project group by a set of paths the workspaces in this group
-/// have.
-///
-/// Paths are mapped to their main worktree path first so we can group
-/// workspaces by main repos.
-#[derive(PartialEq, Eq, Hash, Clone)]
-pub struct ProjectGroupName {
-    path_list: PathList,
-}
-
-impl ProjectGroupName {
-    pub fn display_name(&self) -> SharedString {
-        let mut names = Vec::with_capacity(self.path_list.paths().len());
-        for abs_path in self.path_list.paths() {
-            if let Some(name) = abs_path.file_name() {
-                names.push(name.to_string_lossy().to_string());
-            }
-        }
-        if names.is_empty() {
-            // TODO: Can we do something better in this case?
-            "Empty Workspace".into()
-        } else {
-            names.join(", ").into()
-        }
-    }
-
-    pub fn path_list(&self) -> &PathList {
-        &self.path_list
-    }
-}
+use workspace::{MultiWorkspace, PathList, ProjectGroupKey, Workspace};
 
 #[derive(Default)]
 pub struct ProjectGroup {
@@ -88,7 +57,7 @@ impl ProjectGroup {
 pub struct ProjectGroupBuilder {
     /// Maps git repositories' work_directory_abs_path to their original_repo_abs_path
     directory_mappings: HashMap<PathBuf, PathBuf>,
-    project_groups: VecMap<ProjectGroupName, ProjectGroup>,
+    project_groups: VecMap<ProjectGroupKey, ProjectGroup>,
 }
 
 impl ProjectGroupBuilder {
@@ -108,18 +77,16 @@ impl ProjectGroupBuilder {
             builder.add_workspace_mappings(workspace.read(cx), cx);
         }
 
-        // Second pass: group each workspace using canonical paths derived
-        // from the full set of mappings.
         for workspace in mw.workspaces() {
-            let group_name = builder.canonical_workspace_paths(workspace, cx);
+            let group_key = workspace.read(cx).project_group_key(cx);
             builder
-                .project_group_entry(&group_name)
-                .add_workspace(workspace, cx);
+                .project_group_key(&group_key)
+                .add_workspace(&workspace, cx);
         }
         builder
     }
 
-    fn project_group_entry(&mut self, name: &ProjectGroupName) -> &mut ProjectGroup {
+    fn project_group_key(&mut self, name: &ProjectGroupKey) -> &mut ProjectGroup {
         self.project_groups.entry_ref(name).or_insert_default()
     }
 
@@ -150,23 +117,6 @@ impl ProjectGroupBuilder {
         }
     }
 
-    /// Derives the canonical group name for a workspace by canonicalizing
-    /// each of its root paths using the builder's directory mappings.
-    fn canonical_workspace_paths(
-        &self,
-        workspace: &Entity<Workspace>,
-        cx: &App,
-    ) -> ProjectGroupName {
-        let root_paths = workspace.read(cx).root_paths(cx);
-        let paths: Vec<_> = root_paths
-            .iter()
-            .map(|p| self.canonicalize_path(p).to_path_buf())
-            .collect();
-        ProjectGroupName {
-            path_list: PathList::new(&paths),
-        }
-    }
-
     pub fn canonicalize_path<'a>(&'a self, path: &'a Path) -> &'a Path {
         self.directory_mappings
             .get(path)
@@ -203,7 +153,7 @@ impl ProjectGroupBuilder {
         PathList::new(&paths)
     }
 
-    pub fn groups(&self) -> impl Iterator<Item = (&ProjectGroupName, &ProjectGroup)> {
+    pub fn groups(&self) -> impl Iterator<Item = (&ProjectGroupKey, &ProjectGroup)> {
         self.project_groups.iter()
     }
 }

crates/sidebar/src/sidebar.rs 🔗

@@ -698,7 +698,7 @@ impl Sidebar {
         };
         let mw = multi_workspace.read(cx);
         let workspaces = mw.workspaces().to_vec();
-        let active_workspace = mw.workspaces().get(mw.active_workspace_index()).cloned();
+        let active_workspace = Some(mw.active_workspace());
 
         let agent_server_store = workspaces
             .first()
@@ -2232,12 +2232,10 @@ impl Sidebar {
             return;
         }
 
-        let active_workspace = self.multi_workspace.upgrade().and_then(|w| {
-            w.read(cx)
-                .workspaces()
-                .get(w.read(cx).active_workspace_index())
-                .cloned()
-        });
+        let active_workspace = self
+            .multi_workspace
+            .upgrade()
+            .map(|w| w.read(cx).active_workspace());
 
         if let Some(workspace) = active_workspace {
             self.activate_thread_locally(&metadata, &workspace, window, cx);

crates/workspace/src/multi_workspace.rs 🔗

@@ -8,13 +8,14 @@ use gpui::{
 use project::DisableAiSettings;
 #[cfg(any(test, feature = "test-support"))]
 use project::Project;
+use remote::RemoteConnectionOptions;
 use settings::Settings;
 pub use settings::SidebarSide;
 use std::future::Future;
 use std::path::PathBuf;
 use std::sync::Arc;
 use ui::prelude::*;
-use util::ResultExt;
+use util::{ResultExt, debug_panic, path_list::PathList};
 use zed_actions::agents_sidebar::{MoveWorkspaceToNewWindow, ToggleThreadSwitcher};
 
 use agent_settings::AgentSettings;
@@ -218,10 +219,49 @@ impl<T: Sidebar> SidebarHandle for Entity<T> {
     }
 }
 
+/// Identifies a project group by a set of paths the workspaces in this group
+/// have.
+///
+/// Paths are mapped to their main worktree path first so we can group
+/// workspaces by main repos.
+#[derive(PartialEq, Eq, Hash, Clone)]
+pub struct ProjectGroupKey {
+    pub main_worktree_paths: PathList,
+    pub host: Option<RemoteConnectionOptions>,
+}
+
+impl ProjectGroupKey {
+    pub fn display_name(&self) -> SharedString {
+        let mut names = Vec::with_capacity(self.main_worktree_paths.paths().len());
+        for abs_path in self.main_worktree_paths.paths() {
+            if let Some(name) = abs_path.file_name() {
+                names.push(name.to_string_lossy().to_string());
+            }
+        }
+        if names.is_empty() {
+            // TODO: Can we do something better in this case?
+            "Empty Workspace".into()
+        } else {
+            names.join(", ").into()
+        }
+    }
+
+    pub fn path_list(&self) -> &PathList {
+        &self.main_worktree_paths
+    }
+
+    pub fn from_paths(paths: &[PathBuf], host: Option<RemoteConnectionOptions>) -> Self {
+        Self {
+            main_worktree_paths: PathList::new(paths),
+            host,
+        }
+    }
+}
+
 pub struct MultiWorkspace {
     window_id: WindowId,
-    workspaces: Vec<Entity<Workspace>>,
-    active_workspace_index: usize,
+    project_groups: Vec<ProjectGroup>,
+    active_workspace: Entity<Workspace>,
     sidebar: Option<Box<dyn SidebarHandle>>,
     sidebar_open: bool,
     sidebar_overlay: Option<AnyView>,
@@ -230,6 +270,20 @@ pub struct MultiWorkspace {
     _subscriptions: Vec<Subscription>,
 }
 
+struct ProjectGroup {
+    key: ProjectGroupKey,
+    workspaces: Vec<Entity<Workspace>>,
+}
+
+impl ProjectGroup {
+    fn from_workspace(workspace: Entity<Workspace>, cx: &App) -> Self {
+        Self {
+            key: workspace.read(cx).project_group_key(cx),
+            workspaces: vec![workspace],
+        }
+    }
+}
+
 impl EventEmitter<MultiWorkspaceEvent> for MultiWorkspace {}
 
 impl MultiWorkspace {
@@ -269,8 +323,8 @@ impl MultiWorkspace {
         });
         Self {
             window_id: window.window_handle().window_id(),
-            workspaces: vec![workspace],
-            active_workspace_index: 0,
+            project_groups: vec![ProjectGroup::from_workspace(workspace.clone(), cx)],
+            active_workspace: workspace,
             sidebar: None,
             sidebar_open: false,
             sidebar_overlay: None,
@@ -381,10 +435,17 @@ impl MultiWorkspace {
         }
     }
 
+    pub fn workspaces(&self) -> Vec<Entity<Workspace>> {
+        self.project_groups
+            .iter()
+            .flat_map(|group| group.workspaces.iter().cloned())
+            .collect()
+    }
+
     pub fn open_sidebar(&mut self, cx: &mut Context<Self>) {
         self.sidebar_open = true;
         let sidebar_focus_handle = self.sidebar.as_ref().map(|s| s.focus_handle(cx));
-        for workspace in &self.workspaces {
+        for workspace in self.workspaces() {
             workspace.update(cx, |workspace, _cx| {
                 workspace.set_sidebar_focus_handle(sidebar_focus_handle.clone());
             });
@@ -395,7 +456,7 @@ impl MultiWorkspace {
 
     pub fn close_sidebar(&mut self, window: &mut Window, cx: &mut Context<Self>) {
         self.sidebar_open = false;
-        for workspace in &self.workspaces {
+        for workspace in self.workspaces() {
             workspace.update(cx, |workspace, _cx| {
                 workspace.set_sidebar_focus_handle(None);
             });
@@ -409,9 +470,8 @@ impl MultiWorkspace {
 
     pub fn close_window(&mut self, _: &CloseWindow, window: &mut Window, cx: &mut Context<Self>) {
         cx.spawn_in(window, async move |this, cx| {
-            let workspaces = this.update(cx, |multi_workspace, _cx| {
-                multi_workspace.workspaces().to_vec()
-            })?;
+            let workspaces =
+                this.update(cx, |multi_workspace, _cx| multi_workspace.workspaces())?;
 
             for workspace in workspaces {
                 let should_continue = workspace
@@ -447,15 +507,21 @@ impl MultiWorkspace {
     }
 
     pub fn workspace(&self) -> &Entity<Workspace> {
-        &self.workspaces[self.active_workspace_index]
+        &self.active_workspace
     }
 
-    pub fn workspaces(&self) -> &[Entity<Workspace>] {
-        &self.workspaces
+    pub fn active_workspace(&self) -> Entity<Workspace> {
+        self.active_workspace.clone()
     }
 
     pub fn active_workspace_index(&self) -> usize {
-        self.active_workspace_index
+        self.workspaces()
+            .iter()
+            .position(|workspace| workspace == &self.active_workspace)
+            .unwrap_or_else(|| {
+                debug_panic!("active workspace was not present in project groups");
+                0
+            })
     }
 
     /// Adds a workspace to this window without changing which workspace is
@@ -481,9 +547,9 @@ impl MultiWorkspace {
             return;
         }
 
-        let index = self.insert_workspace(workspace, &*window, cx);
-        let changed = self.active_workspace_index != index;
-        self.active_workspace_index = index;
+        self.insert_workspace(workspace.clone(), &*window, cx);
+        let changed = self.active_workspace != workspace;
+        self.active_workspace = workspace;
         if changed {
             cx.emit(MultiWorkspaceEvent::ActiveWorkspaceChanged);
             self.serialize(cx);
@@ -505,9 +571,9 @@ impl MultiWorkspace {
             return;
         }
 
-        if let Some(index) = self.workspaces.iter().position(|w| *w == workspace) {
-            let changed = self.active_workspace_index != index;
-            self.active_workspace_index = index;
+        if let Some(index) = self.workspaces().iter().position(|w| *w == workspace) {
+            let changed = self.active_workspace_index() != index;
+            self.active_workspace = self.workspaces()[index].clone();
             if changed {
                 cx.emit(MultiWorkspaceEvent::ActiveWorkspaceChanged);
                 self.serialize(cx);
@@ -516,10 +582,7 @@ impl MultiWorkspace {
             return;
         }
 
-        let old_workspace = std::mem::replace(
-            &mut self.workspaces[self.active_workspace_index],
-            workspace.clone(),
-        );
+        let old_workspace = std::mem::replace(&mut self.active_workspace, workspace.clone());
 
         let old_entity_id = old_workspace.entity_id();
         self.detach_workspace(&old_workspace, cx);
@@ -535,35 +598,42 @@ impl MultiWorkspace {
     }
 
     fn set_single_workspace(&mut self, workspace: Entity<Workspace>, cx: &mut Context<Self>) {
-        self.workspaces[0] = workspace;
-        self.active_workspace_index = 0;
+        self.active_workspace = workspace;
         cx.emit(MultiWorkspaceEvent::ActiveWorkspaceChanged);
         cx.notify();
     }
 
-    /// Inserts a workspace into the list if not already present. Returns the
-    /// index of the workspace (existing or newly inserted). Does not change
-    /// the active workspace index.
+    /// Inserts a workspace into the matching project group, creating a new
+    /// group if none exists. Does nothing if the workspace is already present.
     fn insert_workspace(
         &mut self,
         workspace: Entity<Workspace>,
         window: &Window,
         cx: &mut Context<Self>,
-    ) -> usize {
-        if let Some(index) = self.workspaces.iter().position(|w| *w == workspace) {
-            index
+    ) {
+        if self.workspaces().iter().any(|w| *w == workspace) {
+            return;
+        }
+
+        Self::subscribe_to_workspace(&workspace, window, cx);
+        self.sync_sidebar_to_workspace(&workspace, cx);
+        let weak_self = cx.weak_entity();
+        workspace.update(cx, |workspace, cx| {
+            workspace.set_multi_workspace(weak_self, cx);
+        });
+
+        let key = workspace.read(cx).project_group_key(cx);
+        if let Some(group) = self.project_groups.iter_mut().find(|g| g.key == key) {
+            group.workspaces.push(workspace.clone());
         } else {
-            Self::subscribe_to_workspace(&workspace, window, cx);
-            self.sync_sidebar_to_workspace(&workspace, cx);
-            let weak_self = cx.weak_entity();
-            workspace.update(cx, |workspace, cx| {
-                workspace.set_multi_workspace(weak_self, cx);
+            self.project_groups.push(ProjectGroup {
+                key,
+                workspaces: vec![workspace.clone()],
             });
-            self.workspaces.push(workspace.clone());
-            cx.emit(MultiWorkspaceEvent::WorkspaceAdded(workspace));
-            cx.notify();
-            self.workspaces.len() - 1
         }
+
+        cx.emit(MultiWorkspaceEvent::WorkspaceAdded(workspace));
+        cx.notify();
     }
 
     /// Clears session state and DB binding for a workspace that is being
@@ -598,13 +668,14 @@ impl MultiWorkspace {
     }
 
     fn cycle_workspace(&mut self, delta: isize, window: &mut Window, cx: &mut Context<Self>) {
-        let count = self.workspaces.len() as isize;
+        let workspaces = self.workspaces();
+        let count = workspaces.len() as isize;
         if count <= 1 {
             return;
         }
-        let current = self.active_workspace_index as isize;
+        let current = self.active_workspace_index() as isize;
         let next = ((current + delta).rem_euclid(count)) as usize;
-        let workspace = self.workspaces[next].clone();
+        let workspace = workspaces[next].clone();
         self.activate(workspace, window, cx);
     }
 
@@ -838,28 +909,37 @@ impl MultiWorkspace {
         window: &mut Window,
         cx: &mut Context<Self>,
     ) -> bool {
-        let Some(index) = self.workspaces.iter().position(|w| w == workspace) else {
-            return false;
-        };
-        if self.workspaces.len() <= 1 {
+        let all_workspaces = self.workspaces();
+        if all_workspaces.len() <= 1 {
             return false;
         }
 
-        let removed_workspace = self.workspaces.remove(index);
-
-        if self.active_workspace_index >= self.workspaces.len() {
-            self.active_workspace_index = self.workspaces.len() - 1;
-        } else if self.active_workspace_index > index {
-            self.active_workspace_index -= 1;
+        let Some(group) = self
+            .project_groups
+            .iter_mut()
+            .find(|g| g.workspaces.contains(workspace))
+        else {
+            return false;
+        };
+        group.workspaces.retain(|w| w != workspace);
+
+        // Remove empty groups.
+        self.project_groups.retain(|g| !g.workspaces.is_empty());
+
+        // If we removed the active workspace, pick a new one.
+        if self.active_workspace == *workspace {
+            self.active_workspace = self
+                .workspaces()
+                .into_iter()
+                .next()
+                .expect("there is always at least one workspace after the len() > 1 check");
         }
 
-        self.detach_workspace(&removed_workspace, cx);
+        self.detach_workspace(workspace, cx);
 
         self.serialize(cx);
         self.focus_active_workspace(window, cx);
-        cx.emit(MultiWorkspaceEvent::WorkspaceRemoved(
-            removed_workspace.entity_id(),
-        ));
+        cx.emit(MultiWorkspaceEvent::WorkspaceRemoved(workspace.entity_id()));
         cx.emit(MultiWorkspaceEvent::ActiveWorkspaceChanged);
         cx.notify();
 

crates/workspace/src/workspace.rs 🔗

@@ -31,8 +31,9 @@ pub use crate::notifications::NotificationFrame;
 pub use dock::Panel;
 pub use multi_workspace::{
     CloseWorkspaceSidebar, DraggedSidebar, FocusWorkspaceSidebar, MultiWorkspace,
-    MultiWorkspaceEvent, NextWorkspace, PreviousWorkspace, Sidebar, SidebarEvent, SidebarHandle,
-    SidebarRenderState, SidebarSide, ToggleWorkspaceSidebar, sidebar_side_context_menu,
+    MultiWorkspaceEvent, NextWorkspace, PreviousWorkspace, ProjectGroupKey, Sidebar, SidebarEvent,
+    SidebarHandle, SidebarRenderState, SidebarSide, ToggleWorkspaceSidebar,
+    sidebar_side_context_menu,
 };
 pub use path_list::{PathList, SerializedPathList};
 pub use toast_layer::{ToastAction, ToastLayer, ToastView};
@@ -6390,6 +6391,28 @@ impl Workspace {
             .collect::<Vec<_>>()
     }
 
+    pub fn project_group_key(&self, cx: &App) -> ProjectGroupKey {
+        let host = self.project().read(cx).remote_connection_options(cx);
+        let repositories = self.project().read(cx).repositories(cx);
+        let paths: Vec<_> = self
+            .root_paths(cx)
+            .iter()
+            .map(|root_path| {
+                repositories
+                    .values()
+                    .find(|repo| repo.read(cx).snapshot().work_directory_abs_path == *root_path)
+                    .map(|repo| {
+                        repo.read(cx)
+                            .snapshot()
+                            .original_repo_abs_path
+                            .to_path_buf()
+                    })
+                    .unwrap_or_else(|| root_path.to_path_buf())
+            })
+            .collect();
+        ProjectGroupKey::from_paths(&paths, host)
+    }
+
     fn remove_panes(&mut self, member: Member, window: &mut Window, cx: &mut Context<Workspace>) {
         match member {
             Member::Axis(PaneAxis { members, .. }) => {