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
This commit is contained in:
Remco Smits authored and GitHub committed
1 parent
9005ced680
commit
7658ca26a9
1 file changed
+160
-27
@@ -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<Self>,
|
||||
worktree_id: WorktreeId,
|
||||
window: &mut Window,
|
||||
cx: &mut Context<Picker<Self>>,
|
||||
) {
|
||||
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::<MultiWorkspace>().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<RecentProjects> = 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<Picker<RecentProjectsDelegate>> = 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:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user