vim: Change approach to fixing vim's temporary mode bug (#42894)

Dino created

The `Vim.exit_temporary_normal` method had been updated
(https://github.com/zed-industries/zed/pull/42742) to expect and
`Option<&Motion>` that would then be used to determine whether to move
the cursor right in case the motion was `Some(EndOfLine { ..})`.
Unfortunately this meant that all callers now had to provide this
argument, even if just `None`.

After merging those changes I remember that we could probably play
around with `clip_at_line_ends` so this commit removes those intial
changes in favor of updating the `vim::normal::Vim.move_cursor` method
so that, if vim is in temporary mode and `EndOfLine` is used, it
disables clipping at line ends so that the newline character can be
selected.

Closes [#42278](https://github.com/zed-industries/zed/issues/42278)

Release Notes:

- N/A

Change summary

crates/editor/src/editor.rs     |  4 +++
crates/vim/src/normal.rs        | 43 +++++++++++++++++-----------------
crates/vim/src/normal/scroll.rs |  2 
crates/vim/src/normal/yank.rs   |  4 +-
4 files changed, 28 insertions(+), 25 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -3028,6 +3028,10 @@ impl Editor {
         range.clone()
     }
 
+    pub fn clip_at_line_ends(&mut self, cx: &mut Context<Self>) -> bool {
+        self.display_map.read(cx).clip_at_line_ends
+    }
+
     pub fn set_clip_at_line_ends(&mut self, clip: bool, cx: &mut Context<Self>) {
         if self.display_map.read(cx).clip_at_line_ends != clip {
             self.display_map

crates/vim/src/normal.rs 🔗

@@ -386,8 +386,6 @@ 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),
@@ -477,7 +475,7 @@ impl Vim {
             }
         }
         // Exit temporary normal mode (if active).
-        self.exit_temporary_normal(Some(&temp_mode_motion), window, cx);
+        self.exit_temporary_normal(window, cx);
     }
 
     pub fn normal_object(
@@ -580,8 +578,21 @@ impl Vim {
         window: &mut Window,
         cx: &mut Context<Self>,
     ) {
-        self.update_editor(cx, |_, editor, cx| {
+        self.update_editor(cx, |vim, editor, cx| {
             let text_layout_details = editor.text_layout_details(window);
+
+            // If vim is in temporary mode and the motion being used is
+            // `EndOfLine` ($), we'll want to disable clipping at line ends so
+            // that the newline character can be selected so that, when moving
+            // back to visual mode, the cursor will be placed after the last
+            // character and not before it.
+            let clip_at_line_ends = editor.clip_at_line_ends(cx);
+            let should_disable_clip = matches!(motion, Motion::EndOfLine { .. }) && vim.temp_mode;
+
+            if should_disable_clip {
+                editor.set_clip_at_line_ends(false, cx)
+            };
+
             editor.change_selections(
                 SelectionEffects::default().nav_history(motion.push_to_jump_list()),
                 window,
@@ -593,7 +604,11 @@ impl Vim {
                             .unwrap_or((cursor, goal))
                     })
                 },
-            )
+            );
+
+            if should_disable_clip {
+                editor.set_clip_at_line_ends(clip_at_line_ends, cx);
+            };
         });
     }
 
@@ -1054,25 +1069,9 @@ impl Vim {
         });
     }
 
-    /// 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>,
-    ) {
+    fn exit_temporary_normal(&mut self, 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);
-            }
         }
     }
 }

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(None, window, cx);
+        self.exit_temporary_normal(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(None, window, cx);
+        self.exit_temporary_normal(window, cx);
     }
 
     pub fn yank_object(
@@ -90,7 +90,7 @@ impl Vim {
                 });
             });
         });
-        self.exit_temporary_normal(None, window, cx);
+        self.exit_temporary_normal(window, cx);
     }
 
     pub fn yank_selections_content(