From 9fbe317588ac48792d29100315fddc6c15c07e31 Mon Sep 17 00:00:00 2001 From: Danilo Leal <67129314+danilo-leal@users.noreply.github.com> Date: Thu, 16 Apr 2026 19:38:01 -0300 Subject: [PATCH] sidebar: Fix cmd-click in the header not taking to the last workspace (#54128) Fixes a bug where we weren't properly recording the last active workspace to power the cmd-click interaction in the sidebar's header. This PR introduces a field in the sidebar struct for that, allowing to store that value and to come back to it when clicking on the header, which is the single caller of the function introduced here. Release Notes: - Agent: Fixed a bug where cmd-clicking on the project header wouldn't actually take you to the last active workspace. --- crates/sidebar/src/sidebar.rs | 28 ++++-- crates/sidebar/src/sidebar_tests.rs | 126 ++++++++++++++++++++++++ crates/workspace/src/multi_workspace.rs | 29 +++++- 3 files changed, 173 insertions(+), 10 deletions(-) diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index 4100fcd85790e6e66d9fba26f8e975480d58179d..2776582125e15f9f2cd0250f40fe749fe76a4fd8 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -1580,13 +1580,7 @@ impl Sidebar { .on_click( cx.listener(move |this, event: &gpui::ClickEvent, window, cx| { if event.modifiers().secondary() { - if let Some(workspace) = this.workspace_for_group(&key_for_focus, cx) { - this.activate_workspace(&workspace, window, cx); - } else { - this.open_workspace_for_group(&key_for_focus, window, cx); - } - this.selection = None; - this.active_entry = None; + this.activate_or_open_workspace_for_group(&key_for_focus, window, cx); } else { this.toggle_collapse(&key_for_toggle, window, cx); } @@ -3877,6 +3871,26 @@ impl Sidebar { } } + pub(crate) fn activate_or_open_workspace_for_group( + &mut self, + key: &ProjectGroupKey, + window: &mut Window, + cx: &mut Context, + ) { + let workspace = self + .multi_workspace + .upgrade() + .and_then(|mw| mw.read(cx).last_active_workspace_for_group(key, cx)) + .or_else(|| self.workspace_for_group(key, cx)); + if let Some(workspace) = workspace { + self.activate_workspace(&workspace, window, cx); + } else { + self.open_workspace_for_group(key, window, cx); + } + self.selection = None; + self.active_entry = None; + } + fn active_project_group_key(&self, cx: &App) -> Option { let multi_workspace = self.multi_workspace.upgrade()?; let multi_workspace = multi_workspace.read(cx); diff --git a/crates/sidebar/src/sidebar_tests.rs b/crates/sidebar/src/sidebar_tests.rs index 66820d513a5ceaa5919038d88877c66136397bc4..2540c50a6f8f3ad7225846fadccb2acdde0ed365 100644 --- a/crates/sidebar/src/sidebar_tests.rs +++ b/crates/sidebar/src/sidebar_tests.rs @@ -10368,3 +10368,129 @@ async fn test_collab_guest_move_thread_paths_is_noop(cx: &mut TestAppContext) { ); }); } + +#[gpui::test] +async fn test_cmd_click_project_header_returns_to_last_active_linked_worktree_workspace( + cx: &mut TestAppContext, +) { + // Regression test for: cmd-clicking a project group header should return + // the user to the workspace they most recently had active in that group, + // including workspaces rooted at a linked worktree. + init_test(cx); + let fs = FakeFs::new(cx.executor()); + + fs.insert_tree( + "/project-a", + serde_json::json!({ + ".git": {}, + "src": {}, + }), + ) + .await; + fs.insert_tree("/project-b", serde_json::json!({ "src": {} })) + .await; + + fs.add_linked_worktree_for_repo( + Path::new("/project-a/.git"), + false, + git::repository::Worktree { + path: std::path::PathBuf::from("/wt-feature-a"), + ref_name: Some("refs/heads/feature-a".into()), + sha: "aaa".into(), + is_main: false, + is_bare: false, + }, + ) + .await; + + cx.update(|cx| ::set_global(fs.clone(), cx)); + + let main_project_a = project::Project::test(fs.clone(), ["/project-a".as_ref()], cx).await; + let worktree_project_a = + project::Project::test(fs.clone(), ["/wt-feature-a".as_ref()], cx).await; + let project_b = project::Project::test(fs.clone(), ["/project-b".as_ref()], cx).await; + + main_project_a + .update(cx, |p, cx| p.git_scans_complete(cx)) + .await; + worktree_project_a + .update(cx, |p, cx| p.git_scans_complete(cx)) + .await; + + // The multi-workspace starts with the main-paths workspace of group A + // as the initially active workspace. + let (multi_workspace, cx) = cx + .add_window_view(|window, cx| MultiWorkspace::test_new(main_project_a.clone(), window, cx)); + + let sidebar = setup_sidebar(&multi_workspace, cx); + + // Capture the initially active workspace (group A's main-paths workspace) + // *before* registering additional workspaces, since `workspaces()` returns + // retained workspaces in registration order — not activation order — and + // the multi-workspace's starting workspace may not be retained yet. + let main_workspace_a = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()); + + // Register the linked-worktree workspace (group A) and the group-B + // workspace. Both get retained by the multi-workspace. + let worktree_workspace_a = multi_workspace.update_in(cx, |mw, window, cx| { + mw.test_add_workspace(worktree_project_a.clone(), window, cx) + }); + let workspace_b = multi_workspace.update_in(cx, |mw, window, cx| { + mw.test_add_workspace(project_b.clone(), window, cx) + }); + + cx.run_until_parked(); + + // Step 1: activate the linked-worktree workspace. The MultiWorkspace + // records this as the last-active workspace for group A on its + // ProjectGroupState. (We don't assert on the initial active workspace + // because `test_add_workspace` may auto-activate newly registered + // workspaces — what matters for this test is the explicit sequence of + // activations below.) + multi_workspace.update_in(cx, |mw, window, cx| { + mw.activate(worktree_workspace_a.clone(), window, cx); + }); + cx.run_until_parked(); + assert_eq!( + multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()), + worktree_workspace_a, + "linked-worktree workspace should be active after step 1" + ); + + // Step 2: switch to the workspace for group B. Group A's last-active + // workspace remains the linked-worktree one (group B getting activated + // records *its own* last-active workspace, not group A's). + multi_workspace.update_in(cx, |mw, window, cx| { + mw.activate(workspace_b.clone(), window, cx); + }); + cx.run_until_parked(); + assert_eq!( + multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()), + workspace_b, + "group B's workspace should be active after step 2" + ); + + // Step 3: simulate cmd-click on group A's header. The project group key + // for group A is derived from the *main-paths* workspace (linked-worktree + // workspaces share the same key because it normalizes to main-worktree + // paths). + let group_a_key = main_workspace_a.read_with(cx, |ws, cx| ws.project_group_key(cx)); + sidebar.update_in(cx, |sidebar, window, cx| { + sidebar.activate_or_open_workspace_for_group(&group_a_key, window, cx); + }); + cx.run_until_parked(); + + // Expected: we're back in the linked-worktree workspace, not the + // main-paths one. + let active_after_cmd_click = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()); + assert_eq!( + active_after_cmd_click, worktree_workspace_a, + "cmd-click on group A's header should return to the last-active \ + linked-worktree workspace, not the main-paths workspace" + ); + assert_ne!( + active_after_cmd_click, main_workspace_a, + "cmd-click must not fall back to the main-paths workspace when a \ + linked-worktree workspace was the last-active one for the group" + ); +} diff --git a/crates/workspace/src/multi_workspace.rs b/crates/workspace/src/multi_workspace.rs index 970285f4d199b0e6a4f890818ab0594129e8cccf..204e2c45f7000264676d13758a7dff6cb88ec60e 100644 --- a/crates/workspace/src/multi_workspace.rs +++ b/crates/workspace/src/multi_workspace.rs @@ -3,8 +3,8 @@ use fs::Fs; use gpui::{ AnyView, App, Context, DragMoveEvent, Entity, EntityId, EventEmitter, FocusHandle, Focusable, - ManagedView, MouseButton, Pixels, Render, Subscription, Task, Tiling, Window, WindowId, - actions, deferred, px, + ManagedView, MouseButton, Pixels, Render, Subscription, Task, Tiling, WeakEntity, Window, + WindowId, actions, deferred, px, }; pub use project::ProjectGroupKey; use project::{DisableAiSettings, Project}; @@ -279,6 +279,7 @@ pub struct SerializedProjectGroupState { pub struct ProjectGroupState { pub key: ProjectGroupKey, pub expanded: bool, + pub last_active_workspace: Option>, } pub struct MultiWorkspace { @@ -634,6 +635,7 @@ impl MultiWorkspace { ProjectGroupState { key, expanded: true, + last_active_workspace: None, }, ); } @@ -756,7 +758,11 @@ impl MultiWorkspace { if restored.iter().any(|group| group.key == key) { continue; } - restored.push(ProjectGroupState { key, expanded }); + restored.push(ProjectGroupState { + key, + expanded, + last_active_workspace: None, + }); } for existing in std::mem::take(&mut self.project_groups) { if !restored.iter().any(|group| group.key == existing.key) { @@ -793,6 +799,17 @@ impl MultiWorkspace { 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) + } + pub fn group_state_by_key(&self, key: &ProjectGroupKey) -> Option<&ProjectGroupState> { self.project_groups.iter().find(|group| group.key == *key) } @@ -1214,6 +1231,11 @@ impl MultiWorkspace { self.active_workspace = workspace; + 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()); + } + if !self.sidebar_open && !old_active_was_retained { self.detach_workspace(&old_active_workspace, cx); } @@ -1501,6 +1523,7 @@ impl MultiWorkspace { self.project_groups.push(ProjectGroupState { key: group.key, expanded: group.expanded, + last_active_workspace: None, }); }