vim: Fix cursor adjustment on scroll (#2859)

Conrad Irwin created 

Fixes: zed-industries/community#1929

Also preserves visual modes correctly.

[[PR Description]]

Release Notes:

- vim: Fix scroll offset on `ctrl-{e,y,u,b}`
([#1929](https://github.com/zed-industries/community/issues/1929)).

Change summary

assets/keymaps/vim.json                        |  1 
crates/editor/src/scroll.rs                    |  3 +
crates/vim/src/motion.rs                       | 13 ++++++
crates/vim/src/normal/scroll.rs                | 40 +++++++++-----------
crates/vim/test_data/test_next_line_start.json |  3 +
5 files changed, 36 insertions(+), 24 deletions(-)

Detailed changes

assets/keymaps/vim.json 🔗 

@@ -104,6 +104,7 @@
       "v": "vim::ToggleVisual",
       "shift-v": "vim::ToggleVisualLine",
       "ctrl-v": "vim::ToggleVisualBlock",
+      "ctrl-q": "vim::ToggleVisualBlock",
       "*": "vim::MoveToNext",
       "#": "vim::MoveToPrev",
       "0": "vim::StartOfLine", // When no number operator present, use start of line motion

crates/editor/src/scroll.rs 🔗 

@@ -29,6 +29,7 @@ use self::{
 };
 
 pub const SCROLL_EVENT_SEPARATION: Duration = Duration::from_millis(28);
+pub const VERTICAL_SCROLL_MARGIN: f32 = 3.;
 const SCROLLBAR_SHOW_INTERVAL: Duration = Duration::from_secs(1);
 
 #[derive(Default)]
@@ -136,7 +137,7 @@ pub struct ScrollManager {
 impl ScrollManager {
     pub fn new() -> Self {
         ScrollManager {
-            vertical_scroll_margin: 3.0,
+            vertical_scroll_margin: VERTICAL_SCROLL_MARGIN,
             anchor: ScrollAnchor::new(),
             ongoing: OngoingScroll::new(),
             autoscroll_request: None,

crates/vim/src/motion.rs 🔗 

@@ -651,7 +651,10 @@ fn find_backward(
 
 fn next_line_start(map: &DisplaySnapshot, point: DisplayPoint, times: usize) -> DisplayPoint {
     let new_row = (point.row() + times as u32).min(map.max_buffer_row());
-    map.clip_point(DisplayPoint::new(new_row, 0), Bias::Left)
+    first_non_whitespace(
+        map,
+        map.clip_point(DisplayPoint::new(new_row, 0), Bias::Left),
+    )
 }
 
 #[cfg(test)]
@@ -799,4 +802,12 @@ mod test {
         cx.simulate_shared_keystrokes([","]).await;
         cx.assert_shared_state("one two thˇree four").await;
     }
+
+    #[gpui::test]
+    async fn test_next_line_start(cx: &mut gpui::TestAppContext) {
+        let mut cx = NeovimBackedTestContext::new(cx).await;
+        cx.set_shared_state("ˇone\n  two\nthree").await;
+        cx.simulate_shared_keystrokes(["enter"]).await;
+        cx.assert_shared_state("one\n  ˇtwo\nthree").await;
+    }
 }

crates/vim/src/normal/scroll.rs 🔗 

@@ -1,7 +1,9 @@
-use std::cmp::Ordering;
-
 use crate::Vim;
-use editor::{display_map::ToDisplayPoint, scroll::scroll_amount::ScrollAmount, Editor};
+use editor::{
+    display_map::ToDisplayPoint,
+    scroll::{scroll_amount::ScrollAmount, VERTICAL_SCROLL_MARGIN},
+    DisplayPoint, Editor,
+};
 use gpui::{actions, AppContext, ViewContext};
 use language::Bias;
 use workspace::Workspace;
@@ -53,13 +55,9 @@ fn scroll(cx: &mut ViewContext<Workspace>, by: fn(c: Option<f32>) -> ScrollAmoun
 
 fn scroll_editor(editor: &mut Editor, amount: &ScrollAmount, cx: &mut ViewContext<Editor>) {
     let should_move_cursor = editor.newest_selection_on_screen(cx).is_eq();
+
     editor.scroll_screen(amount, cx);
     if should_move_cursor {
-        let selection_ordering = editor.newest_selection_on_screen(cx);
-        if selection_ordering.is_eq() {
-            return;
-        }
-
         let visible_rows = if let Some(visible_rows) = editor.visible_line_count() {
             visible_rows as u32
         } else {
@@ -69,21 +67,19 @@ fn scroll_editor(editor: &mut Editor, amount: &ScrollAmount, cx: &mut ViewContex
         let top_anchor = editor.scroll_manager.anchor().anchor;
 
         editor.change_selections(None, cx, |s| {
-            s.replace_cursors_with(|snapshot| {
-                let mut new_point = top_anchor.to_display_point(&snapshot);
-
-                match selection_ordering {
-                    Ordering::Less => {
-                        new_point = snapshot.clip_point(new_point, Bias::Right);
-                    }
-                    Ordering::Greater => {
-                        *new_point.row_mut() += visible_rows - 1;
-                        new_point = snapshot.clip_point(new_point, Bias::Left);
-                    }
-                    Ordering::Equal => unreachable!(),
-                }
+            s.move_heads_with(|map, head, goal| {
+                let top = top_anchor.to_display_point(map);
+                let min_row = top.row() + VERTICAL_SCROLL_MARGIN as u32;
+                let max_row = top.row() + visible_rows - VERTICAL_SCROLL_MARGIN as u32 - 1;
 
-                vec![new_point]
+                let new_head = if head.row() < min_row {
+                    map.clip_point(DisplayPoint::new(min_row, head.column()), Bias::Left)
+                } else if head.row() > max_row {
+                    map.clip_point(DisplayPoint::new(max_row, head.column()), Bias::Left)
+                } else {
+                    head
+                };
+                (new_head, goal)
             })
         });
     }