terminal: Fix drag-and-drop in vertical terminal panels (#49825)

claire created

Closes #49800

Adds `handle_drop` to Item & ItemHandle, which allows an active item in
a pane to consume drop events before the pane does.

Release Notes:

- terminal: Fix drag-and-drop not working in vertical terminal panels

Change summary

crates/debugger_ui/src/persistence.rs      |  37 +
crates/debugger_ui/src/session/running.rs  | 328 ++++++++++------
crates/terminal/src/terminal.rs            |  15 
crates/terminal_view/Cargo.toml            |   1 
crates/terminal_view/src/terminal_panel.rs | 147 -------
crates/terminal_view/src/terminal_view.rs  | 448 ++++++++++++++++++++++-
crates/workspace/src/item.rs               |  33 +
crates/workspace/src/pane.rs               | 222 +++++++++--
8 files changed, 889 insertions(+), 342 deletions(-)

Detailed changes

crates/debugger_ui/src/persistence.rs 🔗

@@ -265,49 +265,72 @@ pub(crate) fn deserialize_pane_layout(
                 pane.entity_id(),
                 cx.subscribe_in(&pane, window, RunningState::handle_pane_event),
             );
+            let running_state = cx.weak_entity();
+            let pane_handle = pane.downgrade();
 
             let sub_views: Vec<_> = serialized_pane
                 .children
                 .iter()
                 .map(|child| match child {
-                    DebuggerPaneItem::Frames => {
-                        Box::new(SubView::stack_frame_list(stack_frame_list.clone(), cx))
-                    }
+                    DebuggerPaneItem::Frames => Box::new(SubView::stack_frame_list(
+                        stack_frame_list.clone(),
+                        running_state.clone(),
+                        pane_handle.clone(),
+                        cx,
+                    )),
                     DebuggerPaneItem::Variables => Box::new(SubView::new(
                         variable_list.focus_handle(cx),
                         variable_list.clone().into(),
                         DebuggerPaneItem::Variables,
+                        running_state.clone(),
+                        pane_handle.clone(),
+                        cx,
+                    )),
+                    DebuggerPaneItem::BreakpointList => Box::new(SubView::breakpoint_list(
+                        breakpoint_list.clone(),
+                        running_state.clone(),
+                        pane_handle.clone(),
                         cx,
                     )),
-                    DebuggerPaneItem::BreakpointList => {
-                        Box::new(SubView::breakpoint_list(breakpoint_list.clone(), cx))
-                    }
                     DebuggerPaneItem::Modules => Box::new(SubView::new(
                         module_list.focus_handle(cx),
                         module_list.clone().into(),
                         DebuggerPaneItem::Modules,
+                        running_state.clone(),
+                        pane_handle.clone(),
                         cx,
                     )),
                     DebuggerPaneItem::LoadedSources => Box::new(SubView::new(
                         loaded_sources.focus_handle(cx),
                         loaded_sources.clone().into(),
                         DebuggerPaneItem::LoadedSources,
+                        running_state.clone(),
+                        pane_handle.clone(),
                         cx,
                     )),
                     DebuggerPaneItem::Console => {
-                        let view = SubView::console(console.clone(), cx);
+                        let view = SubView::console(
+                            console.clone(),
+                            running_state.clone(),
+                            pane_handle.clone(),
+                            cx,
+                        );
                         Box::new(view)
                     }
                     DebuggerPaneItem::Terminal => Box::new(SubView::new(
                         terminal.focus_handle(cx),
                         terminal.clone().into(),
                         DebuggerPaneItem::Terminal,
+                        running_state.clone(),
+                        pane_handle.clone(),
                         cx,
                     )),
                     DebuggerPaneItem::MemoryView => Box::new(SubView::new(
                         memory_view.focus_handle(cx),
                         memory_view.clone().into(),
                         DebuggerPaneItem::MemoryView,
+                        running_state.clone(),
+                        pane_handle.clone(),
                         cx,
                     )),
                 })

crates/debugger_ui/src/session/running.rs 🔗

@@ -7,7 +7,6 @@ pub mod stack_frame_list;
 pub mod variable_list;
 use std::{
     any::Any,
-    ops::ControlFlow,
     path::PathBuf,
     sync::{Arc, LazyLock},
     time::Duration,
@@ -72,6 +71,7 @@ pub struct RunningState {
     focus_handle: FocusHandle,
     _remote_id: Option<ViewId>,
     workspace: WeakEntity<Workspace>,
+    project: WeakEntity<Project>,
     session_id: SessionId,
     variable_list: Entity<variable_list::VariableList>,
     _subscriptions: Vec<Subscription>,
@@ -144,6 +144,8 @@ pub(crate) struct SubView {
     inner: AnyView,
     item_focus_handle: FocusHandle,
     kind: DebuggerPaneItem,
+    running_state: WeakEntity<RunningState>,
+    host_pane: WeakEntity<Pane>,
     show_indicator: Box<dyn Fn(&App) -> bool>,
     actions: Option<Box<dyn FnMut(&mut Window, &mut App) -> AnyElement>>,
     hovered: bool,
@@ -154,12 +156,16 @@ impl SubView {
         item_focus_handle: FocusHandle,
         view: AnyView,
         kind: DebuggerPaneItem,
+        running_state: WeakEntity<RunningState>,
+        host_pane: WeakEntity<Pane>,
         cx: &mut App,
     ) -> Entity<Self> {
         cx.new(|_| Self {
             kind,
             inner: view,
             item_focus_handle,
+            running_state,
+            host_pane,
             show_indicator: Box::new(|_| false),
             actions: None,
             hovered: false,
@@ -168,6 +174,8 @@ impl SubView {
 
     pub(crate) fn stack_frame_list(
         stack_frame_list: Entity<StackFrameList>,
+        running_state: WeakEntity<RunningState>,
+        host_pane: WeakEntity<Pane>,
         cx: &mut App,
     ) -> Entity<Self> {
         let weak_list = stack_frame_list.downgrade();
@@ -175,6 +183,8 @@ impl SubView {
             stack_frame_list.focus_handle(cx),
             stack_frame_list.into(),
             DebuggerPaneItem::Frames,
+            running_state,
+            host_pane,
             cx,
         );
 
@@ -189,12 +199,19 @@ impl SubView {
         this
     }
 
-    pub(crate) fn console(console: Entity<Console>, cx: &mut App) -> Entity<Self> {
+    pub(crate) fn console(
+        console: Entity<Console>,
+        running_state: WeakEntity<RunningState>,
+        host_pane: WeakEntity<Pane>,
+        cx: &mut App,
+    ) -> Entity<Self> {
         let weak_console = console.downgrade();
         let this = Self::new(
             console.focus_handle(cx),
             console.into(),
             DebuggerPaneItem::Console,
+            running_state,
+            host_pane,
             cx,
         );
         this.update(cx, |this, _| {
@@ -207,13 +224,20 @@ impl SubView {
         this
     }
 
-    pub(crate) fn breakpoint_list(list: Entity<BreakpointList>, cx: &mut App) -> Entity<Self> {
+    pub(crate) fn breakpoint_list(
+        list: Entity<BreakpointList>,
+        running_state: WeakEntity<RunningState>,
+        host_pane: WeakEntity<Pane>,
+        cx: &mut App,
+    ) -> Entity<Self> {
         let weak_list = list.downgrade();
         let focus_handle = list.focus_handle(cx);
         let this = Self::new(
             focus_handle,
             list.into(),
             DebuggerPaneItem::BreakpointList,
+            running_state,
+            host_pane,
             cx,
         );
 
@@ -239,6 +263,10 @@ impl SubView {
     ) {
         self.actions = Some(actions);
     }
+
+    fn set_host_pane(&mut self, host_pane: WeakEntity<Pane>) {
+        self.host_pane = host_pane;
+    }
 }
 impl Focusable for SubView {
     fn focus_handle(&self, _: &App) -> FocusHandle {
@@ -281,6 +309,75 @@ impl Item for SubView {
 
         label.into_any_element()
     }
+
+    fn handle_drop(
+        &self,
+        active_pane: &Pane,
+        dropped: &dyn Any,
+        window: &mut Window,
+        cx: &mut App,
+    ) -> bool {
+        let Some(tab) = dropped.downcast_ref::<DraggedTab>() else {
+            return true;
+        };
+        let Some(this_pane) = self.host_pane.upgrade() else {
+            return true;
+        };
+        let item = if tab.pane == this_pane {
+            active_pane.item_for_index(tab.ix)
+        } else {
+            tab.pane.read(cx).item_for_index(tab.ix)
+        };
+        let Some(item) = item.filter(|item| item.downcast::<SubView>().is_some()) else {
+            return true;
+        };
+        let Some(split_direction) = active_pane.drag_split_direction() else {
+            return false;
+        };
+
+        let source = tab.pane.clone();
+        let item_id_to_move = item.item_id();
+        let weak_running = self.running_state.clone();
+
+        // Source pane may be the one currently updated, so defer the move.
+        window.defer(cx, move |window, cx| {
+            let new_pane = weak_running.update(cx, |running, cx| {
+                let Some(project) = running.project.upgrade() else {
+                    return Err(anyhow!("Debugger project has been dropped"));
+                };
+
+                let new_pane = new_debugger_pane(running.workspace.clone(), project, window, cx);
+                let _previous_subscription = running.pane_close_subscriptions.insert(
+                    new_pane.entity_id(),
+                    cx.subscribe_in(&new_pane, window, RunningState::handle_pane_event),
+                );
+                debug_assert!(_previous_subscription.is_none());
+                running
+                    .panes
+                    .split(&this_pane, &new_pane, split_direction, cx);
+                anyhow::Ok(new_pane)
+            });
+
+            match new_pane.and_then(|result| result) {
+                Ok(new_pane) => {
+                    move_item(
+                        &source,
+                        &new_pane,
+                        item_id_to_move,
+                        new_pane.read(cx).active_item_index(),
+                        true,
+                        window,
+                        cx,
+                    );
+                }
+                Err(err) => {
+                    log::error!("{err:?}");
+                }
+            }
+        });
+
+        true
+    }
 }
 
 impl Render for SubView {
@@ -311,83 +408,18 @@ pub(crate) fn new_debugger_pane(
     cx: &mut Context<RunningState>,
 ) -> Entity<Pane> {
     let weak_running = cx.weak_entity();
-    let custom_drop_handle = {
-        let workspace = workspace.clone();
-        let project = project.downgrade();
-        let weak_running = weak_running.clone();
-        move |pane: &mut Pane, any: &dyn Any, window: &mut Window, cx: &mut Context<Pane>| {
-            let Some(tab) = any.downcast_ref::<DraggedTab>() else {
-                return ControlFlow::Break(());
-            };
-            let Some(project) = project.upgrade() else {
-                return ControlFlow::Break(());
-            };
-            let this_pane = cx.entity();
-            let item = if tab.pane == this_pane {
-                pane.item_for_index(tab.ix)
-            } else {
-                tab.pane.read(cx).item_for_index(tab.ix)
-            };
-            let Some(item) = item.filter(|item| item.downcast::<SubView>().is_some()) else {
-                return ControlFlow::Break(());
-            };
-
-            let source = tab.pane.clone();
-            let item_id_to_move = item.item_id();
-
-            let Some(split_direction) = pane.drag_split_direction() else {
-                // If we drop into existing pane or current pane,
-                // regular pane drop handler will take care of it,
-                // using the right tab index for the operation.
-                return ControlFlow::Continue(());
-            };
-
-            let workspace = workspace.clone();
-            let weak_running = weak_running.clone();
-            // Source pane may be the one currently updated, so defer the move.
-            window.defer(cx, move |window, cx| {
-                let new_pane = weak_running.update(cx, |running, cx| {
-                    let new_pane =
-                        new_debugger_pane(workspace.clone(), project.clone(), window, cx);
-                    let _previous_subscription = running.pane_close_subscriptions.insert(
-                        new_pane.entity_id(),
-                        cx.subscribe_in(&new_pane, window, RunningState::handle_pane_event),
-                    );
-                    debug_assert!(_previous_subscription.is_none());
-                    running
-                        .panes
-                        .split(&this_pane, &new_pane, split_direction, cx);
-                    new_pane
-                });
-
-                match new_pane {
-                    Ok(new_pane) => {
-                        move_item(
-                            &source,
-                            &new_pane,
-                            item_id_to_move,
-                            new_pane.read(cx).active_item_index(),
-                            true,
-                            window,
-                            cx,
-                        );
-                    }
-                    Err(err) => {
-                        log::error!("{err:?}");
-                    }
-                };
-            });
-
-            ControlFlow::Break(())
-        }
-    };
 
     cx.new(move |cx| {
+        let can_drop_predicate: Arc<dyn Fn(&dyn Any, &mut Window, &mut App) -> bool> =
+            Arc::new(|any, _window, _cx| {
+                any.downcast_ref::<DraggedTab>()
+                    .is_some_and(|dragged_tab| dragged_tab.item.downcast::<SubView>().is_some())
+            });
         let mut pane = Pane::new(
             workspace.clone(),
             project.clone(),
             Default::default(),
-            None,
+            Some(can_drop_predicate),
             NoAction.boxed_clone(),
             true,
             window,
@@ -426,7 +458,6 @@ pub(crate) fn new_debugger_pane(
         })));
         pane.set_can_toggle_zoom(false, cx);
         pane.display_nav_history_buttons(None);
-        pane.set_custom_drop_handle(cx, custom_drop_handle);
         pane.set_should_display_tab_bar(|_, _| true);
         pane.set_render_tab_bar_buttons(cx, |_, _, _| (None, None));
         pane.set_render_tab_bar(cx, {
@@ -466,8 +497,17 @@ pub(crate) fn new_debugger_pane(
                             })
                             .on_drop(cx.listener(
                                 move |this, dragged_tab: &DraggedTab, window, cx| {
+                                    if dragged_tab.item.downcast::<SubView>().is_none() {
+                                        return;
+                                    }
                                     this.drag_split_direction = None;
-                                    this.handle_tab_drop(dragged_tab, this.items_len(), window, cx)
+                                    this.handle_tab_drop(
+                                        dragged_tab,
+                                        this.items_len(),
+                                        false,
+                                        window,
+                                        cx,
+                                    )
                                 },
                             ))
                             .children(pane.items().enumerate().map(|(ix, item)| {
@@ -516,8 +556,11 @@ pub(crate) fn new_debugger_pane(
                                     ))
                                     .on_drop(cx.listener(
                                         move |this, dragged_tab: &DraggedTab, window, cx| {
+                                            if dragged_tab.item.downcast::<SubView>().is_none() {
+                                                return;
+                                            }
                                             this.drag_split_direction = None;
-                                            this.handle_tab_drop(dragged_tab, ix, window, cx)
+                                            this.handle_tab_drop(dragged_tab, ix, false, window, cx)
                                         },
                                     ))
                                     .on_drag(
@@ -729,6 +772,7 @@ impl RunningState {
     ) -> Self {
         let focus_handle = cx.focus_handle();
         let session_id = session.read(cx).session_id();
+        let weak_project = project.downgrade();
         let weak_state = cx.weak_entity();
         let stack_frame_list = cx.new(|cx| {
             StackFrameList::new(
@@ -904,6 +948,7 @@ impl RunningState {
             memory_view,
             session,
             workspace,
+            project: weak_project,
             focus_handle,
             variable_list,
             _subscriptions,
@@ -1304,48 +1349,71 @@ impl RunningState {
     fn create_sub_view(
         &self,
         item_kind: DebuggerPaneItem,
-        _pane: &Entity<Pane>,
+        pane: &Entity<Pane>,
         cx: &mut Context<Self>,
     ) -> Box<dyn ItemHandle> {
+        let running_state = cx.weak_entity();
+        let host_pane = pane.downgrade();
+
         match item_kind {
-            DebuggerPaneItem::Console => Box::new(SubView::console(self.console.clone(), cx)),
+            DebuggerPaneItem::Console => Box::new(SubView::console(
+                self.console.clone(),
+                running_state,
+                host_pane,
+                cx,
+            )),
             DebuggerPaneItem::Variables => Box::new(SubView::new(
                 self.variable_list.focus_handle(cx),
                 self.variable_list.clone().into(),
                 item_kind,
+                running_state,
+                host_pane,
+                cx,
+            )),
+            DebuggerPaneItem::BreakpointList => Box::new(SubView::breakpoint_list(
+                self.breakpoint_list.clone(),
+                running_state,
+                host_pane,
                 cx,
             )),
-            DebuggerPaneItem::BreakpointList => {
-                Box::new(SubView::breakpoint_list(self.breakpoint_list.clone(), cx))
-            }
             DebuggerPaneItem::Frames => Box::new(SubView::new(
                 self.stack_frame_list.focus_handle(cx),
                 self.stack_frame_list.clone().into(),
                 item_kind,
+                running_state,
+                host_pane,
                 cx,
             )),
             DebuggerPaneItem::Modules => Box::new(SubView::new(
                 self.module_list.focus_handle(cx),
                 self.module_list.clone().into(),
                 item_kind,
+                running_state,
+                host_pane,
                 cx,
             )),
             DebuggerPaneItem::LoadedSources => Box::new(SubView::new(
                 self.loaded_sources_list.focus_handle(cx),
                 self.loaded_sources_list.clone().into(),
                 item_kind,
+                running_state,
+                host_pane,
                 cx,
             )),
             DebuggerPaneItem::Terminal => Box::new(SubView::new(
                 self.debug_terminal.focus_handle(cx),
                 self.debug_terminal.clone().into(),
                 item_kind,
+                running_state,
+                host_pane,
                 cx,
             )),
             DebuggerPaneItem::MemoryView => Box::new(SubView::new(
                 self.memory_view.focus_handle(cx),
                 self.memory_view.clone().into(),
                 item_kind,
+                running_state,
+                host_pane,
                 cx,
             )),
         }
@@ -1454,6 +1522,13 @@ impl RunningState {
     ) {
         this.serialize_layout(window, cx);
         match event {
+            Event::AddItem { item } => {
+                if let Some(sub_view) = item.downcast::<SubView>() {
+                    sub_view.update(cx, |sub_view, _| {
+                        sub_view.set_host_pane(source_pane.downgrade());
+                    });
+                }
+            }
             Event::Remove { .. } => {
                 let _did_find_pane = this.panes.remove(source_pane, cx).is_ok();
                 debug_assert!(_did_find_pane);
@@ -1795,23 +1870,28 @@ impl RunningState {
         window: &mut Window,
         cx: &mut Context<'_, RunningState>,
     ) -> Member {
+        let running_state = cx.weak_entity();
+
         let leftmost_pane = new_debugger_pane(workspace.clone(), project.clone(), window, cx);
+        let leftmost_pane_handle = leftmost_pane.downgrade();
+        let leftmost_frames = SubView::new(
+            stack_frame_list.focus_handle(cx),
+            stack_frame_list.clone().into(),
+            DebuggerPaneItem::Frames,
+            running_state.clone(),
+            leftmost_pane_handle.clone(),
+            cx,
+        );
+        let leftmost_breakpoints = SubView::breakpoint_list(
+            breakpoints.clone(),
+            running_state.clone(),
+            leftmost_pane_handle,
+            cx,
+        );
         leftmost_pane.update(cx, |this, cx| {
+            this.add_item(Box::new(leftmost_frames), true, false, None, window, cx);
             this.add_item(
-                Box::new(SubView::new(
-                    this.focus_handle(cx),
-                    stack_frame_list.clone().into(),
-                    DebuggerPaneItem::Frames,
-                    cx,
-                )),
-                true,
-                false,
-                None,
-                window,
-                cx,
-            );
-            this.add_item(
-                Box::new(SubView::breakpoint_list(breakpoints.clone(), cx)),
+                Box::new(leftmost_breakpoints),
                 true,
                 false,
                 None,
@@ -1820,44 +1900,42 @@ impl RunningState {
             );
             this.activate_item(0, false, false, window, cx);
         });
+
         let center_pane = new_debugger_pane(workspace.clone(), project.clone(), window, cx);
+        let center_pane_handle = center_pane.downgrade();
+        let center_console = SubView::console(
+            console.clone(),
+            running_state.clone(),
+            center_pane_handle.clone(),
+            cx,
+        );
+        let center_variables = SubView::new(
+            variable_list.focus_handle(cx),
+            variable_list.clone().into(),
+            DebuggerPaneItem::Variables,
+            running_state.clone(),
+            center_pane_handle,
+            cx,
+        );
 
         center_pane.update(cx, |this, cx| {
-            let view = SubView::console(console.clone(), cx);
+            this.add_item(Box::new(center_console), true, false, None, window, cx);
 
-            this.add_item(Box::new(view), true, false, None, window, cx);
-
-            this.add_item(
-                Box::new(SubView::new(
-                    variable_list.focus_handle(cx),
-                    variable_list.clone().into(),
-                    DebuggerPaneItem::Variables,
-                    cx,
-                )),
-                true,
-                false,
-                None,
-                window,
-                cx,
-            );
+            this.add_item(Box::new(center_variables), true, false, None, window, cx);
             this.activate_item(0, false, false, window, cx);
         });
 
         let rightmost_pane = new_debugger_pane(workspace.clone(), project, window, cx);
+        let rightmost_terminal = SubView::new(
+            debug_terminal.focus_handle(cx),
+            debug_terminal.clone().into(),
+            DebuggerPaneItem::Terminal,
+            running_state,
+            rightmost_pane.downgrade(),
+            cx,
+        );
         rightmost_pane.update(cx, |this, cx| {
-            this.add_item(
-                Box::new(SubView::new(
-                    debug_terminal.focus_handle(cx),
-                    debug_terminal.clone().into(),
-                    DebuggerPaneItem::Terminal,
-                    cx,
-                )),
-                false,
-                false,
-                None,
-                window,
-                cx,
-            );
+            this.add_item(Box::new(rightmost_terminal), false, false, None, window, cx);
         });
 
         subscriptions.extend(

crates/terminal/src/terminal.rs 🔗

@@ -415,6 +415,8 @@ impl TerminalBuilder {
             event_loop_task: Task::ready(Ok(())),
             background_executor: background_executor.clone(),
             path_style,
+            #[cfg(any(test, feature = "test-support"))]
+            input_log: Vec::new(),
         };
 
         Ok(TerminalBuilder {
@@ -646,6 +648,8 @@ impl TerminalBuilder {
                 event_loop_task: Task::ready(Ok(())),
                 background_executor,
                 path_style,
+                #[cfg(any(test, feature = "test-support"))]
+                input_log: Vec::new(),
             };
 
             if !activation_script.is_empty() && no_task {
@@ -870,6 +874,8 @@ pub struct Terminal {
     event_loop_task: Task<Result<(), anyhow::Error>>,
     background_executor: BackgroundExecutor,
     path_style: PathStyle,
+    #[cfg(any(test, feature = "test-support"))]
+    input_log: Vec<Vec<u8>>,
 }
 
 struct CopyTemplate {
@@ -1451,9 +1457,18 @@ impl Terminal {
             .push_back(InternalEvent::Scroll(AlacScroll::Bottom));
         self.events.push_back(InternalEvent::SetSelection(None));
 
+        let input = input.into();
+        #[cfg(any(test, feature = "test-support"))]
+        self.input_log.push(input.to_vec());
+
         self.write_to_pty(input);
     }
 
+    #[cfg(any(test, feature = "test-support"))]
+    pub fn take_input_log(&mut self) -> Vec<Vec<u8>> {
+        std::mem::take(&mut self.input_log)
+    }
+
     pub fn toggle_vi_mode(&mut self) {
         self.events.push_back(InternalEvent::ToggleViMode);
     }

crates/terminal_view/Cargo.toml 🔗

@@ -53,6 +53,7 @@ editor = { workspace = true, features = ["test-support"] }
 gpui = { workspace = true, features = ["test-support"] }
 project = { workspace = true, features = ["test-support"] }
 rand.workspace = true
+terminal = { workspace = true, features = ["test-support"] }
 workspace = { workspace = true, features = ["test-support"] }
 
 [package.metadata.cargo-machete]

crates/terminal_view/src/terminal_panel.rs 🔗

@@ -1,4 +1,4 @@
-use std::{cmp, ops::ControlFlow, path::PathBuf, process::ExitStatus, sync::Arc, time::Duration};
+use std::{cmp, path::PathBuf, process::ExitStatus, sync::Arc, time::Duration};
 
 use crate::{
     TerminalView, default_working_directory,
@@ -12,11 +12,11 @@ use db::kvp::KEY_VALUE_STORE;
 use futures::{channel::oneshot, future::join_all};
 use gpui::{
     Action, AnyView, App, AsyncApp, AsyncWindowContext, Context, Corner, Entity, EventEmitter,
-    ExternalPaths, FocusHandle, Focusable, IntoElement, ParentElement, Pixels, Render, Styled,
-    Task, WeakEntity, Window, actions,
+    FocusHandle, Focusable, IntoElement, ParentElement, Pixels, Render, Styled, Task, WeakEntity,
+    Window, actions,
 };
 use itertools::Itertools;
-use project::{Fs, Project, ProjectEntryId};
+use project::{Fs, Project};
 
 use settings::{Settings, TerminalDockPosition};
 use task::{RevealStrategy, RevealTarget, Shell, ShellBuilder, SpawnInTerminal, TaskId};
@@ -28,13 +28,13 @@ use ui::{
 use util::{ResultExt, TryFutureExt};
 use workspace::{
     ActivateNextPane, ActivatePane, ActivatePaneDown, ActivatePaneLeft, ActivatePaneRight,
-    ActivatePaneUp, ActivatePreviousPane, DraggedSelection, DraggedTab, ItemId, MoveItemToPane,
+    ActivatePaneUp, ActivatePreviousPane, DraggedTab, ItemId, MoveItemToPane,
     MoveItemToPaneInDirection, MovePaneDown, MovePaneLeft, MovePaneRight, MovePaneUp, Pane,
     PaneGroup, SplitDirection, SplitDown, SplitLeft, SplitMode, SplitRight, SplitUp, SwapPaneDown,
     SwapPaneLeft, SwapPaneRight, SwapPaneUp, ToggleZoom, Workspace,
     dock::{DockPosition, Panel, PanelEvent, PanelHandle},
     item::SerializableItem,
-    move_active_item, move_item, pane,
+    move_active_item, pane,
 };
 
 use anyhow::{Result, anyhow};
@@ -133,7 +133,11 @@ impl TerminalPanel {
         }
     }
 
-    fn apply_tab_bar_buttons(&self, terminal_pane: &Entity<Pane>, cx: &mut Context<Self>) {
+    pub(crate) fn apply_tab_bar_buttons(
+        &self,
+        terminal_pane: &Entity<Pane>,
+        cx: &mut Context<Self>,
+    ) {
         let assistant_tab_bar_button = self.assistant_tab_bar_button.clone();
         terminal_pane.update(cx, |pane, cx| {
             pane.set_render_tab_bar_buttons(cx, move |pane, window, cx| {
@@ -1187,7 +1191,6 @@ pub fn new_terminal_pane(
     window: &mut Window,
     cx: &mut Context<TerminalPanel>,
 ) -> Entity<Pane> {
-    let is_local = project.read(cx).is_local();
     let terminal_panel = cx.entity();
     let pane = cx.new(|cx| {
         let mut pane = Pane::new(
@@ -1245,113 +1248,6 @@ pub fn new_terminal_pane(
             toolbar.add_item(breadcrumbs, window, cx);
         });
 
-        let drop_closure_project = project.downgrade();
-        let drop_closure_terminal_panel = terminal_panel.downgrade();
-        pane.set_custom_drop_handle(cx, move |pane, dropped_item, window, cx| {
-            let Some(project) = drop_closure_project.upgrade() else {
-                return ControlFlow::Break(());
-            };
-            if let Some(tab) = dropped_item.downcast_ref::<DraggedTab>() {
-                let this_pane = cx.entity();
-                let item = if tab.pane == this_pane {
-                    pane.item_for_index(tab.ix)
-                } else {
-                    tab.pane.read(cx).item_for_index(tab.ix)
-                };
-                if let Some(item) = item {
-                    if item.downcast::<TerminalView>().is_some() {
-                        let source = tab.pane.clone();
-                        let item_id_to_move = item.item_id();
-
-                        // If no split direction, let the regular pane drop handler take care of it
-                        let Some(split_direction) = pane.drag_split_direction() else {
-                            return ControlFlow::Continue(());
-                        };
-
-                        // Gather data synchronously before deferring
-                        let is_zoomed = drop_closure_terminal_panel
-                            .upgrade()
-                            .map(|terminal_panel| {
-                                let terminal_panel = terminal_panel.read(cx);
-                                if terminal_panel.active_pane == this_pane {
-                                    pane.is_zoomed()
-                                } else {
-                                    terminal_panel.active_pane.read(cx).is_zoomed()
-                                }
-                            })
-                            .unwrap_or(false);
-
-                        let workspace = workspace.clone();
-                        let terminal_panel = drop_closure_terminal_panel.clone();
-
-                        // Defer the split operation to avoid re-entrancy panic.
-                        // The pane may be the one currently being updated, so we cannot
-                        // call mark_positions (via split) synchronously.
-                        cx.spawn_in(window, async move |_, cx| {
-                            cx.update(|window, cx| {
-                                let Ok(new_pane) =
-                                    terminal_panel.update(cx, |terminal_panel, cx| {
-                                        let new_pane = new_terminal_pane(
-                                            workspace, project, is_zoomed, window, cx,
-                                        );
-                                        terminal_panel.apply_tab_bar_buttons(&new_pane, cx);
-                                        terminal_panel.center.split(
-                                            &this_pane,
-                                            &new_pane,
-                                            split_direction,
-                                            cx,
-                                        );
-                                        new_pane
-                                    })
-                                else {
-                                    return;
-                                };
-
-                                move_item(
-                                    &source,
-                                    &new_pane,
-                                    item_id_to_move,
-                                    new_pane.read(cx).active_item_index(),
-                                    true,
-                                    window,
-                                    cx,
-                                );
-                            })
-                            .ok();
-                        })
-                        .detach();
-                    } else if let Some(project_path) = item.project_path(cx)
-                        && let Some(entry_path) = project.read(cx).absolute_path(&project_path, cx)
-                    {
-                        add_paths_to_terminal(pane, &[entry_path], window, cx);
-                    }
-                }
-            } else if let Some(selection) = dropped_item.downcast_ref::<DraggedSelection>() {
-                let project = project.read(cx);
-                let paths_to_add = selection
-                    .items()
-                    .map(|selected_entry| selected_entry.entry_id)
-                    .filter_map(|entry_id| project.path_for_entry(entry_id, cx))
-                    .filter_map(|project_path| project.absolute_path(&project_path, cx))
-                    .collect::<Vec<_>>();
-                if !paths_to_add.is_empty() {
-                    add_paths_to_terminal(pane, &paths_to_add, window, cx);
-                }
-            } else if let Some(&entry_id) = dropped_item.downcast_ref::<ProjectEntryId>() {
-                if let Some(entry_path) = project
-                    .read(cx)
-                    .path_for_entry(entry_id, cx)
-                    .and_then(|project_path| project.read(cx).absolute_path(&project_path, cx))
-                {
-                    add_paths_to_terminal(pane, &[entry_path], window, cx);
-                }
-            } else if is_local && let Some(paths) = dropped_item.downcast_ref::<ExternalPaths>() {
-                add_paths_to_terminal(pane, paths.paths(), window, cx);
-            }
-
-            ControlFlow::Break(())
-        });
-
         pane
     });
 
@@ -1376,27 +1272,6 @@ async fn wait_for_terminals_tasks(
     join_all(pending_tasks).await;
 }
 
-fn add_paths_to_terminal(
-    pane: &mut Pane,
-    paths: &[PathBuf],
-    window: &mut Window,
-    cx: &mut Context<Pane>,
-) {
-    if let Some(terminal_view) = pane
-        .active_item()
-        .and_then(|item| item.downcast::<TerminalView>())
-    {
-        window.focus(&terminal_view.focus_handle(cx), cx);
-        let mut new_text = paths.iter().map(|path| format!(" {path:?}")).join("");
-        new_text.push(' ');
-        terminal_view.update(cx, |terminal_view, cx| {
-            terminal_view.terminal().update(cx, |terminal, _| {
-                terminal.paste(&new_text);
-            });
-        });
-    }
-}
-
 struct FailedToSpawnTerminal {
     error: String,
     focus_handle: FocusHandle,

crates/terminal_view/src/terminal_view.rs 🔗

@@ -8,18 +8,20 @@ mod terminal_slash_command;
 use assistant_slash_command::SlashCommandRegistry;
 use editor::{Editor, EditorSettings, actions::SelectAll, blink_manager::BlinkManager};
 use gpui::{
-    Action, AnyElement, App, ClipboardEntry, DismissEvent, Entity, EventEmitter, FocusHandle,
-    Focusable, KeyContext, KeyDownEvent, Keystroke, MouseButton, MouseDownEvent, Pixels, Point,
-    Render, ScrollWheelEvent, Styled, Subscription, Task, WeakEntity, actions, anchored, deferred,
-    div,
+    Action, AnyElement, App, ClipboardEntry, DismissEvent, Entity, EventEmitter, ExternalPaths,
+    FocusHandle, Focusable, KeyContext, KeyDownEvent, Keystroke, MouseButton, MouseDownEvent,
+    Pixels, Point, Render, ScrollWheelEvent, Styled, Subscription, Task, WeakEntity, actions,
+    anchored, deferred, div,
 };
+use itertools::Itertools;
 use menu;
 use persistence::TERMINAL_DB;
-use project::{Project, search::SearchQuery};
+use project::{Project, ProjectEntryId, search::SearchQuery};
 use schemars::JsonSchema;
 use serde::Deserialize;
 use settings::{Settings, SettingsStore, TerminalBlink, WorkingDirectory};
 use std::{
+    any::Any,
     cmp,
     ops::{Range, RangeInclusive},
     path::{Path, PathBuf},
@@ -50,8 +52,8 @@ use ui::{
 };
 use util::ResultExt;
 use workspace::{
-    CloseActiveItem, NewCenterTerminal, NewTerminal, ToolbarItemLocation, Workspace, WorkspaceId,
-    delete_unloaded_items,
+    CloseActiveItem, DraggedSelection, DraggedTab, NewCenterTerminal, NewTerminal, Pane,
+    ToolbarItemLocation, Workspace, WorkspaceId, delete_unloaded_items,
     item::{
         BreadcrumbText, Item, ItemEvent, SerializableItem, TabContentParams, TabTooltipContent,
     },
@@ -833,6 +835,15 @@ impl TerminalView {
         });
     }
 
+    fn add_paths_to_terminal(&self, paths: &[PathBuf], window: &mut Window, cx: &mut App) {
+        let mut text = paths.iter().map(|path| format!(" {path:?}")).join("");
+        text.push(' ');
+        window.focus(&self.focus_handle(cx), cx);
+        self.terminal.update(cx, |terminal, _| {
+            terminal.paste(&text);
+        });
+    }
+
     fn send_text(&mut self, text: &SendText, _: &mut Window, cx: &mut Context<Self>) {
         self.clear_bell(cx);
         self.terminal.update(cx, |term, _| {
@@ -1412,6 +1423,154 @@ impl Item for TerminalView {
         None
     }
 
+    fn handle_drop(
+        &self,
+        active_pane: &Pane,
+        dropped: &dyn Any,
+        window: &mut Window,
+        cx: &mut App,
+    ) -> bool {
+        let Some(project) = self.project.upgrade() else {
+            return false;
+        };
+
+        if let Some(paths) = dropped.downcast_ref::<ExternalPaths>() {
+            let is_local = project.read(cx).is_local();
+            if is_local {
+                self.add_paths_to_terminal(paths.paths(), window, cx);
+                return true;
+            }
+
+            return false;
+        } else if let Some(tab) = dropped.downcast_ref::<DraggedTab>() {
+            let Some(self_handle) = self.self_handle.upgrade() else {
+                return false;
+            };
+
+            let Some(workspace) = self.workspace.upgrade() else {
+                return false;
+            };
+
+            let Some(this_pane) = workspace.read(cx).pane_for(&self_handle) else {
+                return false;
+            };
+
+            let item = if tab.pane == this_pane {
+                active_pane.item_for_index(tab.ix)
+            } else {
+                tab.pane.read(cx).item_for_index(tab.ix)
+            };
+
+            let Some(item) = item else {
+                return false;
+            };
+
+            if item.downcast::<TerminalView>().is_some() {
+                let Some(split_direction) = active_pane.drag_split_direction() else {
+                    return false;
+                };
+
+                let Some(terminal_panel) = workspace.read(cx).panel::<TerminalPanel>(cx) else {
+                    return false;
+                };
+
+                if !terminal_panel.read(cx).center.panes().contains(&&this_pane) {
+                    return false;
+                }
+
+                let source = tab.pane.clone();
+                let item_id_to_move = item.item_id();
+                let is_zoomed = {
+                    let terminal_panel = terminal_panel.read(cx);
+                    if terminal_panel.active_pane == this_pane {
+                        active_pane.is_zoomed()
+                    } else {
+                        terminal_panel.active_pane.read(cx).is_zoomed()
+                    }
+                };
+
+                let workspace = workspace.downgrade();
+                let terminal_panel = terminal_panel.downgrade();
+                // Defer the split operation to avoid re-entrancy panic.
+                // The pane may be the one currently being updated, so we cannot
+                // call mark_positions (via split) synchronously.
+                window
+                    .spawn(cx, async move |cx| {
+                        cx.update(|window, cx| {
+                            let Ok(new_pane) = terminal_panel.update(cx, |terminal_panel, cx| {
+                                let new_pane = terminal_panel::new_terminal_pane(
+                                    workspace, project, is_zoomed, window, cx,
+                                );
+                                terminal_panel.apply_tab_bar_buttons(&new_pane, cx);
+                                terminal_panel.center.split(
+                                    &this_pane,
+                                    &new_pane,
+                                    split_direction,
+                                    cx,
+                                );
+                                anyhow::Ok(new_pane)
+                            }) else {
+                                return;
+                            };
+
+                            let Some(new_pane) = new_pane.log_err() else {
+                                return;
+                            };
+
+                            workspace::move_item(
+                                &source,
+                                &new_pane,
+                                item_id_to_move,
+                                new_pane.read(cx).active_item_index(),
+                                true,
+                                window,
+                                cx,
+                            );
+                        })
+                        .ok();
+                    })
+                    .detach();
+
+                return true;
+            } else {
+                if let Some(project_path) = item.project_path(cx)
+                    && let Some(path) = project.read(cx).absolute_path(&project_path, cx)
+                {
+                    self.add_paths_to_terminal(&[path], window, cx);
+                    return true;
+                }
+            }
+
+            return false;
+        } else if let Some(selection) = dropped.downcast_ref::<DraggedSelection>() {
+            let project = project.read(cx);
+            let paths = selection
+                .items()
+                .map(|selected_entry| selected_entry.entry_id)
+                .filter_map(|entry_id| project.path_for_entry(entry_id, cx))
+                .filter_map(|project_path| project.absolute_path(&project_path, cx))
+                .collect::<Vec<_>>();
+
+            if !paths.is_empty() {
+                self.add_paths_to_terminal(&paths, window, cx);
+            }
+
+            return true;
+        } else if let Some(&entry_id) = dropped.downcast_ref::<ProjectEntryId>() {
+            let project = project.read(cx);
+            if let Some(path) = project
+                .path_for_entry(entry_id, cx)
+                .and_then(|project_path| project.absolute_path(&project_path, cx))
+            {
+                self.add_paths_to_terminal(&[path], window, cx);
+            }
+
+            return true;
+        }
+
+        false
+    }
+
     fn tab_extra_context_menu_actions(
         &self,
         _window: &mut Window,
@@ -1840,10 +1999,46 @@ mod tests {
     use super::*;
     use gpui::TestAppContext;
     use project::{Entry, Project, ProjectPath, Worktree};
-    use std::path::Path;
+    use std::path::{Path, PathBuf};
     use util::paths::PathStyle;
     use util::rel_path::RelPath;
-    use workspace::{AppState, MultiWorkspace};
+    use workspace::item::test::{TestItem, TestProjectItem};
+    use workspace::{AppState, MultiWorkspace, SelectedEntry};
+
+    fn expected_drop_text(paths: &[PathBuf]) -> String {
+        let mut text = String::new();
+        for path in paths {
+            text.push(' ');
+            text.push_str(&format!("{path:?}"));
+        }
+        text.push(' ');
+        text
+    }
+
+    fn assert_drop_writes_to_terminal(
+        pane: &Entity<Pane>,
+        terminal_view_index: usize,
+        terminal: &Entity<Terminal>,
+        dropped: &dyn Any,
+        expected_text: &str,
+        window: &mut Window,
+        cx: &mut Context<MultiWorkspace>,
+    ) {
+        let _ = terminal.update(cx, |terminal, _| terminal.take_input_log());
+
+        let handled = pane.update(cx, |pane, cx| {
+            pane.item_for_index(terminal_view_index)
+                .unwrap()
+                .handle_drop(pane, dropped, window, cx)
+        });
+        assert!(handled, "handle_drop should return true for {:?}", dropped);
+
+        let mut input_log = terminal.update(cx, |terminal, _| terminal.take_input_log());
+        assert_eq!(input_log.len(), 1, "expected exactly one write to terminal");
+        let written =
+            String::from_utf8(input_log.remove(0)).expect("terminal write should be valid UTF-8");
+        assert_eq!(written, expected_text);
+    }
 
     // Working directory calculation tests
 
@@ -1972,24 +2167,7 @@ mod tests {
         let (project, _workspace) = init_test(cx).await;
 
         let (wt, _entry) = create_folder_wt(project.clone(), "/root/", cx).await;
-        let entry = cx
-            .update(|cx| {
-                wt.update(cx, |wt, cx| {
-                    wt.create_entry(
-                        RelPath::new(Path::new("src/main.rs"), PathStyle::local())
-                            .unwrap()
-                            .as_ref()
-                            .into(),
-                        false,
-                        None,
-                        cx,
-                    )
-                })
-            })
-            .await
-            .unwrap()
-            .into_included()
-            .unwrap();
+        let entry = create_file_in_worktree(wt.clone(), "src/main.rs", cx).await;
         insert_active_entry_for(wt, entry, project.clone(), cx);
 
         cx.update(|cx| {
@@ -2014,6 +2192,18 @@ mod tests {
 
     /// Creates a worktree with 1 file: /root.txt
     pub async fn init_test(cx: &mut TestAppContext) -> (Entity<Project>, Entity<Workspace>) {
+        let (project, workspace, _) = init_test_with_window(cx).await;
+        (project, workspace)
+    }
+
+    /// Creates a worktree with 1 file /root.txt and returns the project, workspace, and window handle.
+    async fn init_test_with_window(
+        cx: &mut TestAppContext,
+    ) -> (
+        Entity<Project>,
+        Entity<Workspace>,
+        gpui::WindowHandle<MultiWorkspace>,
+    ) {
         let params = cx.update(AppState::test);
         cx.update(|cx| {
             theme::init(theme::LoadThemes::JustBase, cx);
@@ -2026,7 +2216,32 @@ mod tests {
             .read_with(cx, |mw, _| mw.workspace().clone())
             .unwrap();
 
-        (project, workspace)
+        (project, workspace, window_handle)
+    }
+
+    /// Creates a file in the given worktree and returns its entry.
+    async fn create_file_in_worktree(
+        worktree: Entity<Worktree>,
+        relative_path: impl AsRef<Path>,
+        cx: &mut TestAppContext,
+    ) -> Entry {
+        cx.update(|cx| {
+            worktree.update(cx, |worktree, cx| {
+                worktree.create_entry(
+                    RelPath::new(relative_path.as_ref(), PathStyle::local())
+                        .unwrap()
+                        .as_ref()
+                        .into(),
+                    false,
+                    None,
+                    cx,
+                )
+            })
+        })
+        .await
+        .unwrap()
+        .into_included()
+        .unwrap()
     }
 
     /// Creates a worktree with 1 folder: /root{suffix}/
@@ -2089,6 +2304,183 @@ mod tests {
         });
     }
 
+    // Terminal drag/drop test
+
+    #[gpui::test]
+    async fn test_handle_drop_writes_paths_for_all_drop_types(cx: &mut TestAppContext) {
+        let (project, _workspace, window_handle) = init_test_with_window(cx).await;
+
+        let (worktree, _) = create_folder_wt(project.clone(), "/root/", cx).await;
+        let first_entry = create_file_in_worktree(worktree.clone(), "first.txt", cx).await;
+        let second_entry = create_file_in_worktree(worktree.clone(), "second.txt", cx).await;
+
+        let worktree_id = worktree.read_with(cx, |worktree, _| worktree.id());
+        let first_path = project
+            .read_with(cx, |project, cx| {
+                project.absolute_path(
+                    &ProjectPath {
+                        worktree_id,
+                        path: first_entry.path.clone(),
+                    },
+                    cx,
+                )
+            })
+            .unwrap();
+        let second_path = project
+            .read_with(cx, |project, cx| {
+                project.absolute_path(
+                    &ProjectPath {
+                        worktree_id,
+                        path: second_entry.path.clone(),
+                    },
+                    cx,
+                )
+            })
+            .unwrap();
+
+        let (active_pane, terminal, terminal_view, tab_item) = window_handle
+            .update(cx, |multi_workspace, window, cx| {
+                let workspace = multi_workspace.workspace().clone();
+                let active_pane = workspace.read(cx).active_pane().clone();
+
+                let terminal = cx.new(|cx| {
+                    terminal::TerminalBuilder::new_display_only(
+                        CursorShape::default(),
+                        terminal::terminal_settings::AlternateScroll::On,
+                        None,
+                        0,
+                        cx.background_executor(),
+                        PathStyle::local(),
+                    )
+                    .unwrap()
+                    .subscribe(cx)
+                });
+                let terminal_view = cx.new(|cx| {
+                    TerminalView::new(
+                        terminal.clone(),
+                        workspace.downgrade(),
+                        None,
+                        project.downgrade(),
+                        window,
+                        cx,
+                    )
+                });
+
+                active_pane.update(cx, |pane, cx| {
+                    pane.add_item(
+                        Box::new(terminal_view.clone()),
+                        true,
+                        false,
+                        None,
+                        window,
+                        cx,
+                    );
+                });
+
+                let tab_project_item = cx.new(|_| TestProjectItem {
+                    entry_id: Some(second_entry.id),
+                    project_path: Some(ProjectPath {
+                        worktree_id,
+                        path: second_entry.path.clone(),
+                    }),
+                    is_dirty: false,
+                });
+                let tab_item =
+                    cx.new(|cx| TestItem::new(cx).with_project_items(&[tab_project_item]));
+                active_pane.update(cx, |pane, cx| {
+                    pane.add_item(Box::new(tab_item.clone()), true, false, None, window, cx);
+                });
+
+                (active_pane, terminal, terminal_view, tab_item)
+            })
+            .unwrap();
+
+        cx.run_until_parked();
+
+        window_handle
+            .update(cx, |multi_workspace, window, cx| {
+                let workspace = multi_workspace.workspace().clone();
+                let terminal_view_index =
+                    active_pane.read(cx).index_for_item(&terminal_view).unwrap();
+                let dragged_tab_index = active_pane.read(cx).index_for_item(&tab_item).unwrap();
+
+                assert!(
+                    workspace.read(cx).pane_for(&terminal_view).is_some(),
+                    "terminal view not registered with workspace after run_until_parked"
+                );
+
+                // Dragging an external file should write its path to the terminal
+                let external_paths = ExternalPaths(vec![first_path.clone()].into());
+                assert_drop_writes_to_terminal(
+                    &active_pane,
+                    terminal_view_index,
+                    &terminal,
+                    &external_paths,
+                    &expected_drop_text(std::slice::from_ref(&first_path)),
+                    window,
+                    cx,
+                );
+
+                // Dragging a tab should write the path of the tab's item to the terminal
+                let dragged_tab = DraggedTab {
+                    pane: active_pane.clone(),
+                    item: Box::new(tab_item.clone()),
+                    ix: dragged_tab_index,
+                    detail: 0,
+                    is_active: false,
+                };
+                assert_drop_writes_to_terminal(
+                    &active_pane,
+                    terminal_view_index,
+                    &terminal,
+                    &dragged_tab,
+                    &expected_drop_text(std::slice::from_ref(&second_path)),
+                    window,
+                    cx,
+                );
+
+                // Dragging multiple selections should write both paths to the terminal
+                let dragged_selection = DraggedSelection {
+                    active_selection: SelectedEntry {
+                        worktree_id,
+                        entry_id: first_entry.id,
+                    },
+                    marked_selections: Arc::from([
+                        SelectedEntry {
+                            worktree_id,
+                            entry_id: first_entry.id,
+                        },
+                        SelectedEntry {
+                            worktree_id,
+                            entry_id: second_entry.id,
+                        },
+                    ]),
+                };
+                assert_drop_writes_to_terminal(
+                    &active_pane,
+                    terminal_view_index,
+                    &terminal,
+                    &dragged_selection,
+                    &expected_drop_text(&[first_path.clone(), second_path.clone()]),
+                    window,
+                    cx,
+                );
+
+                // Dropping a project entry should write the entry's path to the terminal
+                let dropped_entry_id = first_entry.id;
+                assert_drop_writes_to_terminal(
+                    &active_pane,
+                    terminal_view_index,
+                    &terminal,
+                    &dropped_entry_id,
+                    &expected_drop_text(&[first_path]),
+                    window,
+                    cx,
+                );
+            })
+            .unwrap();
+    }
+
     // Terminal rename tests
 
     #[gpui::test]

crates/workspace/src/item.rs 🔗

@@ -366,6 +366,18 @@ pub trait Item: Focusable + EventEmitter<Self::Event> + Render + Sized {
         true
     }
 
+    /// Called when the containing pane receives a drop on the item or the item's tab.
+    /// Returns `true` to consume it and suppress the pane's default drop behavior.
+    fn handle_drop(
+        &self,
+        _active_pane: &Pane,
+        _dropped: &dyn Any,
+        _window: &mut Window,
+        _cx: &mut App,
+    ) -> bool {
+        false
+    }
+
     /// Returns additional actions to add to the tab's context menu.
     /// Each entry is a label and an action to dispatch.
     fn tab_extra_context_menu_actions(
@@ -545,6 +557,13 @@ pub trait ItemHandle: 'static + Send {
     fn preserve_preview(&self, cx: &App) -> bool;
     fn include_in_nav_history(&self) -> bool;
     fn relay_action(&self, action: Box<dyn Action>, window: &mut Window, cx: &mut App);
+    fn handle_drop(
+        &self,
+        active_pane: &Pane,
+        dropped: &dyn Any,
+        window: &mut Window,
+        cx: &mut App,
+    ) -> bool;
     fn tab_extra_context_menu_actions(
         &self,
         window: &mut Window,
@@ -1110,6 +1129,20 @@ impl<T: Item> ItemHandle for Entity<T> {
         })
     }
 
+    /// Called when the containing pane receives a drop on the item or the item's tab.
+    /// Returns `true` if the item handled it and the pane should skip its default drop behavior.
+    fn handle_drop(
+        &self,
+        active_pane: &Pane,
+        dropped: &dyn Any,
+        window: &mut Window,
+        cx: &mut App,
+    ) -> bool {
+        self.update(cx, |this, cx| {
+            this.handle_drop(active_pane, dropped, window, cx)
+        })
+    }
+
     fn tab_extra_context_menu_actions(
         &self,
         window: &mut Window,

crates/workspace/src/pane.rs 🔗

@@ -34,7 +34,6 @@ use std::{
     any::Any,
     cmp, fmt, mem,
     num::NonZeroUsize,
-    ops::ControlFlow,
     path::PathBuf,
     rc::Rc,
     sync::{
@@ -382,9 +381,6 @@ pub struct Pane {
     project: WeakEntity<Project>,
     pub drag_split_direction: Option<SplitDirection>,
     can_drop_predicate: Option<Arc<dyn Fn(&dyn Any, &mut Window, &mut App) -> bool>>,
-    custom_drop_handle: Option<
-        Arc<dyn Fn(&mut Pane, &dyn Any, &mut Window, &mut Context<Pane>) -> ControlFlow<(), ()>>,
-    >,
     can_split_predicate:
         Option<Arc<dyn Fn(&mut Self, &dyn Any, &mut Window, &mut Context<Self>) -> bool>>,
     can_toggle_zoom: bool,
@@ -567,7 +563,6 @@ impl Pane {
             workspace,
             project: project.downgrade(),
             can_drop_predicate,
-            custom_drop_handle: None,
             can_split_predicate: None,
             can_toggle_zoom: true,
             should_display_tab_bar: Rc::new(|_, cx| TabBarSettings::get_global(cx).show),
@@ -846,15 +841,6 @@ impl Pane {
         cx.notify();
     }
 
-    pub fn set_custom_drop_handle<F>(&mut self, cx: &mut Context<Self>, handle: F)
-    where
-        F: 'static
-            + Fn(&mut Pane, &dyn Any, &mut Window, &mut Context<Pane>) -> ControlFlow<(), ()>,
-    {
-        self.custom_drop_handle = Some(Arc::new(handle));
-        cx.notify();
-    }
-
     pub fn nav_history_for_item<T: Item>(&self, item: &Entity<T>) -> ItemNavHistory {
         ItemNavHistory {
             history: self.nav_history.clone(),
@@ -2901,7 +2887,7 @@ impl Pane {
             .on_drop(
                 cx.listener(move |this, dragged_tab: &DraggedTab, window, cx| {
                     this.drag_split_direction = None;
-                    this.handle_tab_drop(dragged_tab, ix, window, cx)
+                    this.handle_tab_drop(dragged_tab, ix, false, window, cx)
                 }),
             )
             .on_drop(
@@ -3550,7 +3536,7 @@ impl Pane {
             .on_drop(
                 cx.listener(move |this, dragged_tab: &DraggedTab, window, cx| {
                     this.drag_split_direction = None;
-                    this.handle_tab_drop(dragged_tab, this.items.len(), window, cx)
+                    this.handle_tab_drop(dragged_tab, this.items.len(), false, window, cx)
                 }),
             )
             .on_drop(
@@ -3691,14 +3677,18 @@ impl Pane {
         &mut self,
         dragged_tab: &DraggedTab,
         ix: usize,
+        is_pane_target: bool,
         window: &mut Window,
         cx: &mut Context<Self>,
     ) {
-        if let Some(custom_drop_handle) = self.custom_drop_handle.clone()
-            && let ControlFlow::Break(()) = custom_drop_handle(self, dragged_tab, window, cx)
+        if is_pane_target
+            && ix == self.active_item_index
+            && let Some(active_item) = self.active_item()
+            && active_item.handle_drop(self, dragged_tab, window, cx)
         {
             return;
         }
+
         let mut to_pane = cx.entity();
         let split_direction = self.drag_split_direction;
         let item_id = dragged_tab.item.item_id();
@@ -3791,7 +3781,7 @@ impl Pane {
         let item_id = dragged_tab.item.item_id();
         let pinned_count = self.pinned_tab_count;
 
-        self.handle_tab_drop(dragged_tab, pinned_count, window, cx);
+        self.handle_tab_drop(dragged_tab, pinned_count, false, window, cx);
 
         let to_pane = cx.entity();
 
@@ -3843,11 +3833,12 @@ impl Pane {
         window: &mut Window,
         cx: &mut Context<Self>,
     ) {
-        if let Some(custom_drop_handle) = self.custom_drop_handle.clone()
-            && let ControlFlow::Break(()) = custom_drop_handle(self, dragged_selection, window, cx)
+        if let Some(active_item) = self.active_item()
+            && active_item.handle_drop(self, dragged_selection, window, cx)
         {
             return;
         }
+
         self.handle_project_entry_drop(
             &dragged_selection.active_selection.entry_id,
             dragged_onto,
@@ -3863,11 +3854,12 @@ impl Pane {
         window: &mut Window,
         cx: &mut Context<Self>,
     ) {
-        if let Some(custom_drop_handle) = self.custom_drop_handle.clone()
-            && let ControlFlow::Break(()) = custom_drop_handle(self, project_entry_id, window, cx)
+        if let Some(active_item) = self.active_item()
+            && active_item.handle_drop(self, project_entry_id, window, cx)
         {
             return;
         }
+
         let mut to_pane = cx.entity();
         let split_direction = self.drag_split_direction;
         let project_entry_id = *project_entry_id;
@@ -3939,11 +3931,12 @@ impl Pane {
         window: &mut Window,
         cx: &mut Context<Self>,
     ) {
-        if let Some(custom_drop_handle) = self.custom_drop_handle.clone()
-            && let ControlFlow::Break(()) = custom_drop_handle(self, paths, window, cx)
+        if let Some(active_item) = self.active_item()
+            && active_item.handle_drop(self, paths, window, cx)
         {
             return;
         }
+
         let mut to_pane = cx.entity();
         let mut split_direction = self.drag_split_direction;
         let paths = paths.paths().to_vec();
@@ -4424,6 +4417,7 @@ impl Render for Pane {
                                 this.handle_tab_drop(
                                     dragged_tab,
                                     this.active_item_index(),
+                                    true,
                                     window,
                                     cx,
                                 )
@@ -4826,7 +4820,7 @@ impl Render for DraggedTab {
 
 #[cfg(test)]
 mod tests {
-    use std::{iter::zip, num::NonZero};
+    use std::{cell::Cell, iter::zip, num::NonZero};
 
     use super::*;
     use crate::{
@@ -4839,6 +4833,65 @@ mod tests {
     use theme::LoadThemes;
     use util::TryFutureExt;
 
+    // drop_call_count is a Cell here because `handle_drop` takes &self, not &mut self.
+    struct CustomDropHandlingItem {
+        focus_handle: gpui::FocusHandle,
+        drop_call_count: Cell<usize>,
+    }
+
+    impl CustomDropHandlingItem {
+        fn new(cx: &mut Context<Self>) -> Self {
+            Self {
+                focus_handle: cx.focus_handle(),
+                drop_call_count: Cell::new(0),
+            }
+        }
+
+        fn drop_call_count(&self) -> usize {
+            self.drop_call_count.get()
+        }
+    }
+
+    impl EventEmitter<()> for CustomDropHandlingItem {}
+
+    impl Focusable for CustomDropHandlingItem {
+        fn focus_handle(&self, _cx: &App) -> gpui::FocusHandle {
+            self.focus_handle.clone()
+        }
+    }
+
+    impl Render for CustomDropHandlingItem {
+        fn render(
+            &mut self,
+            _window: &mut Window,
+            _cx: &mut Context<Self>,
+        ) -> impl gpui::IntoElement {
+            gpui::Empty
+        }
+    }
+
+    impl Item for CustomDropHandlingItem {
+        type Event = ();
+
+        fn tab_content_text(&self, _detail: usize, _cx: &App) -> gpui::SharedString {
+            "custom_drop_handling_item".into()
+        }
+
+        fn handle_drop(
+            &self,
+            _active_pane: &Pane,
+            dropped: &dyn std::any::Any,
+            _window: &mut Window,
+            _cx: &mut App,
+        ) -> bool {
+            let is_dragged_tab = dropped.downcast_ref::<DraggedTab>().is_some();
+            if is_dragged_tab {
+                self.drop_call_count.set(self.drop_call_count.get() + 1);
+            }
+            is_dragged_tab
+        }
+    }
+
     #[gpui::test]
     async fn test_add_item_capped_to_max_tabs(cx: &mut TestAppContext) {
         init_test(cx);
@@ -5664,6 +5717,83 @@ mod tests {
         assert_item_labels(&pane, ["C", "A", "B*"], cx);
     }
 
+    #[gpui::test]
+    async fn test_handle_tab_drop_respects_is_pane_target(cx: &mut TestAppContext) {
+        init_test(cx);
+        let fs = FakeFs::new(cx.executor());
+        let project = Project::test(fs, None, cx).await;
+        let (workspace, cx) =
+            cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx));
+        let source_pane = workspace.read_with(cx, |workspace, _| workspace.active_pane().clone());
+
+        let item_a = add_labeled_item(&source_pane, "A", false, cx);
+        let item_b = add_labeled_item(&source_pane, "B", false, cx);
+
+        let target_pane = workspace.update_in(cx, |workspace, window, cx| {
+            workspace.split_pane(source_pane.clone(), SplitDirection::Right, window, cx)
+        });
+
+        let custom_item = target_pane.update_in(cx, |pane, window, cx| {
+            let custom_item = Box::new(cx.new(CustomDropHandlingItem::new));
+            pane.add_item(custom_item.clone(), true, true, None, window, cx);
+            custom_item
+        });
+
+        let moved_item_id = item_a.item_id();
+        let other_item_id = item_b.item_id();
+        let custom_item_id = custom_item.item_id();
+
+        let pane_item_ids = |pane: &Entity<Pane>, cx: &mut VisualTestContext| {
+            pane.read_with(cx, |pane, _| {
+                pane.items().map(|item| item.item_id()).collect::<Vec<_>>()
+            })
+        };
+
+        let source_before_item_ids = pane_item_ids(&source_pane, cx);
+        assert_eq!(source_before_item_ids, vec![moved_item_id, other_item_id]);
+
+        let target_before_item_ids = pane_item_ids(&target_pane, cx);
+        assert_eq!(target_before_item_ids, vec![custom_item_id]);
+
+        let dragged_tab = DraggedTab {
+            pane: source_pane.clone(),
+            item: item_a.boxed_clone(),
+            ix: 0,
+            detail: 0,
+            is_active: true,
+        };
+
+        // Dropping item_a onto the target pane itself means the
+        // custom item handles the drop and no tab move should occur
+        target_pane.update_in(cx, |pane, window, cx| {
+            pane.handle_tab_drop(&dragged_tab, pane.active_item_index(), true, window, cx);
+        });
+        cx.run_until_parked();
+
+        assert_eq!(
+            custom_item.read_with(cx, |item, _| item.drop_call_count()),
+            1
+        );
+        assert_eq!(pane_item_ids(&source_pane, cx), source_before_item_ids);
+        assert_eq!(pane_item_ids(&target_pane, cx), target_before_item_ids);
+
+        // Dropping item_a onto the tab target means the custom handler
+        // should be skipped and the pane's default tab drop behavior should run.
+        target_pane.update_in(cx, |pane, window, cx| {
+            pane.handle_tab_drop(&dragged_tab, pane.active_item_index(), false, window, cx);
+        });
+        cx.run_until_parked();
+
+        assert_eq!(
+            custom_item.read_with(cx, |item, _| item.drop_call_count()),
+            1
+        );
+        assert_eq!(pane_item_ids(&source_pane, cx), vec![other_item_id]);
+
+        let target_item_ids = pane_item_ids(&target_pane, cx);
+        assert_eq!(target_item_ids, vec![moved_item_id, custom_item_id]);
+    }
+
     #[gpui::test]
     async fn test_drag_unpinned_tab_to_split_creates_pane_with_unpinned_tab(
         cx: &mut TestAppContext,
@@ -5699,7 +5829,7 @@ mod tests {
                 detail: 0,
                 is_active: true,
             };
-            pane.handle_tab_drop(&dragged_tab, 0, window, cx);
+            pane.handle_tab_drop(&dragged_tab, 0, true, window, cx);
         });
 
         // A should be moved to new pane. B should remain pinned, A should not be pinned
@@ -5748,7 +5878,7 @@ mod tests {
                 detail: 0,
                 is_active: true,
             };
-            pane.handle_tab_drop(&dragged_tab, 0, window, cx);
+            pane.handle_tab_drop(&dragged_tab, 0, true, window, cx);
         });
 
         // A should be moved to new pane. Both A and B should still be pinned
@@ -5798,7 +5928,7 @@ mod tests {
                 detail: 0,
                 is_active: true,
             };
-            pane.handle_tab_drop(&dragged_tab, 0, window, cx);
+            pane.handle_tab_drop(&dragged_tab, 0, false, window, cx);
         });
 
         // A should stay pinned
@@ -5846,7 +5976,7 @@ mod tests {
                 detail: 0,
                 is_active: true,
             };
-            pane.handle_tab_drop(&dragged_tab, 1, window, cx);
+            pane.handle_tab_drop(&dragged_tab, 1, false, window, cx);
         });
 
         // A should become pinned
@@ -5890,7 +6020,7 @@ mod tests {
                 detail: 0,
                 is_active: true,
             };
-            pane.handle_tab_drop(&dragged_tab, 0, window, cx);
+            pane.handle_tab_drop(&dragged_tab, 0, false, window, cx);
         });
 
         // A should stay pinned
@@ -5952,7 +6082,7 @@ mod tests {
                 detail: 0,
                 is_active: true,
             };
-            pane.handle_tab_drop(&dragged_tab, 0, window, cx);
+            pane.handle_tab_drop(&dragged_tab, 0, false, window, cx);
         });
 
         // E (unpinned) should be closed, leaving 3 pinned items
@@ -5987,7 +6117,7 @@ mod tests {
                 detail: 0,
                 is_active: true,
             };
-            pane.handle_tab_drop(&dragged_tab, 1, window, cx);
+            pane.handle_tab_drop(&dragged_tab, 1, false, window, cx);
         });
 
         // A should still be pinned and active
@@ -6027,7 +6157,7 @@ mod tests {
                 detail: 0,
                 is_active: true,
             };
-            pane.handle_tab_drop(&dragged_tab, 2, window, cx);
+            pane.handle_tab_drop(&dragged_tab, 2, false, window, cx);
         });
 
         // A stays pinned
@@ -6064,7 +6194,7 @@ mod tests {
                 detail: 0,
                 is_active: true,
             };
-            pane.handle_tab_drop(&dragged_tab, 1, window, cx);
+            pane.handle_tab_drop(&dragged_tab, 1, false, window, cx);
         });
 
         // Neither are pinned
@@ -6101,7 +6231,7 @@ mod tests {
                 detail: 0,
                 is_active: true,
             };
-            pane.handle_tab_drop(&dragged_tab, 2, window, cx);
+            pane.handle_tab_drop(&dragged_tab, 2, false, window, cx);
         });
 
         // A becomes unpinned
@@ -6138,7 +6268,7 @@ mod tests {
                 detail: 0,
                 is_active: true,
             };
-            pane.handle_tab_drop(&dragged_tab, 0, window, cx);
+            pane.handle_tab_drop(&dragged_tab, 0, false, window, cx);
         });
 
         // A becomes unpinned
@@ -6174,7 +6304,7 @@ mod tests {
                 detail: 0,
                 is_active: true,
             };
-            pane.handle_tab_drop(&dragged_tab, 1, window, cx);
+            pane.handle_tab_drop(&dragged_tab, 1, false, window, cx);
         });
 
         // A stays pinned, B and C remain unpinned
@@ -6215,7 +6345,7 @@ mod tests {
                 detail: 0,
                 is_active: true,
             };
-            pane.handle_tab_drop(&dragged_tab, 0, window, cx);
+            pane.handle_tab_drop(&dragged_tab, 0, false, window, cx);
         });
 
         // A should become pinned since it was dropped in the pinned region
@@ -6257,7 +6387,7 @@ mod tests {
                 detail: 0,
                 is_active: true,
             };
-            pane.handle_tab_drop(&dragged_tab, 1, window, cx);
+            pane.handle_tab_drop(&dragged_tab, 1, true, window, cx);
         });
 
         // A should remain unpinned since it was dropped outside the pinned region
@@ -6304,7 +6434,7 @@ mod tests {
                 detail: 0,
                 is_active: true,
             };
-            pane.handle_tab_drop(&dragged_tab, 1, window, cx);
+            pane.handle_tab_drop(&dragged_tab, 1, false, window, cx);
         });
 
         // A should be after B and all are pinned
@@ -6319,7 +6449,7 @@ mod tests {
                 detail: 0,
                 is_active: true,
             };
-            pane.handle_tab_drop(&dragged_tab, 2, window, cx);
+            pane.handle_tab_drop(&dragged_tab, 2, false, window, cx);
         });
 
         // A should be after C and all are pinned
@@ -6334,7 +6464,7 @@ mod tests {
                 detail: 0,
                 is_active: true,
             };
-            pane.handle_tab_drop(&dragged_tab, 1, window, cx);
+            pane.handle_tab_drop(&dragged_tab, 1, false, window, cx);
         });
 
         // A should be before C and all are pinned
@@ -6349,7 +6479,7 @@ mod tests {
                 detail: 0,
                 is_active: true,
             };
-            pane.handle_tab_drop(&dragged_tab, 0, window, cx);
+            pane.handle_tab_drop(&dragged_tab, 0, false, window, cx);
         });
 
         // A should be before B and all are pinned
@@ -6381,7 +6511,7 @@ mod tests {
                 detail: 0,
                 is_active: true,
             };
-            pane.handle_tab_drop(&dragged_tab, 2, window, cx);
+            pane.handle_tab_drop(&dragged_tab, 2, false, window, cx);
         });
 
         // A should be at the end
@@ -6413,7 +6543,7 @@ mod tests {
                 detail: 0,
                 is_active: true,
             };
-            pane.handle_tab_drop(&dragged_tab, 0, window, cx);
+            pane.handle_tab_drop(&dragged_tab, 0, false, window, cx);
         });
 
         // C should be at the beginning