From 3d679ddb26009d5e8afd867ccfde28571f55625b Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 4 May 2023 12:04:30 +0200 Subject: [PATCH] Avoid re-allocating `KeymapContext` after every view notification --- crates/collab_ui/src/contact_list.rs | 7 ++-- crates/context_menu/src/context_menu.rs | 7 ++-- crates/editor/src/editor.rs | 26 ++++++++------ crates/gpui/src/app.rs | 25 ++++++------- crates/gpui/src/app/window.rs | 16 ++++----- .../gpui/src/keymap_matcher/keymap_context.rs | 5 +++ crates/picker/src/picker.rs | 7 ++-- crates/project_panel/src/project_panel.rs | 7 ++-- crates/terminal_view/src/terminal_view.rs | 35 +++++++++---------- crates/vim/src/vim.rs | 4 +-- crates/workspace/src/pane.rs | 5 ++- crates/workspace/src/workspace.rs | 5 --- 12 files changed, 74 insertions(+), 75 deletions(-) diff --git a/crates/collab_ui/src/contact_list.rs b/crates/collab_ui/src/contact_list.rs index 87aa41b7a4..452867b8c4 100644 --- a/crates/collab_ui/src/contact_list.rs +++ b/crates/collab_ui/src/contact_list.rs @@ -1306,10 +1306,9 @@ impl View for ContactList { "ContactList" } - fn keymap_context(&self, _: &AppContext) -> KeymapContext { - let mut cx = Self::default_keymap_context(); - cx.add_identifier("menu"); - cx + fn update_keymap_context(&self, keymap: &mut KeymapContext, _: &AppContext) { + Self::reset_to_default_keymap_context(keymap); + keymap.add_identifier("menu"); } fn render(&mut self, cx: &mut ViewContext) -> AnyElement { diff --git a/crates/context_menu/src/context_menu.rs b/crates/context_menu/src/context_menu.rs index e0429bd01b..0a4cd59384 100644 --- a/crates/context_menu/src/context_menu.rs +++ b/crates/context_menu/src/context_menu.rs @@ -140,10 +140,9 @@ impl View for ContextMenu { "ContextMenu" } - fn keymap_context(&self, _: &AppContext) -> KeymapContext { - let mut cx = Self::default_keymap_context(); - cx.add_identifier("menu"); - cx + fn update_keymap_context(&self, keymap: &mut KeymapContext, _: &AppContext) { + Self::reset_to_default_keymap_context(keymap); + keymap.add_identifier("menu"); } fn render(&mut self, cx: &mut ViewContext) -> AnyElement { diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index be57596584..641ef4d133 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -1425,13 +1425,19 @@ impl Editor { } } - pub fn set_keymap_context_layer(&mut self, context: KeymapContext) { + pub fn set_keymap_context_layer( + &mut self, + context: KeymapContext, + cx: &mut ViewContext, + ) { self.keymap_context_layers .insert(TypeId::of::(), context); + cx.notify(); } - pub fn remove_keymap_context_layer(&mut self) { + pub fn remove_keymap_context_layer(&mut self, cx: &mut ViewContext) { self.keymap_context_layers.remove(&TypeId::of::()); + cx.notify(); } pub fn set_input_enabled(&mut self, input_enabled: bool) { @@ -7157,28 +7163,26 @@ impl View for Editor { false } - fn keymap_context(&self, _: &AppContext) -> KeymapContext { - let mut context = Self::default_keymap_context(); + fn update_keymap_context(&self, keymap: &mut KeymapContext, _: &AppContext) { + Self::reset_to_default_keymap_context(keymap); let mode = match self.mode { EditorMode::SingleLine => "single_line", EditorMode::AutoHeight { .. } => "auto_height", EditorMode::Full => "full", }; - context.add_key("mode", mode); + keymap.add_key("mode", mode); if self.pending_rename.is_some() { - context.add_identifier("renaming"); + keymap.add_identifier("renaming"); } match self.context_menu.as_ref() { - Some(ContextMenu::Completions(_)) => context.add_identifier("showing_completions"), - Some(ContextMenu::CodeActions(_)) => context.add_identifier("showing_code_actions"), + Some(ContextMenu::Completions(_)) => keymap.add_identifier("showing_completions"), + Some(ContextMenu::CodeActions(_)) => keymap.add_identifier("showing_code_actions"), None => {} } for layer in self.keymap_context_layers.values() { - context.extend(layer); + keymap.extend(layer); } - - context } fn text_for_range(&self, range_utf16: Range, cx: &AppContext) -> Option { diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index f44012685b..fa538d3e44 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -83,14 +83,15 @@ pub trait View: Entity + Sized { false } - fn keymap_context(&self, _: &AppContext) -> keymap_matcher::KeymapContext { - Self::default_keymap_context() + fn update_keymap_context(&self, keymap: &mut keymap_matcher::KeymapContext, _: &AppContext) { + Self::reset_to_default_keymap_context(keymap); } - fn default_keymap_context() -> keymap_matcher::KeymapContext { - let mut cx = keymap_matcher::KeymapContext::default(); - cx.add_identifier(Self::ui_name()); - cx + + fn reset_to_default_keymap_context(keymap: &mut keymap_matcher::KeymapContext) { + keymap.clear(); + keymap.add_identifier(Self::ui_name()); } + fn debug_json(&self, _: &AppContext) -> serde_json::Value { serde_json::Value::Null } @@ -1797,7 +1798,7 @@ impl AppContext { .insert(observed_view_id); } - view_metadata.keymap_context = view.keymap_context(self); + view.update_keymap_context(&mut view_metadata.keymap_context, self); self.views.insert(view_key, view); self.views_metadata.insert(view_key, view_metadata); @@ -2380,7 +2381,7 @@ pub trait AnyView { cx: &mut WindowContext, view_id: usize, ) -> bool; - fn keymap_context(&self, cx: &AppContext) -> KeymapContext; + fn update_keymap_context(&self, keymap: &mut KeymapContext, cx: &AppContext); fn debug_json(&self, cx: &WindowContext) -> serde_json::Value; fn text_for_range(&self, range: Range, cx: &WindowContext) -> Option; @@ -2506,8 +2507,8 @@ where View::modifiers_changed(self, event, &mut cx) } - fn keymap_context(&self, cx: &AppContext) -> KeymapContext { - View::keymap_context(self, cx) + fn update_keymap_context(&self, keymap: &mut KeymapContext, cx: &AppContext) { + View::update_keymap_context(self, keymap, cx) } fn debug_json(&self, cx: &WindowContext) -> serde_json::Value { @@ -5572,8 +5573,8 @@ mod tests { "View" } - fn keymap_context(&self, _: &AppContext) -> KeymapContext { - self.keymap_context.clone() + fn update_keymap_context(&self, keymap: &mut KeymapContext, _: &AppContext) { + *keymap = self.keymap_context.clone(); } } diff --git a/crates/gpui/src/app/window.rs b/crates/gpui/src/app/window.rs index 091a2ab1b2..94c4df2385 100644 --- a/crates/gpui/src/app/window.rs +++ b/crates/gpui/src/app/window.rs @@ -2,7 +2,7 @@ use crate::{ elements::AnyRootElement, geometry::rect::RectF, json::ToJson, - keymap_matcher::{Binding, Keystroke, MatchResult}, + keymap_matcher::{Binding, KeymapContext, Keystroke, MatchResult}, platform::{ self, Appearance, CursorStyle, Event, KeyDownEvent, KeyUpEvent, ModifiersChangedEvent, MouseButton, MouseMovedEvent, PromptLevel, WindowBounds, @@ -406,10 +406,9 @@ impl<'a> WindowContext<'a> { let mut contexts = Vec::new(); let mut handler_depths_by_action_type = HashMap::::default(); for (depth, view_id) in self.ancestors(view_id).enumerate() { - if let Some(view) = self.views.get(&(window_id, view_id)) { - contexts.push(view.keymap_context(self)); - let view_type = view.as_any().type_id(); - if let Some(actions) = self.actions.get(&view_type) { + if let Some(view_metadata) = self.views_metadata.get(&(window_id, view_id)) { + contexts.push(view_metadata.keymap_context.clone()); + if let Some(actions) = self.actions.get(&view_metadata.type_id) { handler_depths_by_action_type.extend( actions .keys() @@ -458,9 +457,9 @@ impl<'a> WindowContext<'a> { let dispatch_path = self .ancestors(focused_view_id) .filter_map(|view_id| { - self.views + self.views_metadata .get(&(window_id, view_id)) - .map(|view| (view_id, view.keymap_context(self))) + .map(|view| (view_id, view.keymap_context.clone())) }) .collect(); @@ -1177,7 +1176,8 @@ impl<'a> WindowContext<'a> { self.parents.insert((window_id, view_id), parent_id); let mut cx = ViewContext::mutable(self, view_id); let handle = if let Some(view) = build_view(&mut cx) { - let keymap_context = view.keymap_context(cx.app_context()); + let mut keymap_context = KeymapContext::default(); + view.update_keymap_context(&mut keymap_context, cx.app_context()); self.views_metadata.insert( (window_id, view_id), ViewMetadata { diff --git a/crates/gpui/src/keymap_matcher/keymap_context.rs b/crates/gpui/src/keymap_matcher/keymap_context.rs index bbf6bfc14b..b1a449edf3 100644 --- a/crates/gpui/src/keymap_matcher/keymap_context.rs +++ b/crates/gpui/src/keymap_matcher/keymap_context.rs @@ -17,6 +17,11 @@ impl KeymapContext { } } + pub fn clear(&mut self) { + self.set.clear(); + self.map.clear(); + } + pub fn extend(&mut self, other: &Self) { for v in &other.set { self.set.insert(v.clone()); diff --git a/crates/picker/src/picker.rs b/crates/picker/src/picker.rs index 62ffd417fe..01749ccf84 100644 --- a/crates/picker/src/picker.rs +++ b/crates/picker/src/picker.rs @@ -126,10 +126,9 @@ impl View for Picker { .into_any_named("picker") } - fn keymap_context(&self, _: &AppContext) -> KeymapContext { - let mut cx = Self::default_keymap_context(); - cx.add_identifier("menu"); - cx + fn update_keymap_context(&self, keymap: &mut KeymapContext, _: &AppContext) { + Self::reset_to_default_keymap_context(keymap); + keymap.add_identifier("menu"); } fn focus_in(&mut self, _: AnyViewHandle, cx: &mut ViewContext) { diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 373417b167..ec80bd245a 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -1316,10 +1316,9 @@ impl View for ProjectPanel { } } - fn keymap_context(&self, _: &AppContext) -> KeymapContext { - let mut cx = Self::default_keymap_context(); - cx.add_identifier("menu"); - cx + fn update_keymap_context(&self, keymap: &mut KeymapContext, _: &AppContext) { + Self::reset_to_default_keymap_context(keymap); + keymap.add_identifier("menu"); } } diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index 5e04fc9825..3fcbf39887 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/crates/terminal_view/src/terminal_view.rs @@ -446,11 +446,11 @@ impl View for TerminalView { }); } - fn keymap_context(&self, cx: &gpui::AppContext) -> KeymapContext { - let mut context = Self::default_keymap_context(); + fn update_keymap_context(&self, keymap: &mut KeymapContext, cx: &gpui::AppContext) { + Self::reset_to_default_keymap_context(keymap); let mode = self.terminal.read(cx).last_content.mode; - context.add_key( + keymap.add_key( "screen", if mode.contains(TermMode::ALT_SCREEN) { "alt" @@ -460,40 +460,40 @@ impl View for TerminalView { ); if mode.contains(TermMode::APP_CURSOR) { - context.add_identifier("DECCKM"); + keymap.add_identifier("DECCKM"); } if mode.contains(TermMode::APP_KEYPAD) { - context.add_identifier("DECPAM"); + keymap.add_identifier("DECPAM"); } else { - context.add_identifier("DECPNM"); + keymap.add_identifier("DECPNM"); } if mode.contains(TermMode::SHOW_CURSOR) { - context.add_identifier("DECTCEM"); + keymap.add_identifier("DECTCEM"); } if mode.contains(TermMode::LINE_WRAP) { - context.add_identifier("DECAWM"); + keymap.add_identifier("DECAWM"); } if mode.contains(TermMode::ORIGIN) { - context.add_identifier("DECOM"); + keymap.add_identifier("DECOM"); } if mode.contains(TermMode::INSERT) { - context.add_identifier("IRM"); + keymap.add_identifier("IRM"); } //LNM is apparently the name for this. https://vt100.net/docs/vt510-rm/LNM.html if mode.contains(TermMode::LINE_FEED_NEW_LINE) { - context.add_identifier("LNM"); + keymap.add_identifier("LNM"); } if mode.contains(TermMode::FOCUS_IN_OUT) { - context.add_identifier("report_focus"); + keymap.add_identifier("report_focus"); } if mode.contains(TermMode::ALTERNATE_SCROLL) { - context.add_identifier("alternate_scroll"); + keymap.add_identifier("alternate_scroll"); } if mode.contains(TermMode::BRACKETED_PASTE) { - context.add_identifier("bracketed_paste"); + keymap.add_identifier("bracketed_paste"); } if mode.intersects(TermMode::MOUSE_MODE) { - context.add_identifier("any_mouse_reporting"); + keymap.add_identifier("any_mouse_reporting"); } { let mouse_reporting = if mode.contains(TermMode::MOUSE_REPORT_CLICK) { @@ -505,7 +505,7 @@ impl View for TerminalView { } else { "off" }; - context.add_key("mouse_reporting", mouse_reporting); + keymap.add_key("mouse_reporting", mouse_reporting); } { let format = if mode.contains(TermMode::SGR_MOUSE) { @@ -515,9 +515,8 @@ impl View for TerminalView { } else { "normal" }; - context.add_key("mouse_format", format); + keymap.add_key("mouse_format", format); } - context } } diff --git a/crates/vim/src/vim.rs b/crates/vim/src/vim.rs index 7ce925944c..a0a12210ec 100644 --- a/crates/vim/src/vim.rs +++ b/crates/vim/src/vim.rs @@ -309,7 +309,7 @@ impl Vim { editor.set_input_enabled(!state.vim_controlled()); editor.selections.line_mode = matches!(state.mode, Mode::Visual { line: true }); let context_layer = state.keymap_context_layer(); - editor.set_keymap_context_layer::(context_layer); + editor.set_keymap_context_layer::(context_layer, cx); } else { Self::unhook_vim_settings(editor, cx); } @@ -321,7 +321,7 @@ impl Vim { editor.set_clip_at_line_ends(false, cx); editor.set_input_enabled(true); editor.selections.line_mode = false; - editor.remove_keymap_context_layer::(); + editor.remove_keymap_context_layer::(cx); } } diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 8bd42fed04..3d1e477941 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -1831,12 +1831,11 @@ impl View for Pane { }); } - fn keymap_context(&self, _: &AppContext) -> KeymapContext { - let mut keymap = Self::default_keymap_context(); + fn update_keymap_context(&self, keymap: &mut KeymapContext, _: &AppContext) { + Self::reset_to_default_keymap_context(keymap); if self.docked.is_some() { keymap.add_identifier("docked"); } - keymap } } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index bb2629862d..e1007043dc 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -37,7 +37,6 @@ use gpui::{ vector::{vec2f, Vector2F}, }, impl_actions, - keymap_matcher::KeymapContext, platform::{ CursorStyle, MouseButton, PathPromptOptions, Platform, PromptLevel, WindowBounds, WindowOptions, @@ -2809,10 +2808,6 @@ impl View for Workspace { } } } - - fn keymap_context(&self, _: &AppContext) -> KeymapContext { - Self::default_keymap_context() - } } impl ViewId {