diff --git a/crates/git_ui_core/src/worktree_picker.rs b/crates/git_ui_core/src/worktree_picker.rs index 67ddbe8c23..21ed8b8c07 100644 --- a/crates/git_ui_core/src/worktree_picker.rs +++ b/crates/git_ui_core/src/worktree_picker.rs @@ -20,7 +20,8 @@ use ui::{ use util::ResultExt as _; use util::paths::PathExt; use workspace::{ - ModalView, MultiWorkspace, Workspace, dock::DockPosition, notifications::DetachAndPromptErr, + ModalView, MultiWorkspace, RemovalIntent, Workspace, dock::DockPosition, + notifications::DetachAndPromptErr, }; use crate::notifications::show_error_toast; @@ -448,19 +449,17 @@ impl WorktreePickerDelegate { && let Some(workspace) = self.workspace.upgrade() { let group_key = workspace.read(cx).project_group_key(cx); - if let Some(group_workspaces) = multi_workspace + for group_workspace in multi_workspace .read(cx) .workspaces_for_project_group(&group_key, cx) { - for group_workspace in group_workspaces { - for worktree in group_workspace - .read(cx) - .project() - .read(cx) - .visible_worktrees(cx) - { - paths.insert(worktree.read(cx).abs_path().to_path_buf()); - } + for worktree in group_workspace + .read(cx) + .project() + .read(cx) + .visible_worktrees(cx) + { + paths.insert(worktree.read(cx).abs_path().to_path_buf()); } } } @@ -660,7 +659,7 @@ impl WorktreePickerDelegate { let group_key = workspace.read(cx).project_group_key(cx); multi_workspace .read(cx) - .workspaces_for_project_group(&group_key, cx)? + .workspaces_for_project_group(&group_key, cx) .into_iter() .find(|group_workspace| { *group_workspace != workspace @@ -693,7 +692,12 @@ impl WorktreePickerDelegate { cx.spawn_in(window, async move |picker, cx| { let removed = window_handle .update(cx, |multi_workspace, window, cx| { - multi_workspace.close_workspace(&workspace_to_remove, window, cx) + multi_workspace.remove( + [workspace_to_remove.clone()], + RemovalIntent::CloseProject, + window, + cx, + ) })? .await?; diff --git a/crates/git_ui_core/src/worktree_service.rs b/crates/git_ui_core/src/worktree_service.rs index fece13da04..fa5c1764dd 100644 --- a/crates/git_ui_core/src/worktree_service.rs +++ b/crates/git_ui_core/src/worktree_service.rs @@ -1194,7 +1194,7 @@ async fn open_worktree_workspace( None }; - let task = multi_workspace.find_or_create_workspace_with_source_workspace( + let task = multi_workspace.find_or_create_workspace( path_list, remote_connection_options, None, @@ -1206,7 +1206,6 @@ async fn open_worktree_workspace( cx, ) }, - &[], init, OpenMode::Add, source_for_transfer.clone(), @@ -1355,7 +1354,7 @@ async fn open_worktree_workspace( } else { // Background open: register the new workspace as a retained tab // but leave the user where they are. - multi_workspace.add_background_workspace(new_workspace.clone(), window, cx); + multi_workspace.add(new_workspace.clone(), window, cx); } if is_creating_new_worktree { diff --git a/crates/recent_projects/src/recent_projects.rs b/crates/recent_projects/src/recent_projects.rs index eb7aa8d1f4..c26d7537b5 100644 --- a/crates/recent_projects/src/recent_projects.rs +++ b/crates/recent_projects/src/recent_projects.rs @@ -1183,9 +1183,9 @@ impl PickerDelegate for RecentProjectsDelegate { cx, ) }, - &[], None, OpenMode::Activate, + None, window, cx, ) diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index d82584c7cb..a98997468f 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -64,8 +64,8 @@ use util::ResultExt as _; use util::path_list::PathList; use workspace::{ CloseWindow, FocusWorkspaceSidebar, MultiWorkspace, MultiWorkspaceEvent, NextProject, - NextThread, Open, OpenMode, PreviousProject, PreviousThread, ProjectGroupKey, SaveIntent, - Sidebar as WorkspaceSidebar, SidebarSide, Toast, ToggleWorkspaceSidebar, Workspace, + NextThread, Open, OpenMode, PreviousProject, PreviousThread, ProjectGroupKey, RemovalIntent, + SaveIntent, Sidebar as WorkspaceSidebar, SidebarSide, Toast, ToggleWorkspaceSidebar, Workspace, notifications::NotificationId, sidebar_side_context_menu, }; @@ -408,7 +408,6 @@ enum ListEntry { enum ActivatableEntry { Thread { metadata: ThreadMetadata, - workspace: ThreadEntryWorkspace, }, Terminal { metadata: TerminalThreadMetadata, @@ -421,7 +420,6 @@ impl ActivatableEntry { match entry { ListEntry::Thread(thread) => Some(Self::Thread { metadata: thread.metadata.clone(), - workspace: thread.workspace.clone(), }), ListEntry::Terminal(terminal) => Some(Self::Terminal { metadata: terminal.metadata.clone(), @@ -430,38 +428,6 @@ impl ActivatableEntry { ListEntry::ProjectHeader { .. } => None, } } - - fn project_location(&self, cx: &App) -> (PathList, ProjectGroupKey) { - match self { - Self::Thread { - workspace: ThreadEntryWorkspace::Open(workspace), - .. - } - | Self::Terminal { - workspace: ThreadEntryWorkspace::Open(workspace), - .. - } => ( - PathList::new(&workspace.read(cx).root_paths(cx)), - workspace.read(cx).project_group_key(cx), - ), - Self::Thread { - workspace: - ThreadEntryWorkspace::Closed { - folder_paths, - project_group_key, - }, - .. - } - | Self::Terminal { - workspace: - ThreadEntryWorkspace::Closed { - folder_paths, - project_group_key, - }, - .. - } => (folder_paths.clone(), project_group_key.clone()), - } - } } #[cfg(test)] @@ -487,9 +453,9 @@ impl ListEntry { ThreadEntryWorkspace::Open(workspace) => vec![workspace.clone()], ThreadEntryWorkspace::Closed { .. } => Vec::new(), }, - ListEntry::ProjectHeader { key, .. } => multi_workspace - .workspaces_for_project_group(key, cx) - .unwrap_or_default(), + ListEntry::ProjectHeader { key, .. } => { + multi_workspace.workspaces_for_project_group(key, cx) + } } } } @@ -1300,9 +1266,9 @@ impl Sidebar { host, provisional_key, |options, window, cx| connect_remote(active_workspace, options, window, cx), - &[], None, OpenMode::Activate, + None, window, cx, ) @@ -1339,9 +1305,9 @@ impl Sidebar { host, provisional_key, |options, window, cx| connect_remote(active_workspace, options, window, cx), - &[], None, OpenMode::Activate, + None, window, cx, ) @@ -2539,7 +2505,7 @@ impl Sidebar { let open_workspaces = self .multi_workspace .upgrade() - .and_then(|mw| mw.read(cx).workspaces_for_project_group(key, cx)) + .map(|mw| mw.read(cx).workspaces_for_project_group(key, cx)) .unwrap_or_default(); if open_workspaces.is_empty() { @@ -2761,7 +2727,8 @@ impl Sidebar { let Some(base) = multi_workspace .read(cx) .workspaces_for_project_group(&key, cx) - .and_then(|workspaces| workspaces.first().cloned()) + .first() + .cloned() else { continue; }; @@ -2856,9 +2823,7 @@ impl Sidebar { let open_workspaces = multi_workspace .read_with(cx, |multi_workspace, cx| { - multi_workspace - .workspaces_for_project_group(&project_group_key, cx) - .unwrap_or_default() + multi_workspace.workspaces_for_project_group(&project_group_key, cx) }) .unwrap_or_default(); @@ -3057,8 +3022,9 @@ impl Sidebar { close_multi_workspace .update(cx, |multi_workspace, cx| { multi_workspace - .close_workspace( - &close_workspace, + .remove( + [close_workspace.clone()], + RemovalIntent::CloseProject, window, cx, ) @@ -4011,9 +3977,9 @@ impl Sidebar { host, provisional_key, |options, window, cx| connect_remote(active_workspace, options, window, cx), - &[], None, OpenMode::Activate, + None, window, cx, ) @@ -4415,21 +4381,43 @@ impl Sidebar { } } - /// 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). + /// Find the entry to select after the entry at `current_position` is + /// removed: the nearest activatable entry in the same project section, + /// below first, then above. Only when that section has no other + /// activatable entry, the nearest one in the whole list. fn neighboring_activatable_entry(&self, current_position: usize) -> Option { - let after = self - .contents - .entries - .get(current_position.checked_add(1)?..)?; - let before = self.contents.entries.get(..current_position)?; - after + let entries = &self.contents.entries; + let is_header = |entry: &ListEntry| matches!(entry, ListEntry::ProjectHeader { .. }); + + let section_start = entries + .get(..current_position)? .iter() - .chain(before.iter().rev()) - .find_map(ActivatableEntry::from_list_entry) + .rposition(is_header) + .map_or(0, |header| header + 1); + let section_end = entries + .get(current_position + 1..)? + .iter() + .position(is_header) + .map_or(entries.len(), |offset| current_position + 1 + offset); + + for (start, end) in [(section_start, section_end), (0, entries.len())] { + let Some(before) = entries.get(start..current_position) else { + continue; + }; + let Some(after) = entries.get(current_position + 1..end) else { + continue; + }; + + let Some(entry) = after + .iter() + .chain(before.iter().rev()) + .find_map(ActivatableEntry::from_list_entry) + else { + continue; + }; + return Some(entry); + } + None } fn activate_entry( @@ -4622,9 +4610,9 @@ impl Sidebar { host, provisional_key, |options, window, cx| connect_remote(active_workspace, options, window, cx), - &[], None, OpenMode::Activate, + None, window, cx, ) @@ -4913,15 +4901,18 @@ impl Sidebar { } } + /// Closed linked-worktree entries need an open workspace so archive root + /// planning can inspect repositories before deleting the worktree. fn open_workspace_for_archive( &mut self, folder_paths: PathList, project_group_key: ProjectGroupKey, window: &mut Window, cx: &mut Context, - ) -> Option<(Task>>, Entity)> { + then: impl FnOnce(&mut Self, Entity, &mut Window, &mut Context) + 'static, + ) { let Some(multi_workspace) = self.multi_workspace.upgrade() else { - return None; + return; }; let host = project_group_key.host(); @@ -4934,70 +4925,21 @@ impl Sidebar { host, Some(project_group_key), |options, window, cx| connect_remote(active_workspace, options, window, cx), - &[], None, OpenMode::Add, + None, window, cx, ) }); - Some((open_task, modal_workspace)) - } - - fn open_workspace_and_archive_thread( - &mut self, - session_id: acp::SessionId, - folder_paths: PathList, - project_group_key: ProjectGroupKey, - window: &mut Window, - cx: &mut Context, - ) { - let Some((open_task, modal_workspace)) = - self.open_workspace_for_archive(folder_paths, project_group_key, window, cx) - else { - return; - }; - cx.spawn_in(window, async move |this, cx| { let result = open_task.await; remote_connection::dismiss_connection_modal(&modal_workspace, cx); let workspace = result?; Self::wait_for_archive_workspace_metadata(&workspace, cx).await; - this.update_in(cx, |this, window, cx| { - this.update_entries(cx); - this.archive_thread(&session_id, window, cx); - })?; - anyhow::Ok(()) - }) - .detach_and_log_err(cx); - } - - fn open_workspace_and_close_terminal( - &mut self, - metadata: TerminalThreadMetadata, - folder_paths: PathList, - project_group_key: ProjectGroupKey, - window: &mut Window, - cx: &mut Context, - ) { - let Some((open_task, modal_workspace)) = - self.open_workspace_for_archive(folder_paths, project_group_key, window, cx) - else { - return; - }; - - cx.spawn_in(window, async move |this, cx| { - let result = open_task.await; - remote_connection::dismiss_connection_modal(&modal_workspace, cx); - let workspace = result?; - Self::wait_for_archive_workspace_metadata(&workspace, cx).await; - - this.update_in(cx, |this, window, cx| { - let workspace = ThreadEntryWorkspace::Open(workspace); - this.close_terminal(&metadata, &workspace, window, cx); - })?; + this.update_in(cx, |this, window, cx| then(this, workspace, window, cx))?; anyhow::Ok(()) }) .detach_and_log_err(cx); @@ -5023,12 +4965,20 @@ impl Sidebar { cx, ) { - self.open_workspace_and_close_terminal( - metadata.clone(), + let metadata = metadata.clone(); + self.open_workspace_for_archive( folder_paths.clone(), project_group_key.clone(), window, cx, + move |this, workspace, window, cx| { + this.close_terminal( + &metadata, + &ThreadEntryWorkspace::Open(workspace), + window, + cx, + ); + }, ); return; } @@ -5078,123 +5028,40 @@ impl Sidebar { cx, ); - if !workspaces_to_remove.is_empty() { - let multi_workspace = self.multi_workspace.upgrade().unwrap(); - let terminal_workspace_removed = matches!( - workspace, - ThreadEntryWorkspace::Open(workspace) if workspaces_to_remove.contains(workspace) - ); - let (fallback_paths, project_group_key) = neighbor - .as_ref() - .map(|neighbor| neighbor.project_location(cx)) - .unwrap_or_else(|| { - workspaces_to_remove - .first() - .map(|workspace| { - let key = workspace.read(cx).project_group_key(cx); - (key.path_list().clone(), key) - }) - .unwrap_or_default() - }); + let terminal_workspace_removed = matches!( + workspace, + ThreadEntryWorkspace::Open(workspace) if workspaces_to_remove.contains(workspace) + ); + let metadata = metadata.clone(); + let workspace = workspace.clone(); - let excluded = workspaces_to_remove.clone(); - let remove_task = multi_workspace.update(cx, |multi_workspace, cx| { - multi_workspace.remove( - workspaces_to_remove, - move |this, window, cx| { - let active_workspace = this.workspace().clone(); - this.find_or_create_workspace( - fallback_paths, - project_group_key.host(), - Some(project_group_key), - |options, window, cx| { - connect_remote(active_workspace, options, window, cx) - }, - &excluded, - None, - OpenMode::Activate, - window, - cx, - ) - }, + self.remove_workspaces_then( + workspaces_to_remove, + close_item_tasks, + window, + cx, + move |this, window, cx| { + if terminal_workspace_removed { + this.delete_empty_drafts_for_archive_paths( + metadata.folder_paths(), + metadata.remote_connection.as_ref(), + cx, + ); + } + // If the terminal's workspace has already been removed, don't + // synthesize a fallback draft in the detached AgentPanel. + this.close_terminal_entry( + &metadata, + &workspace, + is_active, + neighbor.as_ref(), + !terminal_workspace_removed, + roots_to_archive, window, cx, - ) - }); - - let metadata = metadata.clone(); - let workspace = workspace.clone(); - cx.spawn_in(window, async move |this, cx| { - if !remove_task.await? { - return anyhow::Ok(()); - } - - for task in close_item_tasks { - let result: anyhow::Result<()> = task.await; - result.log_err(); - } - - this.update_in(cx, |this, window, cx| { - if terminal_workspace_removed { - this.delete_empty_drafts_for_archive_paths( - metadata.folder_paths(), - metadata.remote_connection.as_ref(), - cx, - ); - } - // If the terminal's workspace has already been removed, - // don't synthesize a fallback draft in the detached - // AgentPanel. - this.close_terminal_entry( - &metadata, - &workspace, - is_active, - neighbor.as_ref(), - !terminal_workspace_removed, - roots_to_archive, - window, - cx, - ); - })?; - anyhow::Ok(()) - }) - .detach_and_log_err(cx); - } else if !close_item_tasks.is_empty() { - let metadata = metadata.clone(); - let workspace = workspace.clone(); - cx.spawn_in(window, async move |this, cx| { - for task in close_item_tasks { - let result: anyhow::Result<()> = task.await; - result.log_err(); - } - - this.update_in(cx, |this, window, cx| { - this.close_terminal_entry( - &metadata, - &workspace, - is_active, - neighbor.as_ref(), - true, - roots_to_archive, - window, - cx, - ); - })?; - anyhow::Ok(()) - }) - .detach_and_log_err(cx); - } else { - self.close_terminal_entry( - metadata, - workspace, - is_active, - neighbor.as_ref(), - true, - roots_to_archive, - window, - cx, - ); - } + ); + }, + ); } fn close_terminal_entry( @@ -5256,6 +5123,48 @@ impl Sidebar { self.update_entries(cx); } + fn remove_workspaces_then( + &mut self, + workspaces_to_remove: Vec>, + close_item_tasks: Vec>>, + window: &mut Window, + cx: &mut Context, + finish: impl FnOnce(&mut Self, &mut Window, &mut Context) + 'static, + ) { + if workspaces_to_remove.is_empty() && close_item_tasks.is_empty() { + finish(self, window, cx); + return; + } + + let remove_task = if workspaces_to_remove.is_empty() { + None + } else { + let Some(multi_workspace) = self.multi_workspace.upgrade() else { + return; + }; + Some(multi_workspace.update(cx, |multi_workspace, cx| { + multi_workspace.remove(workspaces_to_remove, RemovalIntent::KeepProject, window, cx) + })) + }; + + cx.spawn_in(window, async move |this, cx| { + if let Some(remove_task) = remove_task + && !remove_task.await? + { + return anyhow::Ok(()); + } + + for task in close_item_tasks { + let result: anyhow::Result<()> = task.await; + result.log_err(); + } + + this.update_in(cx, |this, window, cx| finish(this, window, cx))?; + anyhow::Ok(()) + }) + .detach_and_log_err(cx); + } + fn close_items_for_archived_worktrees( &self, roots_to_archive: &[thread_worktree_archive::RootPlan], @@ -5397,12 +5306,16 @@ impl Sidebar { cx, ) { - self.open_workspace_and_archive_thread( - session_id.clone(), + let session_id = session_id.clone(); + self.open_workspace_for_archive( folder_paths, project_group_key, window, cx, + move |this, _workspace, window, cx| { + this.update_entries(cx); + this.archive_thread(&session_id, window, cx); + }, ); return; } @@ -5466,133 +5379,40 @@ impl Sidebar { cx, ); - if !workspaces_to_remove.is_empty() { - let multi_workspace = self.multi_workspace.upgrade().unwrap(); - let session_id = session_id.clone(); + let removed_workspace = !workspaces_to_remove.is_empty(); + let session_id = session_id.clone(); + let thread_remote_connection = metadata + .as_ref() + .and_then(|metadata| metadata.remote_connection.clone()); - let (fallback_paths, project_group_key) = neighbor - .as_ref() - .map(|neighbor| neighbor.project_location(cx)) - .unwrap_or_else(|| { - workspaces_to_remove - .first() - .map(|workspace| { - let key = workspace.read(cx).project_group_key(cx); - (key.path_list().clone(), key) - }) - .unwrap_or_default() - }); - - let excluded = workspaces_to_remove.clone(); - let remove_task = multi_workspace.update(cx, |mw, cx| { - mw.remove( - workspaces_to_remove, - move |this, window, cx| { - let active_workspace = this.workspace().clone(); - this.find_or_create_workspace( - fallback_paths, - project_group_key.host(), - Some(project_group_key), - |options, window, cx| { - connect_remote(active_workspace, options, window, cx) - }, - &excluded, - None, - OpenMode::Activate, - window, - cx, - ) - }, + self.remove_workspaces_then( + workspaces_to_remove, + close_item_tasks, + window, + cx, + move |this, window, cx| { + if removed_workspace && let Some(thread_folder_paths) = thread_folder_paths.as_ref() + { + this.delete_empty_drafts_for_archive_paths( + thread_folder_paths, + thread_remote_connection.as_ref(), + cx, + ); + } + let in_flight = thread_id + .and_then(|tid| this.start_archive_worktree_task(tid, roots_to_archive, cx)); + this.archive_and_activate( + &session_id, + thread_id, + neighbor.as_ref(), + thread_folder_paths.as_ref(), + thread_remote_connection.as_ref(), + in_flight, window, cx, - ) - }); - - let thread_folder_paths = thread_folder_paths.clone(); - let thread_remote_connection = metadata - .as_ref() - .and_then(|metadata| metadata.remote_connection.clone()); - cx.spawn_in(window, async move |this, cx| { - if !remove_task.await? { - return anyhow::Ok(()); - } - - for task in close_item_tasks { - let result: anyhow::Result<()> = task.await; - result.log_err(); - } - - this.update_in(cx, |this, window, cx| { - if let Some(thread_folder_paths) = thread_folder_paths.as_ref() { - this.delete_empty_drafts_for_archive_paths( - thread_folder_paths, - thread_remote_connection.as_ref(), - cx, - ); - } - let in_flight = thread_id.and_then(|tid| { - this.start_archive_worktree_task(tid, roots_to_archive, cx) - }); - this.archive_and_activate( - &session_id, - thread_id, - neighbor.as_ref(), - thread_folder_paths.as_ref(), - thread_remote_connection.as_ref(), - in_flight, - window, - cx, - ); - })?; - anyhow::Ok(()) - }) - .detach_and_log_err(cx); - } else if !close_item_tasks.is_empty() { - let session_id = session_id.clone(); - let thread_folder_paths = thread_folder_paths.clone(); - let thread_remote_connection = metadata - .as_ref() - .and_then(|metadata| metadata.remote_connection.clone()); - cx.spawn_in(window, async move |this, cx| { - for task in close_item_tasks { - let result: anyhow::Result<()> = task.await; - result.log_err(); - } - - this.update_in(cx, |this, window, cx| { - let in_flight = thread_id.and_then(|tid| { - this.start_archive_worktree_task(tid, roots_to_archive, cx) - }); - this.archive_and_activate( - &session_id, - thread_id, - neighbor.as_ref(), - thread_folder_paths.as_ref(), - thread_remote_connection.as_ref(), - in_flight, - window, - cx, - ); - })?; - anyhow::Ok(()) - }) - .detach_and_log_err(cx); - } else { - let in_flight = thread_id - .and_then(|tid| self.start_archive_worktree_task(tid, roots_to_archive, cx)); - self.archive_and_activate( - session_id, - thread_id, - neighbor.as_ref(), - thread_folder_paths.as_ref(), - metadata - .as_ref() - .and_then(|metadata| metadata.remote_connection.as_ref()), - in_flight, - window, - cx, - ); - } + ); + }, + ); } /// Archive a thread and activate the nearest neighbor or a draft. @@ -6831,37 +6651,6 @@ impl Sidebar { } } - /// Closed linked-worktree drafts need an open workspace so archive root - /// planning can inspect repositories before deleting the worktree. - fn open_workspace_and_remove_draft( - &mut self, - draft_id: ThreadId, - folder_paths: PathList, - project_group_key: ProjectGroupKey, - window: &mut Window, - cx: &mut Context, - ) { - let Some((open_task, modal_workspace)) = - self.open_workspace_for_archive(folder_paths, project_group_key, window, cx) - else { - return; - }; - - cx.spawn_in(window, async move |this, cx| { - let result = open_task.await; - remote_connection::dismiss_connection_modal(&modal_workspace, cx); - let workspace = result?; - Self::wait_for_archive_workspace_metadata(&workspace, cx).await; - - this.update_in(cx, |this, window, cx| { - let workspace = ThreadEntryWorkspace::Open(workspace); - this.remove_draft(draft_id, &workspace, window, cx); - })?; - anyhow::Ok(()) - }) - .detach_and_log_err(cx); - } - fn remove_draft( &mut self, draft_id: ThreadId, @@ -6889,12 +6678,14 @@ impl Sidebar { cx, ) { - self.open_workspace_and_remove_draft( - draft_id, + self.open_workspace_for_archive( folder_paths.clone(), project_group_key.clone(), window, cx, + move |this, workspace, window, cx| { + this.remove_draft(draft_id, &ThreadEntryWorkspace::Open(workspace), window, cx); + }, ); return; } @@ -6959,122 +6750,39 @@ impl Sidebar { cx, ); - if !workspaces_to_remove.is_empty() { - let Some(multi_workspace) = self.multi_workspace.upgrade() else { - return; - }; - let draft_workspace_removed = matches!( - workspace, - ThreadEntryWorkspace::Open(workspace) if workspaces_to_remove.contains(workspace) - ); - let (fallback_paths, project_group_key) = neighbor - .as_ref() - .map(|neighbor| neighbor.project_location(cx)) - .unwrap_or_else(|| { - workspaces_to_remove - .first() - .map(|workspace| { - let key = workspace.read(cx).project_group_key(cx); - (key.path_list().clone(), key) - }) - .unwrap_or_default() - }); + let draft_workspace_removed = matches!( + workspace, + ThreadEntryWorkspace::Open(workspace) if workspaces_to_remove.contains(workspace) + ); + let workspace = workspace.clone(); - let excluded = workspaces_to_remove.clone(); - let remove_task = multi_workspace.update(cx, |multi_workspace, cx| { - multi_workspace.remove( - workspaces_to_remove, - move |this, window, cx| { - let active_workspace = this.workspace().clone(); - this.find_or_create_workspace( - fallback_paths, - project_group_key.host(), - Some(project_group_key), - |options, window, cx| { - connect_remote(active_workspace, options, window, cx) - }, - &excluded, - None, - OpenMode::Activate, - window, - cx, - ) - }, + self.remove_workspaces_then( + workspaces_to_remove, + close_item_tasks, + window, + cx, + move |this, window, cx| { + if draft_workspace_removed + && let Some(draft_folder_paths) = draft_folder_paths.as_ref() + { + this.delete_empty_drafts_for_archive_paths( + draft_folder_paths, + draft_remote_connection.as_ref(), + cx, + ); + } + this.remove_draft_entry( + draft_id, + &workspace, + was_active, + neighbor.as_ref(), + !draft_workspace_removed, + roots_to_archive, window, cx, - ) - }); - - let workspace = workspace.clone(); - cx.spawn_in(window, async move |this, cx| { - if !remove_task.await? { - return anyhow::Ok(()); - } - - for task in close_item_tasks { - let result: anyhow::Result<()> = task.await; - result.log_err(); - } - - this.update_in(cx, |this, window, cx| { - if draft_workspace_removed { - if let Some(draft_folder_paths) = draft_folder_paths.as_ref() { - this.delete_empty_drafts_for_archive_paths( - draft_folder_paths, - draft_remote_connection.as_ref(), - cx, - ); - } - } - this.remove_draft_entry( - draft_id, - &workspace, - was_active, - neighbor.as_ref(), - !draft_workspace_removed, - roots_to_archive, - window, - cx, - ); - })?; - anyhow::Ok(()) - }) - .detach_and_log_err(cx); - } else if !close_item_tasks.is_empty() { - let workspace = workspace.clone(); - cx.spawn_in(window, async move |this, cx| { - for task in close_item_tasks { - let result: anyhow::Result<()> = task.await; - result.log_err(); - } - - this.update_in(cx, |this, window, cx| { - this.remove_draft_entry( - draft_id, - &workspace, - was_active, - neighbor.as_ref(), - true, - roots_to_archive, - window, - cx, - ); - })?; - anyhow::Ok(()) - }) - .detach_and_log_err(cx); - } else { - self.remove_draft_entry( - draft_id, - workspace, - was_active, - neighbor.as_ref(), - true, - roots_to_archive, - window, - cx, - ); - } + ); + }, + ); } fn remove_draft_entry( diff --git a/crates/sidebar/src/sidebar_tests.rs b/crates/sidebar/src/sidebar_tests.rs index b88b00281e..aff7eaff3f 100644 --- a/crates/sidebar/src/sidebar_tests.rs +++ b/crates/sidebar/src/sidebar_tests.rs @@ -1055,6 +1055,80 @@ async fn test_collapse_state_survives_worktree_key_change(cx: &mut TestAppContex ); } +#[gpui::test] +async fn test_neighboring_activatable_entry_stays_within_project(cx: &mut TestAppContext) { + let project = init_test_project("/my-project", cx).await; + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project, window, cx)); + let sidebar = setup_sidebar(&multi_workspace, cx); + let workspace = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()); + + let header = |path: &str| ListEntry::ProjectHeader { + key: ProjectGroupKey::new(None, PathList::new(&[std::path::PathBuf::from(path)])), + label: path.into(), + highlight_positions: Vec::new(), + has_running_threads: false, + waiting_thread_count: 0, + has_notifications: false, + is_active: false, + has_threads: true, + }; + let thread = |name: &str| { + ListEntry::Thread(Arc::new(ThreadEntry { + metadata: ThreadMetadata { + thread_id: ThreadId::new(), + session_id: Some(acp::SessionId::new(Arc::from(name))), + agent_id: AgentId::new("zed-agent"), + worktree_paths: WorktreePaths::default(), + title: Some(name.to_string().into()), + title_override: None, + updated_at: Utc::now(), + created_at: Some(Utc::now()), + interacted_at: None, + archived: false, + remote_connection: None, + }, + icon: IconName::ZedAgent, + icon_from_external_svg: None, + status: AgentThreadStatus::Completed, + workspace: ThreadEntryWorkspace::Open(workspace.clone()), + is_live: false, + is_background: false, + is_title_generating: false, + draft: None, + highlight_positions: Vec::new(), + worktrees: Vec::new(), + diff_stats: DiffStats::default(), + })) + }; + + sidebar.update_in(cx, |s, _window, _cx| { + s.contents.entries = vec![ + header("/project-a"), + thread("a-newest"), + thread("a-oldest"), + header("/project-b"), + thread("b-newest"), + ]; + + let neighbor_session = |position: usize| match s.neighboring_activatable_entry(position) { + Some(ActivatableEntry::Thread { metadata, .. }) => metadata.session_id, + _ => None, + }; + + assert_eq!( + neighbor_session(2), + Some(acp::SessionId::new(Arc::from("a-newest"))), + "the neighbor should be the sibling in the same project, not the next project's thread" + ); + assert_eq!( + neighbor_session(4), + Some(acp::SessionId::new(Arc::from("a-oldest"))), + "an empty project should fall back to another project" + ); + }); +} + #[gpui::test] async fn test_visible_entries_as_strings(cx: &mut TestAppContext) { use workspace::ProjectGroup; @@ -8236,6 +8310,7 @@ async fn test_archive_last_worktree_thread_removes_workspace(cx: &mut TestAppCon 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 main_workspace = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()); let _worktree_workspace = multi_workspace.update_in(cx, |mw, window, cx| { mw.test_add_workspace(worktree_project.clone(), window, cx) @@ -8265,6 +8340,38 @@ async fn test_archive_last_worktree_thread_removes_workspace(cx: &mut TestAppCon ); cx.run_until_parked(); + let remote_host = + remote::RemoteConnectionOptions::Mock(remote::MockConnectionOptions { id: 99 }); + multi_workspace.update(cx, |mw, _cx| { + mw.test_add_project_group(workspace::ProjectGroup { + key: ProjectGroupKey::new( + Some(remote_host.clone()), + PathList::new(&[PathBuf::from("/remote/project")]), + ), + workspaces: Vec::new(), + expanded: true, + }); + }); + cx.update(|_window, cx| { + let metadata = ThreadMetadata { + thread_id: ThreadId::new(), + session_id: Some(acp::SessionId::new(Arc::from("remote-thread"))), + agent_id: agent::ZED_AGENT_ID.clone(), + title: Some("Remote Thread".into()), + title_override: None, + updated_at: chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 3, 0, 0, 0).unwrap(), + created_at: None, + interacted_at: None, + worktree_paths: WorktreePaths::from_folder_paths(&PathList::new(&[PathBuf::from( + "/remote/project", + )])), + archived: false, + remote_connection: Some(remote_host), + }; + ThreadMetadataStore::global(cx).update(cx, |store, cx| store.save(metadata, cx)); + }); + cx.run_until_parked(); + multi_workspace.update_in(cx, |_, _window, cx| cx.notify()); cx.run_until_parked(); @@ -8294,6 +8401,14 @@ async fn test_archive_last_worktree_thread_removes_workspace(cx: &mut TestAppCon "linked worktree workspace should be removed after archiving its last thread" ); + multi_workspace.read_with(cx, |mw, _| { + assert_eq!( + mw.workspace(), + &main_workspace, + "archiving the worktree's last thread should activate its own project, not the remote one" + ); + }); + // The linked worktree checkout directory should also be removed from disk. assert!( !fs.is_dir(Path::new("/worktrees/project/feature-a/project")) @@ -8746,12 +8861,10 @@ async fn test_restore_worktree_thread_uses_main_repo_project_group_key(cx: &mut cx.run_until_parked(); // Remove the worktree workspace and delete the worktree from disk. - let main_workspace = - multi_workspace.read_with(cx, |mw, _| mw.workspaces().next().unwrap().clone()); let remove_task = multi_workspace.update_in(cx, |mw, window, cx| { mw.remove( vec![worktree_workspace], - move |_this, _window, _cx| Task::ready(Ok(main_workspace)), + RemovalIntent::KeepProject, window, cx, ) @@ -12459,7 +12572,8 @@ mod property_test { let key = &keys[project_group_index]; let ws = mw .workspaces_for_project_group(key, cx) - .and_then(|ws| ws.first().cloned()) + .first() + .cloned() .unwrap_or_else(|| mw.workspace().clone()); let project = ws.read(cx).project().clone(); (ws, project) @@ -12612,7 +12726,8 @@ mod property_test { let keys = mw.project_group_keys(); let key = &keys[index]; mw.workspaces_for_project_group(key, cx) - .and_then(|ws| ws.first().cloned()) + .first() + .cloned() .unwrap_or_else(|| mw.workspace().clone()) }); multi_workspace.update_in(cx, |mw, window, cx| { @@ -12681,7 +12796,8 @@ mod property_test { let keys = mw.project_group_keys(); let key = &keys[project_group_index]; mw.workspaces_for_project_group(key, cx) - .and_then(|ws| ws.first().cloned()) + .first() + .cloned() .unwrap() }); let main_project = main_workspace.read_with(cx, |ws, _| ws.project().clone()); @@ -12700,8 +12816,7 @@ mod property_test { let workspace = multi_workspace.read_with(cx, |mw, cx| { let keys = mw.project_group_keys(); let key = &keys[project_group_index]; - mw.workspaces_for_project_group(key, cx) - .and_then(|ws| ws.first().cloned()) + mw.workspaces_for_project_group(key, cx).first().cloned() }); let Some(workspace) = workspace else { return }; let project = workspace.read_with(cx, |ws, _| ws.project().clone()); @@ -12728,8 +12843,7 @@ mod property_test { let workspace = multi_workspace.read_with(cx, |mw, cx| { let keys = mw.project_group_keys(); let key = &keys[project_group_index]; - mw.workspaces_for_project_group(key, cx) - .and_then(|ws| ws.first().cloned()) + mw.workspaces_for_project_group(key, cx).first().cloned() }); let Some(workspace) = workspace else { return }; let project = workspace.read_with(cx, |ws, _| ws.project().clone()); @@ -14978,9 +15092,9 @@ async fn test_find_or_create_workspace_returns_the_created_remote_workspace( Some(opts), Some(key), move |_, _, _| Task::ready(Ok(Some(remote_client))), - &[], None, workspace::OpenMode::Activate, + None, window, cx, ) diff --git a/crates/workspace/src/multi_workspace.rs b/crates/workspace/src/multi_workspace.rs index 4e6ec5b38c..df49cf8fca 100644 --- a/crates/workspace/src/multi_workspace.rs +++ b/crates/workspace/src/multi_workspace.rs @@ -281,14 +281,28 @@ pub struct SerializedProjectGroupState { pub struct ProjectGroupState { pub key: ProjectGroupKey, pub expanded: bool, - pub last_active_workspace: Option>, +} + +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum RemovalIntent { + KeepProject, + CloseProject, +} + +/// One row per workspace held by this window. The displayed workspace is +/// always one of these rows. `pinned` records whether the workspace survives +/// being navigated away from; `activated_at` records when it was last +/// displayed. +struct HeldWorkspace { + workspace: Entity, + pinned: bool, + activated_at: Option, } pub struct MultiWorkspace { window_id: WindowId, - retained_workspaces: Vec>, + held: Vec, project_groups: Vec, - active_workspace: Entity, /// Source of truth for which workspace is presented in this window, shared /// with each member `Workspace` so they can tell whether they own the /// platform window's title and edited indicator. This only exists to prevent @@ -351,9 +365,12 @@ impl MultiWorkspace { }); Self { window_id: window.window_handle().window_id(), - retained_workspaces: Vec::new(), + held: vec![HeldWorkspace { + workspace, + pinned: false, + activated_at: Some(0), + }], project_groups: Vec::new(), - active_workspace: workspace, active_workspace_id, sidebar: None, sidebar_open: false, @@ -488,7 +505,7 @@ impl MultiWorkspace { self.sidebar_open = true; self.retain_active_workspace(cx); let sidebar_focus_handle = self.sidebar.as_ref().map(|s| s.focus_handle(cx)); - for workspace in self.retained_workspaces.clone() { + for workspace in self.workspaces().cloned().collect::>() { workspace.update(cx, |workspace, _cx| { workspace.set_sidebar_focus_handle(sidebar_focus_handle.clone()); }); @@ -504,7 +521,7 @@ impl MultiWorkspace { }; telemetry::event!("Sidebar Toggled", action = "close", side = side); self.sidebar_open = false; - for workspace in self.retained_workspaces.clone() { + for workspace in self.workspaces().cloned().collect::>() { workspace.update(cx, |workspace, _cx| { workspace.set_sidebar_focus_handle(None); }); @@ -619,18 +636,30 @@ impl MultiWorkspace { cx.notify(); } - pub fn is_workspace_retained(&self, workspace: &Entity) -> bool { - self.retained_workspaces + fn held_index(&self, workspace: &Entity) -> Option { + self.held .iter() - .any(|retained| retained == workspace) + .position(|held| held.workspace == *workspace) + } + + pub fn is_workspace_retained(&self, workspace: &Entity) -> bool { + self.held + .iter() + .any(|held| held.pinned && held.workspace == *workspace) } pub fn active_workspace_is_retained(&self) -> bool { - self.is_workspace_retained(&self.active_workspace) + self.held[self.displayed_index()].pinned } - pub fn retained_workspaces(&self) -> &[Entity] { - &self.retained_workspaces + /// The displayed workspace is the most recently activated row. + fn displayed_index(&self) -> usize { + self.held + .iter() + .enumerate() + .max_by_key(|(_, held)| held.activated_at) + .expect("a window always holds at least one workspace") + .0 } /// Ensures a project group exists for `key`, creating one if needed. @@ -648,7 +677,6 @@ impl MultiWorkspace { ProjectGroupState { key, expanded: true, - last_active_workspace: None, }, ); } @@ -685,7 +713,7 @@ impl MultiWorkspace { } if new_key_exists { - let active_key = self.active_workspace.read(cx).project_group_key(cx); + let active_key = self.workspace().read(cx).project_group_key(cx); if active_key == *new_key { self.project_groups.retain(|g| g.key != *old_key); } else { @@ -704,27 +732,46 @@ impl MultiWorkspace { // linked worktree workspace), re-create the old group so it // remains reachable in the sidebar. let other_workspace_needs_old_key = self - .retained_workspaces + .held .iter() - .any(|ws| ws.read(cx).project_group_key(cx) == *old_key); + .any(|held| held.pinned && held.workspace.read(cx).project_group_key(cx) == *old_key); if other_workspace_needs_old_key { self.ensure_project_group_state(old_key.clone()); } } - pub(crate) fn retain_workspace( + /// Ensures `workspace` has a row in `held`, registering it on first + /// insert, and returns the row's index. + fn hold( &mut self, workspace: Entity, - key: ProjectGroupKey, + window: &Window, cx: &mut Context, - ) { - self.ensure_project_group_state(key); - if self.is_workspace_retained(&workspace) { + ) -> usize { + if let Some(index) = self.held_index(&workspace) { + return index; + } + self.register_workspace(&workspace, window, cx); + self.held.push(HeldWorkspace { + workspace, + pinned: false, + activated_at: None, + }); + self.held.len() - 1 + } + + /// Pins the row so the workspace survives navigating away, recording + /// `group` as the project group it was pinned under. No-op if already + /// pinned. + fn pin(&mut self, index: usize, group: ProjectGroupKey, cx: &mut Context) { + if self.held[index].pinned { return; } - - self.retained_workspaces.push(workspace.clone()); - cx.emit(MultiWorkspaceEvent::WorkspaceAdded(workspace)); + self.held[index].pinned = true; + self.ensure_project_group_state(group); + cx.emit(MultiWorkspaceEvent::WorkspaceAdded( + self.held[index].workspace.clone(), + )); } pub(crate) fn activate_provisional_workspace( @@ -734,17 +781,9 @@ impl MultiWorkspace { window: &mut Window, cx: &mut Context, ) { - if workspace != self.active_workspace { - self.register_workspace(&workspace, window, cx); - } - - self.ensure_project_group_state(provisional_key); - if !self.is_workspace_retained(&workspace) { - self.retained_workspaces.push(workspace.clone()); - } - - self.activate(workspace.clone(), None, window, cx); - cx.emit(MultiWorkspaceEvent::WorkspaceAdded(workspace)); + let index = self.hold(workspace.clone(), window, cx); + self.pin(index, provisional_key, cx); + self.activate(workspace, None, window, cx); } fn register_workspace( @@ -792,11 +831,7 @@ impl MultiWorkspace { if restored.iter().any(|group| group.key == key) { continue; } - restored.push(ProjectGroupState { - key, - expanded, - last_active_workspace: None, - }); + restored.push(ProjectGroupState { key, expanded }); } for existing in std::mem::take(&mut self.project_groups) { if !restored.iter().any(|group| group.key == existing.key) { @@ -813,35 +848,28 @@ impl MultiWorkspace { .collect() } - fn derived_project_groups(&self, cx: &App) -> Vec { + pub fn project_groups(&self, cx: &App) -> Vec { self.project_groups .iter() .map(|group| ProjectGroup { key: group.key.clone(), - workspaces: self - .retained_workspaces - .iter() - .filter(|workspace| workspace.read(cx).project_group_key(cx) == group.key) - .cloned() - .collect(), + workspaces: self.workspaces_for_project_group(&group.key, cx), expanded: group.expanded, }) .collect() } - pub fn project_groups(&self, cx: &App) -> Vec { - self.derived_project_groups(cx) - } - pub fn last_active_workspace_for_group( &self, key: &ProjectGroupKey, cx: &App, ) -> Option> { - let group = self.project_groups.iter().find(|g| g.key == *key)?; - let weak = group.last_active_workspace.as_ref()?; - let workspace = weak.upgrade()?; - (workspace.read(cx).project_group_key(cx) == *key).then_some(workspace) + self.held + .iter() + .filter(|held| held.workspace.read(cx).project_group_key(cx) == *key) + .filter_map(|held| Some((held.activated_at?, &held.workspace))) + .max_by_key(|(activated_at, _)| *activated_at) + .map(|(_, workspace)| workspace.clone()) } pub fn group_state_by_key(&self, key: &ProjectGroupKey) -> Option<&ProjectGroupState> { @@ -907,57 +935,13 @@ impl MultiWorkspace { &self, key: &ProjectGroupKey, cx: &App, - ) -> Option>> { - let has_group = self.project_groups.iter().any(|group| group.key == *key) - || self - .retained_workspaces - .iter() - .any(|workspace| workspace.read(cx).project_group_key(cx) == *key); - - has_group.then(|| { - self.retained_workspaces - .iter() - .filter(|workspace| workspace.read(cx).project_group_key(cx) == *key) - .cloned() - .collect() - }) - } - - pub fn close_workspace( - &mut self, - workspace: &Entity, - window: &mut Window, - cx: &mut Context, - ) -> Task> { - let group_key = workspace.read(cx).project_group_key(cx); - let excluded_workspace = workspace.clone(); - - self.remove( - [workspace.clone()], - move |this, window, cx| { - if let Some(workspace) = this - .workspaces_for_project_group(&group_key, cx) - .unwrap_or_default() - .into_iter() - .find(|candidate| candidate != &excluded_workspace) - { - return Task::ready(Ok(workspace)); - } - - let group_index = this - .project_groups - .iter() - .position(|group| group.key == group_key); - this.fallback_workspace( - group_index, - std::slice::from_ref(&excluded_workspace), - window, - cx, - ) - }, - window, - cx, - ) + ) -> Vec> { + self.held + .iter() + .filter(|held| held.pinned) + .map(|held| held.workspace.clone()) + .filter(|workspace| workspace.read(cx).project_group_key(cx) == *key) + .collect() } pub fn remove_project_group( @@ -966,97 +950,27 @@ impl MultiWorkspace { window: &mut Window, cx: &mut Context, ) -> Task> { - let group_index = self - .project_groups - .iter() - .position(|group| group.key == *group_key); - - // The active workspace can remain provisional while the sidebar is - // closed. Retain it before removal so switching to the fallback does - // not recreate the project group that is being removed. + // The active workspace can remain unpinned while the sidebar is + // closed. Pin it first: this puts it in the removal set below, and + // stops `activate` from pinning it while switching to the + // replacement, which would recreate the project group row this + // function just deleted. let active_workspace = self.workspace().clone(); if active_workspace.read(cx).project_group_key(cx) == *group_key && !self.is_workspace_retained(&active_workspace) { - self.retain_workspace(active_workspace, group_key.clone(), cx); + let index = self.hold(active_workspace, window, cx); + self.pin(index, group_key.clone(), cx); } - let workspaces = self - .workspaces_for_project_group(group_key, cx) - .unwrap_or_default(); - let excluded_workspaces = workspaces.clone(); - let removal_task = self.remove( - workspaces, - move |this, window, cx| { - this.fallback_workspace(group_index, &excluded_workspaces, window, cx) - }, - window, - cx, - ); + let workspaces = self.workspaces_for_project_group(group_key, cx); + + let task = self.remove(workspaces, RemovalIntent::CloseProject, window, cx); self.project_groups.retain(|group| group.key != *group_key); cx.emit(MultiWorkspaceEvent::ProjectGroupsChanged); - removal_task - } - - /// Finds a fallback workspace outside the project group at `group_index`. - /// - /// Prefers the nearest retained workspace in another group. If none exists, - /// opens the immediately neighboring group when it is local, and otherwise - /// creates an empty workspace. - fn fallback_workspace( - &mut self, - group_index: Option, - excluded_workspaces: &[Entity], - window: &mut Window, - cx: &mut Context, - ) -> Task>> { - if let Some(group_index) = group_index { - if let Some(workspace) = - self.nearest_retained_workspace(group_index, excluded_workspaces, cx) - { - return Task::ready(Ok(workspace)); - } - - let neighboring_group_key = self - .project_groups - .get(group_index + 1) - .or_else(|| { - group_index - .checked_sub(1) - .and_then(|previous| self.project_groups.get(previous)) - }) - .map(|group| group.key.clone()); - - if let Some(key) = neighboring_group_key - && key.host().is_none() - { - return self.find_or_create_local_workspace( - key.path_list().clone(), - Some(key), - excluded_workspaces, - None, - OpenMode::Activate, - window, - cx, - ); - } - } - - let app_state = self.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)) + task } /// Returns the nearest retained workspace outside the project group at @@ -1070,43 +984,36 @@ impl MultiWorkspace { /// /// The `group_index` must identify a project group that is still present in /// [`Self::project_groups`]. + /// The keys of the other project groups, nearest first, preferring the + /// following group over the preceding group at equal distances. Without an + /// index, every group key in display order. + fn neighbor_group_keys(&self, group_index: Option) -> Vec { + let Some(index) = group_index else { + return self + .project_groups + .iter() + .map(|group| group.key.clone()) + .collect(); + }; + + (1..self.project_groups.len()) + .flat_map(|distance| [index.checked_add(distance), index.checked_sub(distance)]) + .flatten() + .filter_map(|index| self.project_groups.get(index)) + .map(|group| group.key.clone()) + .collect() + } + + #[cfg(test)] pub(super) fn nearest_retained_workspace( &self, group_index: usize, excluded_workspaces: &[Entity], cx: &App, ) -> Option> { - for distance in 1..self.project_groups.len() { - for index in [ - group_index.checked_add(distance), - group_index.checked_sub(distance), - ] + self.neighbor_group_keys(Some(group_index)) .into_iter() - .flatten() - { - if let Some(group) = self.project_groups.get(index) { - let workspace_is_available = |workspace: &Entity| { - !excluded_workspaces.contains(workspace) - && !workspace.read(cx).project().read(cx).is_disconnected(cx) - }; - let workspace = self - .last_active_workspace_for_group(&group.key, cx) - .filter(&workspace_is_available) - .or_else(|| { - self.workspaces_for_project_group(&group.key, cx) - .unwrap_or_default() - .into_iter() - .find(workspace_is_available) - }); - - if workspace.is_some() { - return workspace; - } - } - } - } - - None + .find_map(|key| self.live_member_for_group(&key, excluded_workspaces, cx)) } /// Goes through sqlite: serialize -> close -> open new window @@ -1124,9 +1031,7 @@ impl MultiWorkspace { let app_state = self.workspace().read(cx).app_state().clone(); - let workspaces: Vec<_> = self - .workspaces_for_project_group(key, cx) - .unwrap_or_default(); + let workspaces: Vec<_> = self.workspaces_for_project_group(key, cx); let mut serialization_tasks = Vec::new(); for workspace in &workspaces { serialization_tasks.push(workspace.update(cx, |workspace, inner_cx| { @@ -1159,21 +1064,8 @@ impl MultiWorkspace { path_list: &PathList, host: Option<&RemoteConnectionOptions>, cx: &App, - ) -> Option> { - self.workspace_for_paths_excluding(path_list, host, &[], cx) - } - - fn workspace_for_paths_excluding( - &self, - path_list: &PathList, - host: Option<&RemoteConnectionOptions>, - excluding: &[Entity], - cx: &App, ) -> Option> { for workspace in self.workspaces() { - if excluding.contains(workspace) { - continue; - } let root_paths = PathList::new(&workspace.read(cx).root_paths(cx)); let key = workspace.read(cx).project_group_key(cx); let host_matches = key.host().as_ref() == host; @@ -1210,56 +1102,21 @@ impl MultiWorkspace { &mut Context, ) -> Task>>> + 'static, - excluding: &[Entity], - init: Option) + Send>>, - open_mode: OpenMode, - window: &mut Window, - cx: &mut Context, - ) -> Task>> { - self.find_or_create_workspace_with_source_workspace( - paths, - host, - provisional_project_group_key, - connect_remote, - excluding, - init, - open_mode, - None, - window, - cx, - ) - } - - pub fn find_or_create_workspace_with_source_workspace( - &mut self, - paths: PathList, - host: Option, - provisional_project_group_key: Option, - connect_remote: impl FnOnce( - RemoteConnectionOptions, - &mut Window, - &mut Context, - ) -> Task>>> - + 'static, - excluding: &[Entity], init: Option) + Send>>, open_mode: OpenMode, source_workspace: Option>, window: &mut Window, cx: &mut Context, ) -> Task>> { - if let Some(workspace) = - self.workspace_for_paths_excluding(&paths, host.as_ref(), excluding, cx) - { + if let Some(workspace) = self.workspace_for_paths(&paths, host.as_ref(), cx) { self.activate(workspace.clone(), source_workspace, window, cx); return Task::ready(Ok(workspace)); } let Some(connection_options) = host else { - return self.find_or_create_local_workspace_with_source_workspace( + return self.find_or_create_local_workspace( paths, provisional_project_group_key, - excluding, init, open_mode, source_workspace, @@ -1344,44 +1201,17 @@ impl MultiWorkspace { /// or creates a new one (deserializing its saved state from the database). /// Never searches other windows or matches workspaces with a superset of /// the requested paths. - /// - /// `excluding` lists workspaces that should be skipped during the search - /// (e.g. workspaces that are about to be removed). pub fn find_or_create_local_workspace( &mut self, path_list: PathList, project_group: Option, - excluding: &[Entity], - init: Option) + Send>>, - open_mode: OpenMode, - window: &mut Window, - cx: &mut Context, - ) -> Task>> { - self.find_or_create_local_workspace_with_source_workspace( - path_list, - project_group, - excluding, - init, - open_mode, - None, - window, - cx, - ) - } - - pub fn find_or_create_local_workspace_with_source_workspace( - &mut self, - path_list: PathList, - project_group: Option, - excluding: &[Entity], init: Option) + Send>>, open_mode: OpenMode, source_workspace: Option>, window: &mut Window, cx: &mut Context, ) -> Task>> { - if let Some(workspace) = self.workspace_for_paths_excluding(&path_list, None, excluding, cx) - { + if let Some(workspace) = self.workspace_for_paths(&path_list, None, cx) { self.activate(workspace.clone(), source_workspace, window, cx); return Task::ready(Ok(workspace)); } @@ -1390,7 +1220,6 @@ impl MultiWorkspace { let app_state = self.workspace().read(cx).app_state().clone(); let requesting_window = window.window_handle().downcast::(); let fs = ::global(cx); - let excluding = excluding.to_vec(); cx.spawn(async move |_this, cx| { let effective_path_list = if let Some(project_group) = project_group { @@ -1420,12 +1249,7 @@ impl MultiWorkspace { && let Some(workspace) = requesting_window .update(cx, |multi_workspace, window, cx| { multi_workspace - .workspace_for_paths_excluding( - &effective_path_list, - None, - &excluding, - cx, - ) + .workspace_for_paths(&effective_path_list, None, cx) .inspect(|workspace| { multi_workspace.activate( workspace.clone(), @@ -1459,14 +1283,11 @@ impl MultiWorkspace { } pub fn workspace(&self) -> &Entity { - &self.active_workspace + &self.held[self.displayed_index()].workspace } pub fn workspaces(&self) -> impl Iterator> { - let active_is_retained = self.is_workspace_retained(&self.active_workspace); - self.retained_workspaces - .iter() - .chain(std::iter::once(&self.active_workspace).filter(move |_| !active_is_retained)) + self.held.iter().map(|held| &held.workspace) } /// Adds a workspace to this window as persistent without changing which @@ -1477,16 +1298,12 @@ impl MultiWorkspace { if self.is_workspace_retained(&workspace) { return; } - - if workspace != self.active_workspace { - self.register_workspace(&workspace, window, cx); - } - let key = workspace.read(cx).project_group_key(cx); - self.retain_workspace(workspace, key, cx); + let index = self.hold(workspace, window, cx); + self.pin(index, key, cx); telemetry::event!( "Workspace Added", - workspace_count = self.retained_workspaces.len() + workspace_count = self.held.iter().filter(|held| held.pinned).count() ); cx.notify(); } @@ -1504,35 +1321,33 @@ impl MultiWorkspace { return; } - let old_active_workspace = self.active_workspace.clone(); + let old_active_workspace = self.workspace().clone(); let old_active_was_retained = self.active_workspace_is_retained(); - let workspace_was_retained = self.is_workspace_retained(&workspace); let should_retain_workspaces = self.multi_workspace_enabled(cx); if should_retain_workspaces && !old_active_was_retained { let key = old_active_workspace.read(cx).project_group_key(cx); - self.retain_workspace(old_active_workspace.clone(), key, cx); + let index = self.hold(old_active_workspace.clone(), window, cx); + self.pin(index, key, cx); } - if !workspace_was_retained { - self.register_workspace(&workspace, window, cx); - - if should_retain_workspaces { - let key = workspace.read(cx).project_group_key(cx); - self.retain_workspace(workspace.clone(), key, cx); - } + let displayed = self.hold(workspace.clone(), window, cx); + if should_retain_workspaces { + let key = workspace.read(cx).project_group_key(cx); + self.pin(displayed, key, cx); } - self.active_workspace = workspace; // Publish the new active workspace before anyone reads the shared cell // to decide who owns the window chrome. - self.active_workspace_id - .set(self.active_workspace.entity_id()); + self.active_workspace_id.set(workspace.entity_id()); - let active_key = self.active_workspace.read(cx).project_group_key(cx); - if let Some(group) = self.project_groups.iter_mut().find(|g| g.key == active_key) { - group.last_active_workspace = Some(self.active_workspace.downgrade()); - } + let stamp = self + .held + .iter() + .filter_map(|held| held.activated_at) + .max() + .map_or(0, |max| max + 1); + self.held[displayed].activated_at = Some(stamp); if !should_retain_workspaces && !old_active_was_retained { self.detach_workspace(&old_active_workspace, cx); @@ -1542,7 +1357,7 @@ impl MultiWorkspace { // The previously-active workspace left the title and edited indicator // reflecting its own state, so re-apply them from the newly-active // workspace (which is now the chrome owner per `owns_window_chrome`). - self.active_workspace.update(cx, |workspace, cx| { + workspace.update(cx, |workspace, cx| { workspace.refresh_window_state(window, cx); }); @@ -1552,40 +1367,16 @@ impl MultiWorkspace { cx.notify(); } - /// Adds `workspace` as a retained background tab without switching the - /// active workspace to it or moving focus. Mirrors the registration and - /// retention bookkeeping `activate` performs for the incoming workspace, - /// but leaves the currently-active workspace focused. - /// - /// Used when something opens a workspace the user should not be yanked - /// into — e.g. the agent's `create_thread` tool spawning a sibling - /// worktree in the background. - pub fn add_background_workspace( - &mut self, - workspace: Entity, - window: &mut Window, - cx: &mut Context, - ) { - if self.workspace() == &workspace || self.is_workspace_retained(&workspace) { - return; - } - self.register_workspace(&workspace, window, cx); - let key = workspace.read(cx).project_group_key(cx); - self.retain_workspace(workspace, key, cx); - cx.notify(); - } - /// Promotes the currently active workspace to persistent if it is /// transient, so it is retained across workspace switches even when /// the sidebar is closed. No-op if the workspace is already persistent. pub fn retain_active_workspace(&mut self, cx: &mut Context) { - let workspace = self.active_workspace.clone(); - if self.is_workspace_retained(&workspace) { + let index = self.displayed_index(); + if self.held[index].pinned { return; } - - let key = workspace.read(cx).project_group_key(cx); - self.retain_workspace(workspace, key, cx); + let key = self.held[index].workspace.read(cx).project_group_key(cx); + self.pin(index, key, cx); self.serialize(cx); cx.notify(); } @@ -1597,14 +1388,16 @@ impl MultiWorkspace { self.close_sidebar(window, cx); } - let active_workspace = self.active_workspace.clone(); - for workspace in self.retained_workspaces.clone() { - if workspace != active_workspace { + let displayed_workspace = self.workspace().clone(); + for workspace in self.workspaces().cloned().collect::>() { + if workspace != displayed_workspace { self.detach_workspace(&workspace, cx); } } - self.retained_workspaces.clear(); + for held in &mut self.held { + held.pinned = false; + } self.project_groups.clear(); cx.notify(); } @@ -1613,18 +1406,13 @@ impl MultiWorkspace { /// group key, and emits `WorkspaceRemoved`. The DB row is preserved /// so the workspace still appears in the recent-projects list. fn detach_workspace(&mut self, workspace: &Entity, cx: &mut Context) { - self.retained_workspaces - .retain(|retained| retained != workspace); - for group in &mut self.project_groups { - if group - .last_active_workspace - .as_ref() - .and_then(WeakEntity::upgrade) - .as_ref() - == Some(workspace) - { - group.last_active_workspace = None; - } + if let Some(index) = self.held_index(workspace) { + assert_ne!( + index, + self.displayed_index(), + "the displayed workspace must be re-pointed before it is detached" + ); + self.held.remove(index); } cx.emit(MultiWorkspaceEvent::WorkspaceRemoved(workspace.entity_id())); workspace.update(cx, |workspace, _cx| { @@ -1813,7 +1601,12 @@ impl MultiWorkspace { #[cfg(any(test, feature = "test-support"))] pub fn assert_project_group_key_integrity(&self, cx: &App) -> anyhow::Result<()> { let mut retained_ids: collections::HashSet = Default::default(); - for workspace in &self.retained_workspaces { + for workspace in self + .held + .iter() + .filter(|held| held.pinned) + .map(|held| &held.workspace) + { anyhow::ensure!( retained_ids.insert(workspace.entity_id()), "workspace {:?} is retained more than once", @@ -1863,7 +1656,6 @@ impl MultiWorkspace { self.project_groups.push(ProjectGroupState { key: group.key, expanded: group.expanded, - last_active_workspace: None, }); } @@ -1951,23 +1743,47 @@ impl MultiWorkspace { tasks } + /// The best replacement candidate within one project group: its most + /// recently displayed member, else its first pinned member, skipping + /// `excluding` and disconnected projects. + fn live_member_for_group( + &self, + key: &ProjectGroupKey, + excluding: &[Entity], + cx: &App, + ) -> Option> { + let available = |workspace: &Entity| { + !excluding.contains(workspace) + && !workspace.read(cx).project().read(cx).is_disconnected(cx) + }; + self.last_active_workspace_for_group(key, cx) + .filter(&available) + .or_else(|| { + self.held + .iter() + .filter(|held| held.pinned) + .map(|held| held.workspace.clone()) + .filter(|workspace| workspace.read(cx).project_group_key(cx) == *key) + .find(available) + }) + } + /// 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. + /// Every workspace is first asked for consent (save prompts); no state + /// changes until all consent. The rows are then deleted and, when the + /// displayed workspace was among them, a replacement is chosen from what + /// remains: another workspace in the same project, then a workspace in the + /// nearest neighboring project, then an empty workspace. When the intent is + /// `KeepProject` and the project has no other workspace, its root worktrees + /// are reopened afterwards; the same applies to the adjacent local project + /// when nothing at all remains. /// /// Returns `true` if any workspaces were actually removed. pub fn remove( &mut self, workspaces: impl IntoIterator>, - fallback_workspace: impl FnOnce( - &mut Self, - &mut Window, - &mut Context, - ) -> Task>>, + intent: RemovalIntent, window: &mut Window, cx: &mut Context, ) -> Task> { @@ -1977,15 +1793,31 @@ impl MultiWorkspace { return Task::ready(Ok(false)); } - let removing_active = workspaces.iter().any(|ws| ws == self.workspace()); let original_active = self.workspace().clone(); + let group_key = original_active.read(cx).project_group_key(cx); - let fallback_task = removing_active.then(|| fallback_workspace(self, window, cx)); + // Record the neighborhood of the project as the user sees it now: + // callers like `remove_project_group` delete the project row itself + // before the removal task runs. + let group_index = self + .project_groups + .iter() + .position(|group| group.key == group_key); + let neighbor_keys = self.neighbor_group_keys(group_index); + let adjacent_key = group_index.and_then(|index| { + self.project_groups + .get(index + 1) + .or_else(|| { + index + .checked_sub(1) + .and_then(|previous| self.project_groups.get(previous)) + }) + .map(|group| group.key.clone()) + }); cx.spawn_in(window, async move |this, cx| { - // Run the standard workspace close lifecycle for every workspace - // being removed from this window. This handles save prompting and - // session cleanup consistently with other replace-in-window flows. + // Consent phase: run the standard close lifecycle for every + // workspace being removed. Prompts only; no state changes. for workspace in &workspaces { let should_continue = workspace .update_in(cx, |workspace, window, cx| { @@ -1998,47 +1830,101 @@ impl MultiWorkspace { } } - // 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, None, window, cx); - })?; - } else { - this.update_in(cx, |this, window, cx| { - if *this.workspace() != original_active { - this.activate(original_active, None, window, cx); - } - })?; - } - - // Actually remove the workspaces. - this.update_in(cx, |this, _, cx| { + // Edit phase: one synchronous update. Delete the rows, then pick + // the replacement from the rows that actually remain. + let (removed_any, reopen_key) = this.update_in(cx, |this, window, cx| { let mut removed_any = false; + let displayed_workspace = this.workspace().clone(); for workspace in &workspaces { - let was_retained = this.is_workspace_retained(workspace); - if was_retained { + if *workspace == displayed_workspace { + continue; + } + if this.held_index(workspace).is_some() { this.detach_workspace(workspace, cx); removed_any = true; } } + let mut reopen_key = None; + if workspaces.contains(&displayed_workspace) { + let doomed = std::slice::from_ref(&displayed_workspace); + + let same_group = this + .held + .iter() + .filter(|held| held.pinned && held.workspace != displayed_workspace) + .map(|held| held.workspace.clone()) + .find(|workspace| workspace.read(cx).project_group_key(cx) == group_key); + if intent == RemovalIntent::KeepProject + && same_group.is_none() + && group_key.host().is_none() + && !group_key.path_list().is_empty() + { + reopen_key = Some(group_key.clone()); + } + + let replacement = same_group + .or_else(|| { + neighbor_keys + .iter() + .find_map(|key| this.live_member_for_group(key, doomed, cx)) + }) + .unwrap_or_else(|| { + if reopen_key.is_none() { + reopen_key = adjacent_key.clone().filter(|key| { + key.host().is_none() && !key.path_list().is_empty() + }); + } + let app_state = displayed_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, + ); + cx.new(|cx| Workspace::new(None, project, app_state, window, cx)) + }); + + this.activate(replacement, None, window, cx); + this.detach_workspace(&displayed_workspace, cx); + removed_any = true; + } else if *this.workspace() != original_active + && !workspaces.contains(&original_active) + { + // Prompting switched the display away from where the user + // was; go back. + this.activate(original_active.clone(), None, window, cx); + } + if removed_any { this.serialize(cx); cx.notify(); } - Ok(removed_any) - })? + (removed_any, reopen_key) + })?; + + if let Some(key) = reopen_key { + this.update_in(cx, |this, window, cx| { + this.find_or_create_local_workspace( + key.path_list().clone(), + Some(key), + None, + OpenMode::Activate, + None, + window, + cx, + ) + })? + .await?; + } + + Ok(removed_any) }) } @@ -2051,7 +1937,7 @@ impl MultiWorkspace { ) -> Task>> { if self.multi_workspace_enabled(cx) { let empty_workspace = if self - .active_workspace + .workspace() .read(cx) .project() .read(cx) @@ -2059,7 +1945,7 @@ impl MultiWorkspace { .next() .is_none() { - Some(self.active_workspace.clone()) + Some(self.workspace().clone()) } else { None }; @@ -2080,16 +1966,18 @@ impl MultiWorkspace { this.find_or_create_local_workspace( PathList::new(&paths), None, - empty_workspace.as_slice(), None, OpenMode::Activate, + None, window, cx, ) })?; let new_workspace = create_task.await?; - if let Some(empty_workspace) = empty_workspace { + if let Some(empty_workspace) = empty_workspace + && empty_workspace != new_workspace + { this.update(cx, |this, cx| { if this.is_workspace_retained(&empty_workspace) { this.detach_workspace(&empty_workspace, cx); diff --git a/crates/workspace/src/multi_workspace_tests.rs b/crates/workspace/src/multi_workspace_tests.rs index e642e1821b..8c767eee1b 100644 --- a/crates/workspace/src/multi_workspace_tests.rs +++ b/crates/workspace/src/multi_workspace_tests.rs @@ -432,9 +432,9 @@ async fn test_find_or_create_local_workspace_reuses_active_workspace_when_sideba mw.find_or_create_local_workspace( PathList::new(&[PathBuf::from("/root_a")]), None, - &[], None, OpenMode::Activate, + None, window, cx, ) @@ -497,9 +497,9 @@ async fn test_find_or_create_workspace_uses_project_group_key_when_paths_are_mis None, Some(project_group_key.clone()), |_options, _window, _cx| Task::ready(Ok(None)), - &[], None, OpenMode::Activate, + None, window, cx, ) @@ -555,22 +555,9 @@ async fn test_remove_fallback_via_find_or_create_skips_removed_workspaces(cx: &m let removed = multi_workspace .update_in(cx, |mw, window, cx| { - let excluded = vec![workspace_a.clone()]; mw.remove( - excluded.clone(), - move |this, window, cx| { - this.find_or_create_workspace( - PathList::new(&[PathBuf::from("/root_a")]), - None, - None, - |_options, _window, _cx| Task::ready(Ok(None)), - &excluded, - None, - OpenMode::Activate, - window, - cx, - ) - }, + vec![workspace_a.clone()], + RemovalIntent::CloseProject, window, cx, ) @@ -593,6 +580,52 @@ async fn test_remove_fallback_via_find_or_create_skips_removed_workspaces(cx: &m }); } +#[gpui::test] +async fn test_remove_keeping_the_project_does_not_switch_projects(cx: &mut TestAppContext) { + init_test(cx); + let fs = FakeFs::new(cx.executor()); + fs.insert_tree("/root_a", json!({ "file.txt": "" })).await; + fs.insert_tree("/root_b", json!({ "file.txt": "" })).await; + cx.update(|cx| ::set_global(fs.clone(), cx)); + let project_a = Project::test(fs.clone(), ["/root_a".as_ref()], cx).await; + let project_b = Project::test(fs, ["/root_b".as_ref()], cx).await; + + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a, window, cx)); + + let workspace_a = multi_workspace.read_with(cx, |mw, _cx| mw.workspace().clone()); + let _workspace_b = multi_workspace.update_in(cx, |mw, window, cx| { + mw.test_add_workspace(project_b, window, cx) + }); + cx.run_until_parked(); + + multi_workspace.update_in(cx, |mw, window, cx| { + mw.activate(workspace_a.clone(), None, window, cx); + }); + cx.run_until_parked(); + + multi_workspace + .update_in(cx, |mw, window, cx| { + mw.remove( + vec![workspace_a.clone()], + RemovalIntent::KeepProject, + window, + cx, + ) + }) + .await + .expect("removing the active workspace should succeed"); + cx.run_until_parked(); + + multi_workspace.read_with(cx, |mw, cx| { + assert_eq!( + PathList::new(&mw.workspace().read(cx).root_paths(cx)), + PathList::new(&[PathBuf::from("/root_a")]), + "the replacement workspace should be in the removed workspace's project" + ); + }); +} + #[gpui::test] async fn test_find_or_create_local_workspace_reuses_active_workspace_after_sidebar_open( cx: &mut TestAppContext, @@ -625,9 +658,9 @@ async fn test_find_or_create_local_workspace_reuses_active_workspace_after_sideb mw.find_or_create_local_workspace( PathList::new(&[PathBuf::from("/root_a")]), None, - &[], None, OpenMode::Activate, + None, window, cx, ) @@ -711,7 +744,12 @@ async fn test_close_workspace_prefers_already_loaded_neighboring_workspace( let closed = multi_workspace .update_in(cx, |multi_workspace, window, cx| { - multi_workspace.close_workspace(&workspace_a, window, cx) + multi_workspace.remove( + [workspace_a.clone()], + RemovalIntent::CloseProject, + window, + cx, + ) }) .await .expect("closing the active workspace should succeed"); @@ -735,7 +773,6 @@ async fn test_close_workspace_prefers_already_loaded_neighboring_workspace( assert!( multi_workspace .workspaces_for_project_group(&project_c_key, cx) - .unwrap_or_default() .is_empty(), "the unloaded neighboring group C should remain unopened" ); @@ -758,7 +795,6 @@ async fn test_close_workspace_prefers_workspace_in_same_project_group(cx: &mut T let (workspace_a_1, workspace_a_2) = multi_workspace.read_with(cx, |multi_workspace, cx| { let mut workspaces = multi_workspace .workspaces_for_project_group(&key_a, cx) - .expect("project group A should have retained workspaces") .into_iter(); let first = workspaces .next() @@ -781,7 +817,12 @@ async fn test_close_workspace_prefers_workspace_in_same_project_group(cx: &mut T let closed = multi_workspace .update_in(cx, |multi_workspace, window, cx| { - multi_workspace.close_workspace(&workspace_a_1, window, cx) + multi_workspace.remove( + [workspace_a_1.clone()], + RemovalIntent::CloseProject, + window, + cx, + ) }) .await .expect("closing the active workspace should succeed"); @@ -795,9 +836,7 @@ async fn test_close_workspace_prefers_workspace_in_same_project_group(cx: &mut T ); assert_eq!( - multi_workspace - .workspaces_for_project_group(&key_a, cx) - .expect("project group A should remain"), + multi_workspace.workspaces_for_project_group(&key_a, cx), vec![workspace_a_2], "only the fallback workspace should remain in project group A" ); @@ -830,7 +869,12 @@ async fn test_close_workspace_opens_unloaded_local_neighbor(cx: &mut TestAppCont let closed = multi_workspace .update_in(cx, |multi_workspace, window, cx| { - multi_workspace.close_workspace(&workspace_a, window, cx) + multi_workspace.remove( + [workspace_a.clone()], + RemovalIntent::CloseProject, + window, + cx, + ) }) .await .expect("closing the active workspace should succeed"); @@ -1294,7 +1338,12 @@ async fn test_close_workspace_with_remote_neighbor_does_not_create_local_workspa // to creating an empty workspace instead. multi_workspace .update_in(cx, |mw, window, cx| { - mw.close_workspace(&workspace_a, window, cx) + mw.remove( + [workspace_a.clone()], + RemovalIntent::CloseProject, + window, + cx, + ) }) .await .expect("close_workspace should succeed"); @@ -1418,15 +1467,16 @@ async fn test_nearest_retained_workspace(cx: &mut TestAppContext) { .expect("project group for project-c should exist"); let workspace_b = multi_workspace .workspaces_for_project_group(&key_b, cx) - .and_then(|workspaces| workspaces.into_iter().next()) + .into_iter() + .next() .expect("workspace for project-b should exist"); let workspace_d = multi_workspace .workspaces_for_project_group(&key_d, cx) - .and_then(|workspaces| workspaces.into_iter().next()) + .into_iter() + .next() .expect("workspace for project-d should exist"); let retained_workspaces_a = multi_workspace - .workspaces_for_project_group(&key_a, cx) - .expect("project group A should have retained workspaces"); + .workspaces_for_project_group(&key_a, cx); assert_eq!( retained_workspaces_a.len(), 2, @@ -1477,23 +1527,20 @@ async fn test_nearest_retained_workspace(cx: &mut TestAppContext) { "the farther group's last active workspace should be preferred" ); - // We'll now clear the project group's last active workspace. - // In that state, the search falls back to the group's first retained - // workspace. - multi_workspace - .group_state_by_key_mut(&key_a) - .expect("project group A should exist") - .last_active_workspace - .take(); - + // With the group's most recently activated workspace excluded, the + // search falls back to the member activated before it. assert_eq!( multi_workspace.nearest_retained_workspace( group_c_index, - &[workspace_b.clone(), workspace_d.clone()], + &[ + workspace_b.clone(), + workspace_d.clone(), + workspace_a_2.clone() + ], cx ), Some(workspace_a_1.clone()), - "the first retained workspace should be used without a last active workspace" + "the previously activated workspace should be used when the last active one is excluded" ); // Excluding every neighboring workspace exhausts the search. diff --git a/crates/workspace/src/persistence.rs b/crates/workspace/src/persistence.rs index c64150eae1..f08aaba6c4 100644 --- a/crates/workspace/src/persistence.rs +++ b/crates/workspace/src/persistence.rs @@ -2764,9 +2764,9 @@ pub fn delete_unloaded_items( #[cfg(test)] mod tests { use super::*; - use crate::OpenMode; use crate::PathList; use crate::ProjectGroupKey; + use crate::RemovalIntent; use crate::{ multi_workspace::MultiWorkspace, persistence::{ @@ -2843,7 +2843,7 @@ mod tests { .workspaces() .find(|ws| *ws != &active) .expect("should have a non-active workspace"); - mw.remove([ws.clone()], |_, _, _| unreachable!(), _window, cx) + mw.remove([ws.clone()], RemovalIntent::CloseProject, _window, cx) .detach_and_log_err(cx); }); @@ -4811,7 +4811,7 @@ 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], |_, _, _| unreachable!(), window, cx) + mw.remove([ws], RemovalIntent::CloseProject, window, cx) .detach_and_log_err(cx); }); @@ -4921,7 +4921,7 @@ 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], |_, _, _| unreachable!(), window, cx) + mw.remove([ws], RemovalIntent::CloseProject, window, cx) .detach_and_log_err(cx); }); @@ -5002,7 +5002,7 @@ 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], |_, _, _| unreachable!(), window, cx) + mw.remove([ws], RemovalIntent::CloseProject, window, cx) .detach_and_log_err(cx); }); @@ -5955,27 +5955,12 @@ mod tests { }); cx.run_until_parked(); - // Remove workspace_a. The fallback searches for the same paths. - // Without the `excluding` parameter, `workspace_for_paths` would - // return workspace_a (first match) and the assert in `remove` - // would fire. With the fix, workspace_a is skipped and - // workspace_b is found instead. - let path_list = PathList::new(std::slice::from_ref(&dir)); - let excluded = vec![workspace_a.clone()]; + // Remove workspace_a. Its replacement is looked up by the same paths, + // so workspace_a itself has to be skipped or it would be picked again. multi_workspace.update_in(cx, |mw, window, cx| { mw.remove( vec![workspace_a.clone()], - move |this, window, cx| { - this.find_or_create_local_workspace( - path_list, - None, - &excluded, - None, - OpenMode::Activate, - window, - cx, - ) - }, + RemovalIntent::CloseProject, window, cx, ) diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 0e79764e19..4c0e31e72e 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -33,8 +33,8 @@ pub use dock::Panel; pub use multi_workspace::{ CloseWorkspaceSidebar, DraggedSidebar, FocusWorkspaceSidebar, MoveProjectToNewWindow, MultiWorkspace, MultiWorkspaceEvent, NewThread, NextProject, NextThread, PreviousProject, - PreviousThread, ProjectGroup, ProjectGroupKey, SerializedProjectGroupState, Sidebar, - SidebarEvent, SidebarHandle, SidebarRenderState, SidebarSide, ToggleWorkspaceSidebar, + PreviousThread, ProjectGroup, ProjectGroupKey, RemovalIntent, SerializedProjectGroupState, + Sidebar, SidebarEvent, SidebarHandle, SidebarRenderState, SidebarSide, ToggleWorkspaceSidebar, sidebar_side_context_menu, }; pub use path_list::{PathList, SerializedPathList}; @@ -12056,7 +12056,12 @@ mod tests { // 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) + mw.remove( + [workspace_b.clone()], + RemovalIntent::CloseProject, + window, + cx, + ) }) .unwrap(); cx.run_until_parked(); @@ -12094,7 +12099,12 @@ mod tests { .update(cx, |mw, window, cx| { // First switch back to A. mw.activate(workspace_a.clone(), None, window, cx); - mw.remove([workspace_b.clone()], |_, _, _| unreachable!(), window, cx) + mw.remove( + [workspace_b.clone()], + RemovalIntent::CloseProject, + window, + cx, + ) }) .unwrap(); cx.run_until_parked();