From 6286b7c5fbd56489f7f81b071516a204df51b22f Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 8 Apr 2026 02:21:39 -0700 Subject: [PATCH] Fix workspace removal, and chase down all the ramifications (#53366) This PR adjusts how project group removal works in the sidebar to be project group aware. Removal is now a batched operation, and supports a contextually appropriate fallback active workspace fallback. In the process of removing this, I had to chase down and fix other broken code: - Removes the "move to new window" actions - Changed the sidebar to store things in "most recently added" order - The recent project UI was still using workspaces, instead of project groups - Adjusted the archive command to use the new APIs to remove the workspace associated with that worktree (cc: @rtfeldman) - The property tests where still using indexes and workspaces instead of project groups Self-Review Checklist: - [ ] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Closes #ISSUE Release Notes: - N/A --- crates/agent_ui/src/agent_panel.rs | 8 + crates/recent_projects/src/recent_projects.rs | 416 +++------ .../src/sidebar_recent_projects.rs | 16 +- crates/sidebar/src/sidebar.rs | 568 +++++++------ crates/sidebar/src/sidebar_tests.rs | 788 ++++++++++++++---- crates/title_bar/src/title_bar.rs | 26 +- crates/workspace/src/multi_workspace.rs | 348 ++++---- crates/workspace/src/multi_workspace_tests.rs | 16 +- crates/workspace/src/persistence.rs | 131 ++- crates/workspace/src/workspace.rs | 116 ++- crates/zed/src/zed.rs | 2 +- 11 files changed, 1490 insertions(+), 945 deletions(-) diff --git a/crates/agent_ui/src/agent_panel.rs b/crates/agent_ui/src/agent_panel.rs index eba76d2a59f1e5848dd33744b9ed8f5a1ee6aa06..52336871d01d5200f4d2a6799bb969b78fde6696 100644 --- a/crates/agent_ui/src/agent_panel.rs +++ b/crates/agent_ui/src/agent_panel.rs @@ -1294,6 +1294,14 @@ impl AgentPanel { .unwrap_or(false) } + /// Reset the panel to the uninitialized state, clearing any active + /// thread without creating a new draft. Running threads are retained + /// in the background. The sidebar suppresses the uninitialized state + /// so no "Draft" entry appears. + pub fn clear_active_thread(&mut self, window: &mut Window, cx: &mut Context) { + self.set_active_view(ActiveView::Uninitialized, false, window, cx); + } + pub fn new_thread(&mut self, _action: &NewThread, window: &mut Window, cx: &mut Context) { self.reset_start_thread_in_to_default(cx); self.external_thread(None, None, None, None, None, true, window, cx); diff --git a/crates/recent_projects/src/recent_projects.rs b/crates/recent_projects/src/recent_projects.rs index 57754dadec20146cb1f21039266de88a0bd5da9f..9bf496817103246abece6891ead8fd32196cef3b 100644 --- a/crates/recent_projects/src/recent_projects.rs +++ b/crates/recent_projects/src/recent_projects.rs @@ -6,7 +6,6 @@ pub mod sidebar_recent_projects; mod ssh_config; use std::{ - collections::HashSet, path::{Path, PathBuf}, sync::Arc, }; @@ -33,7 +32,7 @@ use picker::{ Picker, PickerDelegate, highlighted_match_with_paths::{HighlightedMatch, HighlightedMatchWithPaths}, }; -use project::{Worktree, git_store::Repository}; +use project::{ProjectGroupKey, Worktree, git_store::Repository}; pub use remote_connections::RemoteSettings; pub use remote_servers::RemoteServerProjects; use settings::{Settings, WorktreeId}; @@ -79,7 +78,7 @@ struct OpenFolderEntry { enum ProjectPickerEntry { Header(SharedString), OpenFolder { index: usize, positions: Vec }, - OpenProject(StringMatch), + ProjectGroup(StringMatch), RecentProject(StringMatch), } @@ -355,10 +354,8 @@ pub fn init(cx: &mut App) { cx.defer(move |cx| { multi_workspace .update(cx, |multi_workspace, window, cx| { - let sibling_workspace_ids: HashSet = multi_workspace - .workspaces() - .filter_map(|ws| ws.read(cx).database_id()) - .collect(); + let window_project_groups: Vec = + multi_workspace.project_group_keys().cloned().collect(); let workspace = multi_workspace.workspace().clone(); workspace.update(cx, |workspace, cx| { @@ -369,7 +366,7 @@ pub fn init(cx: &mut App) { RecentProjects::open( workspace, create_new_window, - sibling_workspace_ids, + window_project_groups, window, focus_handle, cx, @@ -394,7 +391,7 @@ pub fn init(cx: &mut App) { RecentProjects::open( workspace, create_new_window, - HashSet::new(), + Vec::new(), window, focus_handle, cx, @@ -611,7 +608,7 @@ impl RecentProjects { pub fn open( workspace: &mut Workspace, create_new_window: bool, - sibling_workspace_ids: HashSet, + window_project_groups: Vec, window: &mut Window, focus_handle: FocusHandle, cx: &mut Context, @@ -627,7 +624,7 @@ impl RecentProjects { create_new_window, focus_handle, open_folders, - sibling_workspace_ids, + window_project_groups, project_connection_options, ProjectPickerStyle::Modal, ); @@ -638,7 +635,7 @@ impl RecentProjects { pub fn popover( workspace: WeakEntity, - sibling_workspace_ids: HashSet, + window_project_groups: Vec, create_new_window: bool, focus_handle: FocusHandle, window: &mut Window, @@ -662,7 +659,7 @@ impl RecentProjects { create_new_window, focus_handle, open_folders, - sibling_workspace_ids, + window_project_groups, project_connection_options, ProjectPickerStyle::Popover, ); @@ -715,17 +712,17 @@ impl RecentProjects { picker.update_matches(query, window, cx); } } - Some(ProjectPickerEntry::OpenProject(hit)) => { - if let Some((workspace_id, ..)) = - picker.delegate.workspaces.get(hit.candidate_id) + Some(ProjectPickerEntry::ProjectGroup(hit)) => { + if let Some(key) = picker + .delegate + .window_project_groups + .get(hit.candidate_id) + .cloned() { - let workspace_id = *workspace_id; - if picker.delegate.is_current_workspace(workspace_id, cx) { + if picker.delegate.is_active_project_group(&key, cx) { return; } - picker - .delegate - .remove_sibling_workspace(workspace_id, window, cx); + picker.delegate.remove_project_group(key, window, cx); let query = picker.query(cx); picker.update_matches(query, window, cx); } @@ -788,7 +785,7 @@ impl Render for RecentProjects { pub struct RecentProjectsDelegate { workspace: WeakEntity, open_folders: Vec, - sibling_workspace_ids: HashSet, + window_project_groups: Vec, workspaces: Vec<( WorkspaceId, SerializedWorkspaceLocation, @@ -814,7 +811,7 @@ impl RecentProjectsDelegate { create_new_window: bool, focus_handle: FocusHandle, open_folders: Vec, - sibling_workspace_ids: HashSet, + window_project_groups: Vec, project_connection_options: Option, style: ProjectPickerStyle, ) -> Self { @@ -822,7 +819,7 @@ impl RecentProjectsDelegate { Self { workspace, open_folders, - sibling_workspace_ids, + window_project_groups, workspaces: Vec::new(), filtered_entries: Vec::new(), selected_index: 0, @@ -901,7 +898,7 @@ impl PickerDelegate for RecentProjectsDelegate { self.filtered_entries.get(ix), Some( ProjectPickerEntry::OpenFolder { .. } - | ProjectPickerEntry::OpenProject(_) + | ProjectPickerEntry::ProjectGroup(_) | ProjectPickerEntry::RecentProject(_) ) ) @@ -938,13 +935,13 @@ impl PickerDelegate for RecentProjectsDelegate { )) }; - let sibling_candidates: Vec<_> = self - .workspaces + let project_group_candidates: Vec<_> = self + .window_project_groups .iter() .enumerate() - .filter(|(_, (id, _, _, _))| self.sibling_workspace_ids.contains(id)) - .map(|(id, (_, _, paths, _))| { - let combined_string = paths + .map(|(id, key)| { + let combined_string = key + .path_list() .ordered_paths() .map(|path| path.compact().to_string_lossy().into_owned()) .collect::>() @@ -953,8 +950,8 @@ impl PickerDelegate for RecentProjectsDelegate { }) .collect(); - let mut sibling_matches = smol::block_on(fuzzy::match_strings( - &sibling_candidates, + let mut project_group_matches = smol::block_on(fuzzy::match_strings( + &project_group_candidates, query, smart_case, true, @@ -962,7 +959,7 @@ impl PickerDelegate for RecentProjectsDelegate { &Default::default(), cx.background_executor().clone(), )); - sibling_matches.sort_unstable_by(|a, b| { + project_group_matches.sort_unstable_by(|a, b| { b.score .partial_cmp(&a.score) .unwrap_or(std::cmp::Ordering::Equal) @@ -1020,29 +1017,27 @@ impl PickerDelegate for RecentProjectsDelegate { } } - let has_siblings_to_show = if is_empty_query { - !sibling_candidates.is_empty() + let has_projects_to_show = if is_empty_query { + !project_group_candidates.is_empty() } else { - !sibling_matches.is_empty() + !project_group_matches.is_empty() }; - if has_siblings_to_show { + if has_projects_to_show { entries.push(ProjectPickerEntry::Header("This Window".into())); if is_empty_query { - for (id, (workspace_id, _, _, _)) in self.workspaces.iter().enumerate() { - if self.sibling_workspace_ids.contains(workspace_id) { - entries.push(ProjectPickerEntry::OpenProject(StringMatch { - candidate_id: id, - score: 0.0, - positions: Vec::new(), - string: String::new(), - })); - } + for id in 0..self.window_project_groups.len() { + entries.push(ProjectPickerEntry::ProjectGroup(StringMatch { + candidate_id: id, + score: 0.0, + positions: Vec::new(), + string: String::new(), + })); } } else { - for m in sibling_matches { - entries.push(ProjectPickerEntry::OpenProject(m)); + for m in project_group_matches { + entries.push(ProjectPickerEntry::ProjectGroup(m)); } } } @@ -1101,32 +1096,23 @@ impl PickerDelegate for RecentProjectsDelegate { } cx.emit(DismissEvent); } - Some(ProjectPickerEntry::OpenProject(selected_match)) => { - let Some((workspace_id, _, _, _)) = - self.workspaces.get(selected_match.candidate_id) - else { + Some(ProjectPickerEntry::ProjectGroup(selected_match)) => { + let Some(key) = self.window_project_groups.get(selected_match.candidate_id) else { return; }; - let workspace_id = *workspace_id; - - if self.is_current_workspace(workspace_id, cx) { - cx.emit(DismissEvent); - return; - } + let path_list = key.path_list().clone(); if let Some(handle) = window.window_handle().downcast::() { cx.defer(move |cx| { - handle + if let Some(task) = handle .update(cx, |multi_workspace, window, cx| { - let workspace = multi_workspace - .workspaces() - .find(|ws| ws.read(cx).database_id() == Some(workspace_id)) - .cloned(); - if let Some(workspace) = workspace { - multi_workspace.activate(workspace, window, cx); - } + multi_workspace + .find_or_create_local_workspace(path_list, window, cx) }) - .log_err(); + .log_err() + { + task.detach_and_log_err(cx); + } }); } cx.emit(DismissEvent); @@ -1354,28 +1340,18 @@ impl PickerDelegate for RecentProjectsDelegate { .into_any_element(), ) } - ProjectPickerEntry::OpenProject(hit) => { - let (workspace_id, location, paths, _) = self.workspaces.get(hit.candidate_id)?; - let workspace_id = *workspace_id; - let is_current = self.is_current_workspace(workspace_id, cx); + ProjectPickerEntry::ProjectGroup(hit) => { + let key = self.window_project_groups.get(hit.candidate_id)?; + let is_active = self.is_active_project_group(key, cx); + let paths = key.path_list(); let ordered_paths: Vec<_> = paths .ordered_paths() .map(|p| p.compact().to_string_lossy().to_string()) .collect(); - let tooltip_path: SharedString = match &location { - SerializedWorkspaceLocation::Remote(options) => { - let host = options.display_name(); - if ordered_paths.len() == 1 { - format!("{} ({})", ordered_paths[0], host).into() - } else { - format!("{}\n({})", ordered_paths.join("\n"), host).into() - } - } - _ => ordered_paths.join("\n").into(), - }; + let tooltip_path: SharedString = ordered_paths.join("\n").into(); let mut path_start_offset = 0; - let (match_labels, paths): (Vec<_>, Vec<_>) = paths + let (match_labels, path_highlights): (Vec<_>, Vec<_>) = paths .ordered_paths() .map(|p| p.compact()) .map(|path| { @@ -1386,43 +1362,35 @@ impl PickerDelegate for RecentProjectsDelegate { }) .unzip(); - let prefix = match &location { - SerializedWorkspaceLocation::Remote(options) => { - Some(SharedString::from(options.display_name())) - } - _ => None, - }; - let highlighted_match = HighlightedMatchWithPaths { - prefix, + prefix: None, match_label: HighlightedMatch::join(match_labels.into_iter().flatten(), ", "), - paths, - active: is_current, + paths: path_highlights, + active: is_active, }; - let icon = icon_for_remote_connection(match location { - SerializedWorkspaceLocation::Local => None, - SerializedWorkspaceLocation::Remote(options) => Some(options), - }); - + let project_group_key = key.clone(); let secondary_actions = h_flex() .gap_1() - .when(!is_current, |this| { + .when(!is_active, |this| { this.child( IconButton::new("remove_open_project", IconName::Close) .icon_size(IconSize::Small) .tooltip(Tooltip::text("Remove Project from Window")) - .on_click(cx.listener(move |picker, _, window, cx| { - cx.stop_propagation(); - window.prevent_default(); - picker.delegate.remove_sibling_workspace( - workspace_id, - window, - cx, - ); - let query = picker.query(cx); - picker.update_matches(query, window, cx); - })), + .on_click({ + let project_group_key = project_group_key.clone(); + cx.listener(move |picker, _, window, cx| { + cx.stop_propagation(); + window.prevent_default(); + picker.delegate.remove_project_group( + project_group_key.clone(), + window, + cx, + ); + let query = picker.query(cx); + picker.update_matches(query, window, cx); + }) + }), ) }) .into_any_element(); @@ -1436,10 +1404,6 @@ impl PickerDelegate for RecentProjectsDelegate { h_flex() .id("open_project_info_container") .gap_3() - .flex_grow() - .when(self.has_any_non_local_projects, |this| { - this.child(Icon::new(icon).color(Color::Muted)) - }) .child({ let mut highlighted = highlighted_match; if !self.render_paths { @@ -1609,7 +1573,7 @@ impl PickerDelegate for RecentProjectsDelegate { let popover_style = matches!(self.style, ProjectPickerStyle::Popover); let is_already_open_entry = matches!( self.filtered_entries.get(self.selected_index), - Some(ProjectPickerEntry::OpenFolder { .. } | ProjectPickerEntry::OpenProject(_)) + Some(ProjectPickerEntry::OpenFolder { .. } | ProjectPickerEntry::ProjectGroup(_)) ); if popover_style { @@ -1655,11 +1619,10 @@ impl PickerDelegate for RecentProjectsDelegate { let selected_entry = self.filtered_entries.get(self.selected_index); let is_current_workspace_entry = - if let Some(ProjectPickerEntry::OpenProject(hit)) = selected_entry { - self.workspaces + if let Some(ProjectPickerEntry::ProjectGroup(hit)) = selected_entry { + self.window_project_groups .get(hit.candidate_id) - .map(|(id, ..)| self.is_current_workspace(*id, cx)) - .unwrap_or(false) + .is_some_and(|key| self.is_active_project_group(key, cx)) } else { false }; @@ -1677,7 +1640,7 @@ impl PickerDelegate for RecentProjectsDelegate { }) .into_any_element(), ), - Some(ProjectPickerEntry::OpenProject(_)) if !is_current_workspace_entry => Some( + Some(ProjectPickerEntry::ProjectGroup(_)) if !is_current_workspace_entry => Some( Button::new("remove_selected", "Remove from Window") .key_binding(KeyBinding::for_action_in( &RemoveSelected, @@ -1961,29 +1924,26 @@ impl RecentProjectsDelegate { } } - fn remove_sibling_workspace( + fn remove_project_group( &mut self, - workspace_id: WorkspaceId, + key: ProjectGroupKey, window: &mut Window, cx: &mut Context>, ) { if let Some(handle) = window.window_handle().downcast::() { + let key_for_remove = key.clone(); cx.defer(move |cx| { handle .update(cx, |multi_workspace, window, cx| { - let workspace = multi_workspace - .workspaces() - .find(|ws| ws.read(cx).database_id() == Some(workspace_id)) - .cloned(); - if let Some(workspace) = workspace { - multi_workspace.remove(&workspace, window, cx); - } + multi_workspace + .remove_project_group(&key_for_remove, window, cx) + .detach_and_log_err(cx); }) .log_err(); }); } - self.sibling_workspace_ids.remove(&workspace_id); + self.window_project_groups.retain(|k| k != &key); } fn is_current_workspace( @@ -2001,13 +1961,17 @@ impl RecentProjectsDelegate { false } - fn is_sibling_workspace( - &self, - workspace_id: WorkspaceId, - cx: &mut Context>, - ) -> bool { - self.sibling_workspace_ids.contains(&workspace_id) - && !self.is_current_workspace(workspace_id, cx) + fn is_active_project_group(&self, key: &ProjectGroupKey, cx: &App) -> bool { + if let Some(workspace) = self.workspace.upgrade() { + return workspace.read(cx).project_group_key(cx) == *key; + } + false + } + + fn is_in_current_window_groups(&self, paths: &PathList) -> bool { + self.window_project_groups + .iter() + .any(|key| key.path_list() == paths) } fn is_open_folder(&self, paths: &PathList) -> bool { @@ -2033,195 +1997,21 @@ impl RecentProjectsDelegate { cx: &mut Context>, ) -> bool { !self.is_current_workspace(workspace_id, cx) - && !self.is_sibling_workspace(workspace_id, cx) + && !self.is_in_current_window_groups(paths) && !self.is_open_folder(paths) } } #[cfg(test)] mod tests { - use std::path::PathBuf; - - use editor::Editor; - use gpui::{TestAppContext, UpdateGlobal, VisualTestContext, WindowHandle}; + use gpui::{TestAppContext, VisualTestContext}; use serde_json::json; - use settings::SettingsStore; use util::path; use workspace::{AppState, open_paths}; use super::*; - #[gpui::test] - async fn test_dirty_workspace_replaced_when_opening_recent_project(cx: &mut TestAppContext) { - let app_state = init_test(cx); - - cx.update(|cx| { - SettingsStore::update_global(cx, |store, cx| { - store.update_user_settings(cx, |settings| { - settings - .session - .get_or_insert_default() - .restore_unsaved_buffers = Some(false) - }); - }); - }); - - app_state - .fs - .as_fake() - .insert_tree( - path!("/dir"), - json!({ - "main.ts": "a" - }), - ) - .await; - app_state - .fs - .as_fake() - .insert_tree(path!("/test/path"), json!({})) - .await; - cx.update(|cx| { - open_paths( - &[PathBuf::from(path!("/dir/main.ts"))], - app_state, - workspace::OpenOptions::default(), - cx, - ) - }) - .await - .unwrap(); - assert_eq!(cx.update(|cx| cx.windows().len()), 1); - - let multi_workspace = cx.update(|cx| cx.windows()[0].downcast::().unwrap()); - multi_workspace - .update(cx, |multi_workspace, _, cx| { - multi_workspace.open_sidebar(cx); - }) - .unwrap(); - multi_workspace - .update(cx, |multi_workspace, _, cx| { - assert!(!multi_workspace.workspace().read(cx).is_edited()) - }) - .unwrap(); - - let editor = multi_workspace - .read_with(cx, |multi_workspace, cx| { - multi_workspace - .workspace() - .read(cx) - .active_item(cx) - .unwrap() - .downcast::() - .unwrap() - }) - .unwrap(); - multi_workspace - .update(cx, |_, window, cx| { - editor.update(cx, |editor, cx| editor.insert("EDIT", window, cx)); - }) - .unwrap(); - multi_workspace - .update(cx, |multi_workspace, _, cx| { - assert!( - multi_workspace.workspace().read(cx).is_edited(), - "After inserting more text into the editor without saving, we should have a dirty project" - ) - }) - .unwrap(); - - let recent_projects_picker = open_recent_projects(&multi_workspace, cx); - multi_workspace - .update(cx, |_, _, cx| { - recent_projects_picker.update(cx, |picker, cx| { - assert_eq!(picker.query(cx), ""); - let delegate = &mut picker.delegate; - delegate.set_workspaces(vec![( - WorkspaceId::default(), - SerializedWorkspaceLocation::Local, - PathList::new(&[path!("/test/path")]), - Utc::now(), - )]); - delegate.filtered_entries = - vec![ProjectPickerEntry::RecentProject(StringMatch { - candidate_id: 0, - score: 1.0, - positions: Vec::new(), - string: "fake candidate".to_string(), - })]; - }); - }) - .unwrap(); - - assert!( - !cx.has_pending_prompt(), - "Should have no pending prompt on dirty project before opening the new recent project" - ); - let dirty_workspace = multi_workspace - .read_with(cx, |multi_workspace, _cx| { - multi_workspace.workspace().clone() - }) - .unwrap(); - - cx.dispatch_action(*multi_workspace, menu::Confirm); - cx.run_until_parked(); - - // In multi-workspace mode, the dirty workspace is kept and a new one is - // opened alongside it — no save prompt needed. - assert!( - !cx.has_pending_prompt(), - "Should not prompt in multi-workspace mode — dirty workspace is kept" - ); - - multi_workspace - .update(cx, |multi_workspace, _, cx| { - assert!( - multi_workspace - .workspace() - .read(cx) - .active_modal::(cx) - .is_none(), - "Should remove the modal after selecting new recent project" - ); - - assert!( - multi_workspace.workspaces().any(|w| w == &dirty_workspace), - "The dirty workspace should still be present in multi-workspace mode" - ); - - assert!( - !multi_workspace.workspace().read(cx).is_edited(), - "The active workspace should be the freshly opened one, not dirty" - ); - }) - .unwrap(); - } - - fn open_recent_projects( - multi_workspace: &WindowHandle, - cx: &mut TestAppContext, - ) -> Entity> { - cx.dispatch_action( - (*multi_workspace).into(), - OpenRecent { - create_new_window: false, - }, - ); - multi_workspace - .update(cx, |multi_workspace, _, cx| { - multi_workspace - .workspace() - .read(cx) - .active_modal::(cx) - .unwrap() - .read(cx) - .picker - .clone() - }) - .unwrap() - } - #[gpui::test] async fn test_open_dev_container_action_with_single_config(cx: &mut TestAppContext) { let app_state = init_test(cx); diff --git a/crates/recent_projects/src/sidebar_recent_projects.rs b/crates/recent_projects/src/sidebar_recent_projects.rs index dec269c07eada3a1d6172482cb886f9ed44d784c..2c697de73fb6ad203ae6f85c89e73050a68fcbd0 100644 --- a/crates/recent_projects/src/sidebar_recent_projects.rs +++ b/crates/recent_projects/src/sidebar_recent_projects.rs @@ -1,4 +1,3 @@ -use std::collections::HashSet; use std::sync::Arc; use chrono::{DateTime, Utc}; @@ -11,6 +10,7 @@ use picker::{ Picker, PickerDelegate, highlighted_match_with_paths::{HighlightedMatch, HighlightedMatchWithPaths}, }; +use project::ProjectGroupKey; use remote::RemoteConnectionOptions; use settings::Settings; use ui::{KeyBinding, ListItem, ListItemSpacing, Tooltip, prelude::*}; @@ -33,7 +33,7 @@ pub struct SidebarRecentProjects { impl SidebarRecentProjects { pub fn popover( workspace: WeakEntity, - sibling_workspace_ids: HashSet, + window_project_groups: Vec, _focus_handle: FocusHandle, window: &mut Window, cx: &mut App, @@ -45,7 +45,7 @@ impl SidebarRecentProjects { cx.new(|cx| { let delegate = SidebarRecentProjectsDelegate { workspace, - sibling_workspace_ids, + window_project_groups, workspaces: Vec::new(), filtered_workspaces: Vec::new(), selected_index: 0, @@ -116,7 +116,7 @@ impl Render for SidebarRecentProjects { pub struct SidebarRecentProjectsDelegate { workspace: WeakEntity, - sibling_workspace_ids: HashSet, + window_project_groups: Vec, workspaces: Vec<( WorkspaceId, SerializedWorkspaceLocation, @@ -207,8 +207,12 @@ impl PickerDelegate for SidebarRecentProjectsDelegate { .workspaces .iter() .enumerate() - .filter(|(_, (id, _, _, _))| { - Some(*id) != current_workspace_id && !self.sibling_workspace_ids.contains(id) + .filter(|(_, (id, _, paths, _))| { + Some(*id) != current_workspace_id + && !self + .window_project_groups + .iter() + .any(|key| key.path_list() == paths) }) .map(|(id, (_, _, paths, _))| { let combined_string = paths diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index a9e9a0ab1b74a7aea53e380dce555d6b752da955..77d2db85f4ea1269111ad49e6b484647faeed0d2 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -43,10 +43,10 @@ use ui::{ use util::ResultExt as _; use util::path_list::{PathList, SerializedPathList}; use workspace::{ - AddFolderToProject, CloseWindow, FocusWorkspaceSidebar, MoveWorkspaceToNewWindow, - MultiWorkspace, MultiWorkspaceEvent, NextProject, NextThread, Open, PreviousProject, - PreviousThread, ShowFewerThreads, ShowMoreThreads, Sidebar as WorkspaceSidebar, SidebarSide, - ToggleWorkspaceSidebar, Workspace, WorkspaceId, sidebar_side_context_menu, + AddFolderToProject, CloseWindow, FocusWorkspaceSidebar, MultiWorkspace, MultiWorkspaceEvent, + NextProject, NextThread, Open, PreviousProject, PreviousThread, ShowFewerThreads, + ShowMoreThreads, Sidebar as WorkspaceSidebar, SidebarSide, ToggleWorkspaceSidebar, Workspace, + sidebar_side_context_menu, }; use zed_actions::OpenRecent; @@ -220,11 +220,13 @@ enum ListEntry { worktrees: Vec, }, /// A convenience row for starting a new thread. Shown when a project group - /// has no threads, or when the active workspace contains linked worktrees - /// with no threads for that specific worktree set. + /// has no threads, or when an open linked worktree workspace has no threads. + /// When `workspace` is `Some`, this entry is for a specific linked worktree + /// workspace and can be dismissed (removing that workspace). NewThread { key: project::ProjectGroupKey, worktrees: Vec, + workspace: Option>, }, } @@ -236,6 +238,48 @@ impl ListEntry { _ => None, } } + + fn reachable_workspaces<'a>( + &'a self, + multi_workspace: &'a workspace::MultiWorkspace, + cx: &'a App, + ) -> Vec> { + match self { + ListEntry::Thread(thread) => match &thread.workspace { + ThreadEntryWorkspace::Open(ws) => vec![ws.clone()], + ThreadEntryWorkspace::Closed(_) => Vec::new(), + }, + ListEntry::DraftThread { .. } => { + vec![multi_workspace.workspace().clone()] + } + ListEntry::ProjectHeader { key, .. } => { + // The header only activates the main worktree workspace + // (the one whose root paths match the group key's path list). + multi_workspace + .workspaces() + .find(|ws| PathList::new(&ws.read(cx).root_paths(cx)) == *key.path_list()) + .cloned() + .into_iter() + .collect() + } + ListEntry::NewThread { key, workspace, .. } => { + // When the NewThread entry is for a specific linked worktree + // workspace, that workspace is reachable. Otherwise fall back + // to the main worktree workspace. + if let Some(ws) = workspace { + vec![ws.clone()] + } else { + multi_workspace + .workspaces() + .find(|ws| PathList::new(&ws.read(cx).root_paths(cx)) == *key.path_list()) + .cloned() + .into_iter() + .collect() + } + } + _ => Vec::new(), + } + } } impl From for ListEntry { @@ -667,13 +711,10 @@ impl Sidebar { result } - /// Finds an open workspace whose project group key matches the given path list. + /// Finds the main worktree workspace for a project group. fn workspace_for_group(&self, path_list: &PathList, cx: &App) -> Option> { let mw = self.multi_workspace.upgrade()?; - let mw = mw.read(cx); - mw.workspaces() - .find(|ws| ws.read(cx).project_group_key(cx).path_list() == path_list) - .cloned() + mw.read(cx).workspace_for_paths(path_list, cx) } /// Opens a new workspace for a group that has no open workspaces. @@ -1065,56 +1106,51 @@ impl Sidebar { } } - // Emit a NewThread entry when: - // 1. The group has zero threads (convenient affordance). - // 2. The active workspace has linked worktrees but no threads - // for the active workspace's specific set of worktrees. + // Emit NewThread entries: + // 1. When the group has zero threads (convenient affordance). + // 2. For each open linked worktree workspace in this group + // that has no threads (makes the workspace reachable and + // dismissable). let group_has_no_threads = threads.is_empty() && !group_workspaces.is_empty(); - let active_ws_has_threadless_linked_worktrees = is_active - && !is_draft_for_group - && active_workspace.as_ref().is_some_and(|active_ws| { - let ws_path_list = workspace_path_list(active_ws, cx); + + if !is_draft_for_group && group_has_no_threads { + entries.push(ListEntry::NewThread { + key: group_key.clone(), + worktrees: Vec::new(), + workspace: None, + }); + } + + // Emit a NewThread for each open linked worktree workspace + // that has no threads. Skip the workspace if it's showing + // the active draft (it already has a DraftThread entry). + if !is_draft_for_group { + let thread_store = ThreadMetadataStore::global(cx); + for ws in &group_workspaces { + let ws_path_list = workspace_path_list(ws, cx); let has_linked_worktrees = worktree_info_from_thread_paths(&ws_path_list, &group_key) .any(|wt| wt.kind == ui::WorktreeKind::Linked); if !has_linked_worktrees { - return false; + continue; } - let thread_store = ThreadMetadataStore::global(cx); - let has_threads_for_ws = thread_store - .read(cx) - .entries_for_path(&ws_path_list) - .next() - .is_some() - || thread_store - .read(cx) + let store = thread_store.read(cx); + let has_threads = store.entries_for_path(&ws_path_list).next().is_some() + || store .entries_for_main_worktree_path(&ws_path_list) .next() .is_some(); - !has_threads_for_ws - }); - - if !is_draft_for_group - && (group_has_no_threads || active_ws_has_threadless_linked_worktrees) - { - let worktrees = if active_ws_has_threadless_linked_worktrees { - active_workspace - .as_ref() - .map(|ws| { - worktree_info_from_thread_paths( - &workspace_path_list(ws, cx), - &group_key, - ) - .collect() - }) - .unwrap_or_default() - } else { - Vec::new() - }; - entries.push(ListEntry::NewThread { - key: group_key.clone(), - worktrees, - }); + if has_threads { + continue; + } + let worktrees: Vec = + worktree_info_from_thread_paths(&ws_path_list, &group_key).collect(); + entries.push(ListEntry::NewThread { + key: group_key.clone(), + worktrees, + workspace: Some(ws.clone()), + }); + } } let total = threads.len(); @@ -1272,9 +1308,11 @@ impl Sidebar { ListEntry::DraftThread { worktrees, .. } => { self.render_draft_thread(ix, is_active, worktrees, is_selected, cx) } - ListEntry::NewThread { key, worktrees, .. } => { - self.render_new_thread(ix, key, worktrees, is_selected, cx) - } + ListEntry::NewThread { + key, + worktrees, + workspace, + } => self.render_new_thread(ix, key, worktrees, workspace.as_ref(), is_selected, cx), }; if is_group_header_after_first { @@ -1388,7 +1426,7 @@ impl Sidebar { .justify_between() .child( h_flex() - .when(!is_active, |this| this.cursor_pointer()) + .cursor_pointer() .relative() .min_w_0() .w_full() @@ -1493,10 +1531,10 @@ impl Sidebar { }, ), ) - .when(!is_active, |this| { + .map(|this| { let path_list = path_list.clone(); this.cursor_pointer() - .hover(|s| s.bg(hover_color)) + .when(!is_active, |this| this.hover(|s| s.bg(hover_color))) .tooltip(Tooltip::text("Open Workspace")) .on_click(cx.listener(move |this, _, window, cx| { if let Some(workspace) = this.workspace_for_group(&path_list, cx) { @@ -1545,7 +1583,7 @@ impl Sidebar { let multi_workspace = multi_workspace.clone(); let project_group_key = project_group_key.clone(); - let menu = ContextMenu::build_persistent(window, cx, move |menu, _window, cx| { + let menu = ContextMenu::build_persistent(window, cx, move |menu, _window, _cx| { let mut menu = menu .header("Project Folders") .end_slot_action(Box::new(menu::EndSlot)); @@ -1598,42 +1636,15 @@ impl Sidebar { }, ); - let group_count = multi_workspace - .upgrade() - .map_or(0, |mw| mw.read(cx).project_group_keys().count()); - let menu = if group_count > 1 { - let project_group_key = project_group_key.clone(); - let multi_workspace = multi_workspace.clone(); - menu.entry( - "Move to New Window", - Some(Box::new(MoveWorkspaceToNewWindow)), - move |window, cx| { - multi_workspace - .update(cx, |multi_workspace, cx| { - multi_workspace.move_project_group_to_new_window( - &project_group_key, - window, - 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, - ); + multi_workspace + .remove_project_group(&project_group_key, window, cx) + .detach_and_log_err(cx); }) .ok(); }) @@ -1968,9 +1979,12 @@ impl Sidebar { ListEntry::DraftThread { .. } => { // Already active — nothing to do. } - ListEntry::NewThread { key, .. } => { + ListEntry::NewThread { key, workspace, .. } => { let path_list = key.path_list().clone(); - if let Some(workspace) = self.workspace_for_group(&path_list, cx) { + if let Some(workspace) = workspace + .clone() + .or_else(|| self.workspace_for_group(&path_list, cx)) + { self.create_new_thread(&workspace, window, cx); } else { self.open_workspace_for_group(&path_list, window, cx); @@ -2360,114 +2374,190 @@ impl Sidebar { window: &mut Window, cx: &mut Context, ) { - ThreadMetadataStore::global(cx).update(cx, |store, cx| store.archive(session_id, cx)); + let thread_folder_paths = ThreadMetadataStore::global(cx) + .read(cx) + .entry(session_id) + .map(|m| m.folder_paths.clone()); + + // Find the neighbor thread in the sidebar (by display position). + // Look below first, then above, for the nearest thread that isn't + // the one being archived. We capture both the neighbor's metadata + // (for activation) and its workspace paths (for the workspace + // removal fallback). + let current_pos = self.contents.entries.iter().position( + |entry| matches!(entry, ListEntry::Thread(t) if &t.metadata.session_id == session_id), + ); + let neighbor = current_pos.and_then(|pos| { + self.contents.entries[pos + 1..] + .iter() + .chain(self.contents.entries[..pos].iter().rev()) + .find_map(|entry| match entry { + ListEntry::Thread(t) if t.metadata.session_id != *session_id => { + let workspace_paths = match &t.workspace { + ThreadEntryWorkspace::Open(ws) => { + PathList::new(&ws.read(cx).root_paths(cx)) + } + ThreadEntryWorkspace::Closed(paths) => paths.clone(), + }; + Some((t.metadata.clone(), workspace_paths)) + } + _ => None, + }) + }); - // If we're archiving the currently focused thread, move focus to the - // nearest thread within the same project group. We never cross group - // boundaries — if the group has no other threads, clear focus and open - // a blank new thread in the panel instead. - if self - .active_entry - .as_ref() - .is_some_and(|e| e.is_active_thread(session_id)) - { - let current_pos = self.contents.entries.iter().position(|entry| { - matches!(entry, ListEntry::Thread(t) if &t.metadata.session_id == session_id) - }); + // Check if archiving this thread would leave its worktree workspace + // with no threads, requiring workspace removal. + let workspace_to_remove = thread_folder_paths.as_ref().and_then(|folder_paths| { + if folder_paths.is_empty() { + return None; + } - // Find the workspace that owns this thread's project group by - // walking backwards to the nearest ProjectHeader and looking up - // an open workspace for that group's path_list. - let group_workspace = current_pos.and_then(|pos| { - let path_list = - self.contents.entries[..pos] - .iter() - .rev() - .find_map(|e| match e { - ListEntry::ProjectHeader { key, .. } => Some(key.path_list()), - _ => None, - })?; - self.workspace_for_group(path_list, cx) - }); + let remaining = ThreadMetadataStore::global(cx) + .read(cx) + .entries_for_path(folder_paths) + .filter(|t| t.session_id != *session_id) + .count(); + if remaining > 0 { + return None; + } - let next_thread = current_pos.and_then(|pos| { - let group_start = self.contents.entries[..pos] - .iter() - .rposition(|e| matches!(e, ListEntry::ProjectHeader { .. })) - .map_or(0, |i| i + 1); - let group_end = self.contents.entries[pos + 1..] - .iter() - .position(|e| matches!(e, ListEntry::ProjectHeader { .. })) - .map_or(self.contents.entries.len(), |i| pos + 1 + i); + let multi_workspace = self.multi_workspace.upgrade()?; + let workspace = multi_workspace + .read(cx) + .workspace_for_paths(folder_paths, cx)?; - let above = self.contents.entries[group_start..pos] - .iter() - .rev() - .find_map(|entry| { - if let ListEntry::Thread(t) = entry { - Some(t) - } else { - None - } - }); + // Don't remove the main worktree workspace — the project + // header always provides access to it. + let group_key = workspace.read(cx).project_group_key(cx); + (group_key.path_list() != folder_paths).then_some(workspace) + }); - above.or_else(|| { - self.contents.entries[pos + 1..group_end] - .iter() - .find_map(|entry| { - if let ListEntry::Thread(t) = entry { - Some(t) - } else { - None - } - }) - }) + if let Some(workspace_to_remove) = workspace_to_remove { + let multi_workspace = self.multi_workspace.upgrade().unwrap(); + let session_id = session_id.clone(); + + // For the workspace-removal fallback, use the neighbor's workspace + // paths if available, otherwise fall back to the project group key. + let fallback_paths = neighbor + .as_ref() + .map(|(_, paths)| paths.clone()) + .unwrap_or_else(|| { + workspace_to_remove + .read(cx) + .project_group_key(cx) + .path_list() + .clone() + }); + + let remove_task = multi_workspace.update(cx, |mw, cx| { + mw.remove( + [workspace_to_remove], + move |this, window, cx| { + this.find_or_create_local_workspace(fallback_paths, window, cx) + }, + window, + cx, + ) }); - if let Some(next) = next_thread { - let next_metadata = next.metadata.clone(); - // Use the thread's own workspace when it has one open (e.g. an absorbed - // linked worktree thread that appears under the main workspace's header - // but belongs to its own workspace). Loading into the wrong panel binds - // the thread to the wrong project, which corrupts its stored folder_paths - // when metadata is saved via ThreadMetadata::from_thread. - let target_workspace = match &next.workspace { - ThreadEntryWorkspace::Open(ws) => Some(ws.clone()), - ThreadEntryWorkspace::Closed(_) => group_workspace, - }; - if let Some(ref ws) = target_workspace { - self.active_entry = Some(ActiveEntry::Thread { - session_id: next_metadata.session_id.clone(), - workspace: ws.clone(), - }); - } - self.record_thread_access(&next_metadata.session_id); - - if let Some(workspace) = target_workspace { - if let Some(agent_panel) = workspace.read(cx).panel::(cx) { - agent_panel.update(cx, |panel, cx| { - panel.load_agent_thread( - Agent::from(next_metadata.agent_id.clone()), - next_metadata.session_id.clone(), - Some(next_metadata.folder_paths.clone()), - Some(next_metadata.title.clone()), - true, - window, - cx, - ); - }); - } + let neighbor_metadata = neighbor.map(|(metadata, _)| metadata); + let thread_folder_paths = thread_folder_paths.clone(); + cx.spawn_in(window, async move |this, cx| { + let removed = remove_task.await?; + if removed { + this.update_in(cx, |this, window, cx| { + this.archive_and_activate( + &session_id, + neighbor_metadata.as_ref(), + thread_folder_paths.as_ref(), + window, + cx, + ); + })?; } - } else { - if let Some(workspace) = &group_workspace { - self.active_entry = Some(ActiveEntry::Draft(workspace.clone())); - if let Some(agent_panel) = workspace.read(cx).panel::(cx) { - agent_panel.update(cx, |panel, cx| { - panel.new_thread(&NewThread, window, cx); - }); + anyhow::Ok(()) + }) + .detach_and_log_err(cx); + } else { + // Simple case: no workspace removal needed. + let neighbor_metadata = neighbor.map(|(metadata, _)| metadata); + self.archive_and_activate( + session_id, + neighbor_metadata.as_ref(), + thread_folder_paths.as_ref(), + window, + cx, + ); + } + } + + /// Archive a thread and activate the nearest neighbor or a draft. + fn archive_and_activate( + &mut self, + session_id: &acp::SessionId, + neighbor: Option<&ThreadMetadata>, + thread_folder_paths: Option<&PathList>, + window: &mut Window, + cx: &mut Context, + ) { + ThreadMetadataStore::global(cx).update(cx, |store, cx| { + store.archive(session_id, cx); + }); + + let is_active = self + .active_entry + .as_ref() + .is_some_and(|e| e.is_active_thread(session_id)); + + if !is_active { + // The user is looking at a different thread/draft. Clear the + // archived thread from its workspace's panel so that switching + // to that workspace later doesn't show a stale thread. + if let Some(folder_paths) = thread_folder_paths { + if let Some(workspace) = self + .multi_workspace + .upgrade() + .and_then(|mw| mw.read(cx).workspace_for_paths(folder_paths, cx)) + { + if let Some(panel) = workspace.read(cx).panel::(cx) { + let panel_shows_archived = panel + .read(cx) + .active_conversation_view() + .and_then(|cv| cv.read(cx).parent_id(cx)) + .is_some_and(|id| id == *session_id); + if panel_shows_archived { + panel.update(cx, |panel, cx| { + panel.clear_active_thread(window, cx); + }); + } } } } + return; + } + + // Try to activate the neighbor thread. If its workspace is open, + // tell the panel to load it. `rebuild_contents` will reconcile + // `active_entry` once the thread finishes loading. + if let Some(metadata) = neighbor { + if let Some(workspace) = self + .multi_workspace + .upgrade() + .and_then(|mw| mw.read(cx).workspace_for_paths(&metadata.folder_paths, cx)) + { + Self::load_agent_thread_in_workspace(&workspace, metadata, true, window, cx); + return; + } + } + + // No neighbor or its workspace isn't open — fall back to a new + // draft on the active workspace so the user has something to work with. + if let Some(workspace) = self.active_entry_workspace().cloned() { + if let Some(panel) = workspace.read(cx).panel::(cx) { + panel.update(cx, |panel, cx| { + panel.new_thread(&NewThread, window, cx); + }); + } } } @@ -2480,16 +2570,45 @@ impl Sidebar { let Some(ix) = self.selection else { return; }; - let Some(ListEntry::Thread(thread)) = self.contents.entries.get(ix) else { - return; - }; - match thread.status { - AgentThreadStatus::Running | AgentThreadStatus::WaitingForConfirmation => return, - AgentThreadStatus::Completed | AgentThreadStatus::Error => {} + match self.contents.entries.get(ix) { + Some(ListEntry::Thread(thread)) => { + match thread.status { + AgentThreadStatus::Running | AgentThreadStatus::WaitingForConfirmation => { + return; + } + AgentThreadStatus::Completed | AgentThreadStatus::Error => {} + } + let session_id = thread.metadata.session_id.clone(); + self.archive_thread(&session_id, window, cx); + } + Some(ListEntry::NewThread { + workspace: Some(workspace), + .. + }) => { + self.remove_worktree_workspace(workspace.clone(), window, cx); + } + _ => {} } + } - let session_id = thread.metadata.session_id.clone(); - self.archive_thread(&session_id, window, cx) + fn remove_worktree_workspace( + &mut self, + workspace: Entity, + window: &mut Window, + cx: &mut Context, + ) { + if let Some(multi_workspace) = self.multi_workspace.upgrade() { + multi_workspace + .update(cx, |mw, cx| { + mw.remove( + [workspace], + |this, _window, _cx| gpui::Task::ready(Ok(this.workspace().clone())), + window, + cx, + ) + }) + .detach_and_log_err(cx); + } } fn record_thread_access(&mut self, session_id: &acp::SessionId) { @@ -2933,14 +3052,9 @@ impl Sidebar { .map(|w| w.read(cx).focus_handle(cx)) .unwrap_or_else(|| cx.focus_handle()); - let sibling_workspace_ids: HashSet = multi_workspace + let window_project_groups: Vec = multi_workspace .as_ref() - .map(|mw| { - mw.read(cx) - .workspaces() - .filter_map(|ws| ws.read(cx).database_id()) - .collect() - }) + .map(|mw| mw.read(cx).project_group_keys().cloned().collect()) .unwrap_or_default(); let popover_handle = self.recent_projects_popover_handle.clone(); @@ -2951,7 +3065,7 @@ impl Sidebar { workspace.as_ref().map(|ws| { SidebarRecentProjects::popover( ws.clone(), - sibling_workspace_ids.clone(), + window_project_groups.clone(), focus_handle.clone(), window, cx, @@ -3274,30 +3388,6 @@ impl Sidebar { self.create_new_thread(&workspace, window, cx); } - fn on_move_workspace_to_new_window( - &mut self, - _: &MoveWorkspaceToNewWindow, - window: &mut Window, - cx: &mut Context, - ) { - let Some(multi_workspace) = self.multi_workspace.upgrade() else { - return; - }; - - let group_count = multi_workspace.read(cx).project_group_keys().count(); - if group_count <= 1 { - return; - } - - let Some(active_key) = self.active_project_group_key(cx) else { - return; - }; - - multi_workspace.update(cx, |multi_workspace, cx| { - multi_workspace.move_project_group_to_new_window(&active_key, window, cx); - }); - } - fn render_draft_thread( &self, ix: usize, @@ -3345,6 +3435,7 @@ impl Sidebar { ix: usize, key: &ProjectGroupKey, worktrees: &[WorktreeInfo], + workspace: Option<&Entity>, is_selected: bool, cx: &mut Context, ) -> AnyElement { @@ -3353,7 +3444,7 @@ impl Sidebar { let id = SharedString::from(format!("new-thread-btn-{}", ix)); - let thread_item = ThreadItem::new(id, label) + let mut thread_item = ThreadItem::new(id, label) .icon(IconName::Plus) .icon_color(Color::Custom(cx.theme().colors().icon_muted.opacity(0.8))) .worktrees( @@ -3378,6 +3469,20 @@ impl Sidebar { } })); + // Linked worktree NewThread entries can be dismissed, which removes + // the workspace from the multi-workspace. + if let Some(workspace) = workspace.cloned() { + thread_item = thread_item.action_slot( + IconButton::new("close-worktree-workspace", IconName::Close) + .icon_size(IconSize::Small) + .icon_color(Color::Muted) + .tooltip(Tooltip::text("Close Workspace")) + .on_click(cx.listener(move |this, _, window, cx| { + this.remove_worktree_workspace(workspace.clone(), window, cx); + })), + ); + } + thread_item.into_any_element() } @@ -3845,10 +3950,6 @@ impl WorkspaceSidebar for Sidebar { self.cycle_thread_impl(forward, window, cx); } - fn move_workspace_to_new_window(&mut self, window: &mut Window, cx: &mut Context) { - self.on_move_workspace_to_new_window(&MoveWorkspaceToNewWindow, window, cx); - } - fn serialized_state(&self, _cx: &App) -> Option { let serialized = SerializedSidebar { width: Some(f32::from(self.width)), @@ -3951,7 +4052,6 @@ impl Render for Sidebar { .on_action(cx.listener(Self::on_show_more_threads)) .on_action(cx.listener(Self::on_show_fewer_threads)) .on_action(cx.listener(Self::on_new_thread)) - .on_action(cx.listener(Self::on_move_workspace_to_new_window)) .on_action(cx.listener(|this, _: &OpenRecent, window, cx| { this.recent_projects_popover_handle.toggle(window, cx); })) diff --git a/crates/sidebar/src/sidebar_tests.rs b/crates/sidebar/src/sidebar_tests.rs index c5043e1bae08194a78e68a70d75970442e344467..860713e61de2af4cc426370872d93b7da114411c 100644 --- a/crates/sidebar/src/sidebar_tests.rs +++ b/crates/sidebar/src/sidebar_tests.rs @@ -522,18 +522,6 @@ async fn test_workspace_lifecycle(cx: &mut TestAppContext) { visible_entries_as_strings(&sidebar, cx), vec!["v [project-a]", " Thread A1",] ); - - // Remove the second workspace - multi_workspace.update_in(cx, |mw, window, cx| { - let workspace = mw.workspaces().nth(1).cloned().unwrap(); - mw.remove(&workspace, window, cx); - }); - cx.run_until_parked(); - - assert_eq!( - visible_entries_as_strings(&sidebar, cx), - vec!["v [project-a]", " Thread A1"] - ); } #[gpui::test] @@ -4133,6 +4121,129 @@ async fn test_archive_thread_uses_next_threads_own_workspace(cx: &mut TestAppCon ); } +#[gpui::test] +async fn test_archive_last_worktree_thread_removes_workspace(cx: &mut TestAppContext) { + // When the last non-archived thread for a linked worktree is archived, + // the linked worktree workspace should be removed from the multi-workspace. + // The main worktree workspace should remain (it's always reachable via + // the project header). + init_test(cx); + let fs = FakeFs::new(cx.executor()); + + fs.insert_tree( + "/project", + serde_json::json!({ + ".git": { + "worktrees": { + "feature-a": { + "commondir": "../../", + "HEAD": "ref: refs/heads/feature-a", + }, + }, + }, + "src": {}, + }), + ) + .await; + + fs.insert_tree( + "/wt-feature-a", + serde_json::json!({ + ".git": "gitdir: /project/.git/worktrees/feature-a", + "src": {}, + }), + ) + .await; + + fs.add_linked_worktree_for_repo( + Path::new("/project/.git"), + false, + git::repository::Worktree { + path: PathBuf::from("/wt-feature-a"), + ref_name: Some("refs/heads/feature-a".into()), + sha: "abc".into(), + is_main: false, + }, + ) + .await; + + cx.update(|cx| ::set_global(fs.clone(), cx)); + + let main_project = project::Project::test(fs.clone(), ["/project".as_ref()], cx).await; + let worktree_project = project::Project::test(fs.clone(), ["/wt-feature-a".as_ref()], cx).await; + + main_project + .update(cx, |p, cx| p.git_scans_complete(cx)) + .await; + worktree_project + .update(cx, |p, cx| p.git_scans_complete(cx)) + .await; + + 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 _worktree_workspace = multi_workspace.update_in(cx, |mw, window, cx| { + mw.test_add_workspace(worktree_project.clone(), window, cx) + }); + + // Save a thread for the main project. + save_thread_metadata( + acp::SessionId::new(Arc::from("main-thread")), + "Main Thread".into(), + chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0).unwrap(), + None, + &main_project, + cx, + ); + + // Save a thread for the linked worktree. + let wt_thread_id = acp::SessionId::new(Arc::from("worktree-thread")); + save_thread_metadata( + wt_thread_id.clone(), + "Worktree Thread".into(), + chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), + None, + &worktree_project, + cx, + ); + cx.run_until_parked(); + + multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); + cx.run_until_parked(); + + // Should have 2 workspaces. + assert_eq!( + multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()), + 2, + "should start with 2 workspaces (main + linked worktree)" + ); + + // Archive the worktree thread (the only thread for /wt-feature-a). + sidebar.update_in(cx, |sidebar: &mut Sidebar, window, cx| { + sidebar.archive_thread(&wt_thread_id, window, cx); + }); + cx.run_until_parked(); + + // The linked worktree workspace should have been removed. + assert_eq!( + multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()), + 1, + "linked worktree workspace should be removed after archiving its last thread" + ); + + // The main thread should still be visible. + let entries = visible_entries_as_strings(&sidebar, cx); + assert!( + entries.iter().any(|e| e.contains("Main Thread")), + "main thread should still be visible: {entries:?}" + ); + assert!( + !entries.iter().any(|e| e.contains("Worktree Thread")), + "archived worktree thread should not be visible: {entries:?}" + ); +} + #[gpui::test] async fn test_linked_worktree_threads_not_duplicated_across_groups(cx: &mut TestAppContext) { // When a multi-root workspace (e.g. [/other, /project]) shares a @@ -4611,6 +4722,176 @@ async fn test_archive_thread_keeps_metadata_but_hides_from_sidebar(cx: &mut Test }); } +#[gpui::test] +async fn test_archive_thread_active_entry_management(cx: &mut TestAppContext) { + // Tests two archive scenarios: + // 1. Archiving a thread in a non-active workspace leaves active_entry + // as the current draft. + // 2. Archiving the thread the user is looking at falls back to a draft + // on the same workspace. + agent_ui::test_support::init_test(cx); + cx.update(|cx| { + ThreadStore::init_global(cx); + ThreadMetadataStore::init_global(cx); + language_model::LanguageModelRegistry::test(cx); + prompt_store::init(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; + cx.update(|cx| ::set_global(fs.clone(), cx)); + + let project_a = project::Project::test(fs.clone(), ["/project-a".as_ref()], cx).await; + let project_b = project::Project::test(fs.clone(), ["/project-b".as_ref()], cx).await; + + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a.clone(), window, cx)); + let (sidebar, panel_a) = setup_sidebar_with_agent_panel(&multi_workspace, cx); + + let workspace_b = multi_workspace.update_in(cx, |mw, window, cx| { + mw.test_add_workspace(project_b.clone(), window, cx) + }); + let panel_b = add_agent_panel(&workspace_b, cx); + cx.run_until_parked(); + + // --- Scenario 1: archive a thread in the non-active workspace --- + + // Create a thread in project-a (non-active — project-b is active). + let connection = acp_thread::StubAgentConnection::new(); + connection.set_next_prompt_updates(vec![acp::SessionUpdate::AgentMessageChunk( + acp::ContentChunk::new("Done".into()), + )]); + agent_ui::test_support::open_thread_with_connection(&panel_a, connection, cx); + agent_ui::test_support::send_message(&panel_a, cx); + let thread_a = agent_ui::test_support::active_session_id(&panel_a, cx); + cx.run_until_parked(); + + sidebar.update_in(cx, |sidebar, window, cx| { + sidebar.archive_thread(&thread_a, window, cx); + }); + cx.run_until_parked(); + + // active_entry should still be a draft on workspace_b (the active one). + sidebar.read_with(cx, |sidebar, _| { + assert!( + matches!(&sidebar.active_entry, Some(ActiveEntry::Draft(ws)) if ws == &workspace_b), + "expected Draft(workspace_b) after archiving non-active thread, got: {:?}", + sidebar.active_entry, + ); + }); + + // --- Scenario 2: archive the thread the user is looking at --- + + // Create a thread in project-b (the active workspace) and verify it + // becomes the active entry. + let connection = acp_thread::StubAgentConnection::new(); + connection.set_next_prompt_updates(vec![acp::SessionUpdate::AgentMessageChunk( + acp::ContentChunk::new("Done".into()), + )]); + agent_ui::test_support::open_thread_with_connection(&panel_b, connection, cx); + agent_ui::test_support::send_message(&panel_b, cx); + let thread_b = agent_ui::test_support::active_session_id(&panel_b, cx); + cx.run_until_parked(); + + sidebar.read_with(cx, |sidebar, _| { + assert!( + matches!(&sidebar.active_entry, Some(ActiveEntry::Thread { session_id, .. }) if *session_id == thread_b), + "expected active_entry to be Thread({thread_b}), got: {:?}", + sidebar.active_entry, + ); + }); + + sidebar.update_in(cx, |sidebar, window, cx| { + sidebar.archive_thread(&thread_b, window, cx); + }); + cx.run_until_parked(); + + // Should fall back to a draft on the same workspace. + sidebar.read_with(cx, |sidebar, _| { + assert!( + matches!(&sidebar.active_entry, Some(ActiveEntry::Draft(ws)) if ws == &workspace_b), + "expected Draft(workspace_b) after archiving active thread, got: {:?}", + sidebar.active_entry, + ); + }); +} + +#[gpui::test] +async fn test_switch_to_workspace_with_archived_thread_shows_draft(cx: &mut TestAppContext) { + // When a thread is archived while the user is in a different workspace, + // the archiving code clears the thread from its panel (via + // `clear_active_thread`). Switching back to that workspace should show + // a draft, not the archived thread. + agent_ui::test_support::init_test(cx); + cx.update(|cx| { + ThreadStore::init_global(cx); + ThreadMetadataStore::init_global(cx); + language_model::LanguageModelRegistry::test(cx); + prompt_store::init(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; + cx.update(|cx| ::set_global(fs.clone(), cx)); + + let project_a = project::Project::test(fs.clone(), ["/project-a".as_ref()], cx).await; + let project_b = project::Project::test(fs.clone(), ["/project-b".as_ref()], cx).await; + + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a.clone(), window, cx)); + let (sidebar, panel_a) = setup_sidebar_with_agent_panel(&multi_workspace, cx); + + let workspace_b = multi_workspace.update_in(cx, |mw, window, cx| { + mw.test_add_workspace(project_b.clone(), window, cx) + }); + let _panel_b = add_agent_panel(&workspace_b, cx); + cx.run_until_parked(); + + // Create a thread in project-a's panel (currently non-active). + let connection = acp_thread::StubAgentConnection::new(); + connection.set_next_prompt_updates(vec![acp::SessionUpdate::AgentMessageChunk( + acp::ContentChunk::new("Done".into()), + )]); + agent_ui::test_support::open_thread_with_connection(&panel_a, connection, cx); + agent_ui::test_support::send_message(&panel_a, cx); + let thread_a = agent_ui::test_support::active_session_id(&panel_a, cx); + cx.run_until_parked(); + + // Archive it while project-b is active. + sidebar.update_in(cx, |sidebar, window, cx| { + sidebar.archive_thread(&thread_a, window, cx); + }); + cx.run_until_parked(); + + // Switch back to project-a. Its panel was cleared during archiving, + // so active_entry should be Draft. + let workspace_a = + multi_workspace.read_with(cx, |mw, _| mw.workspaces().next().unwrap().clone()); + multi_workspace.update_in(cx, |mw, window, cx| { + mw.activate(workspace_a.clone(), window, cx); + }); + cx.run_until_parked(); + + sidebar.update_in(cx, |sidebar, _window, cx| { + sidebar.update_entries(cx); + }); + cx.run_until_parked(); + + sidebar.read_with(cx, |sidebar, _| { + assert!( + matches!(&sidebar.active_entry, Some(ActiveEntry::Draft(ws)) if ws == &workspace_a), + "expected Draft(workspace_a) after switching to workspace with archived thread, got: {:?}", + sidebar.active_entry, + ); + }); +} + #[gpui::test] async fn test_archived_threads_excluded_from_sidebar_entries(cx: &mut TestAppContext) { let project = init_test_project("/my-project", cx).await; @@ -4673,6 +4954,140 @@ async fn test_archived_threads_excluded_from_sidebar_entries(cx: &mut TestAppCon }); } +#[gpui::test] +async fn test_linked_worktree_workspace_reachable_and_dismissable(cx: &mut TestAppContext) { + // When a linked worktree is opened as its own workspace and the user + // switches away, the workspace must still be reachable from a NewThread + // sidebar entry. Pressing RemoveSelectedThread (shift-backspace) on that + // entry should remove the workspace. + init_test(cx); + let fs = FakeFs::new(cx.executor()); + + fs.insert_tree( + "/project", + serde_json::json!({ + ".git": { + "worktrees": { + "feature-a": { + "commondir": "../../", + "HEAD": "ref: refs/heads/feature-a", + }, + }, + }, + "src": {}, + }), + ) + .await; + + fs.insert_tree( + "/wt-feature-a", + serde_json::json!({ + ".git": "gitdir: /project/.git/worktrees/feature-a", + "src": {}, + }), + ) + .await; + + fs.add_linked_worktree_for_repo( + Path::new("/project/.git"), + false, + git::repository::Worktree { + path: PathBuf::from("/wt-feature-a"), + ref_name: Some("refs/heads/feature-a".into()), + sha: "aaa".into(), + is_main: false, + }, + ) + .await; + + cx.update(|cx| ::set_global(fs.clone(), cx)); + + let main_project = project::Project::test(fs.clone(), ["/project".as_ref()], cx).await; + let worktree_project = project::Project::test(fs.clone(), ["/wt-feature-a".as_ref()], cx).await; + + main_project + .update(cx, |p, cx| p.git_scans_complete(cx)) + .await; + worktree_project + .update(cx, |p, cx| p.git_scans_complete(cx)) + .await; + + 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); + + // Open the linked worktree as a separate workspace (simulates cmd-o). + let worktree_workspace = multi_workspace.update_in(cx, |mw, window, cx| { + mw.test_add_workspace(worktree_project.clone(), window, cx) + }); + add_agent_panel(&worktree_workspace, cx); + cx.run_until_parked(); + + // Switch back to the main workspace. + multi_workspace.update_in(cx, |mw, window, cx| { + let main_ws = mw.workspaces().next().unwrap().clone(); + mw.activate(main_ws, window, cx); + }); + cx.run_until_parked(); + + sidebar.update_in(cx, |sidebar, _window, cx| { + sidebar.update_entries(cx); + }); + cx.run_until_parked(); + + // The linked worktree workspace must be reachable from some sidebar entry. + let worktree_ws_id = worktree_workspace.entity_id(); + let reachable: Vec = sidebar.read_with(cx, |sidebar, cx| { + let mw = multi_workspace.read(cx); + sidebar + .contents + .entries + .iter() + .flat_map(|entry| entry.reachable_workspaces(mw, cx)) + .map(|ws| ws.entity_id()) + .collect() + }); + assert!( + reachable.contains(&worktree_ws_id), + "linked worktree workspace should be reachable, but reachable are: {reachable:?}" + ); + + // Find the NewThread entry for the linked worktree and dismiss it. + let new_thread_ix = sidebar.read_with(cx, |sidebar, _| { + sidebar + .contents + .entries + .iter() + .position(|entry| { + matches!( + entry, + ListEntry::NewThread { + workspace: Some(_), + .. + } + ) + }) + .expect("expected a NewThread entry for the linked worktree") + }); + + assert_eq!( + multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()), + 2 + ); + + sidebar.update_in(cx, |sidebar, window, cx| { + sidebar.selection = Some(new_thread_ix); + sidebar.remove_selected_thread(&RemoveSelectedThread, window, cx); + }); + cx.run_until_parked(); + + assert_eq!( + multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()), + 1, + "linked worktree workspace should be removed after dismissing NewThread entry" + ); +} + #[gpui::test] async fn test_linked_worktree_workspace_shows_main_worktree_threads(cx: &mut TestAppContext) { // When only a linked worktree workspace is open (not the main repo), @@ -5051,35 +5466,25 @@ mod property_test { workspace_counter: u32, worktree_counter: u32, saved_thread_ids: Vec, - workspace_paths: Vec, - main_repo_indices: Vec, unopened_worktrees: Vec, } impl TestState { - fn new(fs: Arc, initial_workspace_path: String) -> Self { + fn new(fs: Arc) -> Self { Self { fs, thread_counter: 0, workspace_counter: 1, worktree_counter: 0, saved_thread_ids: Vec::new(), - workspace_paths: vec![initial_workspace_path], - main_repo_indices: vec![0], unopened_worktrees: Vec::new(), } } - fn next_thread_id(&mut self) -> acp::SessionId { + fn next_metadata_only_thread_id(&mut self) -> acp::SessionId { let id = self.thread_counter; self.thread_counter += 1; - let session_id = acp::SessionId::new(Arc::from(format!("prop-thread-{id}"))); - self.saved_thread_ids.push(session_id.clone()); - session_id - } - - fn remove_thread(&mut self, index: usize) -> acp::SessionId { - self.saved_thread_ids.remove(index) + acp::SessionId::new(Arc::from(format!("prop-thread-{id}"))) } fn next_workspace_path(&mut self) -> String { @@ -5097,99 +5502,81 @@ mod property_test { #[derive(Debug)] enum Operation { - SaveThread { workspace_index: usize }, + SaveThread { project_group_index: usize }, SaveWorktreeThread { worktree_index: usize }, - DeleteThread { index: usize }, ToggleAgentPanel, CreateDraftThread, - AddWorkspace, - OpenWorktreeAsWorkspace { worktree_index: usize }, - RemoveWorkspace { index: usize }, - SwitchWorkspace { index: usize }, - AddLinkedWorktree { workspace_index: usize }, + AddProject { use_worktree: bool }, + ArchiveThread { index: usize }, + SwitchToThread { index: usize }, + SwitchToProjectGroup { index: usize }, + AddLinkedWorktree { project_group_index: usize }, } // Distribution (out of 20 slots): - // SaveThread: 5 slots (~23%) - // SaveWorktreeThread: 2 slots (~9%) - // DeleteThread: 2 slots (~9%) - // ToggleAgentPanel: 2 slots (~9%) - // CreateDraftThread: 2 slots (~9%) - // AddWorkspace: 1 slot (~5%) - // OpenWorktreeAsWorkspace: 1 slot (~5%) - // RemoveWorkspace: 1 slot (~5%) - // SwitchWorkspace: 2 slots (~9%) - // AddLinkedWorktree: 4 slots (~18%) - const DISTRIBUTION_SLOTS: u32 = 22; + // SaveThread: 5 slots (~25%) + // SaveWorktreeThread: 2 slots (~10%) + // ToggleAgentPanel: 1 slot (~5%) + // CreateDraftThread: 1 slot (~5%) + // AddProject: 1 slot (~5%) + // ArchiveThread: 2 slots (~10%) + // SwitchToThread: 2 slots (~10%) + // SwitchToProjectGroup: 2 slots (~10%) + // AddLinkedWorktree: 4 slots (~20%) + const DISTRIBUTION_SLOTS: u32 = 20; impl TestState { - fn generate_operation(&self, raw: u32) -> Operation { + fn generate_operation(&self, raw: u32, project_group_count: usize) -> Operation { let extra = (raw / DISTRIBUTION_SLOTS) as usize; - let workspace_count = self.workspace_paths.len(); match raw % DISTRIBUTION_SLOTS { 0..=4 => Operation::SaveThread { - workspace_index: extra % workspace_count, + project_group_index: extra % project_group_count, }, 5..=6 if !self.unopened_worktrees.is_empty() => Operation::SaveWorktreeThread { worktree_index: extra % self.unopened_worktrees.len(), }, 5..=6 => Operation::SaveThread { - workspace_index: extra % workspace_count, + project_group_index: extra % project_group_count, }, - 7..=8 if !self.saved_thread_ids.is_empty() => Operation::DeleteThread { + 7 => Operation::ToggleAgentPanel, + 8 => Operation::CreateDraftThread, + 9 => Operation::AddProject { + use_worktree: !self.unopened_worktrees.is_empty(), + }, + 10..=11 if !self.saved_thread_ids.is_empty() => Operation::ArchiveThread { index: extra % self.saved_thread_ids.len(), }, - 7..=8 => Operation::SaveThread { - workspace_index: extra % workspace_count, + 10..=11 => Operation::AddProject { + use_worktree: !self.unopened_worktrees.is_empty(), }, - 9..=10 => Operation::ToggleAgentPanel, - 11..=12 => Operation::CreateDraftThread, - 13 if !self.unopened_worktrees.is_empty() => Operation::OpenWorktreeAsWorkspace { - worktree_index: extra % self.unopened_worktrees.len(), + 12..=13 if !self.saved_thread_ids.is_empty() => Operation::SwitchToThread { + index: extra % self.saved_thread_ids.len(), }, - 13 => Operation::AddWorkspace, - 14 if workspace_count > 1 => Operation::RemoveWorkspace { - index: extra % workspace_count, + 12..=13 => Operation::SwitchToProjectGroup { + index: extra % project_group_count, }, - 14 => Operation::AddWorkspace, - 15..=16 => Operation::SwitchWorkspace { - index: extra % workspace_count, + 14..=15 => Operation::SwitchToProjectGroup { + index: extra % project_group_count, }, - 17..=21 if !self.main_repo_indices.is_empty() => { - let main_index = self.main_repo_indices[extra % self.main_repo_indices.len()]; - Operation::AddLinkedWorktree { - workspace_index: main_index, - } - } - 17..=21 => Operation::SaveThread { - workspace_index: extra % workspace_count, + 16..=19 if project_group_count > 0 => Operation::AddLinkedWorktree { + project_group_index: extra % project_group_count, + }, + 16..=19 => Operation::SaveThread { + project_group_index: extra % project_group_count, }, _ => unreachable!(), } } } - fn save_thread_to_path( - state: &mut TestState, - project: &Entity, - cx: &mut gpui::VisualTestContext, - ) { - let session_id = state.next_thread_id(); - let title: SharedString = format!("Thread {}", session_id).into(); - let updated_at = chrono::TimeZone::with_ymd_and_hms(&chrono::Utc, 2024, 1, 1, 0, 0, 0) - .unwrap() - + chrono::Duration::seconds(state.thread_counter as i64); - save_thread_metadata(session_id, title, updated_at, None, project, cx); - } - fn save_thread_to_path_with_main( state: &mut TestState, path_list: PathList, main_worktree_paths: PathList, cx: &mut gpui::VisualTestContext, ) { - let session_id = state.next_thread_id(); + let session_id = state.next_metadata_only_thread_id(); let title: SharedString = format!("Thread {}", session_id).into(); let updated_at = chrono::TimeZone::with_ymd_and_hms(&chrono::Utc, 2024, 1, 1, 0, 0, 0) .unwrap() @@ -5215,20 +5602,48 @@ mod property_test { operation: Operation, state: &mut TestState, multi_workspace: &Entity, - _sidebar: &Entity, + sidebar: &Entity, cx: &mut gpui::VisualTestContext, ) { match operation { - Operation::SaveThread { workspace_index } => { - let project = multi_workspace.read_with(cx, |mw, cx| { - mw.workspaces() - .nth(workspace_index) - .unwrap() - .read(cx) - .project() - .clone() + Operation::SaveThread { + project_group_index, + } => { + // Find a workspace for this project group and create a real + // thread via its agent panel. + let (workspace, project) = multi_workspace.read_with(cx, |mw, cx| { + let key = mw.project_group_keys().nth(project_group_index).unwrap(); + let ws = mw + .workspaces_for_project_group(key, cx) + .next() + .unwrap_or(mw.workspace()) + .clone(); + let project = ws.read(cx).project().clone(); + (ws, project) }); - save_thread_to_path(state, &project, cx); + + let panel = + workspace.read_with(cx, |workspace, cx| workspace.panel::(cx)); + if let Some(panel) = panel { + let connection = StubAgentConnection::new(); + connection.set_next_prompt_updates(vec![ + acp::SessionUpdate::AgentMessageChunk(acp::ContentChunk::new( + "Done".into(), + )), + ]); + open_thread_with_connection(&panel, connection, cx); + send_message(&panel, cx); + let session_id = active_session_id(&panel, cx); + state.saved_thread_ids.push(session_id.clone()); + + let title: SharedString = format!("Thread {}", state.thread_counter).into(); + state.thread_counter += 1; + let updated_at = + chrono::TimeZone::with_ymd_and_hms(&chrono::Utc, 2024, 1, 1, 0, 0, 0) + .unwrap() + + chrono::Duration::seconds(state.thread_counter as i64); + save_thread_metadata(session_id, title, updated_at, None, &project, cx); + } } Operation::SaveWorktreeThread { worktree_index } => { let worktree = &state.unopened_worktrees[worktree_index]; @@ -5237,13 +5652,7 @@ mod property_test { PathList::new(&[std::path::PathBuf::from(&worktree.main_workspace_path)]); save_thread_to_path_with_main(state, path_list, main_worktree_paths, cx); } - Operation::DeleteThread { index } => { - let session_id = state.remove_thread(index); - cx.update(|_, cx| { - ThreadMetadataStore::global(cx) - .update(cx, |store, cx| store.delete(session_id, cx)); - }); - } + Operation::ToggleAgentPanel => { let workspace = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()); let panel_open = @@ -5269,18 +5678,26 @@ mod property_test { workspace.focus_panel::(window, cx); }); } - Operation::AddWorkspace => { - let path = state.next_workspace_path(); - state - .fs - .insert_tree( - &path, - serde_json::json!({ - ".git": {}, - "src": {}, - }), - ) - .await; + Operation::AddProject { use_worktree } => { + let path = if use_worktree { + // Open an existing linked worktree as a project (simulates Cmd+O + // on a worktree directory). + state.unopened_worktrees.remove(0).path + } else { + // Create a brand new project. + let path = state.next_workspace_path(); + state + .fs + .insert_tree( + &path, + serde_json::json!({ + ".git": {}, + "src": {}, + }), + ) + .await; + path + }; let project = project::Project::test( state.fs.clone() as Arc, [path.as_ref()], @@ -5288,53 +5705,62 @@ mod property_test { ) .await; project.update(cx, |p, cx| p.git_scans_complete(cx)).await; - let workspace = multi_workspace.update_in(cx, |mw, window, cx| { + multi_workspace.update_in(cx, |mw, window, cx| { mw.test_add_workspace(project.clone(), window, cx) }); - add_agent_panel(&workspace, cx); - let new_index = state.workspace_paths.len(); - state.workspace_paths.push(path); - state.main_repo_indices.push(new_index); } - Operation::OpenWorktreeAsWorkspace { worktree_index } => { - let worktree = state.unopened_worktrees.remove(worktree_index); - let project = project::Project::test( - state.fs.clone() as Arc, - [worktree.path.as_ref()], - cx, - ) - .await; - project.update(cx, |p, cx| p.git_scans_complete(cx)).await; - let workspace = multi_workspace.update_in(cx, |mw, window, cx| { - mw.test_add_workspace(project.clone(), window, cx) + Operation::ArchiveThread { index } => { + let session_id = state.saved_thread_ids[index].clone(); + sidebar.update_in(cx, |sidebar: &mut Sidebar, window, cx| { + sidebar.archive_thread(&session_id, window, cx); }); - add_agent_panel(&workspace, cx); - state.workspace_paths.push(worktree.path); + cx.run_until_parked(); + state.saved_thread_ids.remove(index); } - Operation::RemoveWorkspace { index } => { - let removed = multi_workspace.update_in(cx, |mw, window, cx| { - let workspace = mw.workspaces().nth(index).unwrap().clone(); - mw.remove(&workspace, window, cx) + Operation::SwitchToThread { index } => { + let session_id = state.saved_thread_ids[index].clone(); + // Find the thread's position in the sidebar entries and select it. + let thread_index = sidebar.read_with(cx, |sidebar, _| { + sidebar.contents.entries.iter().position(|entry| { + matches!( + entry, + ListEntry::Thread(t) if t.metadata.session_id == session_id + ) + }) }); - if removed { - state.workspace_paths.remove(index); - state.main_repo_indices.retain(|i| *i != index); - for i in &mut state.main_repo_indices { - if *i > index { - *i -= 1; - } - } + if let Some(ix) = thread_index { + sidebar.update_in(cx, |sidebar, window, cx| { + sidebar.selection = Some(ix); + sidebar.confirm(&Confirm, window, cx); + }); + cx.run_until_parked(); } } - Operation::SwitchWorkspace { index } => { - let workspace = multi_workspace - .read_with(cx, |mw, _| mw.workspaces().nth(index).unwrap().clone()); + Operation::SwitchToProjectGroup { index } => { + let workspace = multi_workspace.read_with(cx, |mw, cx| { + let key = mw.project_group_keys().nth(index).unwrap(); + mw.workspaces_for_project_group(key, cx) + .next() + .unwrap_or(mw.workspace()) + .clone() + }); multi_workspace.update_in(cx, |mw, window, cx| { mw.activate(workspace, window, cx); }); } - Operation::AddLinkedWorktree { workspace_index } => { - let main_path = state.workspace_paths[workspace_index].clone(); + Operation::AddLinkedWorktree { + project_group_index, + } => { + // Get the main worktree path from the project group key. + let main_path = multi_workspace.read_with(cx, |mw, _| { + let key = mw.project_group_keys().nth(project_group_index).unwrap(); + key.path_list() + .paths() + .first() + .unwrap() + .to_string_lossy() + .to_string() + }); let dot_git = format!("{}/.git", main_path); let worktree_name = state.next_worktree_name(); let worktree_path = format!("/worktrees/{}", worktree_name); @@ -5378,8 +5804,12 @@ mod property_test { .await; // Re-scan the main workspace's project so it discovers the new worktree. - let main_workspace = multi_workspace.read_with(cx, |mw, _| { - mw.workspaces().nth(workspace_index).unwrap().clone() + let main_workspace = multi_workspace.read_with(cx, |mw, cx| { + let key = mw.project_group_keys().nth(project_group_index).unwrap(); + mw.workspaces_for_project_group(key, cx) + .next() + .unwrap() + .clone() }); let main_project = main_workspace.read_with(cx, |ws, _| ws.project().clone()); main_project @@ -5414,13 +5844,14 @@ mod property_test { } fn validate_sidebar_properties(sidebar: &Sidebar, cx: &App) -> anyhow::Result<()> { - verify_every_workspace_in_multiworkspace_is_shown(sidebar, cx)?; + verify_every_group_in_multiworkspace_is_shown(sidebar, cx)?; verify_all_threads_are_shown(sidebar, cx)?; verify_active_state_matches_current_workspace(sidebar, cx)?; + verify_all_workspaces_are_reachable(sidebar, cx)?; Ok(()) } - fn verify_every_workspace_in_multiworkspace_is_shown( + fn verify_every_group_in_multiworkspace_is_shown( sidebar: &Sidebar, cx: &App, ) -> anyhow::Result<()> { @@ -5633,8 +6064,41 @@ mod property_test { Ok(()) } + /// Every workspace in the multi-workspace should be "reachable" from + /// the sidebar — meaning there is at least one entry (thread, draft, + /// new-thread, or project header) that, when clicked, would activate + /// that workspace. + fn verify_all_workspaces_are_reachable(sidebar: &Sidebar, cx: &App) -> anyhow::Result<()> { + let Some(multi_workspace) = sidebar.multi_workspace.upgrade() else { + anyhow::bail!("sidebar should still have an associated multi-workspace"); + }; + + let mw = multi_workspace.read(cx); + + let reachable_workspaces: HashSet = sidebar + .contents + .entries + .iter() + .flat_map(|entry| entry.reachable_workspaces(mw, cx)) + .map(|ws| ws.entity_id()) + .collect(); + + let all_workspace_ids: HashSet = + mw.workspaces().map(|ws| ws.entity_id()).collect(); + + let unreachable = &all_workspace_ids - &reachable_workspaces; + + anyhow::ensure!( + unreachable.is_empty(), + "The following workspaces are not reachable from any sidebar entry: {:?}", + unreachable, + ); + + Ok(()) + } + #[gpui::property_test(config = ProptestConfig { - cases: 10, + cases: 50, ..Default::default() })] async fn test_sidebar_invariants( @@ -5648,6 +6112,20 @@ mod property_test { ThreadMetadataStore::init_global(cx); language_model::LanguageModelRegistry::test(cx); prompt_store::init(cx); + + // Auto-add an AgentPanel to every workspace so that implicitly + // created workspaces (e.g. from thread activation) also have one. + cx.observe_new( + |workspace: &mut Workspace, + window: Option<&mut Window>, + cx: &mut gpui::Context| { + if let Some(window) = window { + let panel = cx.new(|cx| AgentPanel::test_new(workspace, window, cx)); + workspace.add_panel(panel, window, cx); + } + }, + ) + .detach(); }); let fs = FakeFs::new(cx.executor()); @@ -5667,13 +6145,15 @@ mod property_test { let (multi_workspace, cx) = cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); - let (sidebar, _panel) = setup_sidebar_with_agent_panel(&multi_workspace, cx); + let sidebar = setup_sidebar(&multi_workspace, cx); - let mut state = TestState::new(fs, "/my-project".to_string()); + let mut state = TestState::new(fs); let mut executed: Vec = Vec::new(); for &raw_op in &raw_operations { - let operation = state.generate_operation(raw_op); + let project_group_count = + multi_workspace.read_with(cx, |mw, _| mw.project_group_keys().count()); + let operation = state.generate_operation(raw_op, project_group_count); executed.push(format!("{:?}", operation)); perform_operation(operation, &mut state, &multi_workspace, &sidebar, cx).await; cx.run_until_parked(); diff --git a/crates/title_bar/src/title_bar.rs b/crates/title_bar/src/title_bar.rs index dfcd933dc20df9a6f6643402719f2ec1143cc7fe..480fb1acfb0a563182dac733af08ee4abab7a9d4 100644 --- a/crates/title_bar/src/title_bar.rs +++ b/crates/title_bar/src/title_bar.rs @@ -36,7 +36,7 @@ use project::{Project, git_store::GitStoreEvent, trusted_worktrees::TrustedWorkt use remote::RemoteConnectionOptions; use settings::Settings; use settings::WorktreeId; -use std::collections::HashSet; + use std::sync::Arc; use theme::ActiveTheme; use title_bar_settings::TitleBarSettings; @@ -47,7 +47,7 @@ use ui::{ use update_version::UpdateVersion; use util::ResultExt; use workspace::{ - MultiWorkspace, ToggleWorktreeSecurity, Workspace, WorkspaceId, notifications::NotifyResultExt, + MultiWorkspace, ToggleWorktreeSecurity, Workspace, notifications::NotifyResultExt, }; use zed_actions::OpenRemote; @@ -733,23 +733,18 @@ impl TitleBar { .map(|w| w.read(cx).focus_handle(cx)) .unwrap_or_else(|| cx.focus_handle()); - let sibling_workspace_ids: HashSet = self + let window_project_groups: Vec<_> = self .multi_workspace .as_ref() .and_then(|mw| mw.upgrade()) - .map(|mw| { - mw.read(cx) - .workspaces() - .filter_map(|ws| ws.read(cx).database_id()) - .collect() - }) + .map(|mw| mw.read(cx).project_group_keys().cloned().collect()) .unwrap_or_default(); PopoverMenu::new("recent-projects-menu") .menu(move |window, cx| { Some(recent_projects::RecentProjects::popover( workspace.clone(), - sibling_workspace_ids.clone(), + window_project_groups.clone(), false, focus_handle.clone(), window, @@ -795,23 +790,18 @@ impl TitleBar { .map(|w| w.read(cx).focus_handle(cx)) .unwrap_or_else(|| cx.focus_handle()); - let sibling_workspace_ids: HashSet = self + let window_project_groups: Vec<_> = self .multi_workspace .as_ref() .and_then(|mw| mw.upgrade()) - .map(|mw| { - mw.read(cx) - .workspaces() - .filter_map(|ws| ws.read(cx).database_id()) - .collect() - }) + .map(|mw| mw.read(cx).project_group_keys().cloned().collect()) .unwrap_or_default(); PopoverMenu::new("sidebar-title-recent-projects-menu") .menu(move |window, cx| { Some(recent_projects::RecentProjects::popover( workspace.clone(), - sibling_workspace_ids.clone(), + window_project_groups.clone(), false, focus_handle.clone(), window, diff --git a/crates/workspace/src/multi_workspace.rs b/crates/workspace/src/multi_workspace.rs index 672397ae2b7c02894a0866e6e5793964941f88fd..e9baccb4d93c9406d7b766db3d8e4ce4cb7198b7 100644 --- a/crates/workspace/src/multi_workspace.rs +++ b/crates/workspace/src/multi_workspace.rs @@ -11,7 +11,6 @@ pub use settings::SidebarSide; use std::future::Future; use std::path::Path; use std::path::PathBuf; -use std::sync::Arc; use ui::prelude::*; use util::ResultExt; use util::path_list::PathList; @@ -23,7 +22,6 @@ use ui::{ContextMenu, right_click_menu}; const SIDEBAR_RESIZE_HANDLE_SIZE: Pixels = px(6.0); -use crate::AppState; use crate::{ CloseIntent, CloseWindow, DockPosition, Event as WorkspaceEvent, Item, ModalView, OpenMode, Panel, Workspace, WorkspaceId, client_side_decorations, @@ -53,8 +51,6 @@ actions!( ShowFewerThreads, /// Creates a new thread in the current workspace. NewThread, - /// Moves the current workspace's project group to a new window. - MoveWorkspaceToNewWindow, ] ); @@ -134,9 +130,6 @@ pub trait Sidebar: Focusable + Render + EventEmitter + Sized { /// Activates the next or previous thread in sidebar order. fn cycle_thread(&mut self, _forward: bool, _window: &mut Window, _cx: &mut Context) {} - /// Moves the active workspace's project group to a new window. - fn move_workspace_to_new_window(&mut self, _window: &mut Window, _cx: &mut Context) {} - /// Return an opaque JSON blob of sidebar-specific state to persist. fn serialized_state(&self, _cx: &App) -> Option { None @@ -164,7 +157,6 @@ pub trait SidebarHandle: 'static + Send + Sync { fn toggle_thread_switcher(&self, select_last: bool, window: &mut Window, cx: &mut App); fn cycle_project(&self, forward: bool, window: &mut Window, cx: &mut App); fn cycle_thread(&self, forward: bool, window: &mut Window, cx: &mut App); - fn move_workspace_to_new_window(&self, window: &mut Window, cx: &mut App); fn is_threads_list_view_active(&self, cx: &App) -> bool; @@ -243,15 +235,6 @@ impl SidebarHandle for Entity { }); } - fn move_workspace_to_new_window(&self, window: &mut Window, cx: &mut App) { - let entity = self.clone(); - window.defer(cx, move |window, cx| { - entity.update(cx, |this, cx| { - this.move_workspace_to_new_window(window, cx); - }); - }); - } - fn is_threads_list_view_active(&self, cx: &App) -> bool { self.read(cx).is_threads_list_view_active() } @@ -285,13 +268,6 @@ enum ActiveWorkspace { } impl ActiveWorkspace { - fn persistent_index(&self) -> Option { - match self { - Self::Persistent(index) => Some(*index), - Self::Transient(_) => None, - } - } - fn transient_workspace(&self) -> Option<&Entity> { match self { Self::Transient(workspace) => Some(workspace), @@ -595,7 +571,8 @@ impl MultiWorkspace { if self.project_group_keys.contains(&project_group_key) { return; } - self.project_group_keys.push(project_group_key); + // Store newest first so the vec is in "most recently added" + self.project_group_keys.insert(0, project_group_key); } pub fn restore_project_group_keys(&mut self, keys: Vec) { @@ -625,7 +602,6 @@ impl MultiWorkspace { let mut groups = self .project_group_keys .iter() - .rev() .map(|key| (key.clone(), Vec::new())) .collect::>(); for workspace in &self.workspaces { @@ -750,19 +726,67 @@ impl MultiWorkspace { key: &ProjectGroupKey, window: &mut Window, cx: &mut Context, - ) { - self.project_group_keys.retain(|k| k != key); - + ) -> Task> { let workspaces: Vec<_> = self .workspaces_for_project_group(key, cx) .cloned() .collect(); - for workspace in workspaces { - self.remove(&workspace, window, cx); - } - self.serialize(cx); - cx.notify(); + // Compute the neighbor while the key is still in the list. + let neighbor_key = { + let pos = self.project_group_keys.iter().position(|k| k == key); + pos.and_then(|pos| { + // Keys are in display order, so pos+1 is below + // and pos-1 is above. Try below first. + self.project_group_keys.get(pos + 1).or_else(|| { + pos.checked_sub(1) + .and_then(|i| self.project_group_keys.get(i)) + }) + }) + .cloned() + }; + + // Now remove the key. + self.project_group_keys.retain(|k| k != key); + + self.remove( + workspaces, + move |this, window, cx| { + if let Some(neighbor_key) = neighbor_key { + return this.find_or_create_local_workspace( + neighbor_key.path_list().clone(), + window, + cx, + ); + } + + // No other project groups remain — create an empty workspace. + let app_state = this.workspace().read(cx).app_state().clone(); + let project = Project::local( + app_state.client.clone(), + app_state.node_runtime.clone(), + app_state.user_store.clone(), + app_state.languages.clone(), + app_state.fs.clone(), + None, + project::LocalProjectFlags::default(), + cx, + ); + let new_workspace = + cx.new(|cx| Workspace::new(None, project, app_state, window, cx)); + Task::ready(Ok(new_workspace)) + }, + window, + cx, + ) + } + + /// Finds an existing workspace whose root paths exactly match the given path list. + pub fn workspace_for_paths(&self, path_list: &PathList, cx: &App) -> Option> { + self.workspaces + .iter() + .find(|ws| PathList::new(&ws.read(cx).root_paths(cx)) == *path_list) + .cloned() } /// Finds an existing workspace in this multi-workspace whose paths match, @@ -775,12 +799,7 @@ impl MultiWorkspace { window: &mut Window, cx: &mut Context, ) -> Task>> { - if let Some(workspace) = self - .workspaces - .iter() - .find(|ws| PathList::new(&ws.read(cx).root_paths(cx)) == path_list) - .cloned() - { + if let Some(workspace) = self.workspace_for_paths(&path_list, cx) { self.activate(workspace.clone(), window, cx); return Task::ready(Ok(workspace)); } @@ -1201,174 +1220,116 @@ impl MultiWorkspace { }) } + /// Removes one or more workspaces from this multi-workspace. + /// + /// If the active workspace is among those being removed, + /// `fallback_workspace` is called **synchronously before the removal + /// begins** to produce a `Task` that resolves to the workspace that + /// should become active. The fallback must not be one of the + /// workspaces being removed. + /// + /// Returns `true` if any workspaces were actually removed. pub fn remove( &mut self, - workspace: &Entity, + workspaces: impl IntoIterator>, + fallback_workspace: impl FnOnce( + &mut Self, + &mut Window, + &mut Context, + ) -> Task>>, window: &mut Window, cx: &mut Context, - ) -> bool { - let Some(index) = self.workspaces.iter().position(|w| w == workspace) else { - return false; - }; - - let old_key = workspace.read(cx).project_group_key(cx); - - if self.workspaces.len() <= 1 { - let has_worktrees = workspace.read(cx).visible_worktrees(cx).next().is_some(); - - if !has_worktrees { - return false; - } - - let old_workspace = workspace.clone(); - let old_entity_id = old_workspace.entity_id(); - - let app_state = old_workspace.read(cx).app_state().clone(); - - let project = Project::local( - app_state.client.clone(), - app_state.node_runtime.clone(), - app_state.user_store.clone(), - app_state.languages.clone(), - app_state.fs.clone(), - None, - project::LocalProjectFlags::default(), - cx, - ); - - let new_workspace = cx.new(|cx| Workspace::new(None, project, app_state, window, cx)); - - self.workspaces[0] = new_workspace.clone(); - self.active_workspace = ActiveWorkspace::Persistent(0); + ) -> Task> { + let workspaces: Vec<_> = workspaces.into_iter().collect(); - Self::subscribe_to_workspace(&new_workspace, window, cx); - - self.sync_sidebar_to_workspace(&new_workspace, cx); + if workspaces.is_empty() { + return Task::ready(Ok(false)); + } - let weak_self = cx.weak_entity(); + let removing_active = workspaces.iter().any(|ws| ws == self.workspace()); + let original_active = self.workspace().clone(); - new_workspace.update(cx, |workspace, cx| { - workspace.set_multi_workspace(weak_self, cx); - }); + let fallback_task = removing_active.then(|| fallback_workspace(self, window, cx)); - self.detach_workspace(&old_workspace, cx); - - cx.emit(MultiWorkspaceEvent::WorkspaceRemoved(old_entity_id)); - cx.emit(MultiWorkspaceEvent::WorkspaceAdded(new_workspace)); - cx.emit(MultiWorkspaceEvent::ActiveWorkspaceChanged); - } else { - let removed_workspace = self.workspaces.remove(index); + cx.spawn_in(window, async move |this, cx| { + // Prompt each workspace for unsaved changes. If any workspace + // has dirty buffers, save_all_internal will emit Activate to + // bring it into view before showing the save dialog. + for workspace in &workspaces { + let should_continue = workspace + .update_in(cx, |workspace, window, cx| { + workspace.save_all_internal(crate::SaveIntent::Close, window, cx) + })? + .await?; - if let Some(active_index) = self.active_workspace.persistent_index() { - if active_index >= self.workspaces.len() { - self.active_workspace = ActiveWorkspace::Persistent(self.workspaces.len() - 1); - } else if active_index > index { - self.active_workspace = ActiveWorkspace::Persistent(active_index - 1); + if !should_continue { + return Ok(false); } } - self.detach_workspace(&removed_workspace, cx); - - cx.emit(MultiWorkspaceEvent::WorkspaceRemoved( - removed_workspace.entity_id(), - )); - cx.emit(MultiWorkspaceEvent::ActiveWorkspaceChanged); - } - - let key_still_in_use = self - .workspaces - .iter() - .any(|ws| ws.read(cx).project_group_key(cx) == old_key); - - if !key_still_in_use { - self.project_group_keys.retain(|k| k != &old_key); - } - - self.serialize(cx); - self.focus_active_workspace(window, cx); - cx.notify(); - - true - } - - pub fn move_workspace_to_new_window( - &mut self, - workspace: &Entity, - window: &mut Window, - cx: &mut Context, - ) { - let workspace = workspace.clone(); - if !self.remove(&workspace, window, cx) { - return; - } - - let app_state: Arc = workspace.read(cx).app_state().clone(); - - cx.defer(move |cx| { - let options = (app_state.build_window_options)(None, cx); - - let Ok(window) = cx.open_window(options, |window, cx| { - cx.new(|cx| MultiWorkspace::new(workspace, window, cx)) - }) else { - return; - }; - - let _ = window.update(cx, |_, window, _| { - window.activate_window(); - }); - }); - } - - pub fn move_project_group_to_new_window( - &mut self, - key: &ProjectGroupKey, - window: &mut Window, - cx: &mut Context, - ) { - let workspaces: Vec<_> = self - .workspaces_for_project_group(key, cx) - .cloned() - .collect(); - if workspaces.is_empty() { - return; - } - - self.project_group_keys.retain(|k| k != key); - - let mut removed = Vec::new(); - for workspace in &workspaces { - if self.remove(workspace, window, cx) { - removed.push(workspace.clone()); + // If we're removing the active workspace, await the + // fallback and switch to it before tearing anything down. + // Otherwise restore the original active workspace in case + // prompting switched away from it. + if let Some(fallback_task) = fallback_task { + let new_active = fallback_task.await?; + + this.update_in(cx, |this, window, cx| { + assert!( + !workspaces.contains(&new_active), + "fallback workspace must not be one of the workspaces being removed" + ); + this.activate(new_active, window, cx); + })?; + } else { + this.update_in(cx, |this, window, cx| { + if *this.workspace() != original_active { + this.activate(original_active, window, cx); + } + })?; } - } - if removed.is_empty() { - return; - } + // Actually remove the workspaces. + this.update_in(cx, |this, _, cx| { + // Save a handle to the active workspace so we can restore + // its index after the removals shift the vec around. + let active_workspace = this.workspace().clone(); - let app_state = removed[0].read(cx).app_state().clone(); + let mut removed_workspaces: Vec> = Vec::new(); - cx.defer(move |cx| { - let options = (app_state.build_window_options)(None, cx); + this.workspaces.retain(|ws| { + if workspaces.contains(ws) { + removed_workspaces.push(ws.clone()); + false + } else { + true + } + }); - let first = removed[0].clone(); - let rest = removed[1..].to_vec(); + for workspace in &removed_workspaces { + this.detach_workspace(workspace, cx); + cx.emit(MultiWorkspaceEvent::WorkspaceRemoved(workspace.entity_id())); + } - let Ok(new_window) = cx.open_window(options, |window, cx| { - cx.new(|cx| MultiWorkspace::new(first, window, cx)) - }) else { - return; - }; + let removed_any = !removed_workspaces.is_empty(); - new_window - .update(cx, |mw, window, cx| { - for workspace in rest { - mw.activate(workspace, window, cx); + if removed_any { + // Restore the active workspace index after removals. + if let Some(new_index) = this + .workspaces + .iter() + .position(|ws| ws == &active_workspace) + { + this.active_workspace = ActiveWorkspace::Persistent(new_index); } - window.activate_window(); - }) - .log_err(); - }); + + this.serialize(cx); + cx.notify(); + } + + Ok(removed_any) + })? + }) } pub fn open_project( @@ -1528,17 +1489,10 @@ impl Render for MultiWorkspace { sidebar.cycle_thread(true, window, cx); } })) - .on_action( - cx.listener(|this: &mut Self, _: &PreviousThread, window, cx| { - if let Some(sidebar) = &this.sidebar { - sidebar.cycle_thread(false, window, cx); - } - }), - ) .on_action(cx.listener( - |this: &mut Self, _: &MoveWorkspaceToNewWindow, window, cx| { + |this: &mut Self, _: &PreviousThread, window, cx| { if let Some(sidebar) = &this.sidebar { - sidebar.move_workspace_to_new_window(window, cx); + sidebar.cycle_thread(false, window, cx); } }, )) diff --git a/crates/workspace/src/multi_workspace_tests.rs b/crates/workspace/src/multi_workspace_tests.rs index 54677a433dc18624f6ca5c8ca152ab90b4c167e8..259346fe097826b3dcc19fb8fad0b8f07ddd0488 100644 --- a/crates/workspace/src/multi_workspace_tests.rs +++ b/crates/workspace/src/multi_workspace_tests.rs @@ -147,8 +147,8 @@ async fn test_project_group_keys_add_workspace(cx: &mut TestAppContext) { 2, "should have two keys after adding a second workspace" ); - assert_eq!(*keys[0], key_a); - assert_eq!(*keys[1], key_b); + assert_eq!(*keys[0], key_b); + assert_eq!(*keys[1], key_a); }); } @@ -228,8 +228,8 @@ async fn test_project_group_keys_on_worktree_added(cx: &mut TestAppContext) { 2, "should have both the original and updated key" ); - assert_eq!(*keys[0], initial_key); - assert_eq!(*keys[1], updated_key); + assert_eq!(*keys[0], updated_key); + assert_eq!(*keys[1], initial_key); }); } @@ -277,8 +277,8 @@ async fn test_project_group_keys_on_worktree_removed(cx: &mut TestAppContext) { 2, "should accumulate both the original and post-removal key" ); - assert_eq!(*keys[0], initial_key); - assert_eq!(*keys[1], updated_key); + assert_eq!(*keys[0], updated_key); + assert_eq!(*keys[1], initial_key); }); } @@ -334,8 +334,8 @@ async fn test_project_group_keys_across_multiple_workspaces_and_worktree_changes 3, "should have key_a, key_b, and the updated key_a with root_c" ); - assert_eq!(*keys[0], key_a); + assert_eq!(*keys[0], key_a_updated); assert_eq!(*keys[1], key_b); - assert_eq!(*keys[2], key_a_updated); + assert_eq!(*keys[2], key_a); }); } diff --git a/crates/workspace/src/persistence.rs b/crates/workspace/src/persistence.rs index a0e64c223deeb5c6f7f536f8365f495a866eb521..67383740a8b3287bb237748776b0c7ab2654d7ba 100644 --- a/crates/workspace/src/persistence.rs +++ b/crates/workspace/src/persistence.rs @@ -2565,10 +2565,11 @@ mod tests { the newly activated workspace's database id" ); - // --- Remove the second workspace (index 1) --- + // --- Remove the first workspace (index 0, which is not the active one) --- multi_workspace.update_in(cx, |mw, window, cx| { - let ws = mw.workspaces().nth(1).unwrap().clone(); - mw.remove(&ws, window, cx); + let ws = mw.workspaces().nth(0).unwrap().clone(); + mw.remove([ws], |_, _, _| unreachable!(), window, cx) + .detach_and_log_err(cx); }); cx.run_until_parked(); @@ -4209,7 +4210,7 @@ mod tests { workspace.update(cx, |ws: &mut crate::Workspace, _cx| { ws.set_database_id(workspace2_db_id) }); - mw.activate(workspace.clone(), window, cx); + mw.add(workspace.clone(), window, cx); }); // Save a full workspace row to the DB directly. @@ -4238,7 +4239,8 @@ mod tests { // Remove workspace at index 1 (the second workspace). multi_workspace.update_in(cx, |mw, window, cx| { let ws = mw.workspaces().nth(1).unwrap().clone(); - mw.remove(&ws, window, cx); + mw.remove([ws], |_, _, _| unreachable!(), window, cx) + .detach_and_log_err(cx); }); cx.run_until_parked(); @@ -4306,7 +4308,7 @@ mod tests { workspace.update(cx, |ws: &mut crate::Workspace, _cx| { ws.set_database_id(ws2_id) }); - mw.activate(workspace.clone(), window, cx); + mw.add(workspace.clone(), window, cx); }); let session_id = "test-zombie-session"; @@ -4347,7 +4349,8 @@ mod tests { // Remove workspace2 (index 1). multi_workspace.update_in(cx, |mw, window, cx| { let ws = mw.workspaces().nth(1).unwrap().clone(); - mw.remove(&ws, window, cx); + mw.remove([ws], |_, _, _| unreachable!(), window, cx) + .detach_and_log_err(cx); }); cx.run_until_parked(); @@ -4404,7 +4407,7 @@ mod tests { workspace.update(cx, |ws: &mut crate::Workspace, _cx| { ws.set_database_id(workspace2_db_id) }); - mw.activate(workspace.clone(), window, cx); + mw.add(workspace.clone(), window, cx); }); // Save a full workspace row to the DB directly and let it settle. @@ -4429,7 +4432,8 @@ mod tests { // Remove workspace2 — this pushes a task to pending_removal_tasks. multi_workspace.update_in(cx, |mw, window, cx| { let ws = mw.workspaces().nth(1).unwrap().clone(); - mw.remove(&ws, window, cx); + mw.remove([ws], |_, _, _| unreachable!(), window, cx) + .detach_and_log_err(cx); }); // Simulate the quit handler pattern: collect flush tasks + pending @@ -4849,8 +4853,8 @@ mod tests { .map(Into::into) .collect(); let expected_keys = vec![ - ProjectGroupKey::new(None, PathList::new(&["/other-project"])), ProjectGroupKey::new(None, PathList::new(&["/repo"])), + ProjectGroupKey::new(None, PathList::new(&["/other-project"])), ]; assert_eq!( restored_keys, expected_keys, @@ -4903,4 +4907,111 @@ mod tests { "The restored active workspace should be the linked worktree project" ); } + + #[gpui::test] + async fn test_remove_project_group_falls_back_to_neighbor(cx: &mut gpui::TestAppContext) { + crate::tests::init_test(cx); + cx.update(|cx| { + cx.set_staff(true); + cx.update_flags(true, vec!["agent-v2".to_string()]); + }); + + let fs = fs::FakeFs::new(cx.executor()); + let dir_a = unique_test_dir(&fs, "group-a").await; + let dir_b = unique_test_dir(&fs, "group-b").await; + let dir_c = unique_test_dir(&fs, "group-c").await; + + let project_a = Project::test(fs.clone(), [dir_a.as_path()], cx).await; + let project_b = Project::test(fs.clone(), [dir_b.as_path()], cx).await; + let project_c = Project::test(fs.clone(), [dir_c.as_path()], cx).await; + + // Create a multi-workspace with project A, then add B and C. + // project_group_keys stores newest first: [C, B, A]. + // Sidebar displays in the same order: C (top), B (middle), A (bottom). + let (multi_workspace, cx) = cx + .add_window_view(|window, cx| MultiWorkspace::test_new(project_a.clone(), window, cx)); + + multi_workspace.update(cx, |mw, cx| mw.open_sidebar(cx)); + + let workspace_b = multi_workspace.update_in(cx, |mw, window, cx| { + mw.test_add_workspace(project_b.clone(), window, cx) + }); + let _workspace_c = multi_workspace.update_in(cx, |mw, window, cx| { + mw.test_add_workspace(project_c.clone(), window, cx) + }); + cx.run_until_parked(); + + let key_a = project_a.read_with(cx, |p, cx| p.project_group_key(cx)); + let key_b = project_b.read_with(cx, |p, cx| p.project_group_key(cx)); + let key_c = project_c.read_with(cx, |p, cx| p.project_group_key(cx)); + + // Activate workspace B so removing its group exercises the fallback. + multi_workspace.update_in(cx, |mw, window, cx| { + mw.activate(workspace_b.clone(), window, cx); + }); + cx.run_until_parked(); + + // --- Remove group B (the middle one). --- + // In the sidebar [C, B, A], "below" B is A. + multi_workspace.update_in(cx, |mw, window, cx| { + mw.remove_project_group(&key_b, window, cx) + .detach_and_log_err(cx); + }); + cx.run_until_parked(); + + let active_paths = + multi_workspace.read_with(cx, |mw, cx| mw.workspace().read(cx).root_paths(cx)); + assert_eq!( + active_paths + .iter() + .map(|p| p.to_path_buf()) + .collect::>(), + vec![dir_a.clone()], + "After removing the middle group, should fall back to the group below (A)" + ); + + // After removing B, keys = [A, C], sidebar = [C, A]. + // Activate workspace A (the bottom) so removing it tests the + // "fall back upward" path. + let workspace_a = + multi_workspace.read_with(cx, |mw, _| mw.workspaces().next().cloned().unwrap()); + multi_workspace.update_in(cx, |mw, window, cx| { + mw.activate(workspace_a.clone(), window, cx); + }); + cx.run_until_parked(); + + // --- Remove group A (the bottom one in sidebar). --- + // Nothing below A, so should fall back upward to C. + multi_workspace.update_in(cx, |mw, window, cx| { + mw.remove_project_group(&key_a, window, cx) + .detach_and_log_err(cx); + }); + cx.run_until_parked(); + + let active_paths = + multi_workspace.read_with(cx, |mw, cx| mw.workspace().read(cx).root_paths(cx)); + assert_eq!( + active_paths + .iter() + .map(|p| p.to_path_buf()) + .collect::>(), + vec![dir_c.clone()], + "After removing the bottom group, should fall back to the group above (C)" + ); + + // --- Remove group C (the only one remaining). --- + // Should create an empty workspace. + multi_workspace.update_in(cx, |mw, window, cx| { + mw.remove_project_group(&key_c, window, cx) + .detach_and_log_err(cx); + }); + cx.run_until_parked(); + + let active_paths = + multi_workspace.read_with(cx, |mw, cx| mw.workspace().read(cx).root_paths(cx)); + assert!( + active_paths.is_empty(), + "After removing the only remaining group, should have an empty workspace" + ); + } } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 82080ab38c0d864ff6accdd91c79dc546bd0a0ec..3d1f67bf69fb818e6155782fdb4779365098cb20 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -31,10 +31,10 @@ mod workspace_settings; pub use crate::notifications::NotificationFrame; pub use dock::Panel; pub use multi_workspace::{ - CloseWorkspaceSidebar, DraggedSidebar, FocusWorkspaceSidebar, MoveWorkspaceToNewWindow, - MultiWorkspace, MultiWorkspaceEvent, NewThread, NextProject, NextThread, PreviousProject, - PreviousThread, ShowFewerThreads, ShowMoreThreads, Sidebar, SidebarEvent, SidebarHandle, - SidebarRenderState, SidebarSide, ToggleWorkspaceSidebar, sidebar_side_context_menu, + CloseWorkspaceSidebar, DraggedSidebar, FocusWorkspaceSidebar, MultiWorkspace, + MultiWorkspaceEvent, NewThread, NextProject, NextThread, PreviousProject, PreviousThread, + ShowFewerThreads, ShowMoreThreads, Sidebar, SidebarEvent, SidebarHandle, SidebarRenderState, + SidebarSide, ToggleWorkspaceSidebar, sidebar_side_context_menu, }; pub use path_list::{PathList, SerializedPathList}; pub use toast_layer::{ToastAction, ToastLayer, ToastView}; @@ -3374,6 +3374,7 @@ impl Workspace { if remaining_dirty_items.len() > 1 { let answer = workspace.update_in(cx, |_, window, cx| { + cx.emit(Event::Activate); let detail = Pane::file_names_for_prompt( &mut remaining_dirty_items.iter().map(|(_, handle)| handle), cx, @@ -10937,6 +10938,113 @@ mod tests { ); } + #[gpui::test] + async fn test_remove_workspace_prompts_for_unsaved_changes(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree("/root", json!({ "one": "" })).await; + + let project_a = Project::test(fs.clone(), ["root".as_ref()], cx).await; + let project_b = Project::test(fs.clone(), ["root".as_ref()], cx).await; + let multi_workspace_handle = + cx.add_window(|window, cx| MultiWorkspace::test_new(project_a.clone(), window, cx)); + cx.run_until_parked(); + + multi_workspace_handle + .update(cx, |mw, _window, cx| mw.open_sidebar(cx)) + .unwrap(); + + let workspace_a = multi_workspace_handle + .read_with(cx, |mw, _| mw.workspace().clone()) + .unwrap(); + + let workspace_b = multi_workspace_handle + .update(cx, |mw, window, cx| { + mw.test_add_workspace(project_b, window, cx) + }) + .unwrap(); + + // Activate workspace A. + multi_workspace_handle + .update(cx, |mw, window, cx| { + mw.activate(workspace_a.clone(), window, cx); + }) + .unwrap(); + + let cx = &mut VisualTestContext::from_window(multi_workspace_handle.into(), cx); + + // Workspace B has a dirty item. + let item_b = cx.new(|cx| TestItem::new(cx).with_dirty(true)); + workspace_b.update_in(cx, |w, window, cx| { + w.add_item_to_active_pane(Box::new(item_b.clone()), None, true, window, cx) + }); + + // Try to remove workspace B. It should prompt because of the dirty item. + let remove_task = multi_workspace_handle + .update(cx, |mw, window, cx| { + mw.remove([workspace_b.clone()], |_, _, _| unreachable!(), window, cx) + }) + .unwrap(); + cx.run_until_parked(); + + // The prompt should have activated workspace B. + multi_workspace_handle + .read_with(cx, |mw, _| { + assert_eq!( + mw.workspace(), + &workspace_b, + "workspace B should be active while prompting" + ); + }) + .unwrap(); + + // Cancel the prompt — user stays on workspace B. + cx.simulate_prompt_answer("Cancel"); + cx.run_until_parked(); + let removed = remove_task.await.unwrap(); + assert!(!removed, "removal should have been cancelled"); + + multi_workspace_handle + .read_with(cx, |mw, _| { + assert_eq!( + mw.workspace(), + &workspace_b, + "user should stay on workspace B after cancelling" + ); + assert_eq!(mw.workspaces().count(), 2, "both workspaces should remain"); + }) + .unwrap(); + + // Try again. This time accept the prompt. + let remove_task = multi_workspace_handle + .update(cx, |mw, window, cx| { + // First switch back to A. + mw.activate(workspace_a.clone(), window, cx); + mw.remove([workspace_b.clone()], |_, _, _| unreachable!(), window, cx) + }) + .unwrap(); + cx.run_until_parked(); + + // Accept the save prompt. + cx.simulate_prompt_answer("Don't Save"); + cx.run_until_parked(); + let removed = remove_task.await.unwrap(); + assert!(removed, "removal should have succeeded"); + + // Should be back on workspace A, and B should be gone. + multi_workspace_handle + .read_with(cx, |mw, _| { + assert_eq!( + mw.workspace(), + &workspace_a, + "should be back on workspace A after removing B" + ); + assert_eq!(mw.workspaces().count(), 1, "only workspace A should remain"); + }) + .unwrap(); + } + #[gpui::test] async fn test_close_window_with_serializable_items(cx: &mut TestAppContext) { init_test(cx); diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 40d87e68787103bdc4d281748577ed6db5e9a5ad..9a8c11530d4c18ba43285db7ba8fe5c0fa707801 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -6172,8 +6172,8 @@ mod tests { assert_eq!( mw.project_group_keys().cloned().collect::>(), vec![ - ProjectGroupKey::new(None, PathList::new(&[dir1])), ProjectGroupKey::new(None, PathList::new(&[dir2])), + ProjectGroupKey::new(None, PathList::new(&[dir1])), ] ); assert_eq!(mw.workspaces().count(), 1);