Store some vim state per-editor

Conrad Irwin created

This fixes a bug where opening and closing command would reset your
selection incorrectly.

Change summary

crates/vim/src/editor_events.rs         |  6 +
crates/vim/src/mode_indicator.rs        |  4 
crates/vim/src/motion.rs                |  4 
crates/vim/src/normal.rs                |  4 
crates/vim/src/normal/case.rs           |  2 
crates/vim/src/normal/search.rs         | 11 +-
crates/vim/src/normal/substitute.rs     |  2 
crates/vim/src/object.rs                |  2 
crates/vim/src/state.rs                 | 12 ++-
crates/vim/src/test/vim_test_context.rs |  4 
crates/vim/src/vim.rs                   | 82 +++++++++++++++++++-------
crates/vim/src/visual.rs                | 21 +++++-
12 files changed, 106 insertions(+), 48 deletions(-)

Detailed changes

crates/vim/src/editor_events.rs 🔗

@@ -1,4 +1,4 @@
-use crate::Vim;
+use crate::{Vim, VimEvent};
 use editor::{EditorBlurred, EditorFocused, EditorReleased};
 use gpui::AppContext;
 
@@ -22,6 +22,9 @@ fn focused(EditorFocused(editor): &EditorFocused, cx: &mut AppContext) {
     editor.window().update(cx, |cx| {
         Vim::update(cx, |vim, cx| {
             vim.set_active_editor(editor.clone(), cx);
+            cx.emit_global(VimEvent::ModeChanged {
+                mode: vim.state().mode,
+            });
         });
     });
 }
@@ -48,6 +51,7 @@ fn released(EditorReleased(editor): &EditorReleased, cx: &mut AppContext) {
                     vim.active_editor = None;
                 }
             }
+            vim.editor_states.remove(&editor.id())
         });
     });
 }

crates/vim/src/mode_indicator.rs 🔗

