workspace: Handle non-cloneable items better (#41215)

Lukas Wirth created

When trying to split and clone a non clone-able workspace item we now
attempt split and move instead of doing nothing. Additionally we disable
the split menu buttons if we can't split the active item at all.

Release Notes:

- Improved handling of unsplittable panes

Change summary

crates/agent_ui/src/agent_diff.rs             |  4 
crates/collab_ui/src/channel_view.rs          |  4 
crates/diagnostics/src/buffer_diagnostics.rs  |  4 
crates/diagnostics/src/diagnostics.rs         |  4 
crates/editor/src/items.rs                    |  4 
crates/extension_host/src/wasm_host.rs        | 24 +++--
crates/git_ui/src/commit_view.rs              |  4 
crates/git_ui/src/project_diff.rs             |  4 
crates/gpui_tokio/src/gpui_tokio.rs           |  3 
crates/image_viewer/src/image_viewer.rs       |  4 
crates/language_tools/src/key_context_view.rs |  4 
crates/language_tools/src/lsp_log_view.rs     |  4 
crates/language_tools/src/syntax_tree_view.rs |  4 
crates/onboarding/src/onboarding.rs           |  4 
crates/repl/src/notebook/notebook_ui.rs       |  4 
crates/search/src/project_search.rs           |  4 
crates/terminal_view/src/terminal_view.rs     |  4 
crates/workspace/src/item.rs                  | 22 ++++
crates/workspace/src/pane.rs                  | 52 ++++++++----
crates/workspace/src/shared_screen.rs         |  4 
crates/workspace/src/theme_preview.rs         |  4 
crates/workspace/src/workspace.rs             | 88 ++++++++++++--------
crates/zed/src/zed/component_preview.rs       |  4 
23 files changed, 196 insertions(+), 65 deletions(-)

Detailed changes

crates/agent_ui/src/agent_diff.rs 🔗

@@ -576,6 +576,10 @@ impl Item for AgentDiffPane {
         });
     }
 
+    fn can_split(&self) -> bool {
+        true
+    }
+
     fn clone_on_split(
         &self,
         _workspace_id: Option<workspace::WorkspaceId>,

crates/diagnostics/src/diagnostics.rs 🔗

@@ -727,6 +727,10 @@ impl Item for ProjectDiagnosticsEditor {
         });
     }
 
+    fn can_split(&self) -> bool {
+        true
+    }
+
     fn clone_on_split(
         &self,
         _workspace_id: Option<workspace::WorkspaceId>,

crates/editor/src/items.rs 🔗

@@ -757,6 +757,10 @@ impl Item for Editor {
         self.buffer.read(cx).is_singleton()
     }
 
+    fn can_split(&self) -> bool {
+        true
+    }
+
     fn clone_on_split(
         &self,
         _workspace_id: Option<WorkspaceId>,

crates/extension_host/src/wasm_host.rs 🔗

@@ -69,6 +69,7 @@ pub struct WasmExtension {
     pub work_dir: Arc<Path>,
     #[allow(unused)]
     pub zed_api_version: SemanticVersion,
+    _task: Arc<Task<Result<(), gpui_tokio::JoinError>>>,
 }
 
 impl Drop for WasmExtension {
@@ -649,21 +650,26 @@ impl WasmHost {
 
             anyhow::Ok((
                 extension_task,
-                WasmExtension {
-                    manifest: manifest.clone(),
-                    work_dir: this.work_dir.join(manifest.id.as_ref()).into(),
-                    tx,
-                    zed_api_version,
-                },
+                manifest.clone(),
+                this.work_dir.join(manifest.id.as_ref()).into(),
+                tx,
+                zed_api_version,
             ))
         };
         cx.spawn(async move |cx| {
-            let (extension_task, extension) = load_extension_task.await?;
+            let (extension_task, manifest, work_dir, tx, zed_api_version) =
+                load_extension_task.await?;
             // we need to run run the task in an extension context as wasmtime_wasi may
             // call into tokio, accessing its runtime handle
-            gpui_tokio::Tokio::spawn(cx, extension_task)?.detach();
+            let task = Arc::new(gpui_tokio::Tokio::spawn(cx, extension_task)?);
 
-            Ok(extension)
+            Ok(WasmExtension {
+                manifest,
+                work_dir,
+                tx,
+                zed_api_version,
+                _task: task,
+            })
         })
     }
 

crates/git_ui/src/commit_view.rs 🔗

@@ -556,6 +556,10 @@ impl Item for CommitView {
         });
     }
 
