From cc20ae3fb54d0563efe99e42507474840a563daf Mon Sep 17 00:00:00 2001 From: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com> Date: Fri, 8 May 2026 21:54:42 -0400 Subject: [PATCH] workspace: Always add projects to windows (#56187) Right now the recent project picker has different confirm behavior depending on if a user had open their sidebar in their current Zed session. If the sidebar had been open the recent project picker adds the selected project to the multi workspace and makes it the active workspace, without removing anything. If the sidebar hadn't been open the recent project picker would replace the active workspace within the multi workspace. This caused confusion because the same UX flow had two different outcomes depending on Zed's state that wasn't obvious to users. This PR mitigates this by always adding the project to the window while AI features are enabled. Future follow ups will include the ability to disable the sidebar, but that's blocked on the agent panel not having a way to view active threads currently. This also caused issues in the parallel agents workflow because replacing a workspace would drop the workspace, thus causing any terminal processes or threads to be dropped as well. Self-Review Checklist: - [x] 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 or Added/Fixed/Improved ... --- crates/recent_projects/src/recent_projects.rs | 205 ++++++++++-------- crates/sidebar/src/sidebar_tests.rs | 39 ++-- crates/workspace/src/multi_workspace.rs | 21 +- crates/workspace/src/multi_workspace_tests.rs | 53 ++++- 4 files changed, 189 insertions(+), 129 deletions(-) diff --git a/crates/recent_projects/src/recent_projects.rs b/crates/recent_projects/src/recent_projects.rs index a53d524885f9cb2be69345880a8aba9de4396690..7ed1db6bfc51bbe1aa38a6c7374e9b2d21a8c4d2 100644 --- a/crates/recent_projects/src/recent_projects.rs +++ b/crates/recent_projects/src/recent_projects.rs @@ -78,8 +78,28 @@ struct OpenFolderEntry { #[derive(Clone, Debug)] enum ProjectPickerEntry { Header(SharedString), - OpenFolder { index: usize, positions: Vec }, + /// A currently open folder from the active workspace's "Current Folders" section. + /// + /// `index` points into `RecentProjectsDelegate::open_folders`, and `positions` stores the + /// fuzzy-match highlight positions for rendering the folder name. + OpenFolder { + index: usize, + positions: Vec, + }, + /// A project group from the current window's "This Window" section. + /// + /// These entries come from `RecentProjectsDelegate::window_project_groups`, not from the + /// recent-project database. Empty queries list every project group known to the current + /// window; non-empty queries list matching project groups. Confirming one activates or loads + /// that project group in the current window, while secondary confirm can move local project + /// groups to a new window when multiple groups are available. ProjectGroup(StringMatch), + /// A workspace from the recent-project database's "Recent Projects" section. + /// + /// The match's `candidate_id` indexes into `RecentProjectsDelegate::workspaces`. Confirming + /// one opens that recent workspace in either the current window or a new window, depending on + /// whether the picker was invoked for new-window behavior and whether this was a primary or + /// secondary confirm. RecentProject(StringMatch), } @@ -1139,99 +1159,8 @@ impl PickerDelegate for RecentProjectsDelegate { cx.emit(DismissEvent); } Some(ProjectPickerEntry::RecentProject(selected_match)) => { - let Some(workspace) = self.workspace.upgrade() else { - return; - }; - let Some(candidate_workspace) = self.workspaces.get(selected_match.candidate_id) - else { - return; - }; - - let replace_current_window = self.create_new_window == secondary; - let candidate_workspace_id = candidate_workspace.workspace_id; - let candidate_workspace_location = candidate_workspace.location.clone(); - let candidate_workspace_paths = candidate_workspace.paths.clone(); - - workspace.update(cx, |workspace, cx| { - if workspace.database_id() == Some(candidate_workspace_id) { - return; - } - match candidate_workspace_location { - SerializedWorkspaceLocation::Local => { - let paths = candidate_workspace_paths.paths().to_vec(); - if replace_current_window { - if let Some(handle) = - window.window_handle().downcast::() - { - cx.defer(move |cx| { - if let Some(task) = handle - .update(cx, |multi_workspace, window, cx| { - multi_workspace.open_project( - paths, - OpenMode::Activate, - window, - cx, - ) - }) - .log_err() - { - task.detach_and_log_err(cx); - } - }); - } - return; - } else { - workspace - .open_workspace_for_paths( - OpenMode::NewWindow, - paths, - window, - cx, - ) - .detach_and_prompt_err( - "Failed to open project", - window, - cx, - |_, _, _| None, - ); - } - } - SerializedWorkspaceLocation::Remote(mut connection) => { - let app_state = workspace.app_state().clone(); - let replace_window = if replace_current_window { - window.window_handle().downcast::() - } else { - None - }; - let open_options = OpenOptions { - requesting_window: replace_window, - ..Default::default() - }; - if let RemoteConnectionOptions::Ssh(connection) = &mut connection { - RemoteSettings::get_global(cx) - .fill_connection_options_from_settings(connection); - }; - let paths = candidate_workspace_paths.paths().to_vec(); - cx.spawn_in(window, async move |_, cx| { - open_remote_project( - connection.clone(), - paths, - app_state, - open_options, - cx, - ) - .await - }) - .detach_and_prompt_err( - "Failed to open project", - window, - cx, - |_, _, _| None, - ); - } - } - }); - cx.emit(DismissEvent); + let candidate_id = selected_match.candidate_id; + self.open_recent_projects(candidate_id, secondary, window, cx); } _ => {} } @@ -2077,6 +2006,94 @@ fn open_local_project( } impl RecentProjectsDelegate { + fn open_recent_projects( + &mut self, + candidate_id: usize, + secondary: bool, + window: &mut Window, + cx: &mut Context>, + ) { + let Some(workspace) = self.workspace.upgrade() else { + return; + }; + let Some(candidate_workspace) = self.workspaces.get(candidate_id) else { + return; + }; + + let replace_current_window = self.create_new_window == secondary; + let candidate_workspace_id = candidate_workspace.workspace_id; + let candidate_workspace_location = candidate_workspace.location.clone(); + let candidate_workspace_paths = candidate_workspace.paths.clone(); + + workspace.update(cx, |workspace, cx| { + if workspace.database_id() == Some(candidate_workspace_id) { + return; + } + match candidate_workspace_location { + SerializedWorkspaceLocation::Local => { + let paths = candidate_workspace_paths.paths().to_vec(); + if replace_current_window { + if let Some(handle) = window.window_handle().downcast::() { + cx.defer(move |cx| { + if let Some(task) = handle + .update(cx, |multi_workspace, window, cx| { + multi_workspace.open_project( + paths, + OpenMode::Activate, + window, + cx, + ) + }) + .log_err() + { + task.detach_and_log_err(cx); + } + }); + } + return; + } else { + workspace + .open_workspace_for_paths(OpenMode::NewWindow, paths, window, cx) + .detach_and_prompt_err( + "Failed to open project", + window, + cx, + |_, _, _| None, + ); + } + } + SerializedWorkspaceLocation::Remote(mut connection) => { + let app_state = workspace.app_state().clone(); + let replace_window = if replace_current_window { + window.window_handle().downcast::() + } else { + None + }; + let open_options = OpenOptions { + requesting_window: replace_window, + ..Default::default() + }; + if let RemoteConnectionOptions::Ssh(connection) = &mut connection { + RemoteSettings::get_global(cx) + .fill_connection_options_from_settings(connection); + }; + let paths = candidate_workspace_paths.paths().to_vec(); + cx.spawn_in(window, async move |_, cx| { + open_remote_project(connection.clone(), paths, app_state, open_options, cx) + .await + }) + .detach_and_prompt_err( + "Failed to open project", + window, + cx, + |_, _, _| None, + ); + } + } + }); + cx.emit(DismissEvent); + } + fn add_paths_to_project( &mut self, paths: Vec, diff --git a/crates/sidebar/src/sidebar_tests.rs b/crates/sidebar/src/sidebar_tests.rs index 1d553d7d5ad625134600476f5212b34e641dd998..8939992f64c250857215d40e8ee1ee39dd06aa5d 100644 --- a/crates/sidebar/src/sidebar_tests.rs +++ b/crates/sidebar/src/sidebar_tests.rs @@ -8198,14 +8198,13 @@ async fn add_test_project( } #[gpui::test] -async fn test_transient_workspace_lifecycle(cx: &mut TestAppContext) { +async fn test_workspace_lifecycle_retains_projects_when_sidebar_is_closed(cx: &mut TestAppContext) { let (fs, project_a) = init_multi_project_test(&["/project-a", "/project-b", "/project-c"], cx).await; let (multi_workspace, cx) = cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a, window, cx)); let _sidebar = setup_sidebar_closed(&multi_workspace, cx); - // Sidebar starts closed. Initial workspace A is transient. let workspace_a = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()); assert!(!multi_workspace.read_with(cx, |mw, _| mw.sidebar_open())); assert_eq!( @@ -8214,25 +8213,25 @@ async fn test_transient_workspace_lifecycle(cx: &mut TestAppContext) { ); assert!(multi_workspace.read_with(cx, |mw, _| mw.workspace() == &workspace_a)); - // Add B — replaces A as the transient workspace. let workspace_b = add_test_project("/project-b", &fs, &multi_workspace, cx).await; assert_eq!( multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()), - 1 + 2 ); assert!(multi_workspace.read_with(cx, |mw, _| mw.workspace() == &workspace_b)); + assert!(multi_workspace.read_with(cx, |mw, _| mw.workspaces().any(|w| w == &workspace_a))); - // Add C — replaces B as the transient workspace. let workspace_c = add_test_project("/project-c", &fs, &multi_workspace, cx).await; assert_eq!( multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()), - 1 + 3 ); assert!(multi_workspace.read_with(cx, |mw, _| mw.workspace() == &workspace_c)); + assert!(multi_workspace.read_with(cx, |mw, _| mw.workspaces().any(|w| w == &workspace_b))); } #[gpui::test] -async fn test_transient_workspace_retained(cx: &mut TestAppContext) { +async fn test_workspaces_remain_retained_after_sidebar_closes(cx: &mut TestAppContext) { let (fs, project_a) = init_multi_project_test( &["/project-a", "/project-b", "/project-c", "/project-d"], cx, @@ -8242,15 +8241,14 @@ async fn test_transient_workspace_retained(cx: &mut TestAppContext) { cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a, window, cx)); let _sidebar = setup_sidebar(&multi_workspace, cx); assert!(multi_workspace.read_with(cx, |mw, _| mw.sidebar_open())); + let workspace_a = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()); - // Add B — retained since sidebar is open. - let workspace_a = add_test_project("/project-b", &fs, &multi_workspace, cx).await; + let workspace_b = add_test_project("/project-b", &fs, &multi_workspace, cx).await; assert_eq!( multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()), 2 ); - // Switch to A — B survives. (Switching from one internal workspace, to another) multi_workspace.update_in(cx, |mw, window, cx| { mw.activate(workspace_a, None, window, cx) }); @@ -8259,8 +8257,8 @@ async fn test_transient_workspace_retained(cx: &mut TestAppContext) { multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()), 2 ); + assert!(multi_workspace.read_with(cx, |mw, _| mw.workspaces().any(|w| w == &workspace_b))); - // Close sidebar — both A and B remain retained. multi_workspace.update_in(cx, |mw, window, cx| mw.close_sidebar(window, cx)); cx.run_until_parked(); assert_eq!( @@ -8268,7 +8266,6 @@ async fn test_transient_workspace_retained(cx: &mut TestAppContext) { 2 ); - // Add C — added as new transient workspace. (switching from retained, to transient) let workspace_c = add_test_project("/project-c", &fs, &multi_workspace, cx).await; assert_eq!( multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()), @@ -8276,52 +8273,50 @@ async fn test_transient_workspace_retained(cx: &mut TestAppContext) { ); assert!(multi_workspace.read_with(cx, |mw, _| mw.workspace() == &workspace_c)); - // Add D — replaces C as the transient workspace (Have retained and transient workspaces, transient workspace is dropped) let workspace_d = add_test_project("/project-d", &fs, &multi_workspace, cx).await; assert_eq!( multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()), - 3 + 4 ); assert!(multi_workspace.read_with(cx, |mw, _| mw.workspace() == &workspace_d)); + assert!(multi_workspace.read_with(cx, |mw, _| mw.workspaces().any(|w| w == &workspace_c))); } #[gpui::test] -async fn test_transient_workspace_promotion(cx: &mut TestAppContext) { +async fn test_sidebar_opening_keeps_existing_retained_workspaces(cx: &mut TestAppContext) { let (fs, project_a) = init_multi_project_test(&["/project-a", "/project-b", "/project-c"], cx).await; let (multi_workspace, cx) = cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a, window, cx)); setup_sidebar_closed(&multi_workspace, cx); - // Add B — replaces A as the transient workspace (A is discarded). + let workspace_a = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()); let workspace_b = add_test_project("/project-b", &fs, &multi_workspace, cx).await; assert_eq!( multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()), - 1 + 2 ); assert!(multi_workspace.read_with(cx, |mw, _| mw.workspace() == &workspace_b)); + assert!(multi_workspace.read_with(cx, |mw, _| mw.workspaces().any(|w| w == &workspace_a))); - // Open sidebar — promotes the transient B to retained. multi_workspace.update_in(cx, |mw, window, cx| { mw.toggle_sidebar(window, cx); }); cx.run_until_parked(); assert_eq!( multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()), - 1 + 2 ); assert!(multi_workspace.read_with(cx, |mw, _| mw.workspaces().any(|w| w == &workspace_b))); - // Close sidebar — the retained B remains. multi_workspace.update_in(cx, |mw, window, cx| { mw.toggle_sidebar(window, cx); }); - // Add C — added as new transient workspace. let workspace_c = add_test_project("/project-c", &fs, &multi_workspace, cx).await; assert_eq!( multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()), - 2 + 3 ); assert!(multi_workspace.read_with(cx, |mw, _| mw.workspace() == &workspace_c)); } diff --git a/crates/workspace/src/multi_workspace.rs b/crates/workspace/src/multi_workspace.rs index 999b4d30413a5c62ab71d646a3825fc54b7982da..b4ba998c771347cfdb083a3eac3ba9f4e5536467 100644 --- a/crates/workspace/src/multi_workspace.rs +++ b/crates/workspace/src/multi_workspace.rs @@ -324,12 +324,15 @@ impl MultiWorkspace { }); let quit_subscription = cx.on_app_quit(Self::app_will_quit); let settings_subscription = cx.observe_global_in::(window, { - let mut previous_disable_ai = DisableAiSettings::get_global(cx).disable_ai; + let mut previous_multi_workspace_enabled = !DisableAiSettings::get_global(cx) + .disable_ai + && AgentSettings::get_global(cx).enabled; move |this, window, cx| { - if DisableAiSettings::get_global(cx).disable_ai != previous_disable_ai { + let multi_workspace_enabled = this.multi_workspace_enabled(cx); + if previous_multi_workspace_enabled && !multi_workspace_enabled { this.collapse_to_single_workspace(window, cx); - previous_disable_ai = DisableAiSettings::get_global(cx).disable_ai; } + previous_multi_workspace_enabled = multi_workspace_enabled; } }); Self::subscribe_to_workspace(&workspace, window, cx); @@ -1428,11 +1431,17 @@ impl MultiWorkspace { let old_active_workspace = self.active_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); + } if !workspace_was_retained { self.register_workspace(&workspace, window, cx); - if self.sidebar_open { + if should_retain_workspaces { let key = workspace.read(cx).project_group_key(cx); self.retain_workspace(workspace.clone(), key, cx); } @@ -1445,7 +1454,7 @@ impl MultiWorkspace { group.last_active_workspace = Some(self.active_workspace.downgrade()); } - if !self.sidebar_open && !old_active_was_retained { + if !should_retain_workspaces && !old_active_was_retained { self.detach_workspace(&old_active_workspace, cx); } @@ -1471,7 +1480,7 @@ impl MultiWorkspace { } /// Collapses to a single workspace, discarding all groups. - /// Used when multi-workspace is disabled (e.g. disable_ai). + /// Used when multi-workspace is disabled by settings. fn collapse_to_single_workspace(&mut self, window: &mut Window, cx: &mut Context) { if self.sidebar_open { self.close_sidebar(window, cx); diff --git a/crates/workspace/src/multi_workspace_tests.rs b/crates/workspace/src/multi_workspace_tests.rs index 3b715fe80ca2b86598a5e0baba2acc9c31f1200c..a5099a70fd1414283330225b69c8f69aa39662b1 100644 --- a/crates/workspace/src/multi_workspace_tests.rs +++ b/crates/workspace/src/multi_workspace_tests.rs @@ -2,12 +2,13 @@ use std::path::PathBuf; use super::*; use crate::item::test::TestItem; +use agent_settings::AgentSettings; use client::proto; use fs::{FakeFs, Fs}; use gpui::{TestAppContext, VisualTestContext}; use project::DisableAiSettings; use serde_json::json; -use settings::SettingsStore; +use settings::{Settings, SettingsStore}; use util::path; fn init_test(cx: &mut TestAppContext) { @@ -90,6 +91,43 @@ async fn test_sidebar_disabled_when_disable_ai_is_enabled(cx: &mut TestAppContex }); } +#[gpui::test] +async fn test_multi_workspace_collapses_when_agent_is_disabled(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; + 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)); + + multi_workspace.update_in(cx, |multi_workspace, window, cx| { + multi_workspace.test_add_workspace(project_b, window, cx); + }); + cx.run_until_parked(); + + multi_workspace.read_with(cx, |multi_workspace, cx| { + assert!(multi_workspace.multi_workspace_enabled(cx)); + assert_eq!(multi_workspace.workspaces().count(), 2); + }); + + cx.update(|_window, cx| { + let mut settings = AgentSettings::get_global(cx).clone(); + settings.enabled = false; + AgentSettings::override_global(settings, cx); + }); + cx.run_until_parked(); + + multi_workspace.read_with(cx, |multi_workspace, cx| { + assert!(!multi_workspace.multi_workspace_enabled(cx)); + assert!(!multi_workspace.sidebar_open()); + assert_eq!(multi_workspace.workspaces().count(), 1); + assert!(multi_workspace.project_group_keys().is_empty()); + }); +} + #[gpui::test] async fn test_project_group_keys_initial(cx: &mut TestAppContext) { init_test(cx); @@ -618,7 +656,7 @@ async fn test_close_workspace_prefers_already_loaded_neighboring_workspace( } #[gpui::test] -async fn test_switching_projects_with_sidebar_closed_detaches_old_active_workspace( +async fn test_switching_projects_with_sidebar_closed_retains_old_active_workspace( cx: &mut TestAppContext, ) { init_test(cx); @@ -648,7 +686,7 @@ async fn test_switching_projects_with_sidebar_closed_detaches_old_active_workspa }); cx.run_until_parked(); - multi_workspace.read_with(cx, |mw, _cx| { + multi_workspace.read_with(cx, |mw, cx| { assert_eq!( mw.workspace().entity_id(), workspace_b.entity_id(), @@ -656,14 +694,15 @@ async fn test_switching_projects_with_sidebar_closed_detaches_old_active_workspa ); assert_eq!( mw.workspaces().count(), - 1, - "only the new active workspace should remain open after switching with the sidebar closed" + 2, + "the previous active workspace should remain open after switching with the sidebar closed" ); + assert_eq!(mw.project_groups(cx).len(), 2); }); assert!( - workspace_a.read_with(cx, |workspace, _cx| workspace.session_id().is_none()), - "the previous active workspace should be detached when switching away with the sidebar closed" + workspace_a.read_with(cx, |workspace, _cx| workspace.session_id().is_some()), + "the previous active workspace should remain attached when switching away with the sidebar closed" ); }