@@ -34,7 +34,7 @@ impl ModeIndicator {
             if settings::get::<VimModeSetting>(cx).0 {
                 mode_indicator.mode = cx
                     .has_global::<Vim>()
-                    .then(|| cx.global::<Vim>().state.mode);
+                    .then(|| cx.global::<Vim>().state().mode);
             } else {
                 mode_indicator.mode.take();
             }
@@ -46,7 +46,7 @@ impl ModeIndicator {
             .has_global::<Vim>()
             .then(|| {
                 let vim = cx.global::<Vim>();
-                vim.enabled.then(|| vim.state.mode)
+                vim.enabled.then(|| vim.state().mode)
             })
             .flatten();
 

crates/vim/src/motion.rs 🔗

@@ -147,7 +147,7 @@ pub(crate) fn motion(motion: Motion, cx: &mut WindowContext) {
 
     let times = Vim::update(cx, |vim, cx| vim.pop_number_operator(cx));
     let operator = Vim::read(cx).active_operator();
-    match Vim::read(cx).state.mode {
+    match Vim::read(cx).state().mode {
         Mode::Normal => normal_motion(motion, operator, times, cx),
         Mode::Visual | Mode::VisualLine | Mode::VisualBlock => visual_motion(motion, times, cx),
         Mode::Insert => {
@@ -158,7 +158,7 @@ pub(crate) fn motion(motion: Motion, cx: &mut WindowContext) {
 }
 
 fn repeat_motion(backwards: bool, cx: &mut WindowContext) {
-    let find = match Vim::read(cx).state.last_find.clone() {
+    let find = match Vim::read(cx).workspace_state.last_find.clone() {
         Some(Motion::FindForward { before, text }) => {
             if backwards {
                 Motion::FindBackward {

crates/vim/src/normal.rs 🔗

@@ -116,8 +116,8 @@ pub fn normal_motion(
 
 pub fn normal_object(object: Object, cx: &mut WindowContext) {
     Vim::update(cx, |vim, cx| {
-        match vim.state.operator_stack.pop() {
-            Some(Operator::Object { around }) => match vim.state.operator_stack.pop() {
+        match vim.maybe_pop_operator() {
+            Some(Operator::Object { around }) => match vim.maybe_pop_operator() {
                 Some(Operator::Change) => change_object(vim, object, around, cx),
                 Some(Operator::Delete) => delete_object(vim, object, around, cx),
                 Some(Operator::Yank) => yank_object(vim, object, around, cx),

crates/vim/src/normal/case.rs 🔗

@@ -13,7 +13,7 @@ pub fn change_case(_: &mut Workspace, _: &ChangeCase, cx: &mut ViewContext<Works
             let mut cursor_positions = Vec::new();
             let snapshot = editor.buffer().read(cx).snapshot(cx);
             for selection in editor.selections.all::<Point>(cx) {
-                match vim.state.mode {
+                match vim.state().mode {
                     Mode::VisualLine => {
                         let start = Point::new(selection.start.row, 0);
                         let end =

crates/vim/src/normal/search.rs 🔗

@@ -70,10 +70,10 @@ fn search(workspace: &mut Workspace, action: &Search, cx: &mut ViewContext<Works
                             cx,
                         );
                     }
-                    vim.state.search = SearchState {
+                    vim.workspace_state.search = SearchState {
                         direction,
                         count,
-                        initial_query: query,
+                        initial_query: query.clone(),
                     };
                 });
             }
@@ -83,7 +83,7 @@ fn search(workspace: &mut Workspace, action: &Search, cx: &mut ViewContext<Works
 
 // hook into the existing to clear out any vim search state on cmd+f or edit -> find.
 fn search_deploy(_: &mut Pane, _: &buffer_search::Deploy, cx: &mut ViewContext<Pane>) {
-    Vim::update(cx, |vim, _| vim.state.search = Default::default());
+    Vim::update(cx, |vim, _| vim.workspace_state.search = Default::default());
     cx.propagate_action();
 }
 
@@ -93,8 +93,9 @@ fn search_submit(workspace: &mut Workspace, _: &SearchSubmit, cx: &mut ViewConte
         pane.update(cx, |pane, cx| {
             if let Some(search_bar) = pane.toolbar().read(cx).item_of_type::<BufferSearchBar>() {
                 search_bar.update(cx, |search_bar, cx| {
-                    let state = &mut vim.state.search;
+                    let state = &mut vim.workspace_state.search;
                     let mut count = state.count;
+                    let direction = state.direction;
 
                     // in the case that the query has changed, the search bar
                     // will have selected the next match already.
@@ -103,8 +104,8 @@ fn search_submit(workspace: &mut Workspace, _: &SearchSubmit, cx: &mut ViewConte
                     {
                         count = count.saturating_sub(1)
                     }
-                    search_bar.select_match(state.direction, count, cx);
                     state.count = 1;
+                    search_bar.select_match(direction, count, cx);
                     search_bar.focus_editor(&Default::default(), cx);
                 });
             }

crates/vim/src/normal/substitute.rs 🔗

@@ -4,7 +4,7 @@ use language::Point;
 use crate::{motion::Motion, utils::copy_selections_content, Mode, Vim};
 
 pub fn substitute(vim: &mut Vim, count: Option<usize>, cx: &mut WindowContext) {
-    let line_mode = vim.state.mode == Mode::VisualLine;
+    let line_mode = vim.state().mode == Mode::VisualLine;
     vim.switch_mode(Mode::Insert, true, cx);
     vim.update_active_editor(cx, |editor, cx| {
         editor.transact(cx, |editor, cx| {

crates/vim/src/object.rs 🔗

@@ -62,7 +62,7 @@ pub fn init(cx: &mut AppContext) {
 }
 
 fn object(object: Object, cx: &mut WindowContext) {
-    match Vim::read(cx).state.mode {
+    match Vim::read(cx).state().mode {
         Mode::Normal => normal_object(object, cx),
         Mode::Visual | Mode::VisualLine | Mode::VisualBlock => visual_object(object, cx),
         Mode::Insert => {

crates/vim/src/state.rs 🔗

@@ -41,16 +41,20 @@ pub enum Operator {
     FindBackward { after: bool },
 }
 
-#[derive(Default)]
-pub struct VimState {
+#[derive(Default, Clone)]
+pub struct EditorState {
     pub mode: Mode,
     pub last_mode: Mode,
     pub operator_stack: Vec<Operator>,
-    pub search: SearchState,
+}
 
+#[derive(Default, Clone)]
+pub struct WorkspaceState {
+    pub search: SearchState,
     pub last_find: Option<Motion>,
 }
 
+#[derive(Clone)]
 pub struct SearchState {
     pub direction: Direction,
     pub count: usize,
@@ -67,7 +71,7 @@ impl Default for SearchState {
     }
 }
 
-impl VimState {
+impl EditorState {
     pub fn cursor_shape(&self) -> CursorShape {
         match self.mode {
             Mode::Normal => {

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

@@ -76,12 +76,12 @@ impl<'a> VimTestContext<'a> {
     }
 
     pub fn mode(&mut self) -> Mode {
-        self.cx.read(|cx| cx.global::<Vim>().state.mode)
+        self.cx.read(|cx| cx.global::<Vim>().state().mode)
     }
 
     pub fn active_operator(&mut self) -> Option<Operator> {
         self.cx
-            .read(|cx| cx.global::<Vim>().state.operator_stack.last().copied())
+            .read(|cx| cx.global::<Vim>().state().operator_stack.last().copied())
     }
 
     pub fn set_state(&mut self, text: &str, mode: Mode) -> ContextHandle {

crates/vim/src/vim.rs 🔗

@@ -12,7 +12,7 @@ mod utils;
 mod visual;
 
 use anyhow::Result;
-use collections::CommandPaletteFilter;
+use collections::{CommandPaletteFilter, HashMap};
 use editor::{movement, Editor, EditorMode, Event};
 use gpui::{
     actions, impl_actions, keymap_matcher::KeymapContext, keymap_matcher::MatchResult, AppContext,
@@ -24,7 +24,7 @@ use motion::Motion;
 use normal::normal_replace;
 use serde::Deserialize;
 use settings::{Setting, SettingsStore};
-use state::{Mode, Operator, VimState};
+use state::{EditorState, Mode, Operator, WorkspaceState};
 use std::sync::Arc;
 use visual::{visual_block_motion, visual_replace};
 use workspace::{self, Workspace};
@@ -127,7 +127,9 @@ pub struct Vim {
     active_editor: Option<WeakViewHandle<Editor>>,
     editor_subscription: Option<Subscription>,
     enabled: bool,
-    state: VimState,
+    editor_states: HashMap<usize, EditorState>,
+    workspace_state: WorkspaceState,
+    default_state: EditorState,
 }
 
 impl Vim {
@@ -143,7 +145,7 @@ impl Vim {
     }
 
     fn set_active_editor(&mut self, editor: ViewHandle<Editor>, cx: &mut WindowContext) {
-        self.active_editor = Some(editor.downgrade());
+        self.active_editor = Some(editor.clone().downgrade());
         self.editor_subscription = Some(cx.subscribe(&editor, |editor, event, cx| match event {
             Event::SelectionsChanged { local: true } => {
                 let editor = editor.read(cx);
@@ -163,7 +165,10 @@ impl Vim {
             let editor_mode = editor.mode();
             let newest_selection_empty = editor.selections.newest::<usize>(cx).is_empty();
 
-            if editor_mode == EditorMode::Full && !newest_selection_empty {
+            if editor_mode == EditorMode::Full
+                && !newest_selection_empty
+                && self.state().mode == Mode::Normal
+            {
                 self.switch_mode(Mode::Visual, true, cx);
             }
         }
@@ -181,11 +186,14 @@ impl Vim {
     }
 
     fn switch_mode(&mut self, mode: Mode, leave_selections: bool, cx: &mut WindowContext) {
-        let last_mode = self.state.mode;
-        let prior_mode = self.state.last_mode;
-        self.state.last_mode = last_mode;
-        self.state.mode = mode;
-        self.state.operator_stack.clear();
+        let state = self.state();
+        let last_mode = state.mode;
+        let prior_mode = state.last_mode;
+        self.update_state(|state| {
+            state.last_mode = last_mode;
+            state.mode = mode;
+            state.operator_stack.clear();
+        });
 
         cx.emit_global(VimEvent::ModeChanged { mode });
 
@@ -207,7 +215,9 @@ impl Vim {
                 // we cheat with visual block mode and use multiple cursors.
                 // the cost of this cheat is we need to convert back to a single
                 // cursor whenever vim would.
-                if last_mode == Mode::VisualBlock && mode != Mode::VisualBlock {
+                if last_mode == Mode::VisualBlock
+                    && (mode != Mode::VisualBlock && mode != Mode::Insert)
+                {
                     let tail = s.oldest_anchor().tail();
                     let head = s.newest_anchor().head();
                     s.select_anchor_ranges(vec![tail..head]);
@@ -237,7 +247,7 @@ impl Vim {
     }
 
     fn push_operator(&mut self, operator: Operator, cx: &mut WindowContext) {
-        self.state.operator_stack.push(operator);
+        self.update_state(|state| state.operator_stack.push(operator));
         self.sync_vim_settings(cx);
     }
 
@@ -250,9 +260,13 @@ impl Vim {
         }
     }
 
+    fn maybe_pop_operator(&mut self) -> Option<Operator> {
+        self.update_state(|state| state.operator_stack.pop())
+    }
+
     fn pop_operator(&mut self, cx: &mut WindowContext) -> Operator {
-        let popped_operator = self.state.operator_stack.pop()
-            .expect("Operator popped when no operator was on the stack. This likely means there is an invalid keymap config");
+        let popped_operator = self.update_state( |state| state.operator_stack.pop()
+        )            .expect("Operator popped when no operator was on the stack. This likely means there is an invalid keymap config");
         self.sync_vim_settings(cx);
         popped_operator
     }
@@ -266,12 +280,12 @@ impl Vim {
     }
 
     fn clear_operator(&mut self, cx: &mut WindowContext) {
-        self.state.operator_stack.clear();
+        self.update_state(|state| state.operator_stack.clear());
         self.sync_vim_settings(cx);
     }
 
     fn active_operator(&self) -> Option<Operator> {
-        self.state.operator_stack.last().copied()
+        self.state().operator_stack.last().copied()
     }
 
     fn active_editor_input_ignored(text: Arc<str>, cx: &mut WindowContext) {
@@ -282,15 +296,19 @@ impl Vim {
         match Vim::read(cx).active_operator() {
             Some(Operator::FindForward { before }) => {
                 let find = Motion::FindForward { before, text };
-                Vim::update(cx, |vim, _| vim.state.last_find = Some(find.clone()));
+                Vim::update(cx, |vim, _| {
+                    vim.workspace_state.last_find = Some(find.clone())
+                });
                 motion::motion(find, cx)
             }
             Some(Operator::FindBackward { after }) => {
                 let find = Motion::FindBackward { after, text };
-                Vim::update(cx, |vim, _| vim.state.last_find = Some(find.clone()));
+                Vim::update(cx, |vim, _| {
+                    vim.workspace_state.last_find = Some(find.clone())
+                });
                 motion::motion(find, cx)
             }
-            Some(Operator::Replace) => match Vim::read(cx).state.mode {
+            Some(Operator::Replace) => match Vim::read(cx).state().mode {
                 Mode::Normal => normal_replace(text, cx),
                 Mode::Visual | Mode::VisualLine | Mode::VisualBlock => visual_replace(text, cx),
                 _ => Vim::update(cx, |vim, cx| vim.clear_operator(cx)),
@@ -302,7 +320,6 @@ impl Vim {
     fn set_enabled(&mut self, enabled: bool, cx: &mut AppContext) {
         if self.enabled != enabled {
             self.enabled = enabled;
-            self.state = Default::default();
 
             cx.update_default_global::<CommandPaletteFilter, _, _>(|filter, _| {
                 if self.enabled {
@@ -329,8 +346,29 @@ impl Vim {
         }
     }
 
+    pub fn state(&self) -> &EditorState {
+        if let Some(active_editor) = self.active_editor.as_ref() {
+            if let Some(state) = self.editor_states.get(&active_editor.id()) {
+                return state;
+            }
+        }
+
+        &self.default_state
+    }
+
+    pub fn update_state<T>(&mut self, func: impl FnOnce(&mut EditorState) -> T) -> T {
+        let mut state = self.state().clone();
+        let ret = func(&mut state);
+
+        if let Some(active_editor) = self.active_editor.as_ref() {
+            self.editor_states.insert(active_editor.id(), state);
+        }
+
+        ret
+    }
+
     fn sync_vim_settings(&self, cx: &mut WindowContext) {
-        let state = &self.state;
+        let state = self.state();
         let cursor_shape = state.cursor_shape();
 
         self.update_active_editor(cx, |editor, cx| {
@@ -391,7 +429,7 @@ impl Setting for VimModeSetting {
 
 fn local_selections_changed(newest_empty: bool, cx: &mut WindowContext) {
     Vim::update(cx, |vim, cx| {
-        if vim.enabled && vim.state.mode == Mode::Normal && !newest_empty {
+        if vim.enabled && vim.state().mode == Mode::Normal && !newest_empty {
             vim.switch_mode(Mode::Visual, false, cx)
         }
     })

crates/vim/src/visual.rs 🔗

@@ -53,7 +53,7 @@ pub fn init(cx: &mut AppContext) {
 pub fn visual_motion(motion: Motion, times: Option<usize>, cx: &mut WindowContext) {
     Vim::update(cx, |vim, cx| {
         vim.update_active_editor(cx, |editor, cx| {
-            if vim.state.mode == Mode::VisualBlock && !matches!(motion, Motion::EndOfLine) {
+            if vim.state().mode == Mode::VisualBlock && !matches!(motion, Motion::EndOfLine) {
                 let is_up_or_down = matches!(motion, Motion::Up | Motion::Down);
                 visual_block_motion(is_up_or_down, editor, cx, |map, point, goal| {
                     motion.move_point(map, point, goal, times)
@@ -85,7 +85,7 @@ pub fn visual_motion(motion: Motion, times: Option<usize>, cx: &mut WindowContex
 
                         // ensure the current character is included in the selection.
                         if !selection.reversed {
-                            let next_point = if vim.state.mode == Mode::VisualBlock {
+                            let next_point = if vim.state().mode == Mode::VisualBlock {
                                 movement::saturating_right(map, selection.end)
                             } else {
                                 movement::right(map, selection.end)
@@ -240,7 +240,7 @@ pub fn visual_object(object: Object, cx: &mut WindowContext) {
 
 fn toggle_mode(mode: Mode, cx: &mut ViewContext<Workspace>) {
     Vim::update(cx, |vim, cx| {
-        if vim.state.mode == mode {
+        if vim.state().mode == mode {
             vim.switch_mode(Mode::Normal, false, cx);
         } else {
             vim.switch_mode(mode, false, cx);
@@ -294,7 +294,7 @@ pub fn delete(_: &mut Workspace, _: &VisualDelete, cx: &mut ViewContext<Workspac
                         let cursor = map.clip_point(cursor.to_display_point(map), Bias::Left);
                         selection.collapse_to(cursor, selection.goal)
                     });
-                    if vim.state.mode == Mode::VisualBlock {
+                    if vim.state().mode == Mode::VisualBlock {
                         s.select_anchors(vec![s.first_anchor()])
                     }
                 });
@@ -313,7 +313,7 @@ pub fn yank(_: &mut Workspace, _: &VisualYank, cx: &mut ViewContext<Workspace>)
                 s.move_with(|_, selection| {
                     selection.collapse_to(selection.start, SelectionGoal::None)
                 });
-                if vim.state.mode == Mode::VisualBlock {
+                if vim.state().mode == Mode::VisualBlock {
                     s.select_anchors(vec![s.first_anchor()])
                 }
             });
@@ -971,4 +971,15 @@ mod test {
         })
         .await;
     }
+
+    #[gpui::test]
+    async fn test_mode_across_command(cx: &mut gpui::TestAppContext) {
+        let mut cx = VimTestContext::new(cx, true).await;
+
+        cx.set_state("aˇbc", Mode::Normal);
+        cx.simulate_keystrokes(["ctrl-v"]);
+        assert_eq!(cx.mode(), Mode::VisualBlock);
+        cx.simulate_keystrokes(["cmd-shift-p", "escape"]);
+        assert_eq!(cx.mode(), Mode::VisualBlock);
+    }
 }