project_panel: Multi-worktree drag reordering and collaboration sync - #60656
project_panel: Multi-worktree drag reordering and collaboration sync#60656eth0net wants to merge 1 commit into
Conversation
cla-bot
Bot
added
the
cla-signed
5096ab8 to
55c559b
Compare
There was a problem hiding this comment.
Pull request overview
Adds multi-worktree drag-and-drop reordering in the project panel, including root-only drop targeting/highlighting rules, “send to end” behavior for blank-area drops, and new WorktreeStore/Project APIs to perform grouped reorders (with collaboration update propagation).
Changes:
- Implement grouped worktree-root reordering (preserving relative order) and a dedicated move-to-end path for root groups.
- Refine drag highlighting and drop acceptance: root-only drags only target roots; copy-modifier behavior is reflected in both drop handling and highlight invalidation.
- Introduce/extend tests covering grouped reorders, rejected nested-entry drops for roots, modifier toggling, and new worktree-store move APIs.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/project/tests/integration/project_tests.rs | Adds integration tests for move_worktrees_to_end and invalid-id error behavior. |
| crates/project/src/worktree_store.rs | Adds move_worktrees / move_worktrees_to_end, updates reorder paths to emit events/notify and send project updates. |
| crates/project/src/project.rs | Exposes new worktree move APIs on Project as wrappers over WorktreeStore. |
| crates/project_panel/src/project_panel.rs | Implements root-group reorder logic, blank-area move-to-end routing, and updated drag highlight/copy-modifier handling. |
| crates/project_panel/src/project_panel_tests.rs | Adds extensive coverage for new drag semantics (group reorder, rejection rules, copy-mode highlighting, modifier-change invalidation). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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); | ||
| } |
There was a problem hiding this comment.
Good catch — fixed in cb2cb1f. You're right that the flow moved non-root entries after reordering (the mixed-drag test only looked like it left files in place because move_worktree_entry treats a cross-worktree move as a no-op when the relative path happens to be identical — a coincidence of the test data, not intended behavior).
drag_onto now checks whether the active selection is a worktree root. If it is, the drag is treated as a pure reorder gesture: it reorders the roots and returns without moving any non-root entries in the same drag. Files are only moved when the drag is initiated from a non-root entry (test_drag_mixed_root_and_file_with_non_root_active / test_drag_marked_root_with_nested_file_keeps_both both use a non-root active selection and still move the file). Updated test_drag_including_worktree_root_only_reorders to assert this deterministically (with explicit file-location checks rather than relying on the relative-path coincidence).
| // 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(()); | ||
| } |
There was a problem hiding this comment.
Fixed in cb2cb1f. Moved the id validation (destination + every source) ahead of the self-drop early-return, so move_worktrees(&[invalid], invalid, ..) now errors with Missing worktree for id <id> instead of being masked by the self-drop no-op. This matches the upfront-validation contract used elsewhere in the PR. Covered by test_move_worktree_with_invalid_source_errors.
55c559b to
cb2cb1f
Compare
Builds on the drag-and-drop reorder regression fix with the larger feature work: - Reorder marked worktree roots together as a contiguous group when dragged, preserving their relative order. Direction (before vs after the destination) follows the active source's original position. - A drag whose active selection is a worktree root is treated as a pure reorder gesture: it reorders the roots and does not move any non-root entries marked in the same drag. Files are only moved when the drag is initiated from a non-root entry. - Root drops are only accepted onto another worktree's root; the drag-over highlight matches for roots-only drags, while mixed drags still highlight directory targets for the file portion. - Blank-area drops of a root group that includes the last worktree send the group to the end instead of no-opping on the self-drop guard. - Honor the copy modifier in root drag feedback and the blank-area path, refreshing highlights on modifier change. - Reorders call `send_project_updates` so collaborators see the new worktree order. - `WorktreeStore::move_worktrees` / `move_worktrees_to_end` batch the reorder and validate every source up front (before the self-drop no-op), erroring on missing IDs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cb2cb1f to
e9ccef2
Compare
Follow-up to #55755 (now merged), which restored single-worktree drag-and-drop reordering. This PR adds the larger feature work that was split out so the regression fix could land quickly. Rebased onto
main, so the diff is feature-only.What this adds
WorktreeStore::move_worktrees/move_worktrees_to_endbatch the reorder and validate every source up front, erroring on missing IDs.Discussion points (raised in #55755 review)
send_project_updatesso collaborators see the new worktree order. Worktree order was intentionally not synced during collaboration so far — guests keep their own panel order — so this is a behavior change worth confirming before merge.Behavior change vs the merged fix
The minimal fix (#55755) added
test_drag_including_worktree_root_only_reorders, which asserted that a drag containing a worktree root only reorders worktrees. This PR deliberately changes that space:test_drag_worktree_root_onto_nested_entry_is_rejected).test_drag_mixed_root_and_file_with_non_root_active); copy-drags filter roots out but still copy the non-root entries (test_copy_drag_mixed_worktree_root_and_file_still_copies_file).I updated
test_drag_including_worktree_root_only_reordersto reflect the new semantics. Flagging it explicitly since it touches a test that just merged.Release Notes: