Fully disable vim mode on start unless it's enabled

Nathan Sobo created

Also: Make some structural adjustments to remove the need for defer. Instead of accessing the global in associated VimState functions, have a single method that allows us to call update instance methods.

Change summary

crates/gpui/src/app.rs          | 26 ++++++---
crates/vim/src/editor_events.rs | 18 +++---
crates/vim/src/insert.rs        | 14 +++--
crates/vim/src/normal.rs        | 34 ++++++++-----
crates/vim/src/vim.rs           | 90 +++++++++++++++++-----------------
5 files changed, 99 insertions(+), 83 deletions(-)

Detailed changes

crates/gpui/src/app.rs 🔗

@@ -782,7 +782,7 @@ type GlobalActionCallback = dyn FnMut(&dyn AnyAction, &mut MutableAppContext);
 type SubscriptionCallback = Box<dyn FnMut(&dyn Any, &mut MutableAppContext) -> bool>;
 type GlobalSubscriptionCallback = Box<dyn FnMut(&dyn Any, &mut MutableAppContext)>;
 type ObservationCallback = Box<dyn FnMut(&mut MutableAppContext) -> bool>;
-type GlobalObservationCallback = Box<dyn FnMut(&mut MutableAppContext)>;
+type GlobalObservationCallback = Box<dyn FnMut(&dyn Any, &mut MutableAppContext)>;
 type ReleaseObservationCallback = Box<dyn FnMut(&dyn Any, &mut MutableAppContext)>;
 
 pub struct MutableAppContext {
@@ -1222,10 +1222,10 @@ impl MutableAppContext {
         }
     }
 
-    pub fn observe_global<G, F>(&mut self, observe: F) -> Subscription
+    pub fn observe_global<G, F>(&mut self, mut observe: F) -> Subscription
     where
         G: Any,
