From 30e31f6561bd49f634d1aa38584dc1be425a3af5 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sat, 26 Mar 2022 19:18:48 -0600 Subject: [PATCH 1/3] Test that vim mode can be disabled on startup --- crates/vim/src/vim_tests.rs | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/crates/vim/src/vim_tests.rs b/crates/vim/src/vim_tests.rs index e35ed9a122..051ff21ce7 100644 --- a/crates/vim/src/vim_tests.rs +++ b/crates/vim/src/vim_tests.rs @@ -11,7 +11,7 @@ use crate::*; #[gpui::test] async fn test_insert_mode(cx: &mut gpui::TestAppContext) { - let mut cx = VimTestAppContext::new(cx, "").await; + let mut cx = VimTestAppContext::new(cx, true, "").await; cx.simulate_keystroke("i"); assert_eq!(cx.mode(), Mode::Insert); cx.simulate_keystrokes(&["T", "e", "s", "t"]); @@ -23,7 +23,7 @@ async fn test_insert_mode(cx: &mut gpui::TestAppContext) { #[gpui::test] async fn test_normal_hjkl(cx: &mut gpui::TestAppContext) { - let mut cx = VimTestAppContext::new(cx, "Test\nTestTest\nTest").await; + let mut cx = VimTestAppContext::new(cx, true, "Test\nTestTest\nTest").await; cx.simulate_keystroke("l"); cx.assert_newest_selection_head(indoc! {" T|est @@ -81,15 +81,17 @@ async fn test_normal_hjkl(cx: &mut gpui::TestAppContext) { #[gpui::test] async fn test_toggle_through_settings(cx: &mut gpui::TestAppContext) { - let mut cx = VimTestAppContext::new(cx, "").await; + let mut cx = VimTestAppContext::new(cx, true, "").await; + + cx.simulate_keystroke("i"); + assert_eq!(cx.mode(), Mode::Insert); // Editor acts as though vim is disabled cx.disable_vim(); - assert_eq!(cx.mode(), Mode::Insert); cx.simulate_keystrokes(&["h", "j", "k", "l"]); cx.assert_newest_selection_head("hjkl|"); - // Enabling dynamically sets vim mode again + // Enabling dynamically sets vim mode again and restores normal mode cx.enable_vim(); assert_eq!(cx.mode(), Mode::Normal); cx.simulate_keystrokes(&["h", "h", "h", "l"]); @@ -106,6 +108,13 @@ async fn test_toggle_through_settings(cx: &mut gpui::TestAppContext) { assert_eq!(cx.mode(), Mode::Normal); } +#[gpui::test] +async fn test_initially_disabled(cx: &mut gpui::TestAppContext) { + let mut cx = VimTestAppContext::new(cx, false, "").await; + cx.simulate_keystrokes(&["h", "j", "k", "l"]); + cx.assert_newest_selection_head("hjkl|"); +} + struct VimTestAppContext<'a> { cx: &'a mut gpui::TestAppContext, window_id: usize, @@ -115,6 +124,7 @@ struct VimTestAppContext<'a> { impl<'a> VimTestAppContext<'a> { async fn new( cx: &'a mut gpui::TestAppContext, + enabled: bool, initial_editor_text: &str, ) -> VimTestAppContext<'a> { cx.update(|cx| { @@ -125,7 +135,7 @@ impl<'a> VimTestAppContext<'a> { cx.update(|cx| { cx.update_global(|settings: &mut Settings, _| { - settings.vim_mode = true; + settings.vim_mode = enabled; }); }); From daf999c3bec39718e4986373410b41192aad20c6 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sat, 26 Mar 2022 14:30:55 -0600 Subject: [PATCH 2/3] Fully disable vim mode on start unless it's enabled 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. --- 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(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index a291ad3c4b..7f0c3ffcde 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -782,7 +782,7 @@ type GlobalActionCallback = dyn FnMut(&dyn AnyAction, &mut MutableAppContext); type SubscriptionCallback = Box bool>; type GlobalSubscriptionCallback = Box; type ObservationCallback = Box bool>; -type GlobalObservationCallback = Box; +type GlobalObservationCallback = Box; type ReleaseObservationCallback = Box; pub struct MutableAppContext { @@ -1222,10 +1222,10 @@ impl MutableAppContext { } } - pub fn observe_global(&mut self, observe: F) -> Subscription + pub fn observe_global(&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::(); 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::({ 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::({ 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; } diff --git a/crates/vim/src/editor_events.rs b/crates/vim/src/editor_events.rs index 2f8a60ef33..7e49b473f9 100644 --- a/crates/vim/src/editor_events.rs +++ b/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) { diff --git a/crates/vim/src/insert.rs b/crates/vim/src/insert.rs index 50fd53b37b..c027ff2c1f 100644 --- a/crates/vim/src/insert.rs +++ b/crates/vim/src/insert.rs @@ -18,11 +18,13 @@ pub fn init(cx: &mut MutableAppContext) { } fn normal_before(_: &mut Workspace, _: &NormalBefore, cx: &mut ViewContext) { - 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); + }) } diff --git a/crates/vim/src/normal.rs b/crates/vim/src/normal.rs index 87218b3f5f..232a76a030 100644 --- a/crates/vim/src/normal.rs +++ b/crates/vim/src/normal.rs @@ -28,31 +28,39 @@ pub fn init(cx: &mut MutableAppContext) { } fn move_left(_: &mut Workspace, _: &MoveLeft, cx: &mut ViewContext) { - 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) { - 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) { - 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) { - 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) + }); }); }); } diff --git a/crates/vim/src/vim.rs b/crates/vim/src/vim.rs index 68ecd4e003..c6905a9c7a 100644 --- a/crates/vim/src/vim.rs +++ b/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::(VimState::settings_changed) - .detach(); + cx.observe_global::(|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(cx: &mut MutableAppContext, update: F) -> S + where + F: FnOnce(&mut Self, &mut MutableAppContext) -> S, + { + cx.update_default_global(update) + } + fn update_active_editor( + &self, cx: &mut MutableAppContext, update: impl FnOnce(&mut Editor, &mut ViewContext) -> S, ) -> Option { - cx.global::() - .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::(); - 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::(context_layer); - } else { - editor.remove_keymap_context_layer::(); - } - }); + 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::(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::(); } - } - }); - }); + }); + } + } } } From c6ad667d492ce8a17ed7580e8abcb6963b37262a Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sat, 26 Mar 2022 18:38:00 -0600 Subject: [PATCH 3/3] Assign normal mode when re-enabling --- crates/vim/src/vim.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/vim/src/vim.rs b/crates/vim/src/vim.rs index c6905a9c7a..26f7e24cf2 100644 --- a/crates/vim/src/vim.rs +++ b/crates/vim/src/vim.rs @@ -65,6 +65,9 @@ impl VimState { fn set_enabled(&mut self, enabled: bool, cx: &mut MutableAppContext) { if self.enabled != enabled { self.enabled = enabled; + if enabled { + self.mode = Mode::Normal; + } self.sync_editor_options(cx); } }