From 121264d35a4e7c1dc095baac96b40cd15d5f8510 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 4 May 2023 14:34:42 +0200 Subject: [PATCH 1/4] Fix panic when opening multiple definitions in a multibuffer The editor is on the stack, so adding an item to the `Pane` containing the editor will cause a double borrow and a consequent panic. This commit fixes the issue by deferring the opening of the definitions. --- crates/editor/src/editor.rs | 44 +++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 641ef4d133a40b11b1605660aa42eaa1510472fe..8771387cb73cb456c1416b1e5d0575b02648d1c4 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -5680,28 +5680,30 @@ impl Editor { } } else if !definitions.is_empty() { let replica_id = self.replica_id(cx); - let title = definitions - .iter() - .find(|definition| definition.origin.is_some()) - .and_then(|definition| { - definition.origin.as_ref().map(|origin| { - let buffer = origin.buffer.read(cx); - format!( - "Definitions for {}", - buffer - .text_for_range(origin.range.clone()) - .collect::() - ) + cx.window_context().defer(move |cx| { + let title = definitions + .iter() + .find(|definition| definition.origin.is_some()) + .and_then(|definition| { + definition.origin.as_ref().map(|origin| { + let buffer = origin.buffer.read(cx); + format!( + "Definitions for {}", + buffer + .text_for_range(origin.range.clone()) + .collect::() + ) + }) }) - }) - .unwrap_or("Definitions".to_owned()); - let locations = definitions - .into_iter() - .map(|definition| definition.target) - .collect(); - workspace.update(cx, |workspace, cx| { - Self::open_locations_in_multibuffer(workspace, locations, replica_id, title, cx) - }) + .unwrap_or("Definitions".to_owned()); + let locations = definitions + .into_iter() + .map(|definition| definition.target) + .collect(); + workspace.update(cx, |workspace, cx| { + Self::open_locations_in_multibuffer(workspace, locations, replica_id, title, cx) + }); + }); } } From b3baebde22a103b23b3beb198d20aefc12247dc9 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 4 May 2023 14:52:34 +0200 Subject: [PATCH 2/4] Filter out vim commands when vim mode is disabled --- crates/vim/src/vim.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/vim/src/vim.rs b/crates/vim/src/vim.rs index a0a12210ec36d73a6fd468331622231f2b7802c0..cc686f851f21c7019c76a43840cea70a8d6f32de 100644 --- a/crates/vim/src/vim.rs +++ b/crates/vim/src/vim.rs @@ -84,7 +84,12 @@ pub fn init(cx: &mut AppContext) { Vim::active_editor_input_ignored("\n".into(), cx) }); - // Any time settings change, update vim mode to match. + // Any time settings change, update vim mode to match. The Vim struct + // will be initialized as disabled by default, so we filter its commands + // out when starting up. + cx.update_default_global::(|filter, _| { + filter.filtered_namespaces.insert("vim"); + }); cx.update_default_global(|vim: &mut Vim, cx: &mut AppContext| { vim.set_enabled(cx.global::().vim_mode, cx) }); From 912a4cf54999ad0d7c72c936374df9f366461e18 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 4 May 2023 16:18:01 +0200 Subject: [PATCH 3/4] Avoid calling `update_window` twice in `blurred` event handler This was preventing us from unhooking vim when performing a rename, which prevented typing in the rename editor. --- crates/vim/src/editor_events.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/vim/src/editor_events.rs b/crates/vim/src/editor_events.rs index 4324bc6054fde65c9384d900d14b1cfce046c310..a11f1cc182c64610ff588d46b51b142563b1abd7 100644 --- a/crates/vim/src/editor_events.rs +++ b/crates/vim/src/editor_events.rs @@ -35,9 +35,7 @@ fn blurred(EditorBlurred(editor): &EditorBlurred, cx: &mut AppContext) { } } - cx.update_window(editor.window_id(), |cx| { - editor.update(cx, |editor, cx| Vim::unhook_vim_settings(editor, cx)) - }); + editor.update(cx, |editor, cx| Vim::unhook_vim_settings(editor, cx)) }); }); } From 64e0c16baa67f4211e84bc3aa84cb1d09b68b322 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 4 May 2023 17:06:37 +0200 Subject: [PATCH 4/4] Use `Workspace::toggle_sidebar_item` when clicking on sidebar button Previously, we were mistakenly using `Sidebar::toggle_item`, which only performs part of the toggle operation. --- crates/workspace/src/sidebar.rs | 28 +++++++++++++++++++++------- crates/workspace/src/workspace.rs | 5 +++-- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/crates/workspace/src/sidebar.rs b/crates/workspace/src/sidebar.rs index 2b114d83eccf6e7a68ae4d8db0ec96e41330d798..ed629745a87efa447e999718352eca3dc1db616c 100644 --- a/crates/workspace/src/sidebar.rs +++ b/crates/workspace/src/sidebar.rs @@ -1,7 +1,7 @@ -use crate::StatusItemView; +use crate::{StatusItemView, Workspace}; use gpui::{ elements::*, impl_actions, platform::CursorStyle, platform::MouseButton, AnyViewHandle, - AppContext, Entity, Subscription, View, ViewContext, ViewHandle, WindowContext, + AppContext, Entity, Subscription, View, ViewContext, ViewHandle, WeakViewHandle, WindowContext, }; use serde::Deserialize; use settings::Settings; @@ -84,6 +84,7 @@ struct Item { pub struct SidebarButtons { sidebar: ViewHandle, + workspace: WeakViewHandle, } #[derive(Clone, Debug, Deserialize, PartialEq)] @@ -210,9 +211,13 @@ impl View for Sidebar { } impl SidebarButtons { - pub fn new(sidebar: ViewHandle, cx: &mut ViewContext) -> Self { + pub fn new( + sidebar: ViewHandle, + workspace: WeakViewHandle, + cx: &mut ViewContext, + ) -> Self { cx.observe(&sidebar, |_, _, cx| cx.notify()).detach(); - Self { sidebar } + Self { sidebar, workspace } } } @@ -279,9 +284,18 @@ impl View for SidebarButtons { .with_style(style.container) }) .with_cursor_style(CursorStyle::PointingHand) - .on_click(MouseButton::Left, move |_, this, cx| { - this.sidebar - .update(cx, |sidebar, cx| sidebar.toggle_item(ix, cx)); + .on_click(MouseButton::Left, { + let action = action.clone(); + move |_, this, cx| { + if let Some(workspace) = this.workspace.upgrade(cx) { + let action = action.clone(); + cx.window_context().defer(move |cx| { + workspace.update(cx, |workspace, cx| { + workspace.toggle_sidebar_item(&action, cx) + }); + }); + } + } }) .with_tooltip::( ix, diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index e1007043dcfc91d0f66204782b2b8cfa158ea76e..cbac09112828839fbe04ea0b11a05c498c559132 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -583,10 +583,11 @@ impl Workspace { let left_sidebar = cx.add_view(|_| Sidebar::new(SidebarSide::Left)); let right_sidebar = cx.add_view(|_| Sidebar::new(SidebarSide::Right)); - let left_sidebar_buttons = cx.add_view(|cx| SidebarButtons::new(left_sidebar.clone(), cx)); + let left_sidebar_buttons = + cx.add_view(|cx| SidebarButtons::new(left_sidebar.clone(), weak_handle.clone(), cx)); let toggle_dock = cx.add_view(|cx| ToggleDockButton::new(handle, cx)); let right_sidebar_buttons = - cx.add_view(|cx| SidebarButtons::new(right_sidebar.clone(), cx)); + cx.add_view(|cx| SidebarButtons::new(right_sidebar.clone(), weak_handle.clone(), cx)); let status_bar = cx.add_view(|cx| { let mut status_bar = StatusBar::new(¢er_pane.clone(), cx); status_bar.add_left_item(left_sidebar_buttons, cx);