From f49d9324dbfb4354756c5816aea3830bbf01e5c9 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Tue, 14 Apr 2026 20:12:11 -0600 Subject: [PATCH] sidebar: Simplify project group menu (#53933) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the ability to add and remove individual folders from project groups in the sidebar. ## Changes **Sidebar project group menu:** - When there are 2+ local project groups, the ellipsis menu shows "Open Project in New Window" and "Remove Project" (no separators, no folder listing) - When there's only one group or it's remote, the ellipsis menu is replaced with a close X button that removes the project directly **Project picker labels:** - "Open Local Project" → "Open Local Folders" - "Open Remote Project" → "Open Remote Folder" - "Add Local Project" → "Add Local Folders" - "Add Remote Project" → "Add Remote Folder" **Dead code removal:** - `MultiWorkspace::remove_folder_from_project_group` - `MultiWorkspace::prompt_to_add_folders_to_project_group` - `MultiWorkspace::add_folders_to_project_group` - `MultiWorkspace::update_project_group_key` - `MultiWorkspaceEvent::ProjectGroupKeyUpdated` - `Sidebar::move_threads_for_key_change` - `ThreadMetadataStore::change_worktree_paths_by_main` - Associated tests for removed functionality Per-workspace thread regrouping (when a workspace's worktree paths change) continues to work via the existing `WorktreePathsChanged` → `move_thread_paths` flow. The existing `test_worktree_add_only_regroups_threads_for_changed_workspace` test (renamed and strengthened) verifies that only the changed workspace's threads are regrouped, while sibling worktree threads remain in their original group. Release Notes: - Improved sidebar project group menu to show a simple close button for single-project windows and a streamlined two-item menu for multi-project windows. --- crates/agent_ui/src/thread_metadata_store.rs | 32 --- crates/recent_projects/src/recent_projects.rs | 8 +- .../src/sidebar_recent_projects.rs | 4 +- crates/sidebar/src/sidebar.rs | 236 +++++++----------- crates/sidebar/src/sidebar_tests.rs | 221 +++------------- crates/workspace/src/multi_workspace.rs | 151 +---------- crates/workspace/src/multi_workspace_tests.rs | 67 ----- 7 files changed, 132 insertions(+), 587 deletions(-) diff --git a/crates/agent_ui/src/thread_metadata_store.rs b/crates/agent_ui/src/thread_metadata_store.rs index 44353111e9da35b97c39295f29ae525456bd8f94..0dc3172b1ff1b06d265d140438c3c1ee58d92b43 100644 --- a/crates/agent_ui/src/thread_metadata_store.rs +++ b/crates/agent_ui/src/thread_metadata_store.rs @@ -820,38 +820,6 @@ impl ThreadMetadataStore { self.mutate_thread_paths(&thread_ids, mutate, cx); } - /// Like `change_worktree_paths`, but looks up threads by their - /// `main_worktree_paths` instead of `folder_paths`. Used when - /// migrating threads for project group key changes where the - /// lookup key is the group key's main paths. - /// When `remote_connection` is provided, only threads with a matching - /// remote connection are affected. - pub fn change_worktree_paths_by_main( - &mut self, - current_main_paths: &PathList, - remote_connection: Option<&RemoteConnectionOptions>, - mutate: impl Fn(&mut WorktreePaths), - cx: &mut Context, - ) { - let thread_ids: Vec<_> = self - .threads_by_main_paths - .get(current_main_paths) - .into_iter() - .flatten() - .filter(|id| { - same_remote_connection_identity( - self.threads - .get(id) - .and_then(|t| t.remote_connection.as_ref()), - remote_connection, - ) - }) - .copied() - .collect(); - - self.mutate_thread_paths(&thread_ids, mutate, cx); - } - fn mutate_thread_paths( &mut self, thread_ids: &[ThreadId], diff --git a/crates/recent_projects/src/recent_projects.rs b/crates/recent_projects/src/recent_projects.rs index a8144969f6aeb12f2ef16e0a1e1e1b382b4e6712..c966a6508fcaffa811b6c61c37be02e5d9ffc322 100644 --- a/crates/recent_projects/src/recent_projects.rs +++ b/crates/recent_projects/src/recent_projects.rs @@ -1639,7 +1639,7 @@ impl PickerDelegate for RecentProjectsDelegate { let open_action = workspace::Open { create_new_window: self.create_new_window, }; - Button::new("open_local_folder", "Open Local Project") + Button::new("open_local_folder", "Open Local Folders") .key_binding(KeyBinding::for_action_in(&open_action, &focus_handle, cx)) .on_click({ let workspace = self.workspace.clone(); @@ -1655,7 +1655,7 @@ impl PickerDelegate for RecentProjectsDelegate { }) }) .child( - Button::new("open_remote_folder", "Open Remote Project") + Button::new("open_remote_folder", "Open Remote Folder") .key_binding(KeyBinding::for_action( &OpenRemote { from_existing_connection: false, @@ -1827,7 +1827,7 @@ impl PickerDelegate for RecentProjectsDelegate { .separator() }) .entry( - "Open Local Project", + "Open Local Folders", Some(open_action.boxed_clone()), { let workspace_handle = workspace_handle.clone(); @@ -1842,7 +1842,7 @@ impl PickerDelegate for RecentProjectsDelegate { }, ) .action( - "Open Remote Project", + "Open Remote Folder", OpenRemote { from_existing_connection: false, create_new_window: false, diff --git a/crates/recent_projects/src/sidebar_recent_projects.rs b/crates/recent_projects/src/sidebar_recent_projects.rs index 7e0634e9e9ccc321c5f0aa888194c2f920d99586..f197ed3cead41e1fb3786e5b5a99727b5ebcf6b9 100644 --- a/crates/recent_projects/src/sidebar_recent_projects.rs +++ b/crates/recent_projects/src/sidebar_recent_projects.rs @@ -426,7 +426,7 @@ impl PickerDelegate for SidebarRecentProjectsDelegate { create_new_window: false, }; - Button::new("open_local_folder", "Add Local Project") + Button::new("open_local_folder", "Add Local Folders") .key_binding(KeyBinding::for_action_in(&open_action, &focus_handle, cx)) .on_click(cx.listener(move |_, _, window, cx| { window.dispatch_action(open_action.boxed_clone(), cx); @@ -434,7 +434,7 @@ impl PickerDelegate for SidebarRecentProjectsDelegate { })) }) .child( - Button::new("open_remote_folder", "Add Remote Project") + Button::new("open_remote_folder", "Add Remote Folder") .key_binding(KeyBinding::for_action( &OpenRemote { from_existing_connection: false, diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index 65bb8dac88234f0168bffcff185c7f298ffd3f3d..549ea0cd97a5881a69e3f379bb327df243d32459 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -45,8 +45,8 @@ use ui::{ use util::ResultExt as _; use util::path_list::PathList; use workspace::{ - AddFolderToProject, CloseWindow, FocusWorkspaceSidebar, MultiWorkspace, MultiWorkspaceEvent, - NextProject, NextThread, Open, PreviousProject, PreviousThread, ProjectGroupKey, SaveIntent, + CloseWindow, FocusWorkspaceSidebar, MultiWorkspace, MultiWorkspaceEvent, NextProject, + NextThread, Open, PreviousProject, PreviousThread, ProjectGroupKey, SaveIntent, ShowFewerThreads, ShowMoreThreads, Sidebar as WorkspaceSidebar, SidebarSide, Toast, ToggleWorkspaceSidebar, Workspace, notifications::NotificationId, sidebar_side_context_menu, }; @@ -473,6 +473,7 @@ impl Sidebar { |this, _multi_workspace, event: &MultiWorkspaceEvent, window, cx| match event { MultiWorkspaceEvent::ActiveWorkspaceChanged => { this.sync_active_entry_from_active_workspace(cx); + this.replace_archived_panel_thread(window, cx); this.observe_draft_editors(cx); this.update_entries(cx); } @@ -483,10 +484,6 @@ impl Sidebar { MultiWorkspaceEvent::WorkspaceRemoved(_) => { this.update_entries(cx); } - MultiWorkspaceEvent::ProjectGroupKeyUpdated { old_key, new_key } => { - this.move_threads_for_key_change(old_key, new_key, cx); - this.update_entries(cx); - } }, ) .detach(); @@ -671,51 +668,6 @@ impl Sidebar { } } - fn move_threads_for_key_change( - &mut self, - old_key: &ProjectGroupKey, - new_key: &ProjectGroupKey, - cx: &mut Context, - ) { - let old_main_paths = old_key.path_list(); - let new_main_paths = new_key.path_list(); - - let added_paths: Vec = new_main_paths - .paths() - .iter() - .filter(|p| !old_main_paths.paths().contains(p)) - .cloned() - .collect(); - - let removed_paths: Vec = old_main_paths - .paths() - .iter() - .filter(|p| !new_main_paths.paths().contains(p)) - .cloned() - .collect(); - - if added_paths.is_empty() && removed_paths.is_empty() { - return; - } - - let remote_connection = old_key.host(); - ThreadMetadataStore::global(cx).update(cx, |store, store_cx| { - store.change_worktree_paths_by_main( - old_main_paths, - remote_connection.as_ref(), - |paths| { - for path in &added_paths { - paths.add_path(path, path); - } - for path in &removed_paths { - paths.remove_main_path(path); - } - }, - store_cx, - ); - }); - } - fn move_thread_paths( &mut self, project: &Entity, @@ -802,6 +754,29 @@ impl Sidebar { } } + /// When switching workspaces, the active panel may still be showing + /// a thread that was archived from a different workspace. In that + /// case, create a fresh draft so the panel has valid content and + /// `active_entry` can point at it. + fn replace_archived_panel_thread(&mut self, window: &mut Window, cx: &mut Context) { + let Some(workspace) = self.active_workspace(cx) else { + return; + }; + let Some(panel) = workspace.read(cx).panel::(cx) else { + return; + }; + let Some(thread_id) = panel.read(cx).active_thread_id(cx) else { + return; + }; + let is_archived = ThreadMetadataStore::global(cx) + .read(cx) + .entry(thread_id) + .is_some_and(|m| m.archived); + if is_archived { + self.create_new_thread(&workspace, window, cx); + } + } + /// Syncs `active_entry` from the agent panel's current state. /// Called from `ActiveViewChanged` — the panel has settled into its /// new view, so we can safely read it without race conditions. @@ -846,14 +821,20 @@ impl Sidebar { } if let Some(thread_id) = panel.active_thread_id(cx) { - let session_id = panel - .active_agent_thread(cx) - .map(|thread| thread.read(cx).session_id().clone()); - self.active_entry = Some(ActiveEntry { - thread_id, - session_id, - workspace: active_workspace, - }); + let is_archived = ThreadMetadataStore::global(cx) + .read(cx) + .entry(thread_id) + .is_some_and(|m| m.archived); + if !is_archived { + let session_id = panel + .active_agent_thread(cx) + .map(|thread| thread.read(cx).session_id().clone()); + self.active_entry = Some(ActiveEntry { + thread_id, + session_id, + workspace: active_workspace, + }); + } } false @@ -1860,11 +1841,39 @@ impl Sidebar { id_prefix: &str, project_group_key: &ProjectGroupKey, cx: &mut Context, - ) -> impl IntoElement { + ) -> AnyElement { let multi_workspace = self.multi_workspace.clone(); - let this = cx.weak_entity(); let project_group_key = project_group_key.clone(); + let show_menu = multi_workspace + .read_with(cx, |mw, _| { + project_group_key.host().is_none() && mw.project_group_keys().len() >= 2 + }) + .unwrap_or(false); + + if !show_menu { + return IconButton::new( + SharedString::from(format!("{id_prefix}-close-project-{ix}")), + IconName::Close, + ) + .icon_size(IconSize::Small) + .tooltip(Tooltip::text("Remove Project")) + .on_click(cx.listener({ + move |_, _, window, cx| { + multi_workspace + .update(cx, |multi_workspace, cx| { + multi_workspace + .remove_project_group(&project_group_key, window, cx) + .detach_and_log_err(cx); + }) + .ok(); + } + })) + .into_any_element(); + } + + let this = cx.weak_entity(); + PopoverMenu::new(format!("{id_prefix}project-header-menu-{ix}")) .trigger( IconButton::new( @@ -1889,108 +1898,44 @@ impl Sidebar { let multi_workspace = multi_workspace.clone(); let project_group_key = project_group_key.clone(); - let has_multiple_projects = multi_workspace - .read_with(cx, |mw, _| mw.project_group_keys().len() >= 2) - .unwrap_or(false); - let menu = ContextMenu::build_persistent(window, cx, move |menu, _window, menu_cx| { let weak_menu = menu_cx.weak_entity(); - let mut menu = menu - .header("Project Folders") - .end_slot_action(Box::new(menu::EndSlot)); - for path in project_group_key.path_list().paths() { - let Some(name) = path.file_name() else { - continue; - }; - let name: SharedString = name.to_string_lossy().into_owned().into(); - let path = path.clone(); - let project_group_key = project_group_key.clone(); - let multi_workspace = multi_workspace.clone(); - let weak_menu = weak_menu.clone(); - menu = menu.entry_with_end_slot_on_hover( - name.clone(), - None, - |_, _| {}, - IconName::Close, - "Remove Folder".into(), - move |_window, cx| { - multi_workspace - .update(cx, |multi_workspace, cx| { - multi_workspace.remove_folder_from_project_group( - &project_group_key, - &path, - cx, - ); - }) - .ok(); - weak_menu.update(cx, |_, cx| cx.emit(DismissEvent)).ok(); - }, - ); - } - - let menu = menu.separator().entry( - "Add Folder to Project", - Some(Box::new(AddFolderToProject)), + let menu = menu.entry( + "Open Project in New Window", + Some(Box::new(workspace::MoveProjectToNewWindow)), { let project_group_key = project_group_key.clone(); let multi_workspace = multi_workspace.clone(); - let weak_menu = weak_menu.clone(); move |window, cx| { multi_workspace .update(cx, |multi_workspace, cx| { - multi_workspace.prompt_to_add_folders_to_project_group( - project_group_key.clone(), - window, - cx, - ); + multi_workspace + .open_project_group_in_new_window( + &project_group_key, + window, + cx, + ) + .detach_and_log_err(cx); }) .ok(); - weak_menu.update(cx, |_, cx| cx.emit(DismissEvent)).ok(); } }, ); - let menu = if project_group_key.host().is_none() && has_multiple_projects { - menu.entry( - "Open Project in New Window", - Some(Box::new(workspace::MoveProjectToNewWindow)), - { - let project_group_key = project_group_key.clone(); - let multi_workspace = multi_workspace.clone(); - move |window, cx| { - multi_workspace - .update(cx, |multi_workspace, cx| { - multi_workspace - .open_project_group_in_new_window( - &project_group_key, - window, - cx, - ) - .detach_and_log_err(cx); - }) - .ok(); - } - }, - ) - } else { - menu - }; - let project_group_key = project_group_key.clone(); let multi_workspace = multi_workspace.clone(); - menu.separator() - .entry("Remove Project", None, move |window, cx| { - multi_workspace - .update(cx, |multi_workspace, cx| { - multi_workspace - .remove_project_group(&project_group_key, window, cx) - .detach_and_log_err(cx); - }) - .ok(); - weak_menu.update(cx, |_, cx| cx.emit(DismissEvent)).ok(); - }) + menu.entry("Remove Project", None, move |window, cx| { + multi_workspace + .update(cx, |multi_workspace, cx| { + multi_workspace + .remove_project_group(&project_group_key, window, cx) + .detach_and_log_err(cx); + }) + .ok(); + weak_menu.update(cx, |_, cx| cx.emit(DismissEvent)).ok(); + }) }); let this = this.clone(); @@ -2011,6 +1956,7 @@ impl Sidebar { x: px(0.), y: px(1.), }) + .into_any_element() } fn render_sticky_header( diff --git a/crates/sidebar/src/sidebar_tests.rs b/crates/sidebar/src/sidebar_tests.rs index c6805cc98b7bbc2fcc82961eb723c3d8c80592f6..53bc0d8c0eaf7d52606cc075cfaae6396c7d5f36 100644 --- a/crates/sidebar/src/sidebar_tests.rs +++ b/crates/sidebar/src/sidebar_tests.rs @@ -880,97 +880,6 @@ async fn test_collapse_state_survives_worktree_key_change(cx: &mut TestAppContex ); } -#[gpui::test] -async fn test_adding_folder_to_non_backed_group_migrates_threads(cx: &mut TestAppContext) { - use workspace::ProjectGroup; - // When a project group has no backing workspace (e.g. the workspace was - // closed but the group and its threads remain), adding a folder via - // `add_folders_to_project_group` should still migrate thread metadata - // to the new key and cause the sidebar to rerender. - let (_fs, project) = - init_multi_project_test(&["/active-project", "/orphan-a", "/orphan-b"], cx).await; - let (multi_workspace, cx) = - cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); - let sidebar = setup_sidebar(&multi_workspace, cx); - - // Insert a standalone project group for [/orphan-a] with no backing - // workspace — simulating a group that persisted after its workspace - // was closed. - let group_key = ProjectGroupKey::new(None, PathList::new(&[PathBuf::from("/orphan-a")])); - multi_workspace.update(cx, |mw, _cx| { - mw.test_add_project_group(ProjectGroup { - key: group_key.clone(), - workspaces: Vec::new(), - expanded: true, - visible_thread_count: None, - }); - }); - - // Verify the group has no backing workspaces. - multi_workspace.read_with(cx, |mw, cx| { - let group = mw - .project_groups(cx) - .into_iter() - .find(|g| g.key == group_key) - .expect("group should exist"); - assert!( - group.workspaces.is_empty(), - "group should have no backing workspaces" - ); - }); - - // Save threads directly into the metadata store under [/orphan-a]. - save_thread_metadata_with_main_paths( - "t-1", - "Thread One", - PathList::new(&[PathBuf::from("/orphan-a")]), - PathList::new(&[PathBuf::from("/orphan-a")]), - chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), - cx, - ); - save_thread_metadata_with_main_paths( - "t-2", - "Thread Two", - PathList::new(&[PathBuf::from("/orphan-a")]), - PathList::new(&[PathBuf::from("/orphan-a")]), - chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 1).unwrap(), - cx, - ); - sidebar.update_in(cx, |sidebar, _window, cx| sidebar.update_entries(cx)); - cx.run_until_parked(); - - // Verify threads show under the standalone group. - assert_eq!( - visible_entries_as_strings(&sidebar, cx), - vec![ - "v [active-project]", - "v [orphan-a]", - " Thread Two", - " Thread One", - ] - ); - - // Add /orphan-b to the non-backed group. - multi_workspace.update(cx, |mw, cx| { - mw.add_folders_to_project_group(&group_key, vec![PathBuf::from("/orphan-b")], cx); - }); - cx.run_until_parked(); - - sidebar.update_in(cx, |sidebar, _window, cx| sidebar.update_entries(cx)); - cx.run_until_parked(); - - // Threads should now appear under the combined key. - assert_eq!( - visible_entries_as_strings(&sidebar, cx), - vec![ - "v [active-project]", - "v [orphan-a, orphan-b]", - " Thread Two", - " Thread One", - ] - ); -} - #[gpui::test] async fn test_visible_entries_as_strings(cx: &mut TestAppContext) { use workspace::ProjectGroup; @@ -3017,94 +2926,6 @@ async fn test_new_thread_button_works_after_adding_folder(cx: &mut TestAppContex ); }); } -#[gpui::test] -async fn test_group_level_folder_add_syncs_siblings_but_individual_add_splits( - cx: &mut TestAppContext, -) { - // Group-level operations (via the "..." menu) should keep all workspaces - // in the group in sync. Individual worktree additions should let a - // workspace diverge from its group. - init_test(cx); - let fs = FakeFs::new(cx.executor()); - fs.insert_tree("/project-a", serde_json::json!({ "src": {} })) - .await; - fs.insert_tree("/project-b", serde_json::json!({ "src": {} })) - .await; - fs.insert_tree("/project-c", serde_json::json!({ "src": {} })) - .await; - cx.update(|cx| ::set_global(fs.clone(), cx)); - - let project_a = project::Project::test(fs.clone(), [Path::new("/project-a")], cx).await; - let (multi_workspace, cx) = - cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a.clone(), window, cx)); - let _sidebar = setup_sidebar(&multi_workspace, cx); - - // Add a second workspace in the same group by adding it with the same - // project so they share a project group key. - let project_a2 = project::Project::test(fs.clone(), [Path::new("/project-a")], cx).await; - multi_workspace.update_in(cx, |mw, window, cx| { - mw.test_add_workspace(project_a2.clone(), window, cx); - }); - cx.run_until_parked(); - - // Both workspaces should be in the same group with key [/project-a]. - multi_workspace.read_with(cx, |mw, _cx| { - assert_eq!(mw.workspaces().count(), 2); - assert_eq!(mw.project_group_keys().len(), 1); - }); - - // --- Group-level add: add /project-b via the group API --- - let group_key = multi_workspace.read_with(cx, |mw, _cx| mw.project_group_keys()[0].clone()); - multi_workspace.update(cx, |mw, cx| { - mw.add_folders_to_project_group(&group_key, vec![PathBuf::from("/project-b")], cx); - }); - cx.run_until_parked(); - - // Both workspaces should now have /project-b as a worktree. - multi_workspace.read_with(cx, |mw, cx| { - for workspace in mw.workspaces() { - let paths = workspace.read(cx).root_paths(cx); - assert!( - paths.iter().any(|p| p.ends_with("project-b")), - "group-level add should propagate /project-b to all siblings, got {:?}", - paths, - ); - } - }); - - // --- Individual add: add /project-c directly to one workspace --- - let first_workspace = - multi_workspace.read_with(cx, |mw, _cx| mw.workspaces().next().unwrap().clone()); - let first_project = first_workspace.read_with(cx, |ws, _cx| ws.project().clone()); - first_project - .update(cx, |project, cx| { - project.find_or_create_worktree("/project-c", true, cx) - }) - .await - .expect("should add worktree"); - cx.run_until_parked(); - - // The first workspace should now have /project-c but the second should not. - let second_workspace = - multi_workspace.read_with(cx, |mw, _cx| mw.workspaces().nth(1).unwrap().clone()); - first_workspace.read_with(cx, |ws, cx| { - let paths = ws.root_paths(cx); - assert!( - paths.iter().any(|p| p.ends_with("project-c")), - "individual add should give /project-c to this workspace, got {:?}", - paths, - ); - }); - second_workspace.read_with(cx, |ws, cx| { - let paths = ws.root_paths(cx); - assert!( - !paths.iter().any(|p| p.ends_with("project-c")), - "individual add should NOT propagate /project-c to sibling, got {:?}", - paths, - ); - }); -} - #[gpui::test] async fn test_draft_title_updates_from_editor_text(cx: &mut TestAppContext) { let project = init_test_project_with_agent_panel("/my-project", cx).await; @@ -8654,8 +8475,6 @@ async fn test_non_archive_thread_paths_migrate_on_worktree_add_and_remove(cx: &m }); // Add a second worktree to the project. - // TODO: Should there be different behavior for calling Project::find_or_create_worktree, - // or MultiWorkspace::add_folders_to_project_group? project .update(cx, |project, cx| { project.find_or_create_worktree("/project-b", true, cx) @@ -8738,12 +8557,12 @@ async fn test_non_archive_thread_paths_migrate_on_worktree_add_and_remove(cx: &m } #[gpui::test] -async fn test_worktree_add_only_migrates_threads_for_same_folder_paths(cx: &mut TestAppContext) { +async fn test_worktree_add_only_regroups_threads_for_changed_workspace(cx: &mut TestAppContext) { // When two workspaces share the same project group (same main path) // but have different folder paths (main repo vs linked worktree), - // adding a worktree to the main workspace should only migrate threads - // whose folder paths match that workspace — not the linked worktree's - // threads. + // adding a worktree to the main workspace should regroup only that + // workspace and its threads into the new project group. Threads for the + // linked worktree workspace should remain under the original group. agent_ui::test_support::init_test(cx); cx.update(|cx| { cx.set_global(agent_ui::MaxIdleRetainedThreads(1)); @@ -8787,7 +8606,7 @@ async fn test_worktree_add_only_migrates_threads_for_same_folder_paths(cx: &mut let (multi_workspace, cx) = cx.add_window_view(|window, cx| MultiWorkspace::test_new(main_project.clone(), window, cx)); - let _sidebar = setup_sidebar(&multi_workspace, cx); + let sidebar = setup_sidebar(&multi_workspace, cx); multi_workspace.update_in(cx, |mw, window, cx| { mw.test_add_workspace(worktree_project.clone(), window, cx); }); @@ -8817,7 +8636,8 @@ async fn test_worktree_add_only_migrates_threads_for_same_folder_paths(cx: &mut let folder_paths_main = PathList::new(&[PathBuf::from("/project")]); let folder_paths_wt = PathList::new(&[PathBuf::from("/wt-feature")]); - // Sanity-check: each thread is indexed under its own folder paths. + // Sanity-check: each thread is indexed under its own folder paths, but + // both appear under the shared sidebar group keyed by the main worktree. cx.update(|_window, cx| { let store = ThreadMetadataStore::global(cx).read(cx); assert_eq!( @@ -8831,6 +8651,16 @@ async fn test_worktree_add_only_migrates_threads_for_same_folder_paths(cx: &mut "one thread under [/wt-feature]" ); }); + sidebar.update_in(cx, |sidebar, _window, cx| sidebar.update_entries(cx)); + cx.run_until_parked(); + assert_eq!( + visible_entries_as_strings(&sidebar, cx), + vec![ + "v [project]", + " Worktree Thread {wt-feature}", + " Main Thread", + ] + ); // Add /project-b to the main project only. main_project @@ -8841,8 +8671,9 @@ async fn test_worktree_add_only_migrates_threads_for_same_folder_paths(cx: &mut .expect("should add worktree"); cx.run_until_parked(); - // Main Thread (folder paths [/project]) should have migrated to - // [/project, /project-b]. Worktree Thread should be unchanged. + // Main Thread (folder paths [/project]) should be regrouped to + // [/project, /project-b]. Worktree Thread should remain under the + // original [/project] group. let folder_paths_main_b = PathList::new(&[PathBuf::from("/project"), PathBuf::from("/project-b")]); cx.update(|_window, cx| { @@ -8863,6 +8694,18 @@ async fn test_worktree_add_only_migrates_threads_for_same_folder_paths(cx: &mut "worktree thread should remain unchanged under [/wt-feature]" ); }); + + sidebar.update_in(cx, |sidebar, _window, cx| sidebar.update_entries(cx)); + cx.run_until_parked(); + assert_eq!( + visible_entries_as_strings(&sidebar, cx), + vec![ + "v [project]", + " Worktree Thread {wt-feature}", + "v [project, project-b]", + " Main Thread", + ] + ); } #[gpui::test] diff --git a/crates/workspace/src/multi_workspace.rs b/crates/workspace/src/multi_workspace.rs index 67e6e71ffd9f7d639973541db69ff6d37cdbb802..80e9cbd6ebdcbdf5489b07e91418363b236071f4 100644 --- a/crates/workspace/src/multi_workspace.rs +++ b/crates/workspace/src/multi_workspace.rs @@ -1,20 +1,19 @@ use anyhow::Result; use fs::Fs; -use gpui::PathPromptOptions; use gpui::{ AnyView, App, Context, DragMoveEvent, Entity, EntityId, EventEmitter, FocusHandle, Focusable, ManagedView, MouseButton, Pixels, Render, Subscription, Task, Tiling, Window, WindowId, actions, deferred, px, }; pub use project::ProjectGroupKey; -use project::{DirectoryLister, DisableAiSettings, Project}; +use project::{DisableAiSettings, Project}; use release_channel::ReleaseChannel; use remote::RemoteConnectionOptions; use settings::Settings; pub use settings::SidebarSide; use std::future::Future; -use std::path::Path; + use std::path::PathBuf; use ui::prelude::*; use util::ResultExt; @@ -106,10 +105,6 @@ pub enum MultiWorkspaceEvent { ActiveWorkspaceChanged, WorkspaceAdded(Entity), WorkspaceRemoved(EntityId), - ProjectGroupKeyUpdated { - old_key: ProjectGroupKey, - new_key: ProjectGroupKey, - }, } pub enum SidebarEvent { @@ -584,8 +579,7 @@ impl MultiWorkspace { return; } - // Re-key the group without emitting ProjectGroupKeyUpdated — - // the Project already emitted WorktreePathsChanged which the + // The Project already emitted WorktreePathsChanged which the // sidebar handles for thread migration. self.rekey_project_group(old_key, &new_key, cx); self.serialize(cx); @@ -685,25 +679,6 @@ impl MultiWorkspace { } } - /// Re-keys a project group and emits `ProjectGroupKeyUpdated` so - /// the sidebar can migrate thread metadata. Used for direct group - /// manipulation (add/remove folder) where no Project event fires. - fn update_project_group_key( - &mut self, - old_key: &ProjectGroupKey, - new_key: &ProjectGroupKey, - cx: &mut Context, - ) { - self.rekey_project_group(old_key, new_key, cx); - - if old_key != new_key && !new_key.path_list().paths().is_empty() { - cx.emit(MultiWorkspaceEvent::ProjectGroupKeyUpdated { - old_key: old_key.clone(), - new_key: new_key.clone(), - }); - } - } - pub(crate) fn retain_workspace( &mut self, workspace: Entity, @@ -855,126 +830,6 @@ impl MultiWorkspace { }) } - pub fn remove_folder_from_project_group( - &mut self, - group_key: &ProjectGroupKey, - path: &Path, - cx: &mut Context, - ) { - let workspaces = self - .workspaces_for_project_group(group_key, cx) - .unwrap_or_default(); - - let Some(group) = self - .project_groups - .iter() - .find(|group| group.key == *group_key) - else { - return; - }; - - let new_path_list = group.key.path_list().without_path(path); - if new_path_list.is_empty() { - return; - } - - let new_key = ProjectGroupKey::new(group.key.host(), new_path_list); - self.update_project_group_key(group_key, &new_key, cx); - - for workspace in workspaces { - let project = workspace.read(cx).project().clone(); - project.update(cx, |project, cx| { - project.remove_worktree_for_main_worktree_path(path, cx); - }); - } - - self.serialize(cx); - cx.notify(); - } - - pub fn prompt_to_add_folders_to_project_group( - &mut self, - group_key: ProjectGroupKey, - window: &mut Window, - cx: &mut Context, - ) { - let paths = self.workspace().update(cx, |workspace, cx| { - workspace.prompt_for_open_path( - PathPromptOptions { - files: false, - directories: true, - multiple: true, - prompt: None, - }, - DirectoryLister::Project(workspace.project().clone()), - window, - cx, - ) - }); - - cx.spawn_in(window, async move |this, cx| { - if let Some(new_paths) = paths.await.ok().flatten() { - if !new_paths.is_empty() { - this.update(cx, |multi_workspace, cx| { - multi_workspace.add_folders_to_project_group(&group_key, new_paths, cx); - })?; - } - } - anyhow::Ok(()) - }) - .detach_and_log_err(cx); - } - - pub fn add_folders_to_project_group( - &mut self, - group_key: &ProjectGroupKey, - new_paths: Vec, - cx: &mut Context, - ) { - let workspaces = self - .workspaces_for_project_group(group_key, cx) - .unwrap_or_default(); - - let Some(group) = self - .project_groups - .iter() - .find(|group| group.key == *group_key) - else { - return; - }; - - let existing_paths = group.key.path_list().paths(); - let new_paths: Vec = new_paths - .into_iter() - .filter(|p| !existing_paths.contains(p)) - .collect(); - - if new_paths.is_empty() { - return; - } - - let mut all_paths: Vec = existing_paths.to_vec(); - all_paths.extend(new_paths.iter().cloned()); - let new_path_list = PathList::new(&all_paths); - let new_key = ProjectGroupKey::new(group.key.host(), new_path_list); - - self.update_project_group_key(group_key, &new_key, cx); - - for workspace in workspaces { - let project = workspace.read(cx).project().clone(); - for path in &new_paths { - project - .update(cx, |project, cx| { - project.find_or_create_worktree(path, true, cx) - }) - .detach_and_log_err(cx); - } - } - - self.serialize(cx); - cx.notify(); - } - pub fn remove_project_group( &mut self, group_key: &ProjectGroupKey, diff --git a/crates/workspace/src/multi_workspace_tests.rs b/crates/workspace/src/multi_workspace_tests.rs index 1972842fb352bec66579e3bac736a36191664fe8..4df5bcfa0a09dbebddd2e2da5081f17a50c000c9 100644 --- a/crates/workspace/src/multi_workspace_tests.rs +++ b/crates/workspace/src/multi_workspace_tests.rs @@ -291,73 +291,6 @@ async fn test_project_group_keys_duplicate_not_added(cx: &mut TestAppContext) { }); } -#[gpui::test] -async fn test_groups_with_same_paths_merge(cx: &mut TestAppContext) { - init_test(cx); - let fs = FakeFs::new(cx.executor()); - fs.insert_tree("/a", json!({ "file.txt": "" })).await; - fs.insert_tree("/b", json!({ "file.txt": "" })).await; - let project_a = Project::test(fs.clone(), ["/a".as_ref()], cx).await; - let project_b = Project::test(fs.clone(), ["/b".as_ref()], cx).await; - - let (multi_workspace, cx) = - cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a, window, cx)); - - // Open the sidebar so workspaces get grouped. - multi_workspace.update(cx, |mw, cx| { - mw.open_sidebar(cx); - }); - cx.run_until_parked(); - - // Add a second workspace, creating group_b with path [/b]. - let group_a_key = multi_workspace.update_in(cx, |mw, window, cx| { - let group_a_key = mw.project_groups(cx)[0].key.clone(); - mw.test_add_workspace(project_b, window, cx); - group_a_key - }); - cx.run_until_parked(); - - // Now add /b to group_a so it has [/a, /b]. - multi_workspace.update(cx, |mw, cx| { - mw.add_folders_to_project_group(&group_a_key, vec!["/b".into()], cx); - }); - cx.run_until_parked(); - - // Verify we have two groups. - multi_workspace.read_with(cx, |mw, cx| { - assert_eq!( - mw.project_groups(cx).len(), - 2, - "should have two groups before the merge" - ); - }); - - // After adding /b, group_a's key changed. Get the updated key. - let group_a_key_updated = multi_workspace.read_with(cx, |mw, cx| { - mw.project_groups(cx) - .iter() - .find(|g| g.key.path_list().paths().contains(&PathBuf::from("/a"))) - .unwrap() - .key - .clone() - }); - - // Remove /a from group_a, making its key [/b] — same as group_b. - multi_workspace.update(cx, |mw, cx| { - mw.remove_folder_from_project_group(&group_a_key_updated, Path::new("/a"), cx); - }); - cx.run_until_parked(); - - // The two groups now have identical keys [/b] and should have been merged. - multi_workspace.read_with(cx, |mw, cx| { - assert_eq!( - mw.project_groups(cx).len(), - 1, - "groups with identical paths should be merged into one" - ); - }); -} - #[gpui::test] async fn test_adding_worktree_updates_project_group_key(cx: &mut TestAppContext) { init_test(cx);