Fix ctrl-d/u issues with scroll_beyond_last_line off (#15395)

Kevin Sweet and Marshall Bowers created

Closes #15356

Release Notes:

- vim: Fixed issues with `ctrl-d`/`ctrl-u` when
`scroll_beyond_last_line` is set to `off`
([#15356](https://github.com/zed-industries/zed/issues/15356)).


https://github.com/user-attachments/assets/d3166393-4a4e-4195-9db6-3ff1d4aeec78

---------

Co-authored-by: Marshall Bowers <elliott.codes@gmail.com>

Change summary

crates/editor/src/editor.rs                            |  2 
crates/editor/src/scroll.rs                            |  2 
crates/vim/src/normal/scroll.rs                        | 81 ++++++++---
crates/vim/test_data/test_scroll_beyond_last_line.json | 13 +
docs/src/vim.md                                        |  2 
5 files changed, 74 insertions(+), 26 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -59,7 +59,7 @@ use convert_case::{Case, Casing};
 use debounced_delay::DebouncedDelay;
 use display_map::*;
 pub use display_map::{DisplayPoint, FoldPlaceholder};
-pub use editor_settings::{CurrentLineHighlight, EditorSettings};
+pub use editor_settings::{CurrentLineHighlight, EditorSettings, ScrollBeyondLastLine};
 pub use editor_settings_controls::*;
 use element::LineWithInvisibles;
 pub use element::{

crates/editor/src/scroll.rs 🔗

@@ -505,7 +505,7 @@ impl Editor {
         }
 
         if let Some(visible_lines) = self.visible_line_count() {
-            if newest_head.row() < DisplayRow(screen_top.row().0 + visible_lines as u32) {
+            if newest_head.row() <= DisplayRow(screen_top.row().0 + visible_lines as u32) {
                 return Ordering::Equal;
             }
         }

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

@@ -91,6 +91,7 @@ fn scroll_editor(
         s.move_with(|map, selection| {
             let mut head = selection.head();
             let top = top_anchor.to_display_point(map);
+            let starting_column = head.column();
 
             let vertical_scroll_margin =
                 (vertical_scroll_margin as u32).min(visible_line_count as u32 / 2);
@@ -99,7 +100,7 @@ fn scroll_editor(
                 let old_top = old_top_anchor.to_display_point(map);
                 let new_row = if old_top.row() == top.row() {
                     DisplayRow(
-                        top.row()
+                        head.row()
                             .0
                             .saturating_add_signed(amount.lines(visible_line_count) as i32),
                     )
@@ -108,25 +109,25 @@ fn scroll_editor(
                 };
                 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)
             };
-            let max_row = DisplayRow(
-                top.row().0
-                    + (visible_line_count as u32)
-                        .saturating_sub(vertical_scroll_margin)
-                        .saturating_sub(1),
-            );
-
-            let new_head = if head.row() < min_row {
-                map.clip_point(DisplayPoint::new(min_row, head.column()), Bias::Left)
+            let max_row = DisplayRow(map.max_point().row().0.max(top.row().0.saturating_add(
+                (visible_line_count as u32).saturating_sub(1 + vertical_scroll_margin),
+            )));
+
+            let new_row = if head.row() < min_row {
+                min_row
             } else if head.row() > max_row {
-                map.clip_point(DisplayPoint::new(max_row, head.column()), Bias::Left)
+                max_row
             } else {
-                head
+                head.row()
             };
+            let new_head = map.clip_point(DisplayPoint::new(new_row, starting_column), Bias::Left);
+
             if selection.is_empty() {
                 selection.collapse_to(new_head, selection.goal)
             } else {
@@ -142,9 +143,24 @@ mod test {
         state::Mode,
         test::{NeovimBackedTestContext, VimTestContext},
     };
+    use editor::{EditorSettings, ScrollBeyondLastLine};
     use gpui::{point, px, size, Context};
     use indoc::indoc;
     use language::Point;
+    use settings::SettingsStore;
+
+    pub fn sample_text(rows: usize, cols: usize, start_char: char) -> String {
+        let mut text = String::new();
+        for row in 0..rows {
+            let c: char = (start_char as u32 + row as u32) as u8 as char;
+            let mut line = c.to_string().repeat(cols);
+            if row < rows - 1 {
+                line.push('\n');
+            }
+            text += &line;
+        }
+        text
+    }
 
     #[gpui::test]
     async fn test_scroll(cx: &mut gpui::TestAppContext) {
@@ -241,18 +257,6 @@ mod test {
 
         cx.set_scroll_height(10).await;
 
-        pub fn sample_text(rows: usize, cols: usize, start_char: char) -> String {
-            let mut text = String::new();
-            for row in 0..rows {
-                let c: char = (start_char as u32 + row as u32) as u8 as char;
-                let mut line = c.to_string().repeat(cols);
-                if row < rows - 1 {
-                    line.push('\n');
-                }
-                text += &line;
-            }
-            text
-        }
         let content = "ˇ".to_owned() + &sample_text(26, 2, 'a');
         cx.set_shared_state(&content).await;
 
@@ -277,4 +281,33 @@ mod test {
             .await;
         cx.shared_state().await.assert_matches();
     }
+
+    #[gpui::test]
+    async fn test_scroll_beyond_last_line(cx: &mut gpui::TestAppContext) {
+        let mut cx = NeovimBackedTestContext::new(cx).await;
+
+        cx.set_scroll_height(10).await;
+        cx.neovim.set_option(&format!("scrolloff={}", 0)).await;
+
+        let content = "ˇ".to_owned() + &sample_text(26, 2, 'a');
+        cx.set_shared_state(&content).await;
+
+        cx.update_global(|store: &mut SettingsStore, cx| {
+            store.update_user_settings::<EditorSettings>(cx, |s| {
+                s.scroll_beyond_last_line = Some(ScrollBeyondLastLine::Off)
+            });
+        });
+
+        // ctrl-d can reach the end and the cursor stays in the first column
+        cx.simulate_shared_keystrokes("shift-g k").await;
+        cx.shared_state().await.assert_matches();
+        cx.simulate_shared_keystrokes("ctrl-d").await;
+        cx.shared_state().await.assert_matches();
+
+        // ctrl-u from the last line
+        cx.simulate_shared_keystrokes("shift-g").await;
+        cx.shared_state().await.assert_matches();
+        cx.simulate_shared_keystrokes("ctrl-u").await;
+        cx.shared_state().await.assert_matches();
+    }
 }

crates/vim/test_data/test_scroll_beyond_last_line.json 🔗

@@ -0,0 +1,13 @@
+{"SetOption":{"value":"scrolloff=3"}}
+{"SetOption":{"value":"lines=12"}}
+{"SetOption":{"value":"scrolloff=0"}}
+{"Put":{"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"}}
+{"Key":"shift-g"}
+{"Key":"k"}
+{"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\nˇyy\nzz","mode":"Normal"}}
+{"Key":"ctrl-d"}
+{"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\nˇzz","mode":"Normal"}}
+{"Key":"shift-g"}
+{"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\nˇzz","mode":"Normal"}}
+{"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\nˇuu\nvv\nww\nxx\nyy\nzz","mode":"Normal"}}

docs/src/vim.md 🔗

@@ -258,6 +258,8 @@ There are also a few Zed settings that you may also enjoy if you use vim mode:
   "relative_line_numbers": true,
   // hide the scroll bar
   "scrollbar": { "show": "never" },
+  // prevent the buffer from scrolling beyond the last line
+  "scroll_beyond_last_line": "off",
   // allow cursor to reach edges of screen
   "vertical_scroll_margin": 0,
   "gutter": {