From 2bd58793457cec1229afb98e87cc030a0c44c1a2 Mon Sep 17 00:00:00 2001 From: eth0net Date: Tue, 5 May 2026 11:52:17 +0100 Subject: [PATCH 1/8] Fix drag and drop to reorder worktrees This functionality was lost previously due to a change filtering out worktree roots from drag and drop events. This change restores the drag and drop behaviour by moving the filter out of disjoint_entries and into disjoint_effective_entries. --- crates/project_panel/src/project_panel.rs | 22 ++-- .../project_panel/src/project_panel_tests.rs | 101 ++++++++++++++++++ 2 files changed, 115 insertions(+), 8 deletions(-) diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 1ae5f424845..5266a3ea5e8 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -3690,7 +3690,13 @@ impl ProjectPanel { } fn disjoint_effective_entries(&self, cx: &App) -> BTreeSet { - self.disjoint_entries(self.effective_entries(), cx) + 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( @@ -3704,13 +3710,13 @@ 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 - }); + let entries_by_worktree: HashMap> = + entries + .into_iter() + .fold(HashMap::default(), |mut map, entry| { + map.entry(entry.worktree_id).or_default().push(entry); + map + }); for (worktree_id, worktree_entries) in entries_by_worktree { if let Some(worktree) = project.worktree_for_id(worktree_id, cx) { diff --git a/crates/project_panel/src/project_panel_tests.rs b/crates/project_panel/src/project_panel_tests.rs index 4897b57937d..c47b6ddd84b 100644 --- a/crates/project_panel/src/project_panel_tests.rs +++ b/crates/project_panel/src/project_panel_tests.rs @@ -4110,6 +4110,107 @@ async fn test_rename_with_hide_root(cx: &mut gpui::TestAppContext) { } } +#[gpui::test] +async fn test_drag_worktree_root_reorders_worktrees(cx: &mut gpui::TestAppContext) { + 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 cx = &mut VisualTestContext::from_window(window.into(), cx); + let panel = workspace.update_in(cx, ProjectPanel::new); + cx.run_until_parked(); + + 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" + ); + + // Drag worktree root1 onto worktree root2: [r1, r2, r3] -> [r2, r1, r3]. + panel.update_in(cx, |panel, window, cx| { + let project = panel.project.read(cx); + let worktrees = project.visible_worktrees(cx).collect::>(); + let source = worktrees[0].read(cx); + let target = worktrees[1].read(cx); + let source_entry = SelectedEntry { + worktree_id: source.id(), + entry_id: source.root_entry().unwrap().id, + }; + let target_entry_id = target.root_entry().unwrap().id; + let drag = DraggedSelection { + active_selection: source_entry, + marked_selections: Arc::new([source_entry]), + }; + panel.drag_onto(&drag, target_entry_id, false, window, cx); + }); + cx.run_until_parked(); + + 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" + ); + + // Drag worktree root3 onto worktree root2 (now first): [r2, r1, r3] -> [r3, r2, r1]. + panel.update_in(cx, |panel, window, cx| { + let project = panel.project.read(cx); + let worktrees = project.visible_worktrees(cx).collect::>(); + let source = worktrees[2].read(cx); + let target = worktrees[0].read(cx); + let source_entry = SelectedEntry { + worktree_id: source.id(), + entry_id: source.root_entry().unwrap().id, + }; + let target_entry_id = target.root_entry().unwrap().id; + let drag = DraggedSelection { + active_selection: source_entry, + marked_selections: Arc::new([source_entry]), + }; + panel.drag_onto(&drag, target_entry_id, false, window, cx); + }); + cx.run_until_parked(); + + 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_multiple_marked_entries(cx: &mut gpui::TestAppContext) { init_test_with_editor(cx); From 0388bc8c74992b3027e026bc25b3f8f416b506b4 Mon Sep 17 00:00:00 2001 From: eth0net Date: Tue, 5 May 2026 15:07:49 +0100 Subject: [PATCH 2/8] Tighten worktree root drag-and-drop semantics - Reorder marked worktree roots together as a group, preserving their relative order; direction follows the active source's position. - Reject worktree-root drops onto nested entries and suppress the drag-over highlight for invalid targets, so the operation matches the visual feedback. - Filter worktree roots from copy-mode drags so a mixed selection still copies its non-root entries instead of being silently cancelled by the `?` in `create_paste_path`. - Replace the per-entry root-routing in drag_onto's move loop with a partition + batch reorder, removing the now-dead `move_entry` and `move_worktree_root` wrappers. - Add regression tests for multi-root group reorder, nested-target rejection, copy-mode mixed selections, and mixed root+file drags with a non-root active selection. --- crates/project/src/project.rs | 14 + crates/project/src/worktree_store.rs | 92 +++-- crates/project_panel/src/project_panel.rs | 103 +++-- .../project_panel/src/project_panel_tests.rs | 373 ++++++++++++++++++ 4 files changed, 529 insertions(+), 53 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 4e74c4cf1fc..e4c379b4796 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -4579,6 +4579,20 @@ impl Project { }) } + /// Moves multiple worktrees as a group to `destination`'s position, + /// preserving their relative order. + pub fn move_worktrees( + &mut self, + sources: &[WorktreeId], + destination: WorktreeId, + active_source: Option, + cx: &mut Context, + ) -> Result<()> { + self.worktree_store.update(cx, |worktree_store, cx| { + worktree_store.move_worktrees(sources, destination, active_source, cx) + }) + } + /// Attempts to convert the input path to a WSL path if this is a wsl remote project and the input path is a host windows path. pub fn try_windows_path_to_wsl( &self, diff --git a/crates/project/src/worktree_store.rs b/crates/project/src/worktree_store.rs index c6abb6e1743..aa1070d56e6 100644 --- a/crates/project/src/worktree_store.rs +++ b/crates/project/src/worktree_store.rs @@ -1038,40 +1038,80 @@ impl WorktreeStore { destination: WorktreeId, cx: &mut Context, ) -> Result<()> { - if source == destination { + self.move_worktrees(&[source], destination, Some(source), cx) + } + + /// Moves multiple worktrees as a group to `destination`'s position, + /// preserving their relative order. The `active_source` (if provided and + /// in `sources`) decides whether the group lands before or after the + /// destination, mirroring the single-source semantics: a source originally + /// before destination ends up after it, and vice versa. When no active + /// source is supplied, the first source's position is used. + pub fn move_worktrees( + &mut self, + sources: &[WorktreeId], + destination: WorktreeId, + active_source: Option, + cx: &mut Context, + ) -> Result<()> { + if sources.is_empty() { return Ok(()); } - let mut source_index = None; - let mut destination_index = None; - for (i, worktree) in self.worktrees.iter().enumerate() { - if let Some(worktree) = worktree.upgrade() { - let worktree_id = worktree.read(cx).id(); - if worktree_id == source { - source_index = Some(i); - if destination_index.is_some() { - break; - } - } else if worktree_id == destination { - destination_index = Some(i); - if source_index.is_some() { - break; - } - } - } - } + let destination_index = self + .worktrees + .iter() + .position(|wt| { + wt.upgrade() + .is_some_and(|wt| wt.read(cx).id() == destination) + }) + .with_context(|| format!("Missing worktree for id {destination}"))?; - let source_index = - source_index.with_context(|| format!("Missing worktree for id {source}"))?; - let destination_index = - destination_index.with_context(|| format!("Missing worktree for id {destination}"))?; + let source_indices: Vec = self + .worktrees + .iter() + .enumerate() + .filter_map(|(i, wt)| { + let id = wt.upgrade()?.read(cx).id(); + (sources.contains(&id) && id != destination).then_some(i) + }) + .collect(); - if source_index == destination_index { + if source_indices.is_empty() { return Ok(()); } - let worktree_to_move = self.worktrees.remove(source_index); - self.worktrees.insert(destination_index, worktree_to_move); + let direction_index = active_source + .filter(|id| sources.contains(id)) + .and_then(|id| { + self.worktrees + .iter() + .position(|wt| wt.upgrade().is_some_and(|wt| wt.read(cx).id() == id)) + }) + .unwrap_or(source_indices[0]); + let insert_after_destination = direction_index < destination_index; + + let mut to_insert = Vec::with_capacity(source_indices.len()); + for &i in source_indices.iter().rev() { + to_insert.push(self.worktrees.remove(i)); + } + to_insert.reverse(); + + let removed_before_destination = source_indices + .iter() + .filter(|&&i| i < destination_index) + .count(); + let new_destination_index = destination_index - removed_before_destination; + let insert_at = if insert_after_destination { + new_destination_index + 1 + } else { + new_destination_index + }; + + for (offset, handle) in to_insert.into_iter().enumerate() { + self.worktrees.insert(insert_at + offset, handle); + } + cx.emit(WorktreeStoreEvent::WorktreeOrderChanged); cx.notify(); Ok(()) diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 5266a3ea5e8..48de9f50755 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -3588,44 +3588,41 @@ impl ProjectPanel { } } - fn move_entry( + fn reorder_worktree_roots( &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, + source_entries: &[ProjectEntryId], destination: ProjectEntryId, + active_entry_id: ProjectEntryId, cx: &mut Context, ) { self.project.update(cx, |project, cx| { - let Some(worktree_to_move) = project.worktree_for_entry(entry_to_move, cx) else { + // Reorder only fires on explicit drops onto a worktree root. + if !project.entry_is_worktree_root(destination, cx) { return; - }; + } let Some(destination_worktree) = project.worktree_for_entry(destination, cx) else { return; }; - - let worktree_id = worktree_to_move.read(cx).id(); let destination_id = destination_worktree.read(cx).id(); + let source_ids: Vec = source_entries + .iter() + .filter_map(|entry_id| { + project + .worktree_for_entry(*entry_id, cx) + .map(|wt| wt.read(cx).id()) + }) + .collect(); + if source_ids.is_empty() { + return; + } + + let active_source = project + .worktree_for_entry(active_entry_id, cx) + .map(|wt| wt.read(cx).id()); + project - .move_worktree(worktree_id, destination_id, cx) + .move_worktrees(&source_ids, destination_id, active_source, cx) .log_err(); }); } @@ -4459,6 +4456,18 @@ impl ProjectPanel { let entries = self.disjoint_entries(resolved_selections, cx); if Self::is_copy_modifier_set(&window.modifiers()) { + // Worktree roots can't be copied — leaving them in would make + // `create_paste_path` return None and `?` would abort the whole copy. + let entries: BTreeSet = { + let project = self.project.read(cx); + entries + .into_iter() + .filter(|entry| !project.entry_is_worktree_root(entry.entry_id, cx)) + .collect() + }; + if entries.is_empty() { + return; + } let _ = maybe!({ let project = self.project.read(cx); let target_worktree = project.worktree_for_entry(target_entry_id, cx)?; @@ -4519,6 +4528,27 @@ impl ProjectPanel { } else { let update_marks = !self.marked_entries.is_empty(); let active_selection = selections.active_selection; + let active_entry_id = self.resolve_entry(active_selection.entry_id); + + // Reorder marked worktree roots together so their relative order is + // preserved; non-roots fall through to the normal per-entry move flow. + let (root_entry_ids, entries) = { + let project = self.project.read(cx); + let mut roots = Vec::new(); + let mut non_roots = BTreeSet::new(); + for entry in entries { + if project.entry_is_worktree_root(entry.entry_id, cx) { + roots.push(entry.entry_id); + } else { + non_roots.insert(entry); + } + } + (roots, non_roots) + }; + + if !root_entry_ids.is_empty() { + self.reorder_worktree_roots(&root_entry_ids, target_entry_id, active_entry_id, cx); + } // For folded selections, track the leaf suffix relative to the resolved // entry so we can refresh it after the move completes. @@ -4575,7 +4605,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)); } } @@ -5191,6 +5223,23 @@ impl ProjectPanel { drag_state: &DraggedSelection, cx: &Context, ) -> Option { + // Worktree-root drags are only meaningful when dropped on another + // worktree's root, so suppress highlights elsewhere — including over + // the source worktree itself, where the drop would be a no-op. + if self + .project + .read(cx) + .entry_is_worktree_root(drag_state.active_selection.entry_id, cx) + { + let root_id = target_worktree.root_entry()?.id; + if target_entry.id == root_id + && target_worktree.id() != drag_state.active_selection.worktree_id + { + return Some(root_id); + } + return None; + } + let target_parent_path = target_entry.path.parent(); // In case of single item drag, we do not highlight existing diff --git a/crates/project_panel/src/project_panel_tests.rs b/crates/project_panel/src/project_panel_tests.rs index c47b6ddd84b..46a7389a4b2 100644 --- a/crates/project_panel/src/project_panel_tests.rs +++ b/crates/project_panel/src/project_panel_tests.rs @@ -4211,6 +4211,379 @@ async fn test_drag_worktree_root_reorders_worktrees(cx: &mut gpui::TestAppContex ); } +// Marked worktree roots reorder together as a group, preserving their +// relative order, with direction (before vs after destination) chosen from +// the active source's original position. +#[gpui::test] +async fn test_drag_multiple_marked_worktree_roots_reorder_as_group( + cx: &mut gpui::TestAppContext, +) { + 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; + fs.insert_tree("/root4", json!({ "d.txt": "" })).await; + + let project = Project::test( + fs.clone(), + [ + "/root1".as_ref(), + "/root2".as_ref(), + "/root3".as_ref(), + "/root4".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 cx = &mut VisualTestContext::from_window(window.into(), cx); + let panel = workspace.update_in(cx, ProjectPanel::new); + cx.run_until_parked(); + + // Non-contiguous group [r1, r3], active=r1, drop onto r4. Active is before + // dest, so the group lands AFTER r4 with relative order (r1, r3) intact. + panel.update_in(cx, |panel, window, cx| { + let project = panel.project.read(cx); + let worktrees = project.visible_worktrees(cx).collect::>(); + let r1 = worktrees[0].read(cx); + let r3 = worktrees[2].read(cx); + let r4 = worktrees[3].read(cx); + let r1_entry = SelectedEntry { + worktree_id: r1.id(), + entry_id: r1.root_entry().unwrap().id, + }; + let r3_entry = SelectedEntry { + worktree_id: r3.id(), + entry_id: r3.root_entry().unwrap().id, + }; + let target = r4.root_entry().unwrap().id; + let drag = DraggedSelection { + active_selection: r1_entry, + marked_selections: Arc::new([r1_entry, r3_entry]), + }; + panel.drag_onto(&drag, target, false, window, cx); + }); + cx.run_until_parked(); + + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &[ + "v root2", + " b.txt", + "v root4", + " d.txt", + "v root1", + " a.txt", + "v root3", + " c.txt", + ], + "marked roots r1 and r3 should land after r4 in their original relative order" + ); + + // Now [r3, r1] (reordered above), active=r1, drop onto r2. Active is after + // dest, so the group lands BEFORE r2 with relative order (r1, r3) intact. + panel.update_in(cx, |panel, window, cx| { + let project = panel.project.read(cx); + let worktrees = project.visible_worktrees(cx).collect::>(); + // After the previous move, order is [r2, r4, r1, r3]. + let r2 = worktrees[0].read(cx); + let r1 = worktrees[2].read(cx); + let r3 = worktrees[3].read(cx); + let r1_entry = SelectedEntry { + worktree_id: r1.id(), + entry_id: r1.root_entry().unwrap().id, + }; + let r3_entry = SelectedEntry { + worktree_id: r3.id(), + entry_id: r3.root_entry().unwrap().id, + }; + let target = r2.root_entry().unwrap().id; + let drag = DraggedSelection { + active_selection: r1_entry, + marked_selections: Arc::new([r1_entry, r3_entry]), + }; + panel.drag_onto(&drag, target, false, window, cx); + }); + cx.run_until_parked(); + + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &[ + "v root1", + " a.txt", + "v root3", + " c.txt", + "v root2", + " b.txt", + "v root4", + " d.txt", + ], + "marked roots should land before r2 in their original relative order" + ); +} + +// Dropping a worktree root onto a file or subdirectory inside another worktree +// previously triggered an implicit reorder relative to the target's worktree. +// Reorder should only happen on an explicit drop onto a worktree root. +#[gpui::test] +async fn test_drag_worktree_root_onto_nested_entry_is_rejected( + cx: &mut gpui::TestAppContext, +) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree("/root1", json!({ "a.txt": "" })).await; + fs.insert_tree("/root2", json!({ "sub": { "b.txt": "" } })) + .await; + + let project = + Project::test(fs.clone(), ["/root1".as_ref(), "/root2".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 cx = &mut VisualTestContext::from_window(window.into(), cx); + let panel = workspace.update_in(cx, ProjectPanel::new); + cx.run_until_parked(); + + toggle_expand_dir(&panel, "root2/sub", cx); + + // Drop root1 onto root2/sub/b.txt — should NOT reorder. + panel.update_in(cx, |panel, window, cx| { + let project = panel.project.read(cx); + let worktrees = project.visible_worktrees(cx).collect::>(); + let r1 = worktrees[0].read(cx); + let r2 = worktrees[1].read(cx); + let r1_root = SelectedEntry { + worktree_id: r1.id(), + entry_id: r1.root_entry().unwrap().id, + }; + let nested_file_id = r2.entry_for_path(rel_path("sub/b.txt")).unwrap().id; + let drag = DraggedSelection { + active_selection: r1_root, + marked_selections: Arc::new([r1_root]), + }; + panel.drag_onto(&drag, nested_file_id, true, window, cx); + }); + cx.run_until_parked(); + + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &[ + "v root1", + " a.txt", + "v root2", + " v sub <== selected", + " b.txt", + ], + "dropping a worktree root onto a nested file should not reorder worktrees" + ); + + // Drop root1 onto root2/sub (a subdirectory) — should also NOT reorder. + panel.update_in(cx, |panel, window, cx| { + let project = panel.project.read(cx); + let worktrees = project.visible_worktrees(cx).collect::>(); + let r1 = worktrees[0].read(cx); + let r2 = worktrees[1].read(cx); + let r1_root = SelectedEntry { + worktree_id: r1.id(), + entry_id: r1.root_entry().unwrap().id, + }; + let sub_dir_id = r2.entry_for_path(rel_path("sub")).unwrap().id; + let drag = DraggedSelection { + active_selection: r1_root, + marked_selections: Arc::new([r1_root]), + }; + panel.drag_onto(&drag, sub_dir_id, false, window, cx); + }); + cx.run_until_parked(); + + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &[ + "v root1", + " a.txt", + "v root2", + " v sub <== selected", + " b.txt", + ], + "dropping a worktree root onto a subdirectory should not reorder worktrees" + ); +} + +// Mixed selection: marked worktree root + non-root entry, with the non-root +// as the active drag. The root reorders, the file moves, both in one gesture. +// +// This test is positioned to exercise the `active_source.filter(|id| +// sources.contains(id))` fallback in `move_worktrees`: the active drag is a +// file in `root4` (idx 3), which lies AFTER the destination `root3` (idx 2). +// Without the filter, direction would follow root4's position and land the +// group before the destination; with the filter, `active_source` is treated +// as None and we fall back to the first source `root1` (idx 0), landing the +// group AFTER the destination. +#[gpui::test] +async fn test_drag_mixed_root_and_file_with_non_root_active( + cx: &mut gpui::TestAppContext, +) { + 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; + fs.insert_tree("/root4", json!({ "sub": { "d.txt": "" } })) + .await; + + let project = Project::test( + fs.clone(), + [ + "/root1".as_ref(), + "/root2".as_ref(), + "/root3".as_ref(), + "/root4".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 cx = &mut VisualTestContext::from_window(window.into(), cx); + let panel = workspace.update_in(cx, ProjectPanel::new); + cx.run_until_parked(); + + let (r1_id, r2_id, r3_id, r4_id) = panel.update_in(cx, |panel, _, cx| { + let worktrees = panel + .project + .read(cx) + .visible_worktrees(cx) + .collect::>(); + ( + worktrees[0].read(cx).id(), + worktrees[1].read(cx).id(), + worktrees[2].read(cx).id(), + worktrees[3].read(cx).id(), + ) + }); + + // Mark root1's root and root4/sub/d.txt; the file is the active selection. + select_path_with_mark(&panel, "root1", cx); + select_path_with_mark(&panel, "root4/sub/d.txt", cx); + + drag_selection_to(&panel, "root3", false, cx); + + // root1 reorders. The active drag's worktree (root4 at idx 3) isn't in + // the root sources [root1]; the filter discards it and direction falls + // back to root1's position (idx 0 < dest idx 2 → group lands after root3). + let worktree_order = panel.update_in(cx, |panel, _, cx| { + panel + .project + .read(cx) + .visible_worktrees(cx) + .map(|wt| wt.read(cx).id()) + .collect::>() + }); + assert_eq!(worktree_order, vec![r2_id, r3_id, r1_id, r4_id]); + + // d.txt should have moved from root4/sub to root3's root. + assert!( + find_project_entry(&panel, "root3/d.txt", cx).is_some(), + "d.txt should land in root3" + ); + assert_eq!( + find_project_entry(&panel, "root4/sub/d.txt", cx), + None, + "d.txt should be removed from root4/sub" + ); + // a.txt stays in root1. + assert!( + find_project_entry(&panel, "root1/a.txt", cx).is_some(), + "a.txt should remain in root1" + ); +} + +// Copying a worktree root has no meaning (`create_paste_path` returns None for +// it). Without filtering, the whole copy aborts via `?` and any sibling files +// in the same drag get silently dropped. Verify mixed selections still copy. +#[gpui::test] +async fn test_copy_drag_mixed_worktree_root_and_file_still_copies_file( + cx: &mut gpui::TestAppContext, +) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree("/root1", json!({ "a.txt": "hello" })).await; + fs.insert_tree("/root2", json!({ "b.txt": "world" })).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 cx = &mut VisualTestContext::from_window(window.into(), cx); + let panel = workspace.update_in(cx, ProjectPanel::new); + cx.run_until_parked(); + + // Hold the platform-appropriate copy modifier. + cx.simulate_modifiers_change(gpui::Modifiers { + alt: true, + control: true, + ..Default::default() + }); + + // Mark r1's root + r2's b.txt (in distinct worktrees so `disjoint_entries` + // doesn't filter the file out as nested under the root). Active = b.txt. + // Drag-copy onto r3's root. + panel.update_in(cx, |panel, window, cx| { + let project = panel.project.read(cx); + let worktrees = project.visible_worktrees(cx).collect::>(); + let r1 = worktrees[0].read(cx); + let r2 = worktrees[1].read(cx); + let r3 = worktrees[2].read(cx); + let r1_root_entry = SelectedEntry { + worktree_id: r1.id(), + entry_id: r1.root_entry().unwrap().id, + }; + let r2_file_entry = SelectedEntry { + worktree_id: r2.id(), + entry_id: r2.entry_for_path(rel_path("b.txt")).unwrap().id, + }; + let target_entry_id = r3.root_entry().unwrap().id; + let drag = DraggedSelection { + active_selection: r2_file_entry, + marked_selections: Arc::new([r1_root_entry, r2_file_entry]), + }; + panel.drag_onto(&drag, target_entry_id, false, window, cx); + }); + cx.run_until_parked(); + + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &[ + "v root1", + " a.txt", + "v root2", + " b.txt", + "v root3", + " b.txt <== selected", + " c.txt", + ], + "b.txt should be copied into root3 even though r1's root was also marked" + ); +} + #[gpui::test] async fn test_multiple_marked_entries(cx: &mut gpui::TestAppContext) { init_test_with_editor(cx); From a8d9c97dc4e39b1413721a85cdd984813e889d5f Mon Sep 17 00:00:00 2001 From: eth0net Date: Tue, 5 May 2026 15:57:47 +0100 Subject: [PATCH 3/8] Address review feedback on worktree drag-and-drop - `move_worktrees` now validates every source up front and errors on missing IDs, matching the original `move_worktree` contract instead of silently dropping unknown sources. - A self-drop of a multi-root selection (dropping the group onto one of its own members) is now a no-op rather than reordering the remaining roots around it. - The reorder path now calls `send_project_updates` so collaborators see the new worktree order. - `disjoint_entries` skips the worktree root when building `dir_paths`, so a marked root no longer silently filters out its own selected descendants before the move/copy branches see them. - The drag-over highlight only restricts itself to worktree-root targets when the entire drag is roots-only; mixed drags continue to highlight directory targets so the file portion still gets feedback. - Adds regression tests: invalid-source error, multi-root self-drop no-op, and a marked-root + nested-file drag that exercises both the reorder and per-entry move paths. --- crates/project/src/worktree_store.rs | 19 ++- .../tests/integration/project_tests.rs | 34 +++++ crates/project_panel/src/project_panel.rs | 22 +-- .../project_panel/src/project_panel_tests.rs | 138 ++++++++++++++++++ 4 files changed, 203 insertions(+), 10 deletions(-) diff --git a/crates/project/src/worktree_store.rs b/crates/project/src/worktree_store.rs index aa1070d56e6..e3d54cb5813 100644 --- a/crates/project/src/worktree_store.rs +++ b/crates/project/src/worktree_store.rs @@ -1057,6 +1057,12 @@ impl WorktreeStore { if sources.is_empty() { return Ok(()); } + // Self-drop of any selection member is a no-op: the user dropping a + // multi-selection onto one of its own roots has no well-defined + // intent. + if sources.contains(&destination) { + return Ok(()); + } let destination_index = self .worktrees @@ -1067,13 +1073,23 @@ impl WorktreeStore { }) .with_context(|| format!("Missing worktree for id {destination}"))?; + for &source in sources { + if !self + .worktrees + .iter() + .any(|wt| wt.upgrade().is_some_and(|wt| wt.read(cx).id() == source)) + { + anyhow::bail!("Missing worktree for id {source}"); + } + } + let source_indices: Vec = self .worktrees .iter() .enumerate() .filter_map(|(i, wt)| { let id = wt.upgrade()?.read(cx).id(); - (sources.contains(&id) && id != destination).then_some(i) + sources.contains(&id).then_some(i) }) .collect(); @@ -1114,6 +1130,7 @@ impl WorktreeStore { cx.emit(WorktreeStoreEvent::WorktreeOrderChanged); cx.notify(); + self.send_project_updates(cx); Ok(()) } diff --git a/crates/project/tests/integration/project_tests.rs b/crates/project/tests/integration/project_tests.rs index 0fbec6c1a1d..4e4d20110c1 100644 --- a/crates/project/tests/integration/project_tests.rs +++ b/crates/project/tests/integration/project_tests.rs @@ -8946,6 +8946,40 @@ async fn test_reordering_worktrees(cx: &mut gpui::TestAppContext) { }); } +#[gpui::test] +async fn test_move_worktree_with_invalid_source_errors(cx: &mut gpui::TestAppContext) { + use worktree::WorktreeId; + + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + "/dir", + json!({ + "a.rs": "", + "b.rs": "", + }), + ) + .await; + + let project = + Project::test(fs, ["/dir/a.rs".as_ref(), "/dir/b.rs".as_ref()], cx).await; + + project.update(cx, |project, cx| { + let valid_id = project.visible_worktrees(cx).next().unwrap().read(cx).id(); + let invalid_id = WorktreeId::from_usize(99_999); + + assert!( + project.move_worktree(invalid_id, valid_id, cx).is_err(), + "moving an unknown source worktree should error" + ); + assert!( + project.move_worktree(valid_id, invalid_id, cx).is_err(), + "moving onto an unknown destination should error" + ); + }); +} + #[gpui::test] async fn test_unstaged_diff_for_buffer(cx: &mut gpui::TestAppContext) { init_test(cx); diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 48de9f50755..909d2cdcefa 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -3718,11 +3718,14 @@ impl ProjectPanel { for (worktree_id, worktree_entries) in entries_by_worktree { if let Some(worktree) = project.worktree_for_id(worktree_id, cx) { let worktree = worktree.read(cx); + // Skip the worktree root: its empty path would consume every + // other selected entry in the same worktree as "nested inside + // a selected directory" and silently drop them. let dir_paths = worktree_entries .iter() .filter_map(|entry| { worktree.entry_for_id(entry.entry_id).and_then(|entry| { - if entry.is_dir() { + if entry.is_dir() && !entry.path.is_empty() { Some(entry.path.as_ref()) } else { None @@ -5223,14 +5226,15 @@ impl ProjectPanel { drag_state: &DraggedSelection, cx: &Context, ) -> Option { - // Worktree-root drags are only meaningful when dropped on another - // worktree's root, so suppress highlights elsewhere — including over - // the source worktree itself, where the drop would be a no-op. - if self - .project - .read(cx) - .entry_is_worktree_root(drag_state.active_selection.entry_id, cx) - { + // Pure worktree-root drags are only meaningful when dropped on + // another worktree's root; suppress highlights elsewhere. Mixed drags + // (e.g. a root with a marked file) fall through so the file portion + // can still receive feedback on directory targets. + let project = self.project.read(cx); + let drag_is_root_only = drag_state + .items() + .all(|entry| project.entry_is_worktree_root(entry.entry_id, cx)); + if drag_is_root_only { let root_id = target_worktree.root_entry()?.id; if target_entry.id == root_id && target_worktree.id() != drag_state.active_selection.worktree_id diff --git a/crates/project_panel/src/project_panel_tests.rs b/crates/project_panel/src/project_panel_tests.rs index 46a7389a4b2..d5d8578d5a1 100644 --- a/crates/project_panel/src/project_panel_tests.rs +++ b/crates/project_panel/src/project_panel_tests.rs @@ -4416,6 +4416,144 @@ async fn test_drag_worktree_root_onto_nested_entry_is_rejected( ); } +// Dropping a multi-root selection onto one of its own members has no +// well-defined intent, so it should leave the order untouched rather than +// reorder the remaining roots around the destination. +#[gpui::test] +async fn test_drag_multiple_marked_worktree_roots_onto_self_is_no_op( + cx: &mut gpui::TestAppContext, +) { + 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 cx = &mut VisualTestContext::from_window(window.into(), cx); + let panel = workspace.update_in(cx, ProjectPanel::new); + cx.run_until_parked(); + + panel.update_in(cx, |panel, window, cx| { + let project = panel.project.read(cx); + let worktrees = project.visible_worktrees(cx).collect::>(); + let r1 = worktrees[0].read(cx); + let r2 = worktrees[1].read(cx); + let r1_entry = SelectedEntry { + worktree_id: r1.id(), + entry_id: r1.root_entry().unwrap().id, + }; + let r2_entry = SelectedEntry { + worktree_id: r2.id(), + entry_id: r2.root_entry().unwrap().id, + }; + let drag = DraggedSelection { + active_selection: r1_entry, + marked_selections: Arc::new([r1_entry, r2_entry]), + }; + // Drop the [r1, r2] selection onto r2, one of its own members. + panel.drag_onto(&drag, r2_entry.entry_id, false, window, cx); + }); + cx.run_until_parked(); + + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &[ + "v root1", + " a.txt", + "v root2", + " b.txt", + "v root3", + " c.txt", + ], + "self-drop of a multi-root selection should leave the order untouched" + ); +} + +// When a worktree root and one of its descendants are both marked, the +// descendant must survive `disjoint_entries`. The root's empty path would +// otherwise classify every other entry in the same worktree as nested +// inside a "selected directory" and silently drop it before the move/copy +// branches see it. +#[gpui::test] +async fn test_drag_marked_root_with_nested_file_keeps_both( + cx: &mut gpui::TestAppContext, +) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree("/root1", json!({ "sub": { "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 cx = &mut VisualTestContext::from_window(window.into(), cx); + let panel = workspace.update_in(cx, ProjectPanel::new); + cx.run_until_parked(); + + let (r1_id, r2_id, r3_id) = panel.update_in(cx, |panel, _, cx| { + let worktrees = panel + .project + .read(cx) + .visible_worktrees(cx) + .collect::>(); + ( + worktrees[0].read(cx).id(), + worktrees[1].read(cx).id(), + worktrees[2].read(cx).id(), + ) + }); + + select_path_with_mark(&panel, "root1", cx); + select_path_with_mark(&panel, "root1/sub/a.txt", cx); + + drag_selection_to(&panel, "root3", false, cx); + + // root1 reorders past root3 (active is the file in root1, so direction + // follows root1's position via the source-side fallback in the active + // filter). + let order = panel.update_in(cx, |panel, _, cx| { + panel + .project + .read(cx) + .visible_worktrees(cx) + .map(|wt| wt.read(cx).id()) + .collect::>() + }); + assert_eq!(order, vec![r2_id, r3_id, r1_id]); + + // The marked file moves to root3 instead of being silently dropped. + assert!( + find_project_entry(&panel, "root3/a.txt", cx).is_some(), + "a.txt should land in root3" + ); + assert_eq!( + find_project_entry(&panel, "root1/sub/a.txt", cx), + None, + "a.txt should be removed from root1/sub" + ); +} + // Mixed selection: marked worktree root + non-root entry, with the non-root // as the active drag. The root reorders, the file moves, both in one gesture. // From 4faab9c17ec3001f690359b6843c42c6da604176 Mon Sep 17 00:00:00 2001 From: eth0net Date: Tue, 5 May 2026 16:25:22 +0100 Subject: [PATCH 4/8] Address follow-up review feedback - Multi-root drag highlight now suppresses every dragged worktree, not just the active one. Hovering a marked-but-not-active root previously showed a misleading highlight even though `move_worktrees` would treat that drop as a no-op. - Update misleading inline comments in the worktree-reorder tests and in `reorder_worktree_roots`. - Add `test_highlight_entry_for_multi_root_drag_excludes_marked_worktrees`. --- crates/project_panel/src/project_panel.rs | 15 ++- .../project_panel/src/project_panel_tests.rs | 105 +++++++++++++++++- 2 files changed, 111 insertions(+), 9 deletions(-) diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 909d2cdcefa..f4d8232f0b7 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -3596,7 +3596,10 @@ impl ProjectPanel { cx: &mut Context, ) { self.project.update(cx, |project, cx| { - // Reorder only fires on explicit drops onto a worktree root. + // Only reorder when the destination resolves to a worktree root. + // Nested entries are rejected; the empty area below the panel + // resolves to the last worktree's root, which still satisfies + // this check. if !project.entry_is_worktree_root(destination, cx) { return; } @@ -5236,9 +5239,13 @@ impl ProjectPanel { .all(|entry| project.entry_is_worktree_root(entry.entry_id, cx)); if drag_is_root_only { let root_id = target_worktree.root_entry()?.id; - if target_entry.id == root_id - && target_worktree.id() != drag_state.active_selection.worktree_id - { + // Hovering any worktree that's part of the drag (active or just + // marked) is a no-op in `move_worktrees`, so don't highlight it. + let target_worktree_id = target_worktree.id(); + let target_in_drag = drag_state + .items() + .any(|entry| entry.worktree_id == target_worktree_id); + if target_entry.id == root_id && !target_in_drag { return Some(root_id); } return None; diff --git a/crates/project_panel/src/project_panel_tests.rs b/crates/project_panel/src/project_panel_tests.rs index d5d8578d5a1..29b9e3cd953 100644 --- a/crates/project_panel/src/project_panel_tests.rs +++ b/crates/project_panel/src/project_panel_tests.rs @@ -4285,8 +4285,9 @@ async fn test_drag_multiple_marked_worktree_roots_reorder_as_group( "marked roots r1 and r3 should land after r4 in their original relative order" ); - // Now [r3, r1] (reordered above), active=r1, drop onto r2. Active is after - // dest, so the group lands BEFORE r2 with relative order (r1, r3) intact. + // After the previous move, the order is [r2, r4, r1, r3]; the selected + // group is [r1, r3], active=r1, drop onto r2. Active is after dest, so + // the group lands BEFORE r2 with relative order (r1, r3) intact. panel.update_in(cx, |panel, window, cx| { let project = panel.project.read(cx); let worktrees = project.visible_worktrees(cx).collect::>(); @@ -4529,9 +4530,10 @@ async fn test_drag_marked_root_with_nested_file_keeps_both( drag_selection_to(&panel, "root3", false, cx); - // root1 reorders past root3 (active is the file in root1, so direction - // follows root1's position via the source-side fallback in the active - // filter). + // root1 reorders past root3. The active drag is the file in root1, so + // `active_source` resolves to root1's worktree id — which is in `sources`, + // so the filter passes through and direction follows root1's own position + // (idx 0 < dest idx 2 → group lands after root3). let order = panel.update_in(cx, |panel, _, cx| { panel .project @@ -9148,6 +9150,99 @@ async fn test_highlight_entry_for_selection_drag_cross_worktree(cx: &mut gpui::T }); } +// For a multi-root drag, hovering any worktree that is part of the drag +// (active OR just marked) is a no-op in `move_worktrees`, so the highlight +// must not advertise it as a valid drop target. +#[gpui::test] +async fn test_highlight_entry_for_multi_root_drag_excludes_marked_worktrees( + cx: &mut gpui::TestAppContext, +) { + 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; + fs.insert_tree("/root4", json!({ "d.txt": "" })).await; + + let project = Project::test( + fs.clone(), + [ + "/root1".as_ref(), + "/root2".as_ref(), + "/root3".as_ref(), + "/root4".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 cx = &mut VisualTestContext::from_window(window.into(), cx); + let panel = workspace.update_in(cx, ProjectPanel::new); + cx.run_until_parked(); + + panel.update(cx, |panel, cx| { + let worktrees: Vec<_> = panel.project.read(cx).visible_worktrees(cx).collect(); + let r1 = worktrees[0].read(cx); + let r2 = worktrees[1].read(cx); + let r3 = worktrees[2].read(cx); + let r4 = worktrees[3].read(cx); + + let r1_root = r1.root_entry().unwrap(); + let r2_root = r2.root_entry().unwrap(); + let r3_root = r3.root_entry().unwrap(); + let r4_root = r4.root_entry().unwrap(); + + let drag = DraggedSelection { + active_selection: SelectedEntry { + worktree_id: r1.id(), + entry_id: r1_root.id, + }, + marked_selections: Arc::new([ + SelectedEntry { + worktree_id: r1.id(), + entry_id: r1_root.id, + }, + SelectedEntry { + worktree_id: r3.id(), + entry_id: r3_root.id, + }, + ]), + }; + + // Hovering r2 (not in the drag): highlight allowed. + assert_eq!( + panel.highlight_entry_for_selection_drag(r2_root, r2, &drag, cx), + Some(r2_root.id), + "non-dragged worktree root should highlight" + ); + + // Hovering r4 (not in the drag): highlight allowed. + assert_eq!( + panel.highlight_entry_for_selection_drag(r4_root, r4, &drag, cx), + Some(r4_root.id), + "non-dragged worktree root should highlight" + ); + + // Hovering r1 (active source): no-op, no highlight. + assert_eq!( + panel.highlight_entry_for_selection_drag(r1_root, r1, &drag, cx), + None, + "active source's worktree should not highlight" + ); + + // Hovering r3 (marked but not active): no-op, no highlight. + assert_eq!( + panel.highlight_entry_for_selection_drag(r3_root, r3, &drag, cx), + None, + "marked source's worktree should not highlight" + ); + }); +} + #[gpui::test] async fn test_should_highlight_background_for_selection_drag(cx: &mut gpui::TestAppContext) { init_test(cx); From f0c7ee837c60d51777ae7cfb8600146f589b9396 Mon Sep 17 00:00:00 2001 From: eth0net Date: Tue, 5 May 2026 16:55:05 +0100 Subject: [PATCH 5/8] Send root group to the end on blank-area drops that include the last worktree Dropping a multi-root selection on the blank area below the panel previously forwarded the gesture as `destination = last_worktree_root_id` and then hit the self-drop guard in `move_worktrees` if the dragged group already contained the last worktree, silently no-opping the operation. Adds `WorktreeStore::move_worktrees_to_end` (remove sources, append in original order) and a panel-side helper that the blank-area drop handler routes to when the drag is roots-only and includes the last worktree. Explicit row drops keep the self-drop no-op for the ambiguous "[A, B] dropped onto B" gesture. --- crates/project/src/project.rs | 13 +++ crates/project/src/worktree_store.rs | 60 ++++++++++++ crates/project_panel/src/project_panel.rs | 57 +++++++++++- .../project_panel/src/project_panel_tests.rs | 93 +++++++++++++++++++ 4 files changed, 222 insertions(+), 1 deletion(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index e4c379b4796..2d1f4de6bbb 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -4593,6 +4593,19 @@ impl Project { }) } + /// Moves multiple worktrees to the end of the worktree list, preserving + /// their relative order. Used for "drop on the blank area below the + /// project panel" gestures. + pub fn move_worktrees_to_end( + &mut self, + sources: &[WorktreeId], + cx: &mut Context, + ) -> Result<()> { + self.worktree_store.update(cx, |worktree_store, cx| { + worktree_store.move_worktrees_to_end(sources, cx) + }) + } + /// Attempts to convert the input path to a WSL path if this is a wsl remote project and the input path is a host windows path. pub fn try_windows_path_to_wsl( &self, diff --git a/crates/project/src/worktree_store.rs b/crates/project/src/worktree_store.rs index e3d54cb5813..4a1858f10bd 100644 --- a/crates/project/src/worktree_store.rs +++ b/crates/project/src/worktree_store.rs @@ -1134,6 +1134,66 @@ impl WorktreeStore { Ok(()) } + /// Removes every source from the worktree list and appends them to the + /// end in their original relative order. Intended for "drop on the empty + /// area below the panel" gestures, where the user's intent is "send this + /// group to the end" rather than reordering relative to a specific row. + pub fn move_worktrees_to_end( + &mut self, + sources: &[WorktreeId], + cx: &mut Context, + ) -> Result<()> { + if sources.is_empty() { + return Ok(()); + } + + for &source in sources { + if !self + .worktrees + .iter() + .any(|wt| wt.upgrade().is_some_and(|wt| wt.read(cx).id() == source)) + { + anyhow::bail!("Missing worktree for id {source}"); + } + } + + let source_indices: Vec = self + .worktrees + .iter() + .enumerate() + .filter_map(|(i, wt)| { + let id = wt.upgrade()?.read(cx).id(); + sources.contains(&id).then_some(i) + }) + .collect(); + + if source_indices.is_empty() { + return Ok(()); + } + + // Already a contiguous suffix → nothing to do. + let len = self.worktrees.len(); + let already_at_end = source_indices + .iter() + .enumerate() + .all(|(offset, &i)| i == len - source_indices.len() + offset); + if already_at_end { + return Ok(()); + } + + let mut to_append = Vec::with_capacity(source_indices.len()); + for &i in source_indices.iter().rev() { + to_append.push(self.worktrees.remove(i)); + } + to_append.reverse(); + self.worktrees.extend(to_append); + + cx.emit(WorktreeStoreEvent::WorktreeOrderChanged); + cx.notify(); + self.send_project_updates(cx); + Ok(()) + } + pub fn disconnected_from_host(&mut self, cx: &mut App) { for worktree in &self.worktrees { if let Some(worktree) = worktree.upgrade() { diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index f4d8232f0b7..9678713c961 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -3630,6 +3630,52 @@ impl ProjectPanel { }); } + /// Moves all root-only entries in the drag to the end of the worktree + /// list. Used by the blank-area drop handler so dropping a multi-root + /// selection that already contains the last worktree (e.g. `[A, D]` in + /// `[A, B, C, D]`) settles the group at the end instead of falling on + /// the self-drop guard in `move_worktrees`. + fn reorder_worktree_roots_to_end( + &mut self, + selections: &DraggedSelection, + cx: &mut Context, + ) { + self.project.update(cx, |project, cx| { + let source_ids: Vec = selections + .items() + .filter_map(|entry| { + project + .worktree_for_entry(entry.entry_id, cx) + .map(|wt| wt.read(cx).id()) + }) + .collect(); + if source_ids.is_empty() { + return; + } + project.move_worktrees_to_end(&source_ids, cx).log_err(); + }); + } + + fn drag_includes_last_worktree(&self, selections: &DraggedSelection, cx: &App) -> bool { + let project = self.project.read(cx); + let drag_is_root_only = selections + .items() + .all(|entry| project.entry_is_worktree_root(entry.entry_id, cx)); + if !drag_is_root_only { + return false; + } + let Some(last_worktree_id) = project + .visible_worktrees(cx) + .next_back() + .map(|wt| wt.read(cx).id()) + else { + return false; + }; + selections + .items() + .any(|entry| entry.worktree_id == last_worktree_id) + } + fn move_worktree_entry( &mut self, entry_to_move: ProjectEntryId, @@ -7083,7 +7129,16 @@ impl Render for ProjectPanel { move |this, selections: &DraggedSelection, window, cx| { this.drag_target_entry = None; this.hover_scroll_task.take(); - if let Some(entry_id) = this.state.last_worktree_root_id { + // A pure-root drag that contains the + // last worktree would otherwise hit + // the self-drop guard in + // `move_worktrees` and become a no-op. + // Send the group to the end instead. + if this.drag_includes_last_worktree(selections, cx) { + this.reorder_worktree_roots_to_end(selections, cx); + } else if let Some(entry_id) = + this.state.last_worktree_root_id + { this.drag_onto(selections, entry_id, false, window, cx); } cx.stop_propagation(); diff --git a/crates/project_panel/src/project_panel_tests.rs b/crates/project_panel/src/project_panel_tests.rs index 29b9e3cd953..0d8a4dc1cf8 100644 --- a/crates/project_panel/src/project_panel_tests.rs +++ b/crates/project_panel/src/project_panel_tests.rs @@ -4417,6 +4417,99 @@ async fn test_drag_worktree_root_onto_nested_entry_is_rejected( ); } +// Dropping a pure-root selection that includes the last worktree onto the +// blank area below the panel should send the group to the end of the +// worktree list, preserving relative order. Without the special-case +// blank-area handling, the destination resolves to the last worktree's +// root, which is one of the dragged roots, and `move_worktrees`'s +// self-drop guard would silently no-op the gesture. +#[gpui::test] +async fn test_drag_root_group_with_last_worktree_to_blank_area( + cx: &mut gpui::TestAppContext, +) { + 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; + fs.insert_tree("/root4", json!({ "d.txt": "" })).await; + + let project = Project::test( + fs.clone(), + [ + "/root1".as_ref(), + "/root2".as_ref(), + "/root3".as_ref(), + "/root4".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 cx = &mut VisualTestContext::from_window(window.into(), cx); + let panel = workspace.update_in(cx, ProjectPanel::new); + cx.run_until_parked(); + + let (r1_id, r2_id, r3_id, r4_id) = panel.update_in(cx, |panel, _, cx| { + let worktrees = panel + .project + .read(cx) + .visible_worktrees(cx) + .collect::>(); + ( + worktrees[0].read(cx).id(), + worktrees[1].read(cx).id(), + worktrees[2].read(cx).id(), + worktrees[3].read(cx).id(), + ) + }); + + // Drag [r1, r4] onto the blank area: route via the panel's blank-area + // helper since the drag includes the last worktree. + panel.update_in(cx, |panel, _, cx| { + let project = panel.project.read(cx); + let worktrees = project.visible_worktrees(cx).collect::>(); + let r1 = worktrees[0].read(cx); + let r4 = worktrees[3].read(cx); + let r1_entry = SelectedEntry { + worktree_id: r1.id(), + entry_id: r1.root_entry().unwrap().id, + }; + let r4_entry = SelectedEntry { + worktree_id: r4.id(), + entry_id: r4.root_entry().unwrap().id, + }; + let drag = DraggedSelection { + active_selection: r1_entry, + marked_selections: Arc::new([r1_entry, r4_entry]), + }; + assert!( + panel.drag_includes_last_worktree(&drag, cx), + "the drag should be flagged as containing the last worktree" + ); + panel.reorder_worktree_roots_to_end(&drag, cx); + }); + cx.run_until_parked(); + + let order = panel.update_in(cx, |panel, _, cx| { + panel + .project + .read(cx) + .visible_worktrees(cx) + .map(|wt| wt.read(cx).id()) + .collect::>() + }); + assert_eq!( + order, + vec![r2_id, r3_id, r1_id, r4_id], + "marked roots [r1, r4] should land at the end with their original relative order" + ); +} + // Dropping a multi-root selection onto one of its own members has no // well-defined intent, so it should leave the order untouched rather than // reorder the remaining roots around the destination. From e495df4172cc464f424be0613077731ac6ffcd2a Mon Sep 17 00:00:00 2001 From: eth0net Date: Tue, 5 May 2026 17:31:48 +0100 Subject: [PATCH 6/8] Honor copy modifier in worktree-root drag feedback and blank-area path - The blank-area "send group to end" fast path now only triggers for move gestures. Holding the copy modifier falls through to `drag_onto` so the copy branch's existing root-filter no-op stays in effect. - `highlight_entry_for_selection_drag` takes an `is_copy_mode` flag and suppresses every target for pure-root drags in copy mode, so users no longer see a copy cursor with a root-row highlight for an operation that will silently do nothing. - Tighten the doc comment on `move_worktrees` to describe the actual fallback (earliest source in worktree order) and trim the implementation-leaky doc on `move_worktrees_to_end`. - Add `test_highlight_entry_for_root_drag_suppressed_in_copy_mode`. --- crates/project/src/worktree_store.rs | 10 +-- crates/project_panel/src/project_panel.rs | 20 ++++- .../project_panel/src/project_panel_tests.rs | 86 ++++++++++++++++--- 3 files changed, 99 insertions(+), 17 deletions(-) diff --git a/crates/project/src/worktree_store.rs b/crates/project/src/worktree_store.rs index 4a1858f10bd..87e3c3cd2d6 100644 --- a/crates/project/src/worktree_store.rs +++ b/crates/project/src/worktree_store.rs @@ -1045,8 +1045,9 @@ impl WorktreeStore { /// preserving their relative order. The `active_source` (if provided and /// in `sources`) decides whether the group lands before or after the /// destination, mirroring the single-source semantics: a source originally - /// before destination ends up after it, and vice versa. When no active - /// source is supplied, the first source's position is used. + /// before destination ends up after it, and vice versa. When no usable + /// active source is supplied, the earliest source in the current worktree + /// order is used as the direction reference. pub fn move_worktrees( &mut self, sources: &[WorktreeId], @@ -1135,9 +1136,8 @@ impl WorktreeStore { } /// Removes every source from the worktree list and appends them to the - /// end in their original relative order. Intended for "drop on the empty - /// area below the panel" gestures, where the user's intent is "send this - /// group to the end" rather than reordering relative to a specific row. + /// end in their original relative order. Returns early when the sources + /// already form a contiguous suffix. pub fn move_worktrees_to_end( &mut self, sources: &[WorktreeId], diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 9678713c961..0afadd865f1 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -5273,6 +5273,7 @@ impl ProjectPanel { target_entry: &Entry, target_worktree: &Worktree, drag_state: &DraggedSelection, + is_copy_mode: bool, cx: &Context, ) -> Option { // Pure worktree-root drags are only meaningful when dropped on @@ -5284,6 +5285,11 @@ impl ProjectPanel { .items() .all(|entry| project.entry_is_worktree_root(entry.entry_id, cx)); if drag_is_root_only { + // Worktree roots can't be copied; in copy mode the drop is a + // guaranteed no-op, so don't highlight any target. + if is_copy_mode { + return None; + } let root_id = target_worktree.root_entry()?.id; // Hovering any worktree that's part of the drag (active or just // marked) is a no-op in `move_worktrees`, so don't highlight it. @@ -5629,6 +5635,7 @@ impl ProjectPanel { this.marked_entries.push(drag_state.active_selection); } + let is_copy_mode = Self::is_copy_modifier_set(&window.modifiers()); let Some((entry_id, highlight_entry_id)) = maybe!({ let target_worktree = this .project @@ -5641,6 +5648,7 @@ impl ProjectPanel { target_entry, target_worktree, drag_state, + is_copy_mode, cx, )?; Some((target_entry.id, highlight_entry_id)) @@ -7133,8 +7141,16 @@ impl Render for ProjectPanel { // last worktree would otherwise hit // the self-drop guard in // `move_worktrees` and become a no-op. - // Send the group to the end instead. - if this.drag_includes_last_worktree(selections, cx) { + // Send the group to the end — but + // only for move gestures; copy mode + // can't operate on roots and should + // continue to no-op via `drag_onto`. + let is_copy_mode = Self::is_copy_modifier_set( + &window.modifiers(), + ); + if !is_copy_mode + && this.drag_includes_last_worktree(selections, cx) + { this.reorder_worktree_roots_to_end(selections, cx); } else if let Some(entry_id) = this.state.last_worktree_root_id diff --git a/crates/project_panel/src/project_panel_tests.rs b/crates/project_panel/src/project_panel_tests.rs index 0d8a4dc1cf8..56f6fc52be1 100644 --- a/crates/project_panel/src/project_panel_tests.rs +++ b/crates/project_panel/src/project_panel_tests.rs @@ -9077,7 +9077,7 @@ async fn test_highlight_entry_for_selection_drag(cx: &mut gpui::TestAppContext) }]), }; let result = - panel.highlight_entry_for_selection_drag(parent_dir, worktree, &dragged_selection, cx); + panel.highlight_entry_for_selection_drag(parent_dir, worktree, &dragged_selection, false, cx); assert_eq!(result, None, "Should not highlight parent of dragged item"); // Test 2: Single item drag, don't highlight sibling files @@ -9085,13 +9085,14 @@ async fn test_highlight_entry_for_selection_drag(cx: &mut gpui::TestAppContext) sibling_file, worktree, &dragged_selection, + false, cx, ); assert_eq!(result, None, "Should not highlight sibling files"); // Test 3: Single item drag, highlight unrelated directory let result = - panel.highlight_entry_for_selection_drag(other_dir, worktree, &dragged_selection, cx); + panel.highlight_entry_for_selection_drag(other_dir, worktree, &dragged_selection, false, cx); assert_eq!( result, Some(other_dir.id), @@ -9100,7 +9101,7 @@ async fn test_highlight_entry_for_selection_drag(cx: &mut gpui::TestAppContext) // Test 4: Single item drag, highlight sibling directory let result = - panel.highlight_entry_for_selection_drag(child_dir, worktree, &dragged_selection, cx); + panel.highlight_entry_for_selection_drag(child_dir, worktree, &dragged_selection, false, cx); assert_eq!( result, Some(child_dir.id), @@ -9125,7 +9126,7 @@ async fn test_highlight_entry_for_selection_drag(cx: &mut gpui::TestAppContext) ]), }; let result = - panel.highlight_entry_for_selection_drag(parent_dir, worktree, &dragged_selection, cx); + panel.highlight_entry_for_selection_drag(parent_dir, worktree, &dragged_selection, false, cx); assert_eq!( result, Some(parent_dir.id), @@ -9134,7 +9135,7 @@ async fn test_highlight_entry_for_selection_drag(cx: &mut gpui::TestAppContext) // Test 6: Target is file in different directory, highlight parent let result = - panel.highlight_entry_for_selection_drag(other_file, worktree, &dragged_selection, cx); + panel.highlight_entry_for_selection_drag(other_file, worktree, &dragged_selection, false, cx); assert_eq!( result, Some(other_dir.id), @@ -9143,7 +9144,7 @@ async fn test_highlight_entry_for_selection_drag(cx: &mut gpui::TestAppContext) // Test 7: Target is directory, always highlight let result = - panel.highlight_entry_for_selection_drag(child_dir, worktree, &dragged_selection, cx); + panel.highlight_entry_for_selection_drag(child_dir, worktree, &dragged_selection, false, cx); assert_eq!( result, Some(child_dir.id), @@ -9220,6 +9221,7 @@ async fn test_highlight_entry_for_selection_drag_cross_worktree(cx: &mut gpui::T src_dir_from_b, worktree_b.read(cx), &dragged_selection, + false, cx, ); assert_eq!( @@ -9233,6 +9235,7 @@ async fn test_highlight_entry_for_selection_drag_cross_worktree(cx: &mut gpui::T main_rs_from_b, worktree_b.read(cx), &dragged_selection, + false, cx, ); assert_eq!( @@ -9308,34 +9311,97 @@ async fn test_highlight_entry_for_multi_root_drag_excludes_marked_worktrees( // Hovering r2 (not in the drag): highlight allowed. assert_eq!( - panel.highlight_entry_for_selection_drag(r2_root, r2, &drag, cx), + panel.highlight_entry_for_selection_drag(r2_root, r2, &drag, false, cx), Some(r2_root.id), "non-dragged worktree root should highlight" ); // Hovering r4 (not in the drag): highlight allowed. assert_eq!( - panel.highlight_entry_for_selection_drag(r4_root, r4, &drag, cx), + panel.highlight_entry_for_selection_drag(r4_root, r4, &drag, false, cx), Some(r4_root.id), "non-dragged worktree root should highlight" ); // Hovering r1 (active source): no-op, no highlight. assert_eq!( - panel.highlight_entry_for_selection_drag(r1_root, r1, &drag, cx), + panel.highlight_entry_for_selection_drag(r1_root, r1, &drag, false, cx), None, "active source's worktree should not highlight" ); // Hovering r3 (marked but not active): no-op, no highlight. assert_eq!( - panel.highlight_entry_for_selection_drag(r3_root, r3, &drag, cx), + panel.highlight_entry_for_selection_drag(r3_root, r3, &drag, false, cx), None, "marked source's worktree should not highlight" ); }); } +// In copy mode, a pure worktree-root drag is a guaranteed no-op (the copy +// branch in `drag_onto` filters worktree roots out). The highlight should +// reflect this by suppressing every target instead of advertising root +// rows as valid drops. +#[gpui::test] +async fn test_highlight_entry_for_root_drag_suppressed_in_copy_mode( + cx: &mut gpui::TestAppContext, +) { + 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; + + let project = + Project::test(fs.clone(), ["/root1".as_ref(), "/root2".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 cx = &mut VisualTestContext::from_window(window.into(), cx); + let panel = workspace.update_in(cx, ProjectPanel::new); + cx.run_until_parked(); + + panel.update(cx, |panel, cx| { + let worktrees: Vec<_> = panel.project.read(cx).visible_worktrees(cx).collect(); + let r1 = worktrees[0].read(cx); + let r2 = worktrees[1].read(cx); + let r1_root = r1.root_entry().unwrap(); + let r2_root = r2.root_entry().unwrap(); + + let drag = DraggedSelection { + active_selection: SelectedEntry { + worktree_id: r1.id(), + entry_id: r1_root.id, + }, + marked_selections: Arc::new([SelectedEntry { + worktree_id: r1.id(), + entry_id: r1_root.id, + }]), + }; + + // Sanity: without the copy modifier, hovering r2's root highlights it. + assert_eq!( + panel.highlight_entry_for_selection_drag(r2_root, r2, &drag, false, cx), + Some(r2_root.id), + "non-copy root drag should highlight a different worktree's root" + ); + + // With the copy modifier, no target highlights — copy can't act on roots. + assert_eq!( + panel.highlight_entry_for_selection_drag(r2_root, r2, &drag, true, cx), + None, + "copy-mode root drag should suppress highlights on other roots" + ); + assert_eq!( + panel.highlight_entry_for_selection_drag(r1_root, r1, &drag, true, cx), + None, + "copy-mode root drag should suppress highlights on its own worktree" + ); + }); +} + #[gpui::test] async fn test_should_highlight_background_for_selection_drag(cx: &mut gpui::TestAppContext) { init_test(cx); From 0611b29559987919d61916193c8cae22b0580502 Mon Sep 17 00:00:00 2001 From: eth0net Date: Wed, 6 May 2026 10:22:24 +0100 Subject: [PATCH 7/8] Refresh worktree-drag highlights on modifier change and in copy mode - `should_highlight_background_for_selection_drag` now takes `is_copy_mode` and returns false for pure-root drags in copy mode, matching the existing per-row highlight logic. The blank-area on_drag_move handler reads the modifier and threads it through. - The on_modifiers_changed listener now also clears the cached drag target so the next drag-move event recomputes the highlight under the new mode. Without this, the on_drag_move fast path skipped the recomputation while the pointer stayed put on the same row. - Extracted the modifier-changed logic into `ProjectPanel::handle_drag_modifiers_changed` so it can be tested directly. - Adds tests for both fixes. --- crates/project_panel/src/project_panel.rs | 52 ++++++-- .../project_panel/src/project_panel_tests.rs | 111 ++++++++++++++++++ 2 files changed, 151 insertions(+), 12 deletions(-) diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 0afadd865f1..7f356cb87ae 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -4467,6 +4467,22 @@ impl ProjectPanel { .detach(); } + fn handle_drag_modifiers_changed( + &mut self, + modifiers: &Modifiers, + window: &mut Window, + cx: &mut Context, + ) { + self.refresh_drag_cursor_style(modifiers, window, cx); + // The copy modifier flips highlight semantics for worktree-root + // drags. Drop the cached target so the next drag-move event + // recomputes under the new mode — otherwise the on_drag_move + // fast path skips the recomputation while the pointer stays put. + if self.drag_target_entry.take().is_some() { + cx.notify(); + } + } + fn refresh_drag_cursor_style( &self, modifiers: &Modifiers, @@ -5342,19 +5358,28 @@ impl ProjectPanel { &self, drag_state: &DraggedSelection, last_root_id: ProjectEntryId, + is_copy_mode: bool, cx: &App, ) -> bool { + let project = self.project.read(cx); + + // Worktree roots can't be copied, so a pure-root copy drag is a + // guaranteed no-op — don't advertise the background as a target. + if is_copy_mode + && drag_state + .items() + .all(|entry| project.entry_is_worktree_root(entry.entry_id, cx)) + { + return false; + } + // Always highlight for multiple entries if drag_state.items().count() > 1 { return true; } // Since root will always have empty relative path - if let Some(entry_path) = self - .project - .read(cx) - .path_for_entry(drag_state.active_selection.entry_id, cx) - { + if let Some(entry_path) = project.path_for_entry(drag_state.active_selection.entry_id, cx) { if let Some(parent_path) = entry_path.path.parent() { if !parent_path.is_empty() { return true; @@ -5363,11 +5388,7 @@ impl ProjectPanel { } // If parent is empty, check if different worktree - if let Some(last_root_worktree_id) = self - .project - .read(cx) - .worktree_id_for_entry(last_root_id, cx) - { + if let Some(last_root_worktree_id) = project.worktree_id_for_entry(last_root_id, cx) { if drag_state.active_selection.worktree_id != last_root_worktree_id { return true; } @@ -6751,7 +6772,7 @@ impl Render for ProjectPanel { .relative() .on_modifiers_changed(cx.listener( |this, event: &ModifiersChangedEvent, window, cx| { - this.refresh_drag_cursor_style(&event.modifiers, window, cx); + this.handle_drag_modifiers_changed(&event.modifiers, window, cx); }, )) .key_context(self.dispatch_context(window, cx)) @@ -7094,16 +7115,23 @@ impl Render for ProjectPanel { }, )) .on_drag_move::(cx.listener( - move |this, event: &DragMoveEvent, _, cx| { + move |this, + event: &DragMoveEvent, + window, + cx| { let Some(last_root_id) = this.state.last_worktree_root_id else { return; }; if event.bounds.contains(&event.event.position) { let drag_state = event.drag(cx); + let is_copy_mode = Self::is_copy_modifier_set( + &window.modifiers(), + ); if this.should_highlight_background_for_selection_drag( &drag_state, last_root_id, + is_copy_mode, cx, ) { this.drag_target_entry = diff --git a/crates/project_panel/src/project_panel_tests.rs b/crates/project_panel/src/project_panel_tests.rs index 56f6fc52be1..e89ba1fc373 100644 --- a/crates/project_panel/src/project_panel_tests.rs +++ b/crates/project_panel/src/project_panel_tests.rs @@ -9480,6 +9480,7 @@ async fn test_should_highlight_background_for_selection_drag(cx: &mut gpui::Test let result = panel.should_highlight_background_for_selection_drag( &multiple_dragged_selection, root1_entry.id, + false, cx, ); assert!(result, "Should highlight background for multiple entries"); @@ -9499,6 +9500,7 @@ async fn test_should_highlight_background_for_selection_drag(cx: &mut gpui::Test let result = panel.should_highlight_background_for_selection_drag( &nested_dragged_selection, root1_entry.id, + false, cx, ); assert!(result, "Should highlight background for nested file"); @@ -9518,6 +9520,7 @@ async fn test_should_highlight_background_for_selection_drag(cx: &mut gpui::Test let result = panel.should_highlight_background_for_selection_drag( &root_file_dragged_selection, root1_entry.id, + false, cx, ); assert!( @@ -9529,6 +9532,7 @@ async fn test_should_highlight_background_for_selection_drag(cx: &mut gpui::Test let result = panel.should_highlight_background_for_selection_drag( &root_file_dragged_selection, root2_entry.id, + false, cx, ); assert!( @@ -9551,6 +9555,7 @@ async fn test_should_highlight_background_for_selection_drag(cx: &mut gpui::Test let result = panel.should_highlight_background_for_selection_drag( &child_file_dragged_selection, root1_entry.id, + false, cx, ); assert!( @@ -9560,6 +9565,112 @@ async fn test_should_highlight_background_for_selection_drag(cx: &mut gpui::Test }); } +// In copy mode, a pure worktree-root drag is a no-op, so the blank area +// below the panel should not light up as a valid drop target. +#[gpui::test] +async fn test_should_highlight_background_suppressed_for_root_drag_in_copy_mode( + cx: &mut gpui::TestAppContext, +) { + 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; + + let project = + Project::test(fs.clone(), ["/root1".as_ref(), "/root2".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 cx = &mut VisualTestContext::from_window(window.into(), cx); + let panel = workspace.update_in(cx, ProjectPanel::new); + cx.run_until_parked(); + + panel.update(cx, |panel, cx| { + let worktrees: Vec<_> = panel.project.read(cx).visible_worktrees(cx).collect(); + let r1 = worktrees[0].read(cx); + let r2 = worktrees[1].read(cx); + let r1_root_id = r1.root_entry().unwrap().id; + let r2_root_id = r2.root_entry().unwrap().id; + + let drag = DraggedSelection { + active_selection: SelectedEntry { + worktree_id: r1.id(), + entry_id: r1_root_id, + }, + marked_selections: Arc::new([SelectedEntry { + worktree_id: r1.id(), + entry_id: r1_root_id, + }]), + }; + + // Sanity: in non-copy mode, the background highlights for a cross- + // worktree root drag (so the user can drop at the end). + assert!( + panel.should_highlight_background_for_selection_drag( + &drag, + r2_root_id, + false, + cx, + ), + "non-copy root drag should highlight background" + ); + + // In copy mode, the same drag is a no-op → no background highlight. + assert!( + !panel.should_highlight_background_for_selection_drag( + &drag, + r2_root_id, + true, + cx, + ), + "copy-mode root drag should suppress background highlight" + ); + }); +} + +// Toggling the copy modifier while the cursor stays still must invalidate +// the cached drag target so the next drag-move event recomputes the +// highlight under the new mode (otherwise the row stays in its old +// highlight state until the pointer leaves and re-enters). +#[gpui::test] +async fn test_modifier_change_clears_drag_target_entry( + cx: &mut gpui::TestAppContext, +) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree("/root1", json!({ "a.txt": "" })).await; + + let project = Project::test(fs.clone(), ["/root1".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 cx = &mut VisualTestContext::from_window(window.into(), cx); + let panel = workspace.update_in(cx, ProjectPanel::new); + cx.run_until_parked(); + + panel.update_in(cx, |panel, window, cx| { + panel.drag_target_entry = Some(DragTarget::Background); + panel.handle_drag_modifiers_changed( + &gpui::Modifiers { + alt: true, + control: true, + ..Default::default() + }, + window, + cx, + ); + assert!( + panel.drag_target_entry.is_none(), + "modifier change should clear drag_target_entry so the next \ + drag-move can recompute highlights under the new mode" + ); + }); +} + #[gpui::test] async fn test_hide_root(cx: &mut gpui::TestAppContext) { init_test(cx); From 465e096e155c3acb55f147439d0e8820c4dc1ef9 Mon Sep 17 00:00:00 2001 From: eth0net Date: Wed, 6 May 2026 10:31:41 +0100 Subject: [PATCH 8/8] Tighten worktree drag-and-drop tests and docs - Add `test_move_worktrees_to_end` to the project integration tests, covering non-contiguous reorder, the already-suffix no-op, single source, and the missing-source error path. - Add `test_copy_drag_root_onto_blank_area_is_no_op` exercising the end-to-end behaviour through `drag_onto`. - Trim the implementation-leaky doc on `Project::move_worktrees_to_end` and reflow the blank-area drop-handler comment. --- crates/project/src/project.rs | 3 +- .../tests/integration/project_tests.rs | 90 +++++++++++++++++++ crates/project_panel/src/project_panel.rs | 18 ++-- .../project_panel/src/project_panel_tests.rs | 65 ++++++++++++++ 4 files changed, 163 insertions(+), 13 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 2d1f4de6bbb..dfb16f75a7a 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -4594,8 +4594,7 @@ impl Project { } /// Moves multiple worktrees to the end of the worktree list, preserving - /// their relative order. Used for "drop on the blank area below the - /// project panel" gestures. + /// their relative order. pub fn move_worktrees_to_end( &mut self, sources: &[WorktreeId], diff --git a/crates/project/tests/integration/project_tests.rs b/crates/project/tests/integration/project_tests.rs index 4e4d20110c1..6672fde3cc2 100644 --- a/crates/project/tests/integration/project_tests.rs +++ b/crates/project/tests/integration/project_tests.rs @@ -8946,6 +8946,96 @@ async fn test_reordering_worktrees(cx: &mut gpui::TestAppContext) { }); } +#[gpui::test] +async fn test_move_worktrees_to_end(cx: &mut gpui::TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + "/dir", + json!({ + "a.rs": "", + "b.rs": "", + "c.rs": "", + "d.rs": "", + }), + ) + .await; + + let project = Project::test( + fs, + [ + "/dir/a.rs".as_ref(), + "/dir/b.rs".as_ref(), + "/dir/c.rs".as_ref(), + "/dir/d.rs".as_ref(), + ], + cx, + ) + .await; + + let (a_id, b_id, c_id, d_id) = project.update(cx, |project, cx| { + let worktrees = project.visible_worktrees(cx).collect::>(); + ( + worktrees[0].read(cx).id(), + worktrees[1].read(cx).id(), + worktrees[2].read(cx).id(), + worktrees[3].read(cx).id(), + ) + }); + + // Non-contiguous selection [a, c] → group lands at end as [b, d, a, c]. + project + .update(cx, |project, cx| { + project.move_worktrees_to_end(&[a_id, c_id], cx) + }) + .expect("moving non-contiguous group to end"); + project.update(cx, |project, cx| { + let order: Vec<_> = project + .visible_worktrees(cx) + .map(|wt| wt.read(cx).id()) + .collect(); + assert_eq!(order, vec![b_id, d_id, a_id, c_id]); + }); + + // Already-suffix selection [a, c] → no-op (current order is [b, d, a, c]). + project + .update(cx, |project, cx| { + project.move_worktrees_to_end(&[a_id, c_id], cx) + }) + .expect("contiguous-suffix call should still succeed"); + project.update(cx, |project, cx| { + let order: Vec<_> = project + .visible_worktrees(cx) + .map(|wt| wt.read(cx).id()) + .collect(); + assert_eq!(order, vec![b_id, d_id, a_id, c_id]); + }); + + // Single source [b] → moves to end: [d, a, c, b]. + project + .update(cx, |project, cx| { + project.move_worktrees_to_end(&[b_id], cx) + }) + .expect("moving a single source to end"); + project.update(cx, |project, cx| { + let order: Vec<_> = project + .visible_worktrees(cx) + .map(|wt| wt.read(cx).id()) + .collect(); + assert_eq!(order, vec![d_id, a_id, c_id, b_id]); + }); + + // Invalid source id → error. + project.update(cx, |project, cx| { + let invalid = worktree::WorktreeId::from_usize(99_999); + assert!( + project.move_worktrees_to_end(&[invalid], cx).is_err(), + "moving an unknown source should error" + ); + }); +} + #[gpui::test] async fn test_move_worktree_with_invalid_source_errors(cx: &mut gpui::TestAppContext) { use worktree::WorktreeId; diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 7f356cb87ae..38df3cc2833 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -7165,17 +7165,13 @@ impl Render for ProjectPanel { move |this, selections: &DraggedSelection, window, cx| { this.drag_target_entry = None; this.hover_scroll_task.take(); - // A pure-root drag that contains the - // last worktree would otherwise hit - // the self-drop guard in - // `move_worktrees` and become a no-op. - // Send the group to the end — but - // only for move gestures; copy mode - // can't operate on roots and should - // continue to no-op via `drag_onto`. - let is_copy_mode = Self::is_copy_modifier_set( - &window.modifiers(), - ); + let is_copy_mode = + Self::is_copy_modifier_set(&window.modifiers()); + // For move drags whose root group includes the last + // worktree, route to the move-to-end path so we don't + // hit the self-drop guard in `move_worktrees`. Copy + // drags fall through to `drag_onto`, which will + // filter the roots out as a no-op. if !is_copy_mode && this.drag_includes_last_worktree(selections, cx) { diff --git a/crates/project_panel/src/project_panel_tests.rs b/crates/project_panel/src/project_panel_tests.rs index e89ba1fc373..41cec1df524 100644 --- a/crates/project_panel/src/project_panel_tests.rs +++ b/crates/project_panel/src/project_panel_tests.rs @@ -4510,6 +4510,71 @@ async fn test_drag_root_group_with_last_worktree_to_blank_area( ); } +// Holding the copy modifier and dropping a pure-root drag onto the blank +// area below the panel should be a no-op rather than reordering — copy +// can't operate on worktree roots. +#[gpui::test] +async fn test_copy_drag_root_onto_blank_area_is_no_op(cx: &mut gpui::TestAppContext) { + 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 cx = &mut VisualTestContext::from_window(window.into(), cx); + let panel = workspace.update_in(cx, ProjectPanel::new); + cx.run_until_parked(); + + cx.simulate_modifiers_change(gpui::Modifiers { + alt: true, + control: true, + ..Default::default() + }); + + // Drag root1 onto root3's root (the blank-area handler resolves to + // `last_worktree_root_id`); copy mode should make this a no-op. + panel.update_in(cx, |panel, window, cx| { + let project = panel.project.read(cx); + let worktrees = project.visible_worktrees(cx).collect::>(); + let r1 = worktrees[0].read(cx); + let r3 = worktrees[2].read(cx); + let r1_entry = SelectedEntry { + worktree_id: r1.id(), + entry_id: r1.root_entry().unwrap().id, + }; + let drag = DraggedSelection { + active_selection: r1_entry, + marked_selections: Arc::new([r1_entry]), + }; + panel.drag_onto(&drag, r3.root_entry().unwrap().id, false, window, cx); + }); + cx.run_until_parked(); + + assert_eq!( + visible_entries_as_strings(&panel, 0..20, cx), + &[ + "v root1", + " a.txt", + "v root2", + " b.txt", + "v root3", + " c.txt", + ], + "copy-drag of a worktree root should not reorder worktrees" + ); +} + // Dropping a multi-root selection onto one of its own members has no // well-defined intent, so it should leave the order untouched rather than // reorder the remaining roots around the destination.