vim: Fix ctrl-d/u going to top bottom (#14620)

Conrad Irwin created

Release Notes:

- vim: Fixed ctrl-d/ctrl-u getting to top/bottom of buffer (#13250)

Change summary

crates/editor/src/scroll.rs               |  5 +
crates/editor/src/scroll/scroll_amount.rs | 20 ++---
crates/vim/src/normal/scroll.rs           | 95 +++++++++++++++----------
crates/vim/test_data/test_ctrl_d_u.json   |  6 +
4 files changed, 75 insertions(+), 51 deletions(-)

Detailed changes

crates/editor/src/scroll.rs 🔗

@@ -476,7 +476,10 @@ impl Editor {
         }
 
         let cur_position = self.scroll_position(cx);
-        let new_pos = cur_position + point(0., amount.lines(self));
+        let Some(visible_line_count) = self.visible_line_count() else {
+            return;
+        };
+        let new_pos = cur_position + point(0., amount.lines(visible_line_count));
         self.set_scroll_position(new_pos, cx);
     }
 

crates/editor/src/scroll/scroll_amount.rs 🔗

@@ -1,4 +1,3 @@
-use crate::Editor;
 use serde::Deserialize;
 use ui::{px, Pixels};
 
@@ -11,19 +10,16 @@ pub enum ScrollAmount {
 }
 
 impl ScrollAmount {
-    pub fn lines(&self, editor: &mut Editor) -> f32 {
+    pub fn lines(&self, mut visible_line_count: f32) -> f32 {
         match self {
             Self::Line(count) => *count,
-            Self::Page(count) => editor
-                .visible_line_count()
-                .map(|mut l| {
-                    // for full pages subtract one to leave an anchor line
-                    if count.abs() == 1.0 {
-                        l -= 1.0
-                    }
-                    (l * count).trunc()
-                })
-                .unwrap_or(0.),
+            Self::Page(count) => {
+                // for full pages subtract one to leave an anchor line
+                if count.abs() == 1.0 {
+                    visible_line_count -= 1.0
+                }
+                (visible_line_count * count).trunc()
+            }
         }
     }
 

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

@@ -74,46 +74,60 @@ fn scroll_editor(
     }
 
     editor.scroll_screen(amount, cx);
-    if should_move_cursor {
-        let visible_rows = if let Some(visible_rows) = editor.visible_line_count() {
-            visible_rows as u32
-        } else {
-            return;
-        };
-
-        let top_anchor = editor.scroll_manager.anchor().anchor;
-        let vertical_scroll_margin = EditorSettings::get_global(cx).vertical_scroll_margin;
-
-        editor.change_selections(None, cx, |s| {
-            s.move_with(|map, selection| {
-                let mut head = selection.head();
-                let top = top_anchor.to_display_point(map);
-
-                if preserve_cursor_position {
-                    let old_top = old_top_anchor.to_display_point(map);
-                    let new_row =
-                        DisplayRow(top.row().0 + selection.head().row().0 - old_top.row().0);
-                    head = map.clip_point(DisplayPoint::new(new_row, head.column()), Bias::Left)
-                }
-                let min_row = DisplayRow(top.row().0 + vertical_scroll_margin as u32);
-                let max_row =
-                    DisplayRow(top.row().0 + visible_rows - vertical_scroll_margin as u32 - 1);
-
-                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
-                };
-                if selection.is_empty() {
-                    selection.collapse_to(new_head, selection.goal)
+    if !should_move_cursor {
+        return;
+    }
+
+    let visible_line_count = if let Some(visible_line_count) = editor.visible_line_count() {
+        visible_line_count
+    } else {
+        return;
+    };
+
+    let top_anchor = editor.scroll_manager.anchor().anchor;
+    let vertical_scroll_margin = EditorSettings::get_global(cx).vertical_scroll_margin;
+
+    editor.change_selections(None, cx, |s| {
+        s.move_with(|map, selection| {
+            let mut head = selection.head();
+            let top = top_anchor.to_display_point(map);
+
+            if preserve_cursor_position {
+                let old_top = old_top_anchor.to_display_point(map);
+                let new_row = if old_top.row() == top.row() {
+                    DisplayRow(
+                        top.row()
+                            .0
+                            .saturating_add_signed(amount.lines(visible_line_count) as i32),
+                    )
                 } else {
-                    selection.set_head(new_head, selection.goal)
+                    DisplayRow(top.row().0 + selection.head().row().0 - old_top.row().0)
                 };
-            })
-        });
-    }
+                head = map.clip_point(DisplayPoint::new(new_row, head.column()), Bias::Left)
+            }
+            let min_row = if top.row().0 == 0 {
+                DisplayRow(0)
+            } else {
+                DisplayRow(top.row().0 + vertical_scroll_margin as u32)
+            };
+            let max_row = DisplayRow(
+                top.row().0 + visible_line_count as u32 - vertical_scroll_margin as u32 - 1,
+            );
+
+            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
+            };
+            if selection.is_empty() {
+                selection.collapse_to(new_head, selection.goal)
+            } else {
+                selection.set_head(new_head, selection.goal)
+            };
+        })
+    });
 }
 
 #[cfg(test)]
@@ -251,5 +265,10 @@ mod test {
         cx.simulate_shared_keystrokes("ctrl-d ctrl-d 4 j ctrl-u ctrl-u")
             .await;
         cx.shared_state().await.assert_matches();
+
+        // test returning to top
+        cx.simulate_shared_keystrokes("g g ctrl-d ctrl-u ctrl-u")
+            .await;
+        cx.shared_state().await.assert_matches();
     }
 }

crates/vim/test_data/test_ctrl_d_u.json 🔗

@@ -20,3 +20,9 @@
 {"Key":"ctrl-u"}
 {"Key":"ctrl-u"}
 {"Get":{"state":"aa\nbb\ncc\ndd\nee\nff\ngg\nˇhh\nii\njj\nkk\nll\nmm\nnn\noo\npp\nqq\nrr\nss\ntt\nuu\nvv\nww\nxx\nyy\nzz","mode":"Normal"}}
+{"Key":"g"}
+{"Key":"g"}
+{"Key":"ctrl-d"}
+{"Key":"ctrl-u"}
+{"Key":"ctrl-u"}
+{"Get":{"state":"ˇaa\nbb\ncc\ndd\nee\nff\ngg\nhh\nii\njj\nkk\nll\nmm\nnn\noo\npp\nqq\nrr\nss\ntt\nuu\nvv\nww\nxx\nyy\nzz","mode":"Normal"}}