vim lifecycle (#7647)

Conrad Irwin created

Release Notes:

- Fixed :0 and :% in vim mode
([#4303](https://github.com/zed-industries/zed/issues/4303)).
- Improved keymap loading to not load vim key bindings unless vim is
enabled

**or**

- N/A

Change summary

assets/keymaps/vim.json                 |   2 
crates/vim/src/editor_events.rs         |  33 +++--
crates/vim/src/mode_indicator.rs        |  11 -
crates/vim/src/state.rs                 |   1 
crates/vim/src/test/vim_test_context.rs |   4 
crates/vim/src/vim.rs                   | 158 +++++++++++---------------
crates/zed/src/zed.rs                   |  12 +
7 files changed, 103 insertions(+), 118 deletions(-)

Detailed changes

assets/keymaps/vim.json 🔗

@@ -498,7 +498,7 @@
     }
   },
   {
-    "context": "BufferSearchBar && !in_replace > VimEnabled",
+    "context": "BufferSearchBar && !in_replace",
     "bindings": {
       "enter": "vim::SearchSubmit",
       "escape": "buffer_search::Dismiss"

crates/vim/src/editor_events.rs 🔗

@@ -1,6 +1,7 @@
-use crate::{insert::NormalBefore, Vim};
+use crate::{insert::NormalBefore, Vim, VimModeSetting};
 use editor::{Editor, EditorEvent};
 use gpui::{Action, AppContext, Entity, EntityId, View, ViewContext, WindowContext};
+use settings::{Settings, SettingsStore};
 
 pub fn init(cx: &mut AppContext) {
     cx.observe_new_views(|_, cx: &mut ViewContext<Editor>| {
@@ -12,6 +13,17 @@ pub fn init(cx: &mut AppContext) {
         })
         .detach();
 
+        let mut enabled = VimModeSetting::get_global(cx).0;
+        cx.observe_global::<SettingsStore>(move |editor, cx| {
+            if VimModeSetting::get_global(cx).0 != enabled {
+                enabled = VimModeSetting::get_global(cx).0;
+                if !enabled {
+                    Vim::unhook_vim_settings(editor, cx);
+                }
+            }
+        })
+        .detach();
+
         let id = cx.view().entity_id();
         cx.on_release(move |_, _, cx| released(id, cx)).detach();
     })
@@ -19,34 +31,25 @@ pub fn init(cx: &mut AppContext) {
 }
 
 fn focused(editor: View<Editor>, cx: &mut WindowContext) {
-    if Vim::read(cx).active_editor.clone().is_some() {
-        Vim::update(cx, |vim, cx| {
-            vim.update_active_editor(cx, |previously_active_editor, cx| {
-                vim.unhook_vim_settings(previously_active_editor, cx)
-            });
-        });
-    }
-
     Vim::update(cx, |vim, cx| {
-        vim.set_active_editor(editor.clone(), cx);
+        if !vim.enabled {
+            return;
+        }
+        vim.activate_editor(editor.clone(), cx);
     });
 }
 
 fn blurred(editor: View<Editor>, cx: &mut WindowContext) {
     Vim::update(cx, |vim, cx| {
-        vim.stop_recording_immediately(NormalBefore.boxed_clone());
         if let Some(previous_editor) = vim.active_editor.clone() {
+            vim.stop_recording_immediately(NormalBefore.boxed_clone());
             if previous_editor
                 .upgrade()
                 .is_some_and(|previous| previous == editor.clone())
             {
                 vim.clear_operator(cx);
-                vim.active_editor = None;
-                vim.editor_subscription = None;
             }
         }
-
-        editor.update(cx, |editor, cx| vim.unhook_vim_settings(editor, cx))
     });
 }
 

crates/vim/src/mode_indicator.rs 🔗

@@ -1,5 +1,4 @@
 use gpui::{div, Element, Render, Subscription, ViewContext};
-use settings::SettingsStore;
 use workspace::{item::ItemHandle, ui::prelude::*, StatusItemView};
 
 use crate::{state::Mode, Vim};
@@ -7,20 +6,16 @@ use crate::{state::Mode, Vim};
 /// The ModeIndicator displays the current mode in the status bar.
 pub struct ModeIndicator {
     pub(crate) mode: Option<Mode>,
-    _subscriptions: Vec<Subscription>,
+    _subscription: Subscription,
 }
 
 impl ModeIndicator {
     /// Construct a new mode indicator in this window.
     pub fn new(cx: &mut ViewContext<Self>) -> Self {
-        let _subscriptions = vec![
-            cx.observe_global::<Vim>(|this, cx| this.update_mode(cx)),
-            cx.observe_global::<SettingsStore>(|this, cx| this.update_mode(cx)),
-        ];
-
+        let _subscription = cx.observe_global::<Vim>(|this, cx| this.update_mode(cx));
         let mut this = Self {
             mode: None,
-            _subscriptions,
+            _subscription,
         };
         this.update_mode(cx);
         this

crates/vim/src/state.rs 🔗

@@ -169,7 +169,6 @@ impl EditorState {
 
     pub fn keymap_context_layer(&self) -> KeyContext {
         let mut context = KeyContext::default();
-        context.add("VimEnabled");
         context.set(
             "vim_mode",
             match self.mode {

crates/vim/src/test/vim_test_context.rs 🔗

@@ -49,7 +49,9 @@ impl VimTestContext {
                 store.update_user_settings::<VimModeSetting>(cx, |s| *s = Some(enabled));
             });
             settings::KeymapFile::load_asset("keymaps/default.json", cx).unwrap();
-            settings::KeymapFile::load_asset("keymaps/vim.json", cx).unwrap();
+            if enabled {
+                settings::KeymapFile::load_asset("keymaps/vim.json", cx).unwrap();
+            }
         });
 
         // Setup search toolbars and keypress hook

crates/vim/src/vim.rs 🔗

@@ -20,8 +20,8 @@ use command_palette::CommandPaletteInterceptor;
 use copilot::CommandPaletteFilter;
 use editor::{movement, Editor, EditorEvent, EditorMode};
 use gpui::{
-    actions, impl_actions, Action, AppContext, EntityId, Global, KeyContext, Subscription, View,
-    ViewContext, WeakView, WindowContext,
+    actions, impl_actions, Action, AppContext, EntityId, Global, Subscription, View, ViewContext,
+    WeakView, WindowContext,
 };
 use language::{CursorShape, Point, Selection, SelectionGoal};
 pub use mode_indicator::ModeIndicator;
@@ -197,7 +197,11 @@ impl Vim {
         cx.update_global(update)
     }
 
-    fn set_active_editor(&mut self, editor: View<Editor>, cx: &mut WindowContext) {
+    fn activate_editor(&mut self, editor: View<Editor>, cx: &mut WindowContext) {
+        if editor.read(cx).mode() != EditorMode::Full {
+            return;
+        }
+
         self.active_editor = Some(editor.clone().downgrade());
         self.editor_subscription = Some(cx.subscribe(&editor, |editor, event, cx| match event {
             EditorEvent::SelectionsChanged { local: true } => {
@@ -219,19 +223,17 @@ impl Vim {
             _ => {}
         }));
 
-        if self.enabled {
-            let editor = editor.read(cx);
-            let editor_mode = editor.mode();
-            let newest_selection_empty = editor.selections.newest::<usize>(cx).is_empty();
+        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
+        if editor_mode == EditorMode::Full
                 && !newest_selection_empty
                 && self.state().mode == Mode::Normal
                 // When following someone, don't switch vim mode.
                 && editor.leader_peer_id().is_none()
-            {
-                self.switch_mode(Mode::Visual, true, cx);
-            }
+        {
+            self.switch_mode(Mode::Visual, true, cx);
         }
 
         self.sync_vim_settings(cx);
@@ -504,43 +506,39 @@ impl Vim {
     }
 
     fn set_enabled(&mut self, enabled: bool, cx: &mut AppContext) {
-        if self.enabled != enabled {
-            self.enabled = enabled;
-
+        if self.enabled == enabled {
+            return;
+        }
+        if !enabled {
+            let _ = cx.remove_global::<CommandPaletteInterceptor>();
             cx.update_global::<CommandPaletteFilter, _>(|filter, _| {
-                if self.enabled {
-                    filter.hidden_namespaces.remove("vim");
-                } else {
-                    filter.hidden_namespaces.insert("vim");
-                }
+                filter.hidden_namespaces.insert("vim");
             });
+            *self = Default::default();
+            return;
+        }
 
-            if self.enabled {
-                cx.set_global::<CommandPaletteInterceptor>(CommandPaletteInterceptor(Box::new(
-                    command::command_interceptor,
-                )));
-            } else if cx.has_global::<CommandPaletteInterceptor>() {
-                let _ = cx.remove_global::<CommandPaletteInterceptor>();
-            }
+        self.enabled = true;
+        cx.update_global::<CommandPaletteFilter, _>(|filter, _| {
+            filter.hidden_namespaces.remove("vim");
+        });
+        cx.set_global::<CommandPaletteInterceptor>(CommandPaletteInterceptor(Box::new(
+            command::command_interceptor,
+        )));
 
-            if let Some(active_window) = cx.active_window() {
-                active_window
-                    .update(cx, |root_view, cx| {
-                        if self.enabled {
-                            let active_editor = root_view
-                                .downcast::<Workspace>()
-                                .ok()
-                                .and_then(|workspace| workspace.read(cx).active_item(cx))
-                                .and_then(|item| item.downcast::<Editor>());
-                            if let Some(active_editor) = active_editor {
-                                self.set_active_editor(active_editor, cx);
-                            }
-                            self.switch_mode(Mode::Normal, false, cx);
-                        }
-                        self.sync_vim_settings(cx);
-                    })
-                    .ok();
-            }
+        if let Some(active_window) = cx
+            .active_window()
+            .and_then(|window| window.downcast::<Workspace>())
+        {
+            active_window
+                .update(cx, |workspace, cx| {
+                    let active_editor = workspace.active_item_as::<Editor>(cx);
+                    if let Some(active_editor) = active_editor {
+                        self.activate_editor(active_editor, cx);
+                        self.switch_mode(Mode::Normal, false, cx);
+                    }
+                })
+                .ok();
         }
     }
 
@@ -569,45 +567,29 @@ impl Vim {
 
     fn sync_vim_settings(&self, cx: &mut WindowContext) {
         let state = self.state();
-        let cursor_shape = state.cursor_shape();
 
         self.update_active_editor(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_ends(), cx);
-                editor.set_collapse_matches(true);
-                editor.set_input_enabled(!state.vim_controlled());
-                editor.set_autoindent(state.should_autoindent());
-                editor.selections.line_mode = matches!(state.mode, Mode::VisualLine);
-                let context_layer = state.keymap_context_layer();
-                editor.set_keymap_context_layer::<Self>(context_layer, cx);
-            } else {
-                // Note: set_collapse_matches is not in unhook_vim_settings, as that method is called on blur,
-                // but we need collapse_matches to persist when the search bar is focused.
-                editor.set_collapse_matches(false);
-                self.unhook_vim_settings(editor, cx);
-            }
+            editor.set_cursor_shape(state.cursor_shape(), cx);
+            editor.set_clip_at_line_ends(state.clip_at_line_ends(), cx);
+            editor.set_collapse_matches(true);
+            editor.set_input_enabled(!state.vim_controlled());
+            editor.set_autoindent(state.should_autoindent());
+            editor.selections.line_mode = matches!(state.mode, Mode::VisualLine);
+            let context_layer = state.keymap_context_layer();
+            editor.set_keymap_context_layer::<Self>(context_layer, cx);
         });
     }
 
-    fn unhook_vim_settings(&self, editor: &mut Editor, cx: &mut ViewContext<Editor>) {
-        editor.set_cursor_shape(CursorShape::Bar, cx);
-        editor.set_clip_at_line_ends(false, cx);
-        editor.set_input_enabled(true);
-        editor.set_autoindent(true);
-        editor.selections.line_mode = false;
-
-        // we set the VimEnabled context on all editors so that we
-        // can distinguish between vim mode and non-vim mode in the BufferSearchBar.
-        // This is a bit of a hack, but currently the search crate does not depend on vim,
-        // and it seems nice to keep it that way.
-        if self.enabled {
-            let mut context = KeyContext::default();
-            context.add("VimEnabled");
-            editor.set_keymap_context_layer::<Self>(context, cx)
-        } else {
-            editor.remove_keymap_context_layer::<Self>(cx);
+    fn unhook_vim_settings(editor: &mut Editor, cx: &mut ViewContext<Editor>) {
+        if editor.mode() == EditorMode::Full {
+            editor.set_cursor_shape(CursorShape::Bar, cx);
+            editor.set_clip_at_line_ends(false, cx);
+            editor.set_collapse_matches(false);
+            editor.set_input_enabled(true);
+            editor.set_autoindent(true);
+            editor.selections.line_mode = false;
         }
+        editor.remove_keymap_context_layer::<Self>(cx)
     }
 }
 
@@ -633,19 +615,17 @@ fn local_selections_changed(
     cx: &mut WindowContext,
 ) {
     Vim::update(cx, |vim, cx| {
-        if vim.enabled {
-            if vim.state().mode == Mode::Normal && !newest.is_empty() {
-                if matches!(newest.goal, SelectionGoal::HorizontalRange { .. }) {
-                    vim.switch_mode(Mode::VisualBlock, false, cx);
-                } else {
-                    vim.switch_mode(Mode::Visual, false, cx)
-                }
-            } else if newest.is_empty()
-                && !is_multicursor
-                && [Mode::Visual, Mode::VisualLine, Mode::VisualBlock].contains(&vim.state().mode)
-            {
-                vim.switch_mode(Mode::Normal, true, cx)
+        if vim.state().mode == Mode::Normal && !newest.is_empty() {
+            if matches!(newest.goal, SelectionGoal::HorizontalRange { .. }) {
+                vim.switch_mode(Mode::VisualBlock, false, cx);
+            } else {
+                vim.switch_mode(Mode::Visual, false, cx)
             }
+        } else if newest.is_empty()
+            && !is_multicursor
+            && [Mode::Visual, Mode::VisualLine, Mode::VisualBlock].contains(&vim.state().mode)
+        {
+            vim.switch_mode(Mode::Normal, true, cx)
         }
     })
 }

crates/zed/src/zed.rs 🔗

@@ -32,6 +32,7 @@ use util::{
     ResultExt,
 };
 use uuid::Uuid;
+use vim::VimModeSetting;
 use welcome::BaseKeymap;
 use workspace::Pane;
 use workspace::{
@@ -495,13 +496,17 @@ pub fn handle_keymap_file_changes(
     cx: &mut AppContext,
 ) {
     BaseKeymap::register(cx);
+    VimModeSetting::register(cx);
 
     let (base_keymap_tx, mut base_keymap_rx) = mpsc::unbounded();
     let mut old_base_keymap = *BaseKeymap::get_global(cx);
+    let mut old_vim_enabled = VimModeSetting::get_global(cx).0;
     cx.observe_global::<SettingsStore>(move |cx| {
         let new_base_keymap = *BaseKeymap::get_global(cx);
-        if new_base_keymap != old_base_keymap {
+        let new_vim_enabled = VimModeSetting::get_global(cx).0;
+        if new_base_keymap != old_base_keymap || new_vim_enabled != old_vim_enabled {
             old_base_keymap = new_base_keymap.clone();
+            old_vim_enabled = new_vim_enabled;
             base_keymap_tx.unbounded_send(()).unwrap();
         }
     })
@@ -538,8 +543,9 @@ fn reload_keymaps(cx: &mut AppContext, keymap_content: &KeymapFile) {
 }
 
 pub fn load_default_keymap(cx: &mut AppContext) {
-    for path in ["keymaps/default.json", "keymaps/vim.json"] {
-        KeymapFile::load_asset(path, cx).unwrap();
+    KeymapFile::load_asset("keymaps/default.json", cx).unwrap();
+    if VimModeSetting::get_global(cx).0 {
+        KeymapFile::load_asset("keymaps/vim.json", cx).unwrap();
     }
 
     if let Some(asset_path) = BaseKeymap::get_global(cx).asset_path() {