vim: Fix cursor position being set to end of line in normal mode (#38161)

Dino and Conrad Irwin created 1 month ago

Address an issue where, in Vim mode, clicking past the end of a line
after selecting the entire line would place the cursor on the newline
character instead of the last character of the line, which is
inconsistent with Vim's normal mode expectations.

I believe the root cause was that the cursor’s position was updated to
the end of the line before the mode switch from Visual to Normal, at
which point `DisplayMap.clip_at_line_ends` was still set to `false`. As
a result, the cursor could end up in an invalid position for Normal
mode. The fix ensures that when switching between these two modes, and
if the selection is empty, the selection point is properly clipped,
preventing the cursor from being placed past the end of the line.

Related #38049 

Release Notes:

- Fixed issue in Vim mode where switching from any mode to normal mode
could end up with the cursor in the newline character

---------

Co-authored-by: Conrad Irwin <[e-mail]>

Change summary

crates/editor/src/editor.rs |  4 +++
crates/vim/src/test.rs      | 49 +++++++++++++++++++++++++++++++++++++++
crates/vim/src/vim.rs       |  4 ++
3 files changed, 56 insertions(+), 1 deletion(-)

Detailed changes

crates/editor/src/editor.rs

@@ -2518,6 +2518,10 @@ impl Editor {
         key_context
     }
 
+    pub fn last_bounds(&self) -> Option<&Bounds<Pixels>> {
+        self.last_bounds.as_ref()
+    }
+
     fn show_mouse_cursor(&mut self, cx: &mut Context<Self>) {
         if self.mouse_cursor_hidden {
             self.mouse_cursor_hidden = false;

crates/vim/src/test.rs

@@ -2213,3 +2213,52 @@ async fn test_multi_cursor_replay(cx: &mut gpui::TestAppContext) {
         Mode::Normal,
     );
 }
+
+#[gpui::test]
+async fn test_clipping_on_mode_change(cx: &mut gpui::TestAppContext) {
+    let mut cx = VimTestContext::new(cx, true).await;
+
+    cx.set_state(
+        indoc! {
+        "
+        ˇverylongline
+        andsomelinebelow
+        "
+        },
+        Mode::Normal,
+    );
+
+    cx.simulate_keystrokes("v e");
+    cx.assert_state(
+        indoc! {
+        "
+        «verylonglineˇ»
+        andsomelinebelow
+        "
+        },
+        Mode::Visual,
+    );
+
+    let mut pixel_position = cx.update_editor(|editor, window, cx| {
+        let snapshot = editor.snapshot(window, cx);
+        let current_head = editor.selections.newest_display(cx).end;
+        editor.last_bounds().unwrap().origin
+            + editor
+                .display_to_pixel_point(current_head, &snapshot, window)
+                .unwrap()
+    });
+    pixel_position.x += px(100.);
+    // click beyond end of the line
+    cx.simulate_click(pixel_position, Modifiers::default());
+    cx.run_until_parked();
+
+    cx.assert_state(
+        indoc! {
+        "
+        verylonglinˇe
+        andsomelinebelow
+        "
+        },
+        Mode::Normal,
+    );
+}

crates/vim/src/vim.rs

@@ -1092,6 +1092,8 @@ impl Vim {
                         let mut point = selection.head();
                         if !selection.reversed && !selection.is_empty() {
                             point = movement::left(map, selection.head());
+                        } else if selection.is_empty() {
+                            point = map.clip_point(point, Bias::Left);
                         }
                         selection.collapse_to(point, selection.goal)
                     } else if !last_mode.is_visual() && mode.is_visual() && selection.is_empty() {
@@ -1608,7 +1610,7 @@ impl Vim {
             && !is_multicursor
             && [Mode::Visual, Mode::VisualLine, Mode::VisualBlock].contains(&self.mode)
         {
-            self.switch_mode(Mode::Normal, true, window, cx);
+            self.switch_mode(Mode::Normal, false, window, cx);
         }
     }