From 7658ca26a9be42381d45f4947be850d01b6d90b4 Mon Sep 17 00:00:00 2001 From: Remco Smits Date: Mon, 20 Jul 2026 05:30:22 +0200 Subject: [PATCH] recent_projects: Fix stale information when removing project from a group (#61062) # Objective Before this PR we would show inside the recent project picker the projects per group (window) which is great, but when you were trying to remove a project from the group it would show you a label that includes the just removed project and also when you clicked on it, it would bring you back to the 2 project's in the same window in this case. ## Solution My solution is when we actually remove the project from a group, we also update the group key that includes the newly updated path list, that should no longer have the removed project path in it. I also refactored it a bit so we have one method that does the project removing part so we don't have to duplicate this logic. ## Testing You can produce this when you do the following: 1. Open Zed 2. Add 2 projects to your window using the project panel 3. Open the recent project picker 4. See that the `This window` header shows 2 projects 5. Remove one of the projects 6. See that below the `This window` header we would show you a entry that shows you `projectA, projectB` where **projectb** should be removed from the group. I also added a regression test that covers this issue, so we can make sure the change actually works and does not regress of couse :). ## 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 adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable ## Showcase **Before** (**Note** before this change we showed the group project names comma seperated) https://github.com/user-attachments/assets/750358f4-f347-4f91-86b7-6500b2e09af9 **After** (**Note** after we now updated the groups key so it does not include the removed project) https://github.com/user-attachments/assets/f15acea8-d936-4aab-b5c2-324ee49f74a2 --- Release Notes: - Recent projects: Fixed that it shows you a removed project from a group --- crates/recent_projects/src/recent_projects.rs | 187 +++++++++++++++--- 1 file changed, 160 insertions(+), 27 deletions(-) diff --git a/crates/recent_projects/src/recent_projects.rs b/crates/recent_projects/src/recent_projects.rs index 02a05f8929..8b82c2dd60 100644 --- a/crates/recent_projects/src/recent_projects.rs +++ b/crates/recent_projects/src/recent_projects.rs @@ -771,20 +771,13 @@ impl RecentProjects { match picker.delegate.filtered_entries.get(ix) { Some(ProjectPickerEntry::OpenFolder { index, .. }) => { - if let Some(folder) = picker.delegate.open_folders.get(*index) { - let worktree_id = folder.worktree_id; - let Some(workspace) = picker.delegate.workspace.upgrade() else { - return; - }; - workspace.update(cx, |workspace, cx| { - let project = workspace.project().clone(); - project.update(cx, |project, cx| { - project.remove_worktree(worktree_id, cx); - }); - }); - picker.delegate.open_folders = get_open_folders(workspace.read(cx), cx); - let query = picker.query(cx); - picker.update_matches(query, window, cx); + if let Some(worktree_id) = picker + .delegate + .open_folders + .get(*index) + .map(|f| f.worktree_id) + { + RecentProjectsDelegate::remove_open_folder(picker, worktree_id, window, cx); } } Some(ProjectPickerEntry::ProjectGroup(hit)) => { @@ -1271,19 +1264,12 @@ impl PickerDelegate for RecentProjectsDelegate { } }) .on_click(cx.listener(move |picker, _, window, cx| { - let Some(workspace) = picker.delegate.workspace.upgrade() else { - return; - }; - workspace.update(cx, |workspace, cx| { - let project = workspace.project().clone(); - project.update(cx, |project, cx| { - project.remove_worktree(worktree_id, cx); - }); - }); - picker.delegate.open_folders = - get_open_folders(workspace.read(cx), cx); - let query = picker.query(cx); - picker.update_matches(query, window, cx); + RecentProjectsDelegate::remove_open_folder( + picker, + worktree_id, + window, + cx, + ); })), ) .into_any_element(); @@ -2383,6 +2369,39 @@ impl RecentProjectsDelegate { } } + fn remove_open_folder( + picker: &mut Picker, + worktree_id: WorktreeId, + window: &mut Window, + cx: &mut Context>, + ) { + let Some(workspace) = picker.delegate.workspace.upgrade() else { + return; + }; + + let old_key = workspace.read(cx).project_group_key(cx); + workspace.update(cx, |workspace, cx| { + let project = workspace.project().clone(); + project.update(cx, |project, cx| { + project.remove_worktree(worktree_id, cx); + }); + }); + + let new_key = workspace.read(cx).project_group_key(cx); + if let Some(entry) = picker + .delegate + .window_project_groups + .iter_mut() + .find(|key| **key == old_key) + { + *entry = new_key; + } + + picker.delegate.open_folders = get_open_folders(workspace.read(cx), cx); + let query = picker.query(cx); + picker.update_matches(query, window, cx); + } + fn remove_project_group( &mut self, key: ProjectGroupKey, @@ -3185,4 +3204,118 @@ mod tests { "remote project group confirm should not create a local workspace" ); } + + #[gpui::test] + async fn test_remove_open_folder_rekeys_this_window_group(cx: &mut TestAppContext) { + // Regression test: removing a folder from the active project while the + // picker is open must update the "This Window" group so it no longer + // lists the removed folder. + let app_state = init_test(cx); + app_state + .fs + .as_fake() + .insert_tree(path!("/a"), json!({ "1.txt": "" })) + .await; + app_state + .fs + .as_fake() + .insert_tree(path!("/b"), json!({ "2.txt": "" })) + .await; + + cx.update(|cx| { + open_paths( + &[PathBuf::from(path!("/a")), PathBuf::from(path!("/b"))], + app_state, + workspace::OpenOptions::default(), + cx, + ) + }) + .await + .unwrap(); + cx.run_until_parked(); + + let mw = cx.update(|cx| cx.windows()[0].downcast::().unwrap()); + let (workspace, active_key, fh) = mw + .read_with(cx, |mw, cx| { + let ws = mw.workspace().clone(); + ( + ws.clone(), + ws.read(cx).project_group_key(cx), + ws.read(cx).focus_handle(cx), + ) + }) + .unwrap(); + + assert_eq!( + active_key.path_list().paths().len(), + 2, + "group should span both folders before removal" + ); + let groups = vec![active_key]; + + let popover: Entity = cx.update(|cx| { + let window = cx.windows()[0]; + window + .update(cx, |_, window, cx| { + RecentProjects::popover( + workspace.downgrade(), + groups, + Some(false), + fh, + window, + cx, + ) + }) + .unwrap() + }); + cx.run_until_parked(); + + let picker: Entity> = cx.update(|cx| { + let window = cx.windows()[0]; + window + .update(cx, |_, _window, cx| popover.read(cx).picker.clone()) + .unwrap() + }); + cx.run_until_parked(); + + let a_worktree_id = workspace + .read_with(cx, |workspace, cx| { + workspace + .project() + .read(cx) + .visible_worktrees(cx) + .find(|wt| wt.read(cx).abs_path().ends_with("a")) + .map(|wt| wt.read(cx).id()) + }) + .expect("a worktree should exist"); + + cx.update(|cx| { + let window = cx.windows()[0]; + window + .update(cx, |_, window, cx| { + picker.update(cx, |picker, cx| { + RecentProjectsDelegate::remove_open_folder( + picker, + a_worktree_id, + window, + cx, + ); + }); + }) + .unwrap(); + }); + cx.run_until_parked(); + + let groups_after = picker.read_with(cx, |picker, _| { + picker.delegate.window_project_groups.clone() + }); + assert!( + !groups_after.iter().any(|key| key + .path_list() + .paths() + .iter() + .any(|path| path.ends_with("a"))), + "the removed folder should no longer appear in any This Window group, got {groups_after:?}" + ); + } }