From 465e096e155c3acb55f147439d0e8820c4dc1ef9 Mon Sep 17 00:00:00 2001 From: eth0net Date: Wed, 6 May 2026 10:31:41 +0100 Subject: [PATCH] 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.