From 022368225e969c2c3ca79ee2c284ad80a8eaa168 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 27 Apr 2023 12:31:42 +0200 Subject: [PATCH] Remove internal actions from `Pane` --- crates/editor/src/editor_tests.rs | 12 +- crates/workspace/src/dock.rs | 13 +- crates/workspace/src/pane.rs | 269 +++++++----------- .../src/pane/dragged_item_receiver.rs | 19 +- crates/workspace/src/workspace.rs | 11 +- 5 files changed, 135 insertions(+), 189 deletions(-) diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index e71ab8ae4b6679f0e89ebf8e70712ea31b7ec51f..af4bc1674b8e010b1f2e357122b63b07b9d8ff8f 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -24,7 +24,7 @@ use util::{ }; use workspace::{ item::{FollowableItem, Item, ItemHandle}, - NavigationEntry, Pane, ViewId, + NavigationEntry, ViewId, }; #[gpui::test] @@ -486,12 +486,15 @@ fn test_clone(cx: &mut TestAppContext) { } #[gpui::test] -fn test_navigation_history(cx: &mut TestAppContext) { +async fn test_navigation_history(cx: &mut TestAppContext) { cx.update(|cx| cx.set_global(Settings::test(cx))); cx.set_global(DragAndDrop::::default()); use workspace::item::Item; - let (_, pane) = cx.add_window(|cx| Pane::new(0, None, || &[], cx)); + let fs = FakeFs::new(cx.background()); + let project = Project::test(fs, [], cx).await; + let (_, workspace) = cx.add_window(|cx| Workspace::test_new(project, cx)); + let pane = workspace.read_with(cx, |workspace, _| workspace.active_pane().clone()); cx.add_view(&pane, |cx| { let buffer = MultiBuffer::build_simple(&sample_text(300, 5, 'a'), cx); let mut editor = build_editor(buffer.clone(), cx); @@ -5576,7 +5579,8 @@ async fn test_following_with_multiple_excerpts(cx: &mut gpui::TestAppContext) { Settings::test_async(cx); let fs = FakeFs::new(cx.background()); let project = Project::test(fs, ["/file.rs".as_ref()], cx).await; - let (_, pane) = cx.add_window(|cx| Pane::new(0, None, || &[], cx)); + let (_, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx)); + let pane = workspace.read_with(cx, |workspace, _| workspace.active_pane().clone()); let leader = pane.update(cx, |_, cx| { let multibuffer = cx.add_model(|_| MultiBuffer::new(0)); diff --git a/crates/workspace/src/dock.rs b/crates/workspace/src/dock.rs index 67cfea3d9ad8bc00a4ef10f00c36fdaec3ab2070..94c690eaa761a20691be8566a4dc6992063046fc 100644 --- a/crates/workspace/src/dock.rs +++ b/crates/workspace/src/dock.rs @@ -182,21 +182,14 @@ pub struct Dock { impl Dock { pub fn new( - workspace_id: usize, default_item_factory: DockDefaultItemFactory, background_actions: BackgroundActions, cx: &mut ViewContext, ) -> Self { let position = DockPosition::Hidden(cx.global::().default_dock_anchor); - - let pane = cx.add_view(|cx| { - Pane::new( - workspace_id, - Some(position.anchor()), - background_actions, - cx, - ) - }); + let workspace = cx.weak_handle(); + let pane = + cx.add_view(|cx| Pane::new(workspace, Some(position.anchor()), background_actions, cx)); pane.update(cx, |pane, cx| { pane.set_active(false, cx); }); diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 3d5cd906e748bb4ea20fa7d20c8aedfec33591e3..41f4d5d111cea8e522ddc9eedb4558a679409bcc 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -20,11 +20,12 @@ use gpui::{ rect::RectF, vector::{vec2f, Vector2F}, }, - impl_actions, impl_internal_actions, + impl_actions, keymap_matcher::KeymapContext, platform::{CursorStyle, MouseButton, NavigationDirection, PromptLevel}, - Action, AnyViewHandle, AnyWeakViewHandle, AppContext, AsyncAppContext, Entity, ModelHandle, - MouseRegion, Quad, Task, View, ViewContext, ViewHandle, WeakViewHandle, WindowContext, + Action, AnyViewHandle, AnyWeakViewHandle, AppContext, AsyncAppContext, Entity, EventContext, + ModelHandle, MouseRegion, Quad, Task, View, ViewContext, ViewHandle, WeakViewHandle, + WindowContext, }; use project::{Project, ProjectEntryId, ProjectPath}; use serde::Deserialize; @@ -74,14 +75,6 @@ actions!( ] ); -#[derive(Clone, PartialEq)] -pub struct MoveItem { - pub item_id: usize, - pub from: WeakViewHandle, - pub to: WeakViewHandle, - pub destination_index: usize, -} - #[derive(Clone, Deserialize, PartialEq)] pub struct GoBack { #[serde(skip_deserializing)] @@ -94,36 +87,7 @@ pub struct GoForward { pub pane: Option>, } -#[derive(Clone, PartialEq)] -pub struct DeploySplitMenu; - -#[derive(Clone, PartialEq)] -pub struct DeployDockMenu; - -#[derive(Clone, PartialEq)] -pub struct DeployNewMenu; - -#[derive(Clone, PartialEq)] -pub struct DeployTabContextMenu { - pub position: Vector2F, - pub item_id: usize, - pub pane: WeakViewHandle, -} - impl_actions!(pane, [GoBack, GoForward, ActivateItem]); -impl_internal_actions!( - pane, - [ - CloseItemById, - CloseItemsToTheLeftById, - CloseItemsToTheRightById, - DeployTabContextMenu, - DeploySplitMenu, - DeployNewMenu, - DeployDockMenu, - MoveItem - ] -); const MAX_NAVIGATION_HISTORY_LEN: usize = 1024; @@ -148,68 +112,10 @@ pub fn init(cx: &mut AppContext) { cx.add_async_action(Pane::close_items_to_the_left); cx.add_async_action(Pane::close_items_to_the_right); cx.add_async_action(Pane::close_all_items); - cx.add_async_action(|workspace: &mut Workspace, action: &CloseItemById, cx| { - let pane = action.pane.upgrade(cx)?; - let task = Pane::close_item_by_id(workspace, pane, action.item_id, cx); - Some(cx.foreground().spawn(async move { - task.await?; - Ok(()) - })) - }); - cx.add_async_action( - |workspace: &mut Workspace, action: &CloseItemsToTheLeftById, cx| { - let pane = action.pane.upgrade(cx)?; - let task = Pane::close_items_to_the_left_by_id(workspace, pane, action.item_id, cx); - Some(cx.foreground().spawn(async move { - task.await?; - Ok(()) - })) - }, - ); - cx.add_async_action( - |workspace: &mut Workspace, action: &CloseItemsToTheRightById, cx| { - let pane = action.pane.upgrade(cx)?; - let task = Pane::close_items_to_the_right_by_id(workspace, pane, action.item_id, cx); - Some(cx.foreground().spawn(async move { - task.await?; - Ok(()) - })) - }, - ); - cx.add_action( - |workspace, - MoveItem { - from, - to, - item_id, - destination_index, - }, - cx| { - // Get item handle to move - let from = if let Some(from) = from.upgrade(cx) { - from - } else { - return; - }; - - // Add item to new pane at given index - let to = if let Some(to) = to.upgrade(cx) { - to - } else { - return; - }; - - Pane::move_item(workspace, from, to, *item_id, *destination_index, cx) - }, - ); cx.add_action(|pane: &mut Pane, _: &SplitLeft, cx| pane.split(SplitDirection::Left, cx)); cx.add_action(|pane: &mut Pane, _: &SplitUp, cx| pane.split(SplitDirection::Up, cx)); cx.add_action(|pane: &mut Pane, _: &SplitRight, cx| pane.split(SplitDirection::Right, cx)); cx.add_action(|pane: &mut Pane, _: &SplitDown, cx| pane.split(SplitDirection::Down, cx)); - cx.add_action(Pane::deploy_split_menu); - cx.add_action(Pane::deploy_dock_menu); - cx.add_action(Pane::deploy_new_menu); - cx.add_action(Pane::deploy_tab_context_menu); cx.add_action(|workspace: &mut Workspace, _: &ReopenClosedItem, cx| { Pane::reopen_closed_item(workspace, cx).detach(); }); @@ -243,7 +149,7 @@ pub struct Pane { tab_context_menu: ViewHandle, docked: Option, _background_actions: BackgroundActions, - _workspace_id: usize, + workspace: WeakViewHandle, } pub struct ItemNavHistory { @@ -315,7 +221,7 @@ impl TabBarContextMenu { impl Pane { pub fn new( - workspace_id: usize, + workspace: WeakViewHandle, docked: Option, background_actions: BackgroundActions, cx: &mut ViewContext, @@ -349,7 +255,7 @@ impl Pane { tab_context_menu: cx.add_view(ContextMenu::new), docked, _background_actions: background_actions, - _workspace_id: workspace_id, + workspace, } } @@ -1223,7 +1129,7 @@ impl Pane { cx.emit(Event::Split(direction)); } - fn deploy_split_menu(&mut self, _: &DeploySplitMenu, cx: &mut ViewContext) { + fn deploy_split_menu(&mut self, cx: &mut ViewContext) { self.tab_bar_context_menu.handle.update(cx, |menu, cx| { menu.show( Default::default(), @@ -1241,7 +1147,7 @@ impl Pane { self.tab_bar_context_menu.kind = TabBarContextMenuKind::Split; } - fn deploy_dock_menu(&mut self, _: &DeployDockMenu, cx: &mut ViewContext) { + fn deploy_dock_menu(&mut self, cx: &mut ViewContext) { self.tab_bar_context_menu.handle.update(cx, |menu, cx| { menu.show( Default::default(), @@ -1258,7 +1164,7 @@ impl Pane { self.tab_bar_context_menu.kind = TabBarContextMenuKind::Dock; } - fn deploy_new_menu(&mut self, _: &DeployNewMenu, cx: &mut ViewContext) { + fn deploy_new_menu(&mut self, cx: &mut ViewContext) { self.tab_bar_context_menu.handle.update(cx, |menu, cx| { menu.show( Default::default(), @@ -1277,19 +1183,19 @@ impl Pane { fn deploy_tab_context_menu( &mut self, - action: &DeployTabContextMenu, + position: Vector2F, + target_item_id: usize, cx: &mut ViewContext, ) { - let target_item_id = action.item_id; - let target_pane = action.pane.clone(); let active_item_id = self.items[self.active_item_index].id(); let is_active_item = target_item_id == active_item_id; + let target_pane = cx.weak_handle(); // The `CloseInactiveItems` action should really be called "CloseOthers" and the behaviour should be dynamically based on the tab the action is ran on. Currenlty, this is a weird action because you can run it on a non-active tab and it will close everything by the actual active tab self.tab_context_menu.update(cx, |menu, cx| { menu.show( - action.position, + position, AnchorCorner::TopLeft, if is_active_item { vec![ @@ -1303,29 +1209,60 @@ impl Pane { } else { // In the case of the user right clicking on a non-active tab, for some item-closing commands, we need to provide the id of the tab, for the others, we can reuse the existing command. vec![ - ContextMenuItem::action( - "Close Inactive Item", - CloseItemById { - item_id: target_item_id, - pane: target_pane.clone(), - }, - ), + ContextMenuItem::handler("Close Inactive Item", { + let workspace = self.workspace.clone(); + let pane = target_pane.clone(); + move |cx| { + if let Some((workspace, pane)) = + workspace.upgrade(cx).zip(pane.upgrade(cx)) + { + workspace.update(cx, |workspace, cx| { + Self::close_item_by_id(workspace, pane, target_item_id, cx) + .detach_and_log_err(cx); + }) + } + } + }), ContextMenuItem::action("Close Inactive Items", CloseInactiveItems), ContextMenuItem::action("Close Clean Items", CloseCleanItems), - ContextMenuItem::action( - "Close Items To The Left", - CloseItemsToTheLeftById { - item_id: target_item_id, - pane: target_pane.clone(), - }, - ), - ContextMenuItem::action( - "Close Items To The Right", - CloseItemsToTheRightById { - item_id: target_item_id, - pane: target_pane.clone(), - }, - ), + ContextMenuItem::handler("Close Items To The Left", { + let workspace = self.workspace.clone(); + let pane = target_pane.clone(); + move |cx| { + if let Some((workspace, pane)) = + workspace.upgrade(cx).zip(pane.upgrade(cx)) + { + workspace.update(cx, |workspace, cx| { + Self::close_items_to_the_left_by_id( + workspace, + pane, + target_item_id, + cx, + ) + .detach_and_log_err(cx); + }) + } + } + }), + ContextMenuItem::handler("Close Items To The Right", { + let workspace = self.workspace.clone(); + let pane = target_pane.clone(); + move |cx| { + if let Some((workspace, pane)) = + workspace.upgrade(cx).zip(pane.upgrade(cx)) + { + workspace.update(cx, |workspace, cx| { + Self::close_items_to_the_right_by_id( + workspace, + pane, + target_item_id, + cx, + ) + .detach_and_log_err(cx); + }) + } + } + }), ContextMenuItem::action("Close All Items", CloseAllItems), ] }, @@ -1407,24 +1344,28 @@ impl Pane { cx.dispatch_action(ActivateItem(ix)); }) .on_click(MouseButton::Middle, { - let item = item.clone(); - let pane = pane.clone(); - move |_, _, cx| { - cx.dispatch_action(CloseItemById { - item_id: item.id(), - pane: pane.clone(), - }) + let item_id = item.id(); + move |_, pane, cx| { + let workspace = pane.workspace.clone(); + let pane = cx.weak_handle(); + cx.window_context().defer(move |cx| { + if let Some((workspace, pane)) = + workspace.upgrade(cx).zip(pane.upgrade(cx)) + { + workspace.update(cx, |workspace, cx| { + Self::close_item_by_id( + workspace, pane, item_id, cx, + ) + .detach_and_log_err(cx); + }); + } + }); } }) .on_down( MouseButton::Right, - move |e, _, cx| { - let item = item.clone(); - cx.dispatch_action(DeployTabContextMenu { - position: e.position, - item_id: item.id(), - pane: pane.clone(), - }); + move |event, pane, cx| { + pane.deploy_tab_context_menu(event.position, item.id(), cx); }, ); @@ -1622,10 +1563,17 @@ impl Pane { .on_click(MouseButton::Left, { let pane = pane.clone(); move |_, _, cx| { - cx.dispatch_action(CloseItemById { - item_id, - pane: pane.clone(), - }) + let pane = pane.clone(); + cx.window_context().defer(move |cx| { + if let Some(pane) = pane.upgrade(cx) { + if let Some(workspace) = pane.read(cx).workspace.upgrade(cx) { + workspace.update(cx, |workspace, cx| { + Self::close_item_by_id(workspace, pane, item_id, cx) + .detach_and_log_err(cx); + }); + } + } + }); } }) .into_any_named("close-tab-icon") @@ -1654,7 +1602,7 @@ impl Pane { 0, "icons/plus_12.svg", cx, - DeployNewMenu, + |pane, cx| pane.deploy_new_menu(cx), self.tab_bar_context_menu .handle_if_kind(TabBarContextMenuKind::New), )) @@ -1668,7 +1616,7 @@ impl Pane { 1, dock_icon, cx, - DeployDockMenu, + |pane, cx| pane.deploy_dock_menu(cx), self.tab_bar_context_menu .handle_if_kind(TabBarContextMenuKind::Dock), ) @@ -1679,17 +1627,22 @@ impl Pane { 2, "icons/split_12.svg", cx, - DeploySplitMenu, + |pane, cx| pane.deploy_split_menu(cx), self.tab_bar_context_menu .handle_if_kind(TabBarContextMenuKind::Split), ) }), ) // Add the close dock button if this pane is a dock - .with_children( - self.docked - .map(|_| render_tab_bar_button(3, "icons/x_mark_8.svg", cx, HideDock, None)), - ) + .with_children(self.docked.map(|_| { + render_tab_bar_button( + 3, + "icons/x_mark_8.svg", + cx, + |_, cx| cx.dispatch_action(HideDock), + None, + ) + })) .contained() .with_style(theme.workspace.tab_bar.pane_button_container) .flex(1., false) @@ -1863,11 +1816,11 @@ impl View for Pane { } } -fn render_tab_bar_button( +fn render_tab_bar_button)>( index: usize, icon: &'static str, cx: &mut ViewContext, - action: A, + on_click: F, context_menu: Option>, ) -> AnyElement { enum TabBarButton {} @@ -1887,9 +1840,7 @@ fn render_tab_bar_button( .with_height(style.button_width) }) .with_cursor_style(CursorStyle::PointingHand) - .on_click(MouseButton::Left, move |_, _, cx| { - cx.dispatch_action(action.clone()); - }), + .on_click(MouseButton::Left, move |_, pane, cx| on_click(pane, cx)), ) .with_children( context_menu.map(|menu| ChildView::new(&menu, cx).aligned().bottom().right()), diff --git a/crates/workspace/src/pane/dragged_item_receiver.rs b/crates/workspace/src/pane/dragged_item_receiver.rs index ae6b250fb625117bf4645bb3dcec16f814b4fd14..bde6d4b371102fea49510c458b8b1175430a6d37 100644 --- a/crates/workspace/src/pane/dragged_item_receiver.rs +++ b/crates/workspace/src/pane/dragged_item_receiver.rs @@ -11,8 +11,7 @@ use project::ProjectEntryId; use settings::Settings; use crate::{ - MoveItem, OpenProjectEntryInPane, Pane, SplitDirection, SplitWithItem, SplitWithProjectEntry, - Workspace, + OpenProjectEntryInPane, Pane, SplitDirection, SplitWithItem, SplitWithProjectEntry, Workspace, }; use super::DraggedItem; @@ -142,12 +141,16 @@ pub fn handle_dropped_item( match action { Action::Move(from, item_id) => { if pane != &from || allow_same_pane { - cx.dispatch_action(MoveItem { - item_id, - from, - to: pane.clone(), - destination_index: index, - }) + let pane = pane.clone(); + cx.window_context().defer(move |cx| { + if let Some((from, to)) = from.upgrade(cx).zip(pane.upgrade(cx)) { + if let Some(workspace) = from.read(cx).workspace.upgrade(cx) { + workspace.update(cx, |workspace, cx| { + Pane::move_item(workspace, from, to, item_id, index, cx); + }) + } + } + }); } else { cx.propagate_event(); } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 7524b84bf8f92b853142a3469d3b94a445f6ca09..b71de516fd67095d3701710841dac3184310ba8a 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -710,7 +710,7 @@ impl Workspace { let weak_handle = cx.weak_handle(); let center_pane = - cx.add_view(|cx| Pane::new(weak_handle.id(), None, background_actions, cx)); + cx.add_view(|cx| Pane::new(weak_handle.clone(), None, background_actions, cx)); let pane_id = center_pane.id(); cx.subscribe(¢er_pane, move |this, _, event, cx| { this.handle_pane_event(pane_id, event, cx) @@ -718,12 +718,7 @@ impl Workspace { .detach(); cx.focus(¢er_pane); cx.emit(Event::PaneAdded(center_pane.clone())); - let dock = Dock::new( - weak_handle.id(), - dock_default_factory, - background_actions, - cx, - ); + let dock = Dock::new(dock_default_factory, background_actions, cx); let dock_pane = dock.pane().clone(); let fs = project.read(cx).fs().clone(); @@ -1562,7 +1557,7 @@ impl Workspace { fn add_pane(&mut self, cx: &mut ViewContext) -> ViewHandle { let pane = - cx.add_view(|cx| Pane::new(self.weak_handle().id(), None, self.background_actions, cx)); + cx.add_view(|cx| Pane::new(self.weak_handle(), None, self.background_actions, cx)); let pane_id = pane.id(); cx.subscribe(&pane, move |this, _, event, cx| { this.handle_pane_event(pane_id, event, cx)