+    fn can_split(&self) -> bool {
+        true
+    }
+
     fn clone_on_split(
         &self,
         _workspace_id: Option<workspace::WorkspaceId>,

crates/git_ui/src/project_diff.rs 🔗

@@ -709,6 +709,10 @@ impl Item for ProjectDiff {
         });
     }
 
+    fn can_split(&self) -> bool {
+        true
+    }
+
     fn clone_on_split(
         &self,
         _workspace_id: Option<workspace::WorkspaceId>,

crates/gpui_tokio/src/gpui_tokio.rs 🔗

@@ -1,9 +1,10 @@
 use std::future::Future;
 
 use gpui::{App, AppContext, Global, ReadGlobal, Task};
-use tokio::task::JoinError;
 use util::defer;
 
+pub use tokio::task::JoinError;
+
 pub fn init(cx: &mut App) {
     cx.set_global(GlobalTokio::new());
 }

crates/onboarding/src/onboarding.rs 🔗

@@ -378,6 +378,10 @@ impl Item for Onboarding {
         false
     }
 
+    fn can_split(&self) -> bool {
+        true
+    }
+
     fn clone_on_split(
         &self,
         _workspace_id: Option<WorkspaceId>,

crates/repl/src/notebook/notebook_ui.rs 🔗

@@ -694,6 +694,10 @@ impl EventEmitter<()> for NotebookEditor {}
 impl Item for NotebookEditor {
     type Event = ();
 
+    fn can_split(&self) -> bool {
+        true
+    }
+
     fn clone_on_split(
         &self,
         _workspace_id: Option<workspace::WorkspaceId>,

crates/search/src/project_search.rs 🔗

@@ -567,6 +567,10 @@ impl Item for ProjectSearchView {
             .update(cx, |editor, cx| editor.reload(project, window, cx))
     }
 
+    fn can_split(&self) -> bool {
+        true
+    }
+
     fn clone_on_split(
         &self,
         _workspace_id: Option<WorkspaceId>,

crates/terminal_view/src/terminal_view.rs 🔗

@@ -1213,6 +1213,10 @@ impl Item for TerminalView {
         workspace::item::ItemBufferKind::Singleton
     }
 
+    fn can_split(&self) -> bool {
+        true
+    }
+
     fn clone_on_split(
         &self,
         workspace_id: Option<WorkspaceId>,

crates/workspace/src/item.rs 🔗

@@ -213,16 +213,21 @@ pub trait Item: Focusable + EventEmitter<Self::Event> + Render + Sized {
         ItemBufferKind::None
     }
     fn set_nav_history(&mut self, _: ItemNavHistory, _window: &mut Window, _: &mut Context<Self>) {}
+
+    fn can_split(&self) -> bool {
+        false
+    }
     fn clone_on_split(
         &self,
-        _workspace_id: Option<WorkspaceId>,
-        _window: &mut Window,
-        _: &mut Context<Self>,
+        workspace_id: Option<WorkspaceId>,
+        window: &mut Window,
+        cx: &mut Context<Self>,
     ) -> Task<Option<Entity<Self>>>
     where
         Self: Sized,
     {
-        Task::ready(None)
+        _ = (workspace_id, window, cx);
+        unimplemented!("clone_on_split() must be implemented if can_split() returns true")
     }
     fn is_dirty(&self, _: &App) -> bool {
         false
@@ -418,6 +423,7 @@ pub trait ItemHandle: 'static + Send {
     );
     fn buffer_kind(&self, cx: &App) -> ItemBufferKind;
     fn boxed_clone(&self) -> Box<dyn ItemHandle>;
+    fn can_split(&self, cx: &App) -> bool;
     fn clone_on_split(
         &self,
         workspace_id: Option<WorkspaceId>,
@@ -631,6 +637,10 @@ impl<T: Item> ItemHandle for Entity<T> {
         Box::new(self.clone())
     }
 
+    fn can_split(&self, cx: &App) -> bool {
+        self.read(cx).can_split()
+    }
+
     fn clone_on_split(
         &self,
         workspace_id: Option<WorkspaceId>,
@@ -1503,6 +1513,10 @@ pub mod test {
             self.push_to_nav_history(cx);
         }
 
+        fn can_split(&self) -> bool {
+            true
+        }
+
         fn clone_on_split(
             &self,
             _workspace_id: Option<WorkspaceId>,

crates/workspace/src/pane.rs 🔗

@@ -3292,18 +3292,22 @@ impl Pane {
                         else {
                             return;
                         };
-                        let task = item.clone_on_split(database_id, window, cx);
-                        let to_pane = to_pane.downgrade();
-                        cx.spawn_in(window, async move |_, cx| {
-                            if let Some(item) = task.await {
-                                to_pane
-                                    .update_in(cx, |pane, window, cx| {
-                                        pane.add_item(item, true, true, None, window, cx)
-                                    })
-                                    .ok();
-                            }
-                        })
-                        .detach();
+                        if item.can_split(cx) {
+                            let task = item.clone_on_split(database_id, window, cx);
+                            let to_pane = to_pane.downgrade();
+                            cx.spawn_in(window, async move |_, cx| {
+                                if let Some(item) = task.await {
+                                    to_pane
+                                        .update_in(cx, |pane, window, cx| {
+                                            pane.add_item(item, true, true, None, window, cx)
+                                        })
+                                        .ok();
+                                }
+                            })
+                            .detach();
+                        } else {
+                            move_item(&from_pane, &to_pane, item_id, ix, true, window, cx);
+                        }
                     } else {
                         move_item(&from_pane, &to_pane, item_id, ix, true, window, cx);
                     }
@@ -3597,6 +3601,11 @@ fn default_render_tab_bar_buttons(
     if !pane.has_focus(window, cx) && !pane.context_menu_focused(window, cx) {
         return (None, None);
     }
+    let (can_clone, can_split_move) = match pane.active_item() {
+        Some(active_item) if active_item.can_split(cx) => (true, false),
+        Some(_) => (false, pane.items_len() > 1),
+        None => (false, false),
+    };
     // Ideally we would return a vec of elements here to pass directly to the [TabBar]'s
     // `end_slot`, but due to needing a view here that isn't possible.
     let right_children = h_flex()
@@ -3633,17 +3642,26 @@ fn default_render_tab_bar_buttons(
         .child(
             PopoverMenu::new("pane-tab-bar-split")
                 .trigger_with_tooltip(
-                    IconButton::new("split", IconName::Split).icon_size(IconSize::Small),
+                    IconButton::new("split", IconName::Split)
+                        .icon_size(IconSize::Small)
+                        .disabled(!can_clone && !can_split_move),
                     Tooltip::text("Split Pane"),
                 )
                 .anchor(Corner::TopRight)
                 .with_handle(pane.split_item_context_menu_handle.clone())
                 .menu(move |window, cx| {
                     ContextMenu::build(window, cx, |menu, _, _| {
-                        menu.action("Split Right", SplitRight.boxed_clone())
-                            .action("Split Left", SplitLeft.boxed_clone())
-                            .action("Split Up", SplitUp.boxed_clone())
-                            .action("Split Down", SplitDown.boxed_clone())
+                        if can_split_move {
+                            menu.action("Split Right", SplitAndMoveRight.boxed_clone())
+                                .action("Split Left", SplitAndMoveLeft.boxed_clone())
+                                .action("Split Up", SplitAndMoveUp.boxed_clone())
+                                .action("Split Down", SplitAndMoveDown.boxed_clone())
+                        } else {
+                            menu.action("Split Right", SplitRight.boxed_clone())
+                                .action("Split Left", SplitLeft.boxed_clone())
+                                .action("Split Up", SplitUp.boxed_clone())
+                                .action("Split Down", SplitDown.boxed_clone())
+                        }
                     })
                     .into()
                 }),

crates/workspace/src/shared_screen.rs 🔗

@@ -109,6 +109,10 @@ impl Item for SharedScreen {
         self.nav_history = Some(history);
     }
 
+    fn can_split(&self) -> bool {
+        true
+    }
+
     fn clone_on_split(
         &self,
         _workspace_id: Option<WorkspaceId>,

crates/workspace/src/theme_preview.rs 🔗

@@ -97,6 +97,10 @@ impl Item for ThemePreview {
         None
     }
 
+    fn can_split(&self) -> bool {
+        true
+    }
+
     fn clone_on_split(
         &self,
         _workspace_id: Option<crate::WorkspaceId>,

crates/workspace/src/workspace.rs 🔗

@@ -3663,24 +3663,31 @@ impl Workspace {
         };
 
         if action.clone {
-            clone_active_item(
-                self.database_id(),
-                &self.active_pane,
-                &destination,
-                action.focus,
-                window,
-                cx,
-            )
-        } else {
-            move_active_item(
-                &self.active_pane,
-                &destination,
-                action.focus,
-                true,
-                window,
-                cx,
-            )
+            if self
+                .active_pane
+                .read(cx)
+                .active_item()
+                .is_some_and(|item| item.can_split(cx))
+            {
+                clone_active_item(
+                    self.database_id(),
+                    &self.active_pane,
+                    &destination,
+                    action.focus,
+                    window,
+                    cx,
+                );
+                return;
+            }
         }
+        move_active_item(
+            &self.active_pane,
+            &destination,
+            action.focus,
+            true,
+            window,
+            cx,
+        )
     }
 
     pub fn activate_next_pane(&mut self, window: &mut Window, cx: &mut App) {
@@ -3841,24 +3848,31 @@ impl Workspace {
         };
 
         if action.clone {
-            clone_active_item(
-                self.database_id(),
-                &self.active_pane,
-                &destination,
-                action.focus,
-                window,
-                cx,
-            )
-        } else {
-            move_active_item(
-                &self.active_pane,
-                &destination,
-                action.focus,
-                true,
-                window,
-                cx,
-            );
+            if self
+                .active_pane
+                .read(cx)
+                .active_item()
+                .is_some_and(|item| item.can_split(cx))
+            {
+                clone_active_item(
+                    self.database_id(),
+                    &self.active_pane,
+                    &destination,
+                    action.focus,
+                    window,
+                    cx,
+                );
+                return;
+            }
         }
+        move_active_item(
+            &self.active_pane,
+            &destination,
+            action.focus,
+            true,
+            window,
+            cx,
+        );
     }
 
     pub fn bounding_box_for_pane(&self, pane: &Entity<Pane>) -> Option<Bounds<Pixels>> {
@@ -4141,6 +4155,9 @@ impl Workspace {
         let Some(item) = pane.read(cx).active_item() else {
             return Task::ready(None);
         };
+        if !item.can_split(cx) {
+            return Task::ready(None);
+        }
         let task = item.clone_on_split(self.database_id(), window, cx);
         cx.spawn_in(window, async move |this, cx| {
             if let Some(clone) = task.await {
@@ -8225,6 +8242,9 @@ pub fn clone_active_item(
     let Some(active_item) = source.read(cx).active_item() else {
         return;
     };
+    if !active_item.can_split(cx) {
+        return;
+    }
     let destination = destination.downgrade();
     let task = active_item.clone_on_split(workspace_id, window, cx);
     window

crates/zed/src/zed/component_preview.rs 🔗

@@ -715,6 +715,10 @@ impl Item for ComponentPreview {
         false
     }
 
+    fn can_split(&self) -> bool {
+        true
+    }
+
     fn clone_on_split(
         &self,
         _workspace_id: Option<WorkspaceId>,