From 8230cb16d1cffc1e6be044f2292d9f3189bf53a3 Mon Sep 17 00:00:00 2001 From: Elliot Thomas Date: Thu, 9 Jul 2026 13:32:00 +0100 Subject: [PATCH] project_panel: Restore worktree drag-and-drop reordering (#55755) Reordering worktree roots by drag-and-drop had silently broken: a worktree-root filter added to `disjoint_entries` for delete-safety stripped roots before the drop handler could see them, so root-to-root drops never reached the existing `move_worktree` reorder path. This PR is the minimal regression fix: - Move the worktree-root filter out of `disjoint_entries` and into `disjoint_effective_entries` (used by cut/copy/delete), so drag-and-drop keeps seeing roots and single-root reorder works again via the existing `move_worktree` path. - Filter worktree roots out of `drag_onto`'s copy branch, so holding the copy modifier over a drag that contains a root no longer returns `None` from `create_paste_path` and silently cancels the whole copy. - Add `test_drag_worktree_root_reorders_worktrees` exercising the drag-onto reorder flow end to end. The larger feature work (multi-root group reordering, blank-area "send to end", copy-mode drag feedback, and syncing worktree order to collaborators) has been split into a separate follow-up PR so this fix can land quickly. Note that worktree order was intentionally not synced during collaboration, so that change is discussed separately. Closes #46699 Release Notes: - Fixed drag and drop to reorder worktrees --------- Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: Smit Barmase --- crates/project_panel/src/project_panel.rs | 55 +++--- .../project_panel/src/project_panel_tests.rs | 177 ++++++++++++++++++ 2 files changed, 206 insertions(+), 26 deletions(-) diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 8b23423a973..e8c4bf2f63d 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -2553,7 +2553,7 @@ impl ProjectPanel { cx: &mut Context, ) { maybe!({ - let items_to_delete = self.disjoint_effective_entries(cx); + let items_to_delete = self.disjoint_effective_entries_excluding_roots(cx); if items_to_delete.is_empty() { return None; } @@ -3266,7 +3266,7 @@ impl ProjectPanel { } fn cut(&mut self, _: &Cut, _: &mut Window, cx: &mut Context) { - let entries = self.disjoint_effective_entries(cx); + let entries = self.disjoint_effective_entries_excluding_roots(cx); if !entries.is_empty() { self.write_entries_to_system_clipboard(&entries, cx); self.clipboard = Some(ClipboardEntry::Cut(entries)); @@ -3275,7 +3275,7 @@ impl ProjectPanel { } fn copy(&mut self, _: &Copy, _: &mut Window, cx: &mut Context) { - let entries = self.disjoint_effective_entries(cx); + let entries = self.disjoint_effective_entries_excluding_roots(cx); if !entries.is_empty() { self.write_entries_to_system_clipboard(&entries, cx); self.clipboard = Some(ClipboardEntry::Copied(entries)); @@ -3870,25 +3870,6 @@ impl ProjectPanel { } } - fn move_entry( - &mut self, - entry_to_move: ProjectEntryId, - destination: ProjectEntryId, - destination_is_file: bool, - cx: &mut Context, - ) -> Option>> { - if self - .project - .read(cx) - .entry_is_worktree_root(entry_to_move, cx) - { - self.move_worktree_root(entry_to_move, destination, cx); - None - } else { - self.move_worktree_entry(entry_to_move, destination, destination_is_file, cx) - } - } - fn move_worktree_root( &mut self, entry_to_move: ProjectEntryId, @@ -3971,8 +3952,14 @@ impl ProjectPanel { self.index_for_entry(selection.entry_id, selection.worktree_id) } - fn disjoint_effective_entries(&self, cx: &App) -> BTreeSet { - self.disjoint_entries(self.effective_entries(), cx) + fn disjoint_effective_entries_excluding_roots(&self, cx: &App) -> BTreeSet { + let project = self.project.read(cx); + let entries = self + .effective_entries() + .into_iter() + .filter(|entry| !project.entry_is_worktree_root(entry.entry_id, cx)) + .collect(); + self.disjoint_entries(entries, cx) } fn disjoint_entries( @@ -3988,7 +3975,6 @@ impl ProjectPanel { let project = self.project.read(cx); let entries_by_worktree: HashMap> = entries .into_iter() - .filter(|entry| !project.entry_is_worktree_root(entry.entry_id, cx)) .fold(HashMap::default(), |mut map, entry| { map.entry(entry.worktree_id).or_default().push(entry); map @@ -4745,6 +4731,21 @@ impl ProjectPanel { .collect::>(); let entries = self.disjoint_entries(resolved_selections, cx); + let root_entries: Vec = { + let project = self.project.read(cx); + entries + .iter() + .filter(|entry| project.entry_is_worktree_root(entry.entry_id, cx)) + .map(|entry| entry.entry_id) + .collect() + }; + if !root_entries.is_empty() { + for entry_id in root_entries { + self.move_worktree_root(entry_id, target_entry_id, cx); + } + return; + } + if Self::is_copy_modifier_set(&window.modifiers()) { let _ = maybe!({ let project = self.project.read(cx); @@ -4862,7 +4863,9 @@ impl ProjectPanel { // results with folded selections that need refreshing. let mut move_tasks: Vec<(ProjectEntryId, Task>)> = Vec::new(); for entry in entries { - if let Some(task) = self.move_entry(entry.entry_id, target_entry_id, is_file, cx) { + if let Some(task) = + self.move_worktree_entry(entry.entry_id, target_entry_id, is_file, cx) + { move_tasks.push((entry.entry_id, task)); } } diff --git a/crates/project_panel/src/project_panel_tests.rs b/crates/project_panel/src/project_panel_tests.rs index 3e4e3788da7..efc5a191b1c 100644 --- a/crates/project_panel/src/project_panel_tests.rs +++ b/crates/project_panel/src/project_panel_tests.rs @@ -4428,6 +4428,141 @@ async fn test_rename_with_hide_root(cx: &mut gpui::TestAppContext) { } } +async fn setup_three_worktree_panel( + cx: &mut gpui::TestAppContext, +) -> (Entity, VisualTestContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree("/root1", json!({ "a.txt": "" })).await; + fs.insert_tree("/root2", json!({ "b.txt": "" })).await; + fs.insert_tree("/root3", json!({ "c.txt": "" })).await; + + let project = Project::test( + fs.clone(), + ["/root1".as_ref(), "/root2".as_ref(), "/root3".as_ref()], + cx, + ) + .await; + let window = cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let workspace = window + .read_with(cx, |mw, _| mw.workspace().clone()) + .unwrap(); + let mut cx = VisualTestContext::from_window(window.into(), cx); + let panel = workspace.update_in(&mut cx, ProjectPanel::new); + cx.run_until_parked(); + (panel, cx) +} + +#[gpui::test] +async fn test_drag_worktree_root_reorders_worktrees(cx: &mut gpui::TestAppContext) { + let (panel, mut cx) = setup_three_worktree_panel(cx).await; + let cx = &mut cx; + + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &[ + "v root1", + " a.txt", + "v root2", + " b.txt", + "v root3", + " c.txt", + ], + "worktrees should start in insertion order" + ); + + // [r1, r2, r3] -> [r2, r1, r3]. + drag_entries_onto(&panel, &["root1"], "root2", false, cx); + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &[ + "v root2", + " b.txt", + "v root1", + " a.txt", + "v root3", + " c.txt", + ], + "dragging root1 onto root2 should swap their positions" + ); + + // [r2, r1, r3] -> [r3, r2, r1]. + drag_entries_onto(&panel, &["root3"], "root2", false, cx); + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &[ + "v root3", + " c.txt", + "v root2", + " b.txt", + "v root1", + " a.txt", + ], + "dragging the last root onto the first should move it to the front" + ); +} + +#[gpui::test] +async fn test_drag_including_worktree_root_only_reorders(cx: &mut gpui::TestAppContext) { + let (panel, mut cx) = setup_three_worktree_panel(cx).await; + let cx = &mut cx; + + // Drag {root1, root2/b.txt} onto root3's root entry: only the worktree + // reorder should happen and b.txt must stay in root2. + drag_entries_onto(&panel, &["root1", "root2/b.txt"], "root3", false, cx); + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &[ + "v root2", + " b.txt", + "v root3", + " c.txt", + "v root1", + " a.txt", + ], + "dropping a mixed selection on a root should only reorder worktrees" + ); + + // Drag {root2, root3/c.txt} onto root1/a.txt (a non-root entry): the root + // still reorders to root1's position and c.txt must stay in root3. + drag_entries_onto(&panel, &["root2", "root3/c.txt"], "root1/a.txt", true, cx); + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &[ + "v root3", + " c.txt", + "v root1", + " a.txt", + "v root2", + " b.txt", + ], + "dropping a mixed selection on a non-root entry should only reorder worktrees" + ); + + // With the copy modifier held, a selection containing a root should still + // only reorder worktrees and copy nothing. + cx.simulate_modifiers_change(gpui::Modifiers { + alt: true, + control: true, + ..Default::default() + }); + drag_entries_onto(&panel, &["root3", "root1/a.txt"], "root2", false, cx); + cx.simulate_modifiers_change(Default::default()); + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &[ + "v root1", + " a.txt", + "v root2", + " b.txt", + "v root3", + " c.txt", + ], + "copy-dragging a mixed selection should only reorder worktrees and copy nothing" + ); +} + #[gpui::test] async fn test_multiple_marked_entries(cx: &mut gpui::TestAppContext) { init_test_with_editor(cx); @@ -10069,6 +10204,48 @@ pub(crate) fn drag_selection_to( cx.executor().run_until_parked(); } +/// Drags the entries at `source_paths` onto `target_path`. Paths are worktree +/// root names optionally followed by a path inside the worktree, e.g. "root1" +/// or "root1/dir/file.txt". The first source path is the active selection. +pub(crate) fn drag_entries_onto( + panel: &Entity, + source_paths: &[&str], + target_path: &str, + target_is_file: bool, + cx: &mut VisualTestContext, +) { + let target_entry_id = find_project_entry(panel, target_path, cx) + .unwrap_or_else(|| panic!("no entry for target path {target_path:?}")); + let selections: Vec = source_paths + .iter() + .map(|path| { + let entry_id = find_project_entry(panel, path, cx) + .unwrap_or_else(|| panic!("no entry for source path {path:?}")); + let worktree_id = panel + .update(cx, |panel, cx| { + panel.project.read(cx).worktree_id_for_entry(entry_id, cx) + }) + .unwrap_or_else(|| panic!("no worktree for source path {path:?}")); + SelectedEntry { + worktree_id, + entry_id, + } + }) + .collect(); + let active_selection = *selections + .first() + .expect("at least one source path is required"); + + panel.update_in(cx, |panel, window, cx| { + let drag = DraggedSelection { + active_selection, + marked_selections: Arc::from(selections), + }; + panel.drag_onto(&drag, target_entry_id, target_is_file, window, cx); + }); + cx.executor().run_until_parked(); +} + pub(crate) fn find_project_entry( panel: &Entity, path: &str,