-        F: 'static + FnMut(&mut MutableAppContext),
+        F: 'static + FnMut(&G, &mut MutableAppContext),
     {
         let type_id = TypeId::of::<G>();
         let id = post_inc(&mut self.next_subscription_id);
@@ -1234,7 +1234,14 @@ impl MutableAppContext {
             .lock()
             .entry(type_id)
             .or_default()
-            .insert(id, Some(Box::new(observe)));
+            .insert(
+                id,
+                Some(
+                    Box::new(move |global: &dyn Any, cx: &mut MutableAppContext| {
+                        observe(global.downcast_ref().unwrap(), cx)
+                    }) as GlobalObservationCallback,
+                ),
+            );
 
         Subscription::GlobalObservation {
             id,
@@ -2075,10 +2082,10 @@ impl MutableAppContext {
     fn notify_global_observers(&mut self, observed_type_id: TypeId) {
         let callbacks = self.global_observations.lock().remove(&observed_type_id);
         if let Some(callbacks) = callbacks {
-            if self.cx.globals.contains_key(&observed_type_id) {
+            if let Some(global) = self.cx.globals.remove(&observed_type_id) {
                 for (id, callback) in callbacks {
                     if let Some(mut callback) = callback {
-                        callback(self);
+                        callback(global.as_ref(), self);
                         match self
                             .global_observations
                             .lock()
@@ -2095,6 +2102,7 @@ impl MutableAppContext {
                         }
                     }
                 }
+                self.cx.globals.insert(observed_type_id, global);
             }
         }
     }
@@ -5232,7 +5240,7 @@ mod tests {
         let observation_count = Rc::new(RefCell::new(0));
         let subscription = cx.observe_global::<Global, _>({
             let observation_count = observation_count.clone();
-            move |_| {
+            move |_, _| {
                 *observation_count.borrow_mut() += 1;
             }
         });
@@ -5262,7 +5270,7 @@ mod tests {
         let observation_count = Rc::new(RefCell::new(0));
         cx.observe_global::<OtherGlobal, _>({
             let observation_count = observation_count.clone();
-            move |_| {
+            move |_, _| {
                 *observation_count.borrow_mut() += 1;
             }
         })
@@ -5636,7 +5644,7 @@ mod tests {
         *subscription.borrow_mut() = Some(cx.observe_global::<(), _>({
             let observation_count = observation_count.clone();
             let subscription = subscription.clone();
-            move |_| {
+            move |_, _| {
                 subscription.borrow_mut().take();
                 *observation_count.borrow_mut() += 1;
             }

crates/vim/src/editor_events.rs 🔗

@@ -13,7 +13,7 @@ pub fn init(cx: &mut MutableAppContext) {
 fn editor_created(EditorCreated(editor): &EditorCreated, cx: &mut MutableAppContext) {
     cx.update_default_global(|vim_state: &mut VimState, cx| {
         vim_state.editors.insert(editor.id(), editor.downgrade());
-        VimState::sync_editor_options(cx);
+        vim_state.sync_editor_options(cx);
     })
 }
 
@@ -24,21 +24,21 @@ fn editor_focused(EditorFocused(editor): &EditorFocused, cx: &mut MutableAppCont
         Mode::Normal
     };
 
-    cx.update_default_global(|vim_state: &mut VimState, _| {
-        vim_state.active_editor = Some(editor.downgrade());
+    VimState::update_global(cx, |state, cx| {
+        state.active_editor = Some(editor.downgrade());
+        state.switch_mode(&SwitchMode(mode), cx);
     });
-    VimState::switch_mode(&SwitchMode(mode), cx);
 }
 
 fn editor_blurred(EditorBlurred(editor): &EditorBlurred, cx: &mut MutableAppContext) {
-    cx.update_default_global(|vim_state: &mut VimState, _| {
-        if let Some(previous_editor) = vim_state.active_editor.clone() {
+    VimState::update_global(cx, |state, cx| {
+        if let Some(previous_editor) = state.active_editor.clone() {
             if previous_editor == editor.clone() {
-                vim_state.active_editor = None;
+                state.active_editor = None;
             }
         }
-    });
-    VimState::sync_editor_options(cx);
+        state.sync_editor_options(cx);
+    })
 }
 
 fn editor_released(EditorReleased(editor): &EditorReleased, cx: &mut MutableAppContext) {

crates/vim/src/insert.rs 🔗

@@ -18,11 +18,13 @@ pub fn init(cx: &mut MutableAppContext) {
 }
 
 fn normal_before(_: &mut Workspace, _: &NormalBefore, cx: &mut ViewContext<Workspace>) {
-    VimState::update_active_editor(cx, |editor, cx| {
-        editor.move_cursors(cx, |map, mut cursor, _| {
-            *cursor.column_mut() = cursor.column().saturating_sub(1);
-            (map.clip_point(cursor, Bias::Left), SelectionGoal::None)
+    VimState::update_global(cx, |state, cx| {
+        state.update_active_editor(cx, |editor, cx| {
+            editor.move_cursors(cx, |map, mut cursor, _| {
+                *cursor.column_mut() = cursor.column().saturating_sub(1);
+                (map.clip_point(cursor, Bias::Left), SelectionGoal::None)
+            });
         });
-    });
-    VimState::switch_mode(&SwitchMode(Mode::Normal), cx);
+        state.switch_mode(&SwitchMode(Mode::Normal), cx);
+    })
 }

crates/vim/src/normal.rs 🔗

@@ -28,31 +28,39 @@ pub fn init(cx: &mut MutableAppContext) {
 }
 
 fn move_left(_: &mut Workspace, _: &MoveLeft, cx: &mut ViewContext<Workspace>) {
-    VimState::update_active_editor(cx, |editor, cx| {
-        editor.move_cursors(cx, |map, mut cursor, _| {
-            *cursor.column_mut() = cursor.column().saturating_sub(1);
-            (map.clip_point(cursor, Bias::Left), SelectionGoal::None)
+    VimState::update_global(cx, |state, cx| {
+        state.update_active_editor(cx, |editor, cx| {
+            editor.move_cursors(cx, |map, mut cursor, _| {
+                *cursor.column_mut() = cursor.column().saturating_sub(1);
+                (map.clip_point(cursor, Bias::Left), SelectionGoal::None)
+            });
         });
-    });
+    })
 }
 
 fn move_down(_: &mut Workspace, _: &MoveDown, cx: &mut ViewContext<Workspace>) {
-    VimState::update_active_editor(cx, |editor, cx| {
-        editor.move_cursors(cx, movement::down);
+    VimState::update_global(cx, |state, cx| {
+        state.update_active_editor(cx, |editor, cx| {
+            editor.move_cursors(cx, movement::down);
+        });
     });
 }
 
 fn move_up(_: &mut Workspace, _: &MoveUp, cx: &mut ViewContext<Workspace>) {
-    VimState::update_active_editor(cx, |editor, cx| {
-        editor.move_cursors(cx, movement::up);
+    VimState::update_global(cx, |state, cx| {
+        state.update_active_editor(cx, |editor, cx| {
+            editor.move_cursors(cx, movement::up);
+        });
     });
 }
 
 fn move_right(_: &mut Workspace, _: &MoveRight, cx: &mut ViewContext<Workspace>) {
-    VimState::update_active_editor(cx, |editor, cx| {
-        editor.move_cursors(cx, |map, mut cursor, _| {
-            *cursor.column_mut() += 1;
-            (map.clip_point(cursor, Bias::Right), SelectionGoal::None)
+    VimState::update_global(cx, |state, cx| {
+        state.update_active_editor(cx, |editor, cx| {
+            editor.move_cursors(cx, |map, mut cursor, _| {
+                *cursor.column_mut() += 1;
+                (map.clip_point(cursor, Bias::Right), SelectionGoal::None)
+            });
         });
     });
 }

crates/vim/src/vim.rs 🔗

@@ -19,10 +19,14 @@ pub fn init(cx: &mut MutableAppContext) {
     insert::init(cx);
     normal::init(cx);
 
-    cx.add_action(|_: &mut Workspace, action: &SwitchMode, cx| VimState::switch_mode(action, cx));
+    cx.add_action(|_: &mut Workspace, action: &SwitchMode, cx| {
+        VimState::update_global(cx, |state, cx| state.switch_mode(action, cx))
+    });
 
-    cx.observe_global::<Settings, _>(VimState::settings_changed)
-        .detach();
+    cx.observe_global::<Settings, _>(|settings, cx| {
+        VimState::update_global(cx, |state, cx| state.set_enabled(settings.vim_mode, cx))
+    })
+    .detach();
 }
 
 #[derive(Default)]
@@ -35,62 +39,56 @@ pub struct VimState {
 }
 
 impl VimState {
+    fn update_global<F, S>(cx: &mut MutableAppContext, update: F) -> S
+    where
+        F: FnOnce(&mut Self, &mut MutableAppContext) -> S,
+    {
+        cx.update_default_global(update)
+    }
+
     fn update_active_editor<S>(
+        &self,
         cx: &mut MutableAppContext,
         update: impl FnOnce(&mut Editor, &mut ViewContext<Editor>) -> S,
     ) -> Option<S> {
-        cx.global::<Self>()
-            .active_editor
+        self.active_editor
             .clone()
             .and_then(|ae| ae.upgrade(cx))
             .map(|ae| ae.update(cx, update))
     }
 
-    fn switch_mode(SwitchMode(mode): &SwitchMode, cx: &mut MutableAppContext) {
-        cx.update_default_global(|this: &mut Self, _| {
-            this.mode = *mode;
-        });
-
-        VimState::sync_editor_options(cx);
+    fn switch_mode(&mut self, SwitchMode(mode): &SwitchMode, cx: &mut MutableAppContext) {
+        self.mode = *mode;
+        self.sync_editor_options(cx);
     }
 
-    fn settings_changed(cx: &mut MutableAppContext) {
-        cx.update_default_global(|this: &mut Self, cx| {
-            let settings = cx.global::<Settings>();
-            if this.enabled != settings.vim_mode {
-                this.enabled = settings.vim_mode;
-                this.mode = if settings.vim_mode {
-                    Mode::Normal
-                } else {
-                    Mode::Insert
-                };
-                Self::sync_editor_options(cx);
-            }
-        });
+    fn set_enabled(&mut self, enabled: bool, cx: &mut MutableAppContext) {
+        if self.enabled != enabled {
+            self.enabled = enabled;
+            self.sync_editor_options(cx);
+        }
     }
 
-    fn sync_editor_options(cx: &mut MutableAppContext) {
-        cx.defer(move |cx| {
-            cx.update_default_global(|this: &mut VimState, cx| {
-                let mode = this.mode;
-                let cursor_shape = mode.cursor_shape();
-                let keymap_layer_active = this.enabled;
-                for editor in this.editors.values() {
-                    if let Some(editor) = editor.upgrade(cx) {
-                        editor.update(cx, |editor, cx| {
-                            editor.set_cursor_shape(cursor_shape, cx);
-                            editor.set_clip_at_line_ends(cursor_shape == CursorShape::Block, cx);
-                            editor.set_input_enabled(mode == Mode::Insert);
-                            if keymap_layer_active {
-                                let context_layer = mode.keymap_context_layer();
-                                editor.set_keymap_context_layer::<Self>(context_layer);
-                            } else {
-                                editor.remove_keymap_context_layer::<Self>();
-                            }
-                        });
+    fn sync_editor_options(&self, cx: &mut MutableAppContext) {
+        let mode = self.mode;
+        let cursor_shape = mode.cursor_shape();
+        for editor in self.editors.values() {
+            if let Some(editor) = editor.upgrade(cx) {
+                editor.update(cx, |editor, cx| {
+                    if self.enabled {
+                        editor.set_cursor_shape(cursor_shape, cx);
+                        editor.set_clip_at_line_ends(cursor_shape == CursorShape::Block, cx);
+                        editor.set_input_enabled(mode == Mode::Insert);
+                        let context_layer = mode.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.remove_keymap_context_layer::<Self>();
                     }
-                }
-            });
-        });
+                });
+            }
+        }
     }
 }