fix issue with single line editors in vim not properly unhooking vim mode bindings

Kay Simmons created

Change summary

assets/keymaps/vim.json         |  3 
crates/editor/src/editor.rs     | 15 ++++-
crates/vim/src/editor_events.rs | 58 ++++++++++++------------
crates/vim/src/vim.rs           | 84 +++++++++++++++++++---------------
4 files changed, 88 insertions(+), 72 deletions(-)

Detailed changes

assets/keymaps/vim.json 🔗

@@ -315,7 +315,8 @@
     {
         "context": "Editor && VimWaiting",
         "bindings": {
-            "*": "gpui::KeyPressed"
+            "*": "gpui::KeyPressed",
+            "escape": "editor::Cancel"
         }
     }
 ]

crates/editor/src/editor.rs 🔗

@@ -400,7 +400,7 @@ pub enum SelectMode {
     All,
 }
 
-#[derive(Copy, Clone, PartialEq, Eq)]
+#[derive(Copy, Clone, PartialEq, Eq, Debug)]
 pub enum EditorMode {
     SingleLine,
     AutoHeight { max_lines: usize },
@@ -1732,11 +1732,13 @@ impl Editor {
     }
 
     pub fn handle_input(&mut self, text: &str, cx: &mut ViewContext<Self>) {
+        let text: Arc<str> = text.into();
+
         if !self.input_enabled {
+            cx.emit(Event::InputIgnored { text });
             return;
         }
 
-        let text: Arc<str> = text.into();
         let selections = self.selections.all_adjusted(cx);
         let mut edits = Vec::new();
         let mut new_selections = Vec::with_capacity(selections.len());
@@ -6187,6 +6189,9 @@ impl Deref for EditorSnapshot {
 
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub enum Event {
+    InputIgnored {
+        text: Arc<str>,
+    },
     ExcerptsAdded {
         buffer: ModelHandle<Buffer>,
         predecessor: ExcerptId,
@@ -6253,8 +6258,10 @@ impl View for Editor {
     }
 
     fn focus_in(&mut self, _: AnyViewHandle, cx: &mut ViewContext<Self>) {
-        let focused_event = EditorFocused(cx.handle());
-        cx.emit_global(focused_event);
+        if cx.is_self_focused() {
+            let focused_event = EditorFocused(cx.handle());
+            cx.emit_global(focused_event);
+        }
         if let Some(rename) = self.pending_rename.as_ref() {
             cx.focus(&rename.editor);
         } else {

crates/vim/src/editor_events.rs 🔗

@@ -1,62 +1,62 @@
-use editor::{EditorBlurred, EditorCreated, EditorFocused, EditorMode, EditorReleased};
+use editor::{EditorBlurred, EditorFocused, EditorMode, EditorReleased};
 use gpui::MutableAppContext;
 
 use crate::{state::Mode, Vim};
 
 pub fn init(cx: &mut MutableAppContext) {
-    cx.subscribe_global(editor_created).detach();
-    cx.subscribe_global(editor_focused).detach();
-    cx.subscribe_global(editor_blurred).detach();
-    cx.subscribe_global(editor_released).detach();
+    cx.subscribe_global(focused).detach();
+    cx.subscribe_global(blurred).detach();
+    cx.subscribe_global(released).detach();
 }
 
-fn editor_created(EditorCreated(editor): &EditorCreated, cx: &mut MutableAppContext) {
-    cx.update_default_global(|vim: &mut Vim, cx| {
-        vim.editors.insert(editor.id(), editor.downgrade());
-        vim.sync_vim_settings(cx);
-    })
-}
-
-fn editor_focused(EditorFocused(editor): &EditorFocused, cx: &mut MutableAppContext) {
+fn focused(EditorFocused(editor): &EditorFocused, cx: &mut MutableAppContext) {
     Vim::update(cx, |vim, cx| {
+        if let Some(previously_active_editor) = vim
+            .active_editor
+            .as_ref()
+            .and_then(|editor| editor.upgrade(cx))
+        {
+            vim.unhook_vim_settings(previously_active_editor, cx);
+        }
+
         vim.active_editor = Some(editor.downgrade());
-        vim.selection_subscription = Some(cx.subscribe(editor, |editor, event, cx| {
+        dbg!("Active editor changed", editor.read(cx).mode());
+        vim.editor_subscription = Some(cx.subscribe(editor, |editor, event, cx| {
             if editor.read(cx).leader_replica_id().is_none() {
                 if let editor::Event::SelectionsChanged { local: true } = event {
                     let newest_empty = editor.read(cx).selections.newest::<usize>(cx).is_empty();
-                    editor_local_selections_changed(newest_empty, cx);
+                    local_selections_changed(newest_empty, cx);
                 }
             }
         }));
 
-        if !vim.enabled {
-            return;
-        }
-
-        let editor = editor.read(cx);
-        let editor_mode = editor.mode();
-        let newest_selection_empty = editor.selections.newest::<usize>(cx).is_empty();
+        if vim.enabled {
+            let editor = editor.read(cx);
+            let editor_mode = editor.mode();
+            let newest_selection_empty = editor.selections.newest::<usize>(cx).is_empty();
 
-        if editor_mode == EditorMode::Full && !newest_selection_empty {
-            vim.switch_mode(Mode::Visual { line: false }, true, cx);
+            if editor_mode == EditorMode::Full && !newest_selection_empty {
+                vim.switch_mode(Mode::Visual { line: false }, true, cx);
+            }
         }
+
+        vim.sync_vim_settings(cx);
     });
 }
 
-fn editor_blurred(EditorBlurred(editor): &EditorBlurred, cx: &mut MutableAppContext) {
+fn blurred(EditorBlurred(editor): &EditorBlurred, cx: &mut MutableAppContext) {
     Vim::update(cx, |vim, cx| {
         if let Some(previous_editor) = vim.active_editor.clone() {
             if previous_editor == editor.clone() {
                 vim.active_editor = None;
             }
         }
-        vim.sync_vim_settings(cx);
+        vim.unhook_vim_settings(editor.clone(), cx);
     })
 }
 
-fn editor_released(EditorReleased(editor): &EditorReleased, cx: &mut MutableAppContext) {
+fn released(EditorReleased(editor): &EditorReleased, cx: &mut MutableAppContext) {
     cx.update_default_global(|vim: &mut Vim, _| {
-        vim.editors.remove(&editor.id());
         if let Some(previous_editor) = vim.active_editor.clone() {
             if previous_editor == editor.clone() {
                 vim.active_editor = None;
@@ -65,7 +65,7 @@ fn editor_released(EditorReleased(editor): &EditorReleased, cx: &mut MutableAppC
     });
 }
 
-fn editor_local_selections_changed(newest_empty: bool, cx: &mut MutableAppContext) {
+fn local_selections_changed(newest_empty: bool, cx: &mut MutableAppContext) {
     Vim::update(cx, |vim, cx| {
         if vim.enabled && vim.state.mode == Mode::Normal && !newest_empty {
             vim.switch_mode(Mode::Visual { line: false }, false, cx)

crates/vim/src/vim.rs 🔗

@@ -10,13 +10,12 @@ mod state;
 mod utils;
 mod visual;
 
-use collections::HashMap;
 use command_palette::CommandPaletteFilter;
 use editor::{Bias, Cancel, Editor, EditorMode};
 use gpui::{
     impl_actions,
     keymap_matcher::{KeyPressed, Keystroke},
-    MutableAppContext, Subscription, ViewContext, WeakViewHandle,
+    MutableAppContext, Subscription, ViewContext, ViewHandle, WeakViewHandle,
 };
 use language::CursorShape;
 use motion::Motion;
@@ -117,9 +116,8 @@ pub fn observe_keypresses(window_id: usize, cx: &mut MutableAppContext) {
 
 #[derive(Default)]
 pub struct Vim {
-    editors: HashMap<usize, WeakViewHandle<Editor>>,
     active_editor: Option<WeakViewHandle<Editor>>,
-    selection_subscription: Option<Subscription>,
+    editor_subscription: Option<Subscription>,
 
     enabled: bool,
     state: VimState,
@@ -160,24 +158,27 @@ impl Vim {
         }
 
         // Adjust selections
-        for editor in self.editors.values() {
-            if let Some(editor) = editor.upgrade(cx) {
-                editor.update(cx, |editor, cx| {
-                    editor.change_selections(None, cx, |s| {
-                        s.move_with(|map, selection| {
-                            if self.state.empty_selections_only() {
-                                let new_head = map.clip_point(selection.head(), Bias::Left);
-                                selection.collapse_to(new_head, selection.goal)
-                            } else {
-                                selection.set_head(
-                                    map.clip_point(selection.head(), Bias::Left),
-                                    selection.goal,
-                                );
-                            }
-                        });
-                    })
+        if let Some(editor) = self
+            .active_editor
+            .as_ref()
+            .and_then(|editor| editor.upgrade(cx))
+        {
+            editor.update(cx, |editor, cx| {
+                dbg!(&mode, editor.mode());
+                editor.change_selections(None, cx, |s| {
+                    s.move_with(|map, selection| {
+                        if self.state.empty_selections_only() {
+                            let new_head = map.clip_point(selection.head(), Bias::Left);
+                            selection.collapse_to(new_head, selection.goal)
+                        } else {
+                            selection.set_head(
+                                map.clip_point(selection.head(), Bias::Left),
+                                selection.goal,
+                            );
+                        }
+                    });
                 })
-            }
+            })
         }
     }
 
@@ -264,26 +265,33 @@ impl Vim {
             }
         });
 
-        for editor in self.editors.values() {
-            if let Some(editor) = editor.upgrade(cx) {
+        if let Some(editor) = self
+            .active_editor
+            .as_ref()
+            .and_then(|editor| editor.upgrade(cx))
+        {
+            if self.enabled && editor.read(cx).mode() == EditorMode::Full {
                 editor.update(cx, |editor, cx| {
-                    if self.enabled && editor.mode() == EditorMode::Full {
-                        editor.set_cursor_shape(cursor_shape, cx);
-                        editor.set_clip_at_line_ends(state.clip_at_line_end(), cx);
-                        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);
-                    } else {
-                        editor.set_cursor_shape(CursorShape::Bar, cx);
-                        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.set_cursor_shape(cursor_shape, cx);
+                    editor.set_clip_at_line_ends(state.clip_at_line_end(), cx);
+                    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);
                 });
+            } else {
+                self.unhook_vim_settings(editor, cx);
             }
         }
     }
+
+    fn unhook_vim_settings(&self, editor: ViewHandle<Editor>, cx: &mut MutableAppContext) {
+        editor.update(cx, |editor, cx| {
+            editor.set_cursor_shape(CursorShape::Bar, cx);
+            editor.set_clip_at_line_ends(false, cx);
+            editor.set_input_enabled(true);
+            editor.selections.line_mode = false;
+            editor.remove_keymap_context_layer::<Self>();
+        });
+    }
 }