Avoid re-allocating `KeymapContext` after every view notification

Antonio Scandurra created

Change summary

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 ++++----
crates/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(-)

Detailed changes

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<Self>) -> AnyElement<Self> {

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<Self>) -> AnyElement<Self> {

crates/editor/src/editor.rs 🔗

@@ -1425,13 +1425,19 @@ impl Editor {
         }
     }
 
-    pub fn set_keymap_context_layer<Tag: 'static>(&mut self, context: KeymapContext) {
+    pub fn set_keymap_context_layer<Tag: 'static>(
+        &mut self,
+        context: KeymapContext,
+        cx: &mut ViewContext<Self>,
+    ) {
         self.keymap_context_layers
             .insert(TypeId::of::<Tag>(), context);
+        cx.notify();
     }
 
-    pub fn remove_keymap_context_layer<Tag: 'static>(&mut self) {
+    pub fn remove_keymap_context_layer<Tag: 'static>(&mut self, cx: &mut ViewContext<Self>) {
         self.keymap_context_layers.remove(&TypeId::of::<Tag>());
+        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<usize>, cx: &AppContext) -> Option<String> {

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<usize>, cx: &WindowContext) -> Option<String>;
@@ -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();
             }
         }
 

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::<TypeId, usize>::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 {

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());

crates/picker/src/picker.rs 🔗

@@ -126,10 +126,9 @@ impl<D: PickerDelegate> View for Picker<D> {
             .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<Self>) {

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");
     }
 }
 

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
     }
 }
 

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::<Self>(context_layer);
+                editor.set_keymap_context_layer::<Self>(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::<Self>();
+        editor.remove_keymap_context_layer::<Self>(cx);
     }
 }
 

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
     }
 }
 

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 {