vim: Fix ctrl-b not moving the cursor (#17808)

Closes #17687

Release Notes:

- Fixed `ctrl-b` not moving the cursor.

---------

Co-authored-by: Abdelhakim Qbaich <abdelhakim@qbaich.com>
Co-authored-by: Pete LeVasseur <plevasseur@gmail.com>

crates/editor/src/scroll/scroll_amount.rs

@@ -1,6 +1,18 @@
 use serde::Deserialize;
 use ui::{px, Pixels};
 
+#[derive(Debug)]
+pub enum ScrollDirection {
+    Upwards,
+    Downwards,
+}
+
+impl ScrollDirection {
+    pub fn is_upwards(&self) -> bool {
+        matches!(self, ScrollDirection::Upwards)
+    }
+}
+
 #[derive(Debug, Clone, PartialEq, Deserialize)]
 pub enum ScrollAmount {
     // Scroll N lines (positive is towards the end of the document)
@@ -15,7 +27,7 @@ impl ScrollAmount {
             Self::Line(count) => *count,
             Self::Page(count) => {
                 // for full pages subtract one to leave an anchor line
-                if count.abs() == 1.0 {
+                if self.is_full_page() {
                     visible_line_count -= 1.0
                 }
                 (visible_line_count * count).trunc()
@@ -29,4 +41,19 @@ impl ScrollAmount {
             ScrollAmount::Page(x) => px(height.0 * x),
         }
     }
+
+    pub fn is_full_page(&self) -> bool {
+        match self {
+            ScrollAmount::Page(count) if count.abs() == 1.0 => true,
+            _ => false,
+        }
+    }
+
+    pub fn direction(&self) -> ScrollDirection {
+        match self {
+            Self::Line(amount) if amount.is_sign_positive() => ScrollDirection::Downwards,
+            Self::Page(amount) if amount.is_sign_positive() => ScrollDirection::Downwards,
+            _ => ScrollDirection::Upwards,
+        }
+    }
 }

crates/vim/src/normal/scroll.rs

@@ -73,14 +73,24 @@ fn scroll_editor(
         return;
     }
 
-    editor.scroll_screen(amount, cx);
+    let full_page_up = amount.is_full_page() && amount.direction().is_upwards();
+    let amount = match (amount.is_full_page(), editor.visible_line_count()) {
+        (true, Some(visible_line_count)) => {
+            if amount.direction().is_upwards() {
+                ScrollAmount::Line(amount.lines(visible_line_count) + 1.0)
+            } else {
+                ScrollAmount::Line(amount.lines(visible_line_count) - 1.0)
+            }
+        }
+        _ => amount.clone(),
+    };
+
+    editor.scroll_screen(&amount, cx);
     if !should_move_cursor {
         return;
     }
 
-    let visible_line_count = if let Some(visible_line_count) = editor.visible_line_count() {
+    let Some(visible_line_count) = editor.visible_line_count() else {
-        visible_line_count
-    } else {
         return;
     };
 
@@ -115,11 +125,18 @@ fn scroll_editor(
             } else {
                 DisplayRow(top.row().0 + vertical_scroll_margin)
             };
-            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 {
+            let max_visible_row = top.row().0.saturating_add(
+                (visible_line_count as u32).saturating_sub(1 + vertical_scroll_margin),
+            );
+            let max_row = DisplayRow(map.max_point().row().0.max(max_visible_row));
+
+            let new_row = if full_page_up {
+                // Special-casing ctrl-b/page-up, which is special-cased by Vim, it seems
+                // to always put the cursor on the last line of the page, even if the cursor
+                // was before that.
+                DisplayRow(max_visible_row)
+            } else if head.row() < min_row {
                 min_row
             } else if head.row() > max_row {
                 max_row
@@ -251,6 +268,7 @@ mod test {
             )
         });
     }
+
     #[gpui::test]
     async fn test_ctrl_d_u(cx: &mut gpui::TestAppContext) {
         let mut cx = NeovimBackedTestContext::new(cx).await;
@@ -282,6 +300,64 @@ mod test {
         cx.shared_state().await.assert_matches();
     }
 
+    #[gpui::test]
+    async fn test_ctrl_f_b(cx: &mut gpui::TestAppContext) {
+        let mut cx = NeovimBackedTestContext::new(cx).await;
+
+        let visible_lines = 10;
+        cx.set_scroll_height(visible_lines).await;
+
+        // First test without vertical scroll margin
+        cx.neovim.set_option(&format!("scrolloff={}", 0)).await;
+        cx.update_global(|store: &mut SettingsStore, cx| {
+            store.update_user_settings::<EditorSettings>(cx, |s| {
+                s.vertical_scroll_margin = Some(0.0)
+            });
+        });
+
+        let content = "ˇ".to_owned() + &sample_text(26, 2, 'a');
+        cx.set_shared_state(&content).await;
+
+        // scroll down: ctrl-f
+        cx.simulate_shared_keystrokes("ctrl-f").await;
+        cx.shared_state().await.assert_matches();
+
+        cx.simulate_shared_keystrokes("ctrl-f").await;
+        cx.shared_state().await.assert_matches();
+
+        // scroll up: ctrl-b
+        cx.simulate_shared_keystrokes("ctrl-b").await;
+        cx.shared_state().await.assert_matches();
+
+        cx.simulate_shared_keystrokes("ctrl-b").await;
+        cx.shared_state().await.assert_matches();
+
+        // Now go back to start of file, and test with vertical scroll margin
+        cx.simulate_shared_keystrokes("g g").await;
+        cx.shared_state().await.assert_matches();
+
+        cx.neovim.set_option(&format!("scrolloff={}", 3)).await;
+        cx.update_global(|store: &mut SettingsStore, cx| {
+            store.update_user_settings::<EditorSettings>(cx, |s| {
+                s.vertical_scroll_margin = Some(3.0)
+            });
+        });
+
+        // scroll down: ctrl-f
+        cx.simulate_shared_keystrokes("ctrl-f").await;
+        cx.shared_state().await.assert_matches();
+
+        cx.simulate_shared_keystrokes("ctrl-f").await;
+        cx.shared_state().await.assert_matches();
+
+        // scroll up: ctrl-b
+        cx.simulate_shared_keystrokes("ctrl-b").await;
+        cx.shared_state().await.assert_matches();
+
+        cx.simulate_shared_keystrokes("ctrl-b").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;

crates/vim/test_data/test_ctrl_f_b.json

@@ -0,0 +1,24 @@
+{"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":"ctrl-f"}
+{"Get":{"state":"aa\nbb\ncc\ndd\nee\nff\ngg\nhh\nˇii\njj\nkk\nll\nmm\nnn\noo\npp\nqq\nrr\nss\ntt\nuu\nvv\nww\nxx\nyy\nzz","mode":"Normal"}}
+{"Key":"ctrl-f"}
+{"Get":{"state":"aa\nbb\ncc\ndd\nee\nff\ngg\nhh\nii\njj\nkk\nll\nmm\nnn\noo\npp\nˇqq\nrr\nss\ntt\nuu\nvv\nww\nxx\nyy\nzz","mode":"Normal"}}
+{"Key":"ctrl-b"}
+{"Get":{"state":"aa\nbb\ncc\ndd\nee\nff\ngg\nhh\nii\njj\nkk\nll\nmm\nnn\noo\npp\nqq\nˇrr\nss\ntt\nuu\nvv\nww\nxx\nyy\nzz","mode":"Normal"}}
+{"Key":"ctrl-b"}
+{"Get":{"state":"aa\nbb\ncc\ndd\nee\nff\ngg\nhh\nii\nˇjj\nkk\nll\nmm\nnn\noo\npp\nqq\nrr\nss\ntt\nuu\nvv\nww\nxx\nyy\nzz","mode":"Normal"}}
+{"Key":"g"}
+{"Key":"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\nzz","mode":"Normal"}}
+{"SetOption":{"value":"scrolloff=3"}}
+{"Key":"ctrl-f"}
+{"Get":{"state":"aa\nbb\ncc\ndd\nee\nff\ngg\nhh\nii\njj\nkk\nˇll\nmm\nnn\noo\npp\nqq\nrr\nss\ntt\nuu\nvv\nww\nxx\nyy\nzz","mode":"Normal"}}
+{"Key":"ctrl-f"}
+{"Get":{"state":"aa\nbb\ncc\ndd\nee\nff\ngg\nhh\nii\njj\nkk\nll\nmm\nnn\noo\npp\nqq\nrr\nss\nˇtt\nuu\nvv\nww\nxx\nyy\nzz","mode":"Normal"}}
+{"Key":"ctrl-b"}
+{"Get":{"state":"aa\nbb\ncc\ndd\nee\nff\ngg\nhh\nii\njj\nkk\nll\nmm\nnn\nˇoo\npp\nqq\nrr\nss\ntt\nuu\nvv\nww\nxx\nyy\nzz","mode":"Normal"}}
+{"Key":"ctrl-b"}
+{"Get":{"state":"aa\nbb\ncc\ndd\nee\nff\nˇgg\nhh\nii\njj\nkk\nll\nmm\nnn\noo\npp\nqq\nrr\nss\ntt\nuu\nvv\nww\nxx\nyy\nzz","mode":"Normal"}}