vim: Fix bug where repeat operator could lead to unrecoverable replaying state (#46376)

Dino and neel created 3 days ago

When a recorded action moves focus away from the editor (e.g.,
`buffer_search::Deploy`), the `EndRepeat` action handler is not invoked
because is node is no longer on the dispatch path. This left
`dot_replaying` set to `true`, causing subsequent repeats to malfunction
and the `VimGlobals.pre_count` value to never be reset.

Reset `dot_replaying` as a fail-safe when the replayer exhausts its
action queue, ensuring the state is always cleaned up regardless of
whether `EndRepeat` was handled.

Release Notes:

- Fixed vim repeat (`.`) breaking when the recorded action moves focus
away from the editor

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

Change summary

crates/vim/src/normal/repeat.rs | 51 ++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+), 1 deletion(-)

Detailed changes

crates/vim/src/normal/repeat.rs

@@ -145,7 +145,13 @@ impl Replayer {
         lock.ix += 1;
         drop(lock);
         let Some(action) = action else {
-            Vim::globals(cx).replayer.take();
+            // The `globals.dot_replaying = false` is a fail-safe to ensure that
+            // this value is always reset, in the case that the focus is moved
+            // away from the editor, effectively preventing the `EndRepeat`
+            // action from being handled.
+            let globals = Vim::globals(cx);
+            globals.replayer.take();
+            globals.dot_replaying = false;
             return;
         };
         match action {
@@ -396,6 +402,7 @@ mod test {
     use gpui::EntityInputHandler;
 
     use crate::{
+        VimGlobals,
         state::Mode,
         test::{NeovimBackedTestContext, VimTestContext},
     };
@@ -739,6 +746,48 @@ mod test {
         cx.shared_state().await.assert_eq("ˇx hello\n");
     }
 
+    #[gpui::test]
+    async fn test_repeat_after_blur_resets_dot_replaying(cx: &mut gpui::TestAppContext) {
+        let mut cx = VimTestContext::new(cx, true).await;
+
+        // Bind `ctrl-f` to the `buffer_search::Deploy` action so that this can
+        // be triggered while in Insert mode, ensuring that an action which
+        // moves the focus away from the editor, gets recorded.
+        cx.update(|_, cx| {
+            cx.bind_keys([gpui::KeyBinding::new(
+                "ctrl-f",
+                search::buffer_search::Deploy::find(),
+                None,
+            )])
+        });
+
+        cx.set_state("ˇhello", Mode::Normal);
+
+        // We're going to enter insert mode, which will start recording, type a
+        // character and then immediately use `ctrl-f` to trigger the buffer
+        // search. Triggering the buffer search will move focus away from the
+        // editor, effectively stopping the recording immediately after
+        // `buffer_search::Deploy` is recorded. The first `escape` is used to
+        // dismiss the search bar, while the second is used to move from Insert
+        // to Normal mode.
+        cx.simulate_keystrokes("i x ctrl-f escape escape");
+        cx.run_until_parked();
+
+        // Using the `.` key will dispatch the `vim::Repeat` action, repeating
+        // the set of recorded actions. This will eventually focus on the search
+        // bar, preventing the `EndRepeat` action from being correctly handled.
+        cx.simulate_keystrokes(".");
+        cx.run_until_parked();
+
+        // After replay finishes, even though the `EndRepeat` action wasn't
+        // handled, seeing as the editor lost focus during replay, the
+        // `dot_replaying` value should be set back to `false`.
+        assert!(
+            !cx.update(|_, cx| cx.global::<VimGlobals>().dot_replaying),
+            "dot_replaying should be false after repeat completes"
+        );
+    }
+
     #[gpui::test]
     async fn test_undo_repeated_insert(cx: &mut gpui::TestAppContext) {
         let mut cx = NeovimBackedTestContext::new(cx).await;