vim: Fix bug where `d . .` freezes the editor (#42145)

AidanV and dino created 3 weeks ago

This bug seems to be caused by pushing an operator (i.e. `d`) followed
by a repeat (i.e. `.`) so the recording includes the push operator and
the repeat. When this is repeated (i.e. `.`) it causes an infinite loop.

This change fixes this bug by pushing a ClearOperator action if there is
an ongoing recording when repeat is called.

Release Notes:

- Fixed bug where pressing `d . .` in Vim mode would freeze the editor.

---------

Co-authored-by: dino <[e-mail]>

Change summary

crates/vim/src/normal/repeat.rs                    | 100 +++++++++++++++
crates/vim/src/state.rs                            |   3 
crates/vim/src/vim.rs                              |   5 
crates/vim/test_data/test_repeat_clear_count.json  |  21 +++
crates/vim/test_data/test_repeat_clear_repeat.json |   8 +
5 files changed, 134 insertions(+), 3 deletions(-)

Detailed changes

crates/vim/src/normal/repeat.rs

@@ -230,8 +230,19 @@ impl Vim {
         window: &mut Window,
         cx: &mut Context<Self>,
     ) {
-        let count = Vim::take_count(cx);
+        if self.active_operator().is_some() {
+            Vim::update_globals(cx, |globals, _| {
+                globals.recording_actions.clear();
+                globals.recording_count = None;
+                globals.dot_recording = false;
+                globals.stop_recording_after_next_action = false;
+            });
+            self.clear_operator(window, cx);
+            return;
+        }
+
         Vim::take_forced_motion(cx);
+        let count = Vim::take_count(cx);
 
         let Some((mut actions, selection, mode)) = Vim::update_globals(cx, |globals, _| {
             let actions = globals.recorded_actions.clone();
@@ -810,4 +821,91 @@ mod test {
         cx.simulate_shared_keystrokes("@ b").await;
         cx.shared_state().await.assert_eq("aaaaaaabbbˇd");
     }
+
+    #[gpui::test]
+    async fn test_repeat_clear(cx: &mut gpui::TestAppContext) {
+        let mut cx = VimTestContext::new(cx, true).await;
+
+        // Check that, when repeat is preceded by something other than a number,
+        // the current operator is cleared, in order to prevent infinite loops.
+        cx.set_state("ˇhello world", Mode::Normal);
+        cx.simulate_keystrokes("d .");
+        assert_eq!(cx.active_operator(), None);
+    }
+
+    #[gpui::test]
+    async fn test_repeat_clear_repeat(cx: &mut gpui::TestAppContext) {
+        let mut cx = NeovimBackedTestContext::new(cx).await;
+
+        cx.set_shared_state(indoc! {
+            "ˇthe quick brown
+            fox jumps over
+            the lazy dog"
+        })
+        .await;
+        cx.simulate_shared_keystrokes("d d").await;
+        cx.shared_state().await.assert_eq(indoc! {
+            "ˇfox jumps over
+            the lazy dog"
+        });
+        cx.simulate_shared_keystrokes("d . .").await;
+        cx.shared_state().await.assert_eq(indoc! {
+            "ˇthe lazy dog"
+        });
+    }
+
+    #[gpui::test]
+    async fn test_repeat_clear_count(cx: &mut gpui::TestAppContext) {
+        let mut cx = NeovimBackedTestContext::new(cx).await;
+
+        cx.set_shared_state(indoc! {
+            "ˇthe quick brown
+            fox jumps over
+            the lazy dog"
+        })
+        .await;
+        cx.simulate_shared_keystrokes("d d").await;
+        cx.shared_state().await.assert_eq(indoc! {
+            "ˇfox jumps over
+            the lazy dog"
+        });
+        cx.simulate_shared_keystrokes("2 d .").await;
+        cx.shared_state().await.assert_eq(indoc! {
+            "ˇfox jumps over
+            the lazy dog"
+        });
+        cx.simulate_shared_keystrokes(".").await;
+        cx.shared_state().await.assert_eq(indoc! {
+            "ˇthe lazy dog"
+        });
+
+        cx.set_shared_state(indoc! {
+            "ˇthe quick brown
+            fox jumps over
+            the lazy dog
+            the quick brown
+            fox jumps over
+            the lazy dog"
+        })
+        .await;
+        cx.simulate_shared_keystrokes("2 d d").await;
+        cx.shared_state().await.assert_eq(indoc! {
+            "ˇthe lazy dog
+            the quick brown
+            fox jumps over
+            the lazy dog"
+        });
+        cx.simulate_shared_keystrokes("5 d .").await;
+        cx.shared_state().await.assert_eq(indoc! {
+            "ˇthe lazy dog
+            the quick brown
+            fox jumps over
+            the lazy dog"
+        });
+        cx.simulate_shared_keystrokes(".").await;
+        cx.shared_state().await.assert_eq(indoc! {
+            "ˇfox jumps over
+            the lazy dog"
+        });
+    }
 }

crates/vim/src/state.rs

@@ -217,6 +217,7 @@ pub struct VimGlobals {
     pub forced_motion: bool,
     pub stop_recording_after_next_action: bool,
     pub ignore_current_insertion: bool,
+    pub recording_count: Option<usize>,
     pub recorded_count: Option<usize>,
     pub recording_actions: Vec<ReplayableAction>,
     pub recorded_actions: Vec<ReplayableAction>,
@@ -898,6 +899,7 @@ impl VimGlobals {
             if self.stop_recording_after_next_action {
                 self.dot_recording = false;
                 self.recorded_actions = std::mem::take(&mut self.recording_actions);
+                self.recorded_count = self.recording_count.take();
                 self.stop_recording_after_next_action = false;
             }
         }
@@ -924,6 +926,7 @@ impl VimGlobals {
             if self.stop_recording_after_next_action {
                 self.dot_recording = false;
                 self.recorded_actions = std::mem::take(&mut self.recording_actions);
+                self.recorded_count = self.recording_count.take();
                 self.stop_recording_after_next_action = false;
             }
         }

crates/vim/src/vim.rs

@@ -1258,7 +1258,7 @@ impl Vim {
         };
 
         if global_state.dot_recording {
-            global_state.recorded_count = count;
+            global_state.recording_count = count;
         }
         count
     }
@@ -1516,7 +1516,7 @@ impl Vim {
             if !globals.dot_replaying {
                 globals.dot_recording = true;
                 globals.recording_actions = Default::default();
-                globals.recorded_count = None;
+                globals.recording_count = None;
 
                 let selections = self.editor().map(|editor| {
                     editor.update(cx, |editor, cx| {
@@ -1586,6 +1586,7 @@ impl Vim {
                 .recording_actions
                 .push(ReplayableAction::Action(action.boxed_clone()));
             globals.recorded_actions = mem::take(&mut globals.recording_actions);
+            globals.recorded_count = globals.recording_count.take();
             globals.dot_recording = false;
             globals.stop_recording_after_next_action = false;
         }

crates/vim/test_data/test_repeat_clear_count.json

@@ -0,0 +1,21 @@
+{"Put":{"state":"ˇthe quick brown\nfox jumps over\nthe lazy dog"}}
+{"Key":"d"}
+{"Key":"d"}
+{"Get":{"state":"ˇfox jumps over\nthe lazy dog","mode":"Normal"}}
+{"Key":"2"}
+{"Key":"d"}
+{"Key":"."}
+{"Get":{"state":"ˇfox jumps over\nthe lazy dog","mode":"Normal"}}
+{"Key":"."}
+{"Get":{"state":"ˇthe lazy dog","mode":"Normal"}}
+{"Put":{"state":"ˇthe quick brown\nfox jumps over\nthe lazy dog\nthe quick brown\nfox jumps over\nthe lazy dog"}}
+{"Key":"2"}
+{"Key":"d"}
+{"Key":"d"}
+{"Get":{"state":"ˇthe lazy dog\nthe quick brown\nfox jumps over\nthe lazy dog","mode":"Normal"}}
+{"Key":"5"}
+{"Key":"d"}
+{"Key":"."}
+{"Get":{"state":"ˇthe lazy dog\nthe quick brown\nfox jumps over\nthe lazy dog","mode":"Normal"}}
+{"Key":"."}
+{"Get":{"state":"ˇfox jumps over\nthe lazy dog","mode":"Normal"}}

crates/vim/test_data/test_repeat_clear_repeat.json

@@ -0,0 +1,8 @@
+{"Put":{"state":"ˇthe quick brown\nfox jumps over\nthe lazy dog"}}
+{"Key":"d"}
+{"Key":"d"}
+{"Get":{"state":"ˇfox jumps over\nthe lazy dog","mode":"Normal"}}
+{"Key":"d"}
+{"Key":"."}
+{"Key":"."}
+{"Get":{"state":"ˇthe lazy dog","mode":"Normal"}}