vim: Fix temporary mode exit on end of line (#42742)

Dino created 1 month ago

When using the end of line motion ($) while in temporary mode, the
cursor would be placed in insert mode just before the last character
instead of after, just like in NeoVim.

This happens because `EndOfLine` kind of assumes that we're in `Normal`
mode and simply places the cursor in the last character instead of the
newline character.

This commit moves the cursor one position to the right when exiting
temporary mode and the motion used was `Motion::EndOfLine`

- Update `vim::normal::Vim.exit_temporary_normal` to now accept a
`Option<&Motion>` argument, in case callers want this new logic to
potentially be applied

Closes #42278 

Release Notes:

- Fixed temporary mode exit when using `$` to move to the end of the
line

Change summary

crates/vim/src/normal.rs                          | 53 ++++++++++++++++
crates/vim/src/normal/scroll.rs                   |  2 
crates/vim/src/normal/yank.rs                     |  4 
crates/vim/src/test/neovim_backed_test_context.rs |  1 
crates/vim/test_data/test_temporary_mode.json     | 27 ++++++++
5 files changed, 82 insertions(+), 5 deletions(-)

Detailed changes

crates/vim/src/normal.rs

@@ -386,6 +386,8 @@ impl Vim {
         window: &mut Window,
         cx: &mut Context<Self>,
     ) {
+        let temp_mode_motion = motion.clone();
+
         match operator {
             None => self.move_cursor(motion, times, window, cx),
             Some(Operator::Change) => self.change_motion(motion, times, forced_motion, window, cx),
@@ -475,7 +477,7 @@ impl Vim {
             }
         }
         // Exit temporary normal mode (if active).
-        self.exit_temporary_normal(window, cx);
+        self.exit_temporary_normal(Some(&temp_mode_motion), window, cx);
     }
 
     pub fn normal_object(
@@ -1052,9 +1054,25 @@ impl Vim {
         });
     }
 
-    fn exit_temporary_normal(&mut self, window: &mut Window, cx: &mut Context<Self>) {
+    /// If temporary mode is enabled, switches back to insert mode, using the
+    /// provided `motion` to determine whether to move the cursor before
+    /// re-enabling insert mode, for example, when `EndOfLine` ($) is used.
+    fn exit_temporary_normal(
+        &mut self,
+        motion: Option<&Motion>,
+        window: &mut Window,
+        cx: &mut Context<Self>,
+    ) {
         if self.temp_mode {
             self.switch_mode(Mode::Insert, true, window, cx);
+
+            // Since we're switching from `Normal` mode to `Insert` mode, we'll
+            // move the cursor one position to the right, to ensure that, for
+            // motions like `EndOfLine` ($), the cursor is actually at the end
+            // of line and not on the last character.
+            if matches!(motion, Some(Motion::EndOfLine { .. })) {
+                self.move_cursor(Motion::Right, Some(1), window, cx);
+            }
         }
     }
 }
@@ -2269,4 +2287,35 @@ mod test {
             assert_eq!(workspace.active_pane().read(cx).active_item_index(), 1);
         });
     }
+
+    #[gpui::test]
+    async fn test_temporary_mode(cx: &mut gpui::TestAppContext) {
+        let mut cx = NeovimBackedTestContext::new(cx).await;
+
+        // Test jumping to the end of the line ($).
+        cx.set_shared_state(indoc! {"lorem ˇipsum"}).await;
+        cx.simulate_shared_keystrokes("i").await;
+        cx.shared_state().await.assert_matches();
+        cx.simulate_shared_keystrokes("ctrl-o $").await;
+        cx.shared_state().await.assert_eq(indoc! {"lorem ipsumˇ"});
+
+        // Test jumping to the next word.
+        cx.set_shared_state(indoc! {"loremˇ ipsum dolor"}).await;
+        cx.simulate_shared_keystrokes("a").await;
+        cx.shared_state().await.assert_matches();
+        cx.simulate_shared_keystrokes("a n d space ctrl-o w").await;
+        cx.shared_state()
+            .await
+            .assert_eq(indoc! {"lorem and ipsum ˇdolor"});
+
+        // Test yanking to end of line ($).
+        cx.set_shared_state(indoc! {"lorem ˇipsum dolor"}).await;
+        cx.simulate_shared_keystrokes("i").await;
+        cx.shared_state().await.assert_matches();
+        cx.simulate_shared_keystrokes("a n d space ctrl-o y $")
+            .await;
+        cx.shared_state()
+            .await
+            .assert_eq(indoc! {"lorem and ˇipsum dolor"});
+    }
 }

crates/vim/src/normal/scroll.rs

@@ -96,7 +96,7 @@ impl Vim {
     ) {
         let amount = by(Vim::take_count(cx).map(|c| c as f32));
         Vim::take_forced_motion(cx);
-        self.exit_temporary_normal(window, cx);
+        self.exit_temporary_normal(None, window, cx);
         self.update_editor(cx, |_, editor, cx| {
             scroll_editor(editor, move_cursor, amount, window, cx)
         });

crates/vim/src/normal/yank.rs

@@ -59,7 +59,7 @@ impl Vim {
                 });
             });
         });
-        self.exit_temporary_normal(window, cx);
+        self.exit_temporary_normal(None, window, cx);
     }
 
     pub fn yank_object(
@@ -90,7 +90,7 @@ impl Vim {
                 });
             });
         });
-        self.exit_temporary_normal(window, cx);
+        self.exit_temporary_normal(None, window, cx);
     }
 
     pub fn yank_selections_content(

crates/vim/src/test/neovim_backed_test_context.rs

@@ -31,6 +31,7 @@ pub struct SharedState {
 }
 
 impl SharedState {
+    /// Assert that both Zed and NeoVim have the same content and mode.
     #[track_caller]
     pub fn assert_matches(&self) {
         if self.neovim != self.editor || self.neovim_mode != self.editor_mode {

crates/vim/test_data/test_temporary_mode.json

@@ -0,0 +1,27 @@
+{"Put":{"state":"lorem ˇipsum"}}
+{"Key":"i"}
+{"Get":{"state":"lorem ˇipsum","mode":"Insert"}}
+{"Key":"ctrl-o"}
+{"Key":"$"}
+{"Get":{"state":"lorem ipsumˇ","mode":"Insert"}}
+{"Put":{"state":"loremˇ ipsum dolor"}}
+{"Key":"a"}
+{"Get":{"state":"lorem ˇipsum dolor","mode":"Insert"}}
+{"Key":"a"}
+{"Key":"n"}
+{"Key":"d"}
+{"Key":"space"}
+{"Key":"ctrl-o"}
+{"Key":"w"}
+{"Get":{"state":"lorem and ipsum ˇdolor","mode":"Insert"}}
+{"Put":{"state":"lorem ˇipsum dolor"}}
+{"Key":"i"}
+{"Get":{"state":"lorem ˇipsum dolor","mode":"Insert"}}
+{"Key":"a"}
+{"Key":"n"}
+{"Key":"d"}
+{"Key":"space"}
+{"Key":"ctrl-o"}
+{"Key":"y"}
+{"Key":"$"}
+{"Get":{"state":"lorem and ˇipsum dolor","mode":"Insert"}}