vim hml (#7298)

Conrad Irwin , Vbhavsar , and fdionisi created

- Add a setting for `vertical_scroll_offset`
- Fix H/M/L in vim with scrolloff



Release Notes:

- Added a settings for `vertical_scroll_offset`
- vim: Fix H/M/L with various values of vertical_scroll_offset

---------

Co-authored-by: Vbhavsar <vbhavsar@gmail.com>
Co-authored-by: fdionisi <code@fdionisi.me>

Change summary

assets/settings/default.json                      |  2 +
crates/editor/src/display_map.rs                  |  3 +
crates/editor/src/editor.rs                       |  7 ++-
crates/editor/src/editor_settings.rs              |  6 +++
crates/editor/src/movement.rs                     |  7 +--
crates/editor/src/scroll.rs                       | 12 +++---
crates/vim/src/motion.rs                          | 32 +++++++++++++---
crates/vim/src/normal/scroll.rs                   | 10 ++--
crates/vim/src/test/neovim_backed_test_context.rs |  6 +--
9 files changed, 57 insertions(+), 28 deletions(-)

Detailed changes

assets/settings/default.json 🔗

@@ -133,6 +133,8 @@
     // Whether to show diagnostic indicators in the scrollbar.
     "diagnostics": true
   },
+  // The number of lines to keep above/below the cursor when scrolling.
+  "vertical_scroll_margin": 3,
   "relative_line_numbers": false,
   // When to populate a new search's query based on the text under the cursor.
   // This setting can take the following three values:

crates/editor/src/display_map.rs 🔗

@@ -586,8 +586,9 @@ impl DisplaySnapshot {
             text_system,
             editor_style,
             rem_size,
-            anchor: _,
+            scroll_anchor: _,
             visible_rows: _,
+            vertical_scroll_margin: _,
         }: &TextLayoutDetails,
     ) -> Arc<LineLayout> {
         let mut runs = Vec::new();

crates/editor/src/editor.rs 🔗

@@ -1430,7 +1430,7 @@ impl Editor {
             buffer: buffer.clone(),
             display_map: display_map.clone(),
             selections,
-            scroll_manager: ScrollManager::new(),
+            scroll_manager: ScrollManager::new(cx),
             columnar_selection_tail: None,
             add_selections_state: None,
             select_next_state: None,
@@ -3086,8 +3086,9 @@ impl Editor {
             text_system: cx.text_system().clone(),
             editor_style: self.style.clone().unwrap(),
             rem_size: cx.rem_size(),
-            anchor: self.scroll_manager.anchor().anchor,
+            scroll_anchor: self.scroll_manager.anchor(),
             visible_rows: self.visible_line_count(),
+            vertical_scroll_margin: self.scroll_manager.vertical_scroll_margin,
         }
     }
 
@@ -8762,6 +8763,8 @@ impl Editor {
             )),
             cx,
         );
+        self.scroll_manager.vertical_scroll_margin =
+            EditorSettings::get_global(cx).vertical_scroll_margin;
         cx.notify();
     }
 

crates/editor/src/editor_settings.rs 🔗

@@ -11,6 +11,7 @@ pub struct EditorSettings {
     pub completion_documentation_secondary_query_debounce: u64,
     pub use_on_type_format: bool,
     pub scrollbar: Scrollbar,
+    pub vertical_scroll_margin: f32,
     pub relative_line_numbers: bool,
     pub seed_search_query_from_cursor: SeedQuerySetting,
     pub redact_private_values: bool,
@@ -87,6 +88,11 @@ pub struct EditorSettingsContent {
     pub use_on_type_format: Option<bool>,
     /// Scrollbar related settings
     pub scrollbar: Option<ScrollbarContent>,
+
+    /// The number of lines to keep above/below the cursor when auto-scrolling.
+    ///
+    /// Default: 3.
+    pub vertical_scroll_margin: Option<f32>,
     /// Whether the line numbers on editors gutter are relative or not.
     ///
     /// Default: false

crates/editor/src/movement.rs 🔗

@@ -2,14 +2,12 @@
 //! in editor given a given motion (e.g. it handles converting a "move left" command into coordinates in editor). It is exposed mostly for use by vim crate.
 
 use super::{Bias, DisplayPoint, DisplaySnapshot, SelectionGoal, ToDisplayPoint};
-use crate::{char_kind, CharKind, EditorStyle, ToOffset, ToPoint};
+use crate::{char_kind, scroll::ScrollAnchor, CharKind, EditorStyle, ToOffset, ToPoint};
 use gpui::{px, Pixels, WindowTextSystem};
 use language::Point;
 
 use std::{ops::Range, sync::Arc};
 
-use multi_buffer::Anchor;
-
 /// Defines search strategy for items in `movement` module.
 /// `FindRange::SingeLine` only looks for a match on a single line at a time, whereas
 /// `FindRange::MultiLine` keeps going until the end of a string.
@@ -25,8 +23,9 @@ pub struct TextLayoutDetails {
     pub(crate) text_system: Arc<WindowTextSystem>,
     pub(crate) editor_style: EditorStyle,
     pub(crate) rem_size: Pixels,
-    pub anchor: Anchor,
+    pub scroll_anchor: ScrollAnchor,
     pub visible_rows: Option<f32>,
+    pub vertical_scroll_margin: f32,
 }
 
 /// Returns a column to the left of the current point, wrapping

crates/editor/src/scroll.rs 🔗

@@ -6,13 +6,14 @@ use crate::{
     display_map::{DisplaySnapshot, ToDisplayPoint},
     hover_popover::hide_hover,
     persistence::DB,
-    Anchor, DisplayPoint, Editor, EditorEvent, EditorMode, InlayHintRefreshReason,
+    Anchor, DisplayPoint, Editor, EditorEvent, EditorMode, EditorSettings, InlayHintRefreshReason,
     MultiBufferSnapshot, ToPoint,
 };
 pub use autoscroll::{Autoscroll, AutoscrollStrategy};
-use gpui::{point, px, AppContext, Entity, Global, Pixels, Task, ViewContext};
+use gpui::{point, px, AppContext, Entity, Global, Pixels, Task, ViewContext, WindowContext};
 use language::{Bias, Point};
 pub use scroll_amount::ScrollAmount;
+use settings::Settings;
 use std::{
     cmp::Ordering,
     time::{Duration, Instant},
@@ -21,7 +22,6 @@ use util::ResultExt;
 use workspace::{ItemId, WorkspaceId};
 
 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)]
@@ -128,7 +128,7 @@ impl OngoingScroll {
 }
 
 pub struct ScrollManager {
-    vertical_scroll_margin: f32,
+    pub(crate) vertical_scroll_margin: f32,
     anchor: ScrollAnchor,
     ongoing: OngoingScroll,
     autoscroll_request: Option<(Autoscroll, bool)>,
@@ -140,9 +140,9 @@ pub struct ScrollManager {
 }
 
 impl ScrollManager {
-    pub fn new() -> Self {
+    pub fn new(cx: &mut WindowContext) -> Self {
         ScrollManager {
-            vertical_scroll_margin: VERTICAL_SCROLL_MARGIN,
+            vertical_scroll_margin: EditorSettings::get_global(cx).vertical_scroll_margin,
             anchor: ScrollAnchor::new(),
             ongoing: OngoingScroll::new(),
             autoscroll_request: None,

crates/vim/src/motion.rs 🔗

@@ -1044,9 +1044,17 @@ fn window_top(
     map: &DisplaySnapshot,
     point: DisplayPoint,
     text_layout_details: &TextLayoutDetails,
-    times: usize,
+    mut times: usize,
 ) -> (DisplayPoint, SelectionGoal) {
-    let first_visible_line = text_layout_details.anchor.to_display_point(map);
+    let first_visible_line = text_layout_details
+        .scroll_anchor
+        .anchor
+        .to_display_point(map);
+
+    if first_visible_line.row() != 0 && text_layout_details.vertical_scroll_margin as usize > times
+    {
+        times = text_layout_details.vertical_scroll_margin.ceil() as usize;
+    }
 
     if let Some(visible_rows) = text_layout_details.visible_rows {
         let bottom_row = first_visible_line.row() + visible_rows as u32;
@@ -1070,7 +1078,10 @@ fn window_middle(
     text_layout_details: &TextLayoutDetails,
 ) -> (DisplayPoint, SelectionGoal) {
     if let Some(visible_rows) = text_layout_details.visible_rows {
-        let first_visible_line = text_layout_details.anchor.to_display_point(map);
+        let first_visible_line = text_layout_details
+            .scroll_anchor
+            .anchor
+            .to_display_point(map);
         let max_rows = (visible_rows as u32).min(map.max_buffer_row());
         let new_row = first_visible_line.row() + (max_rows.div_euclid(2));
         let new_col = point.column().min(map.line_len(new_row));
@@ -1085,11 +1096,20 @@ fn window_bottom(
     map: &DisplaySnapshot,
     point: DisplayPoint,
     text_layout_details: &TextLayoutDetails,
-    times: usize,
+    mut times: usize,
 ) -> (DisplayPoint, SelectionGoal) {
     if let Some(visible_rows) = text_layout_details.visible_rows {
-        let first_visible_line = text_layout_details.anchor.to_display_point(map);
-        let bottom_row = first_visible_line.row() + (visible_rows) as u32;
+        let first_visible_line = text_layout_details
+            .scroll_anchor
+            .anchor
+            .to_display_point(map);
+        let bottom_row = first_visible_line.row()
+            + (visible_rows + text_layout_details.scroll_anchor.offset.y - 1.).floor() as u32;
+        if bottom_row < map.max_buffer_row()
+            && text_layout_details.vertical_scroll_margin as usize > times
+        {
+            times = text_layout_details.vertical_scroll_margin.ceil() as usize;
+        }
         let bottom_row_capped = bottom_row.min(map.max_buffer_row());
         let new_row = if bottom_row_capped.saturating_sub(times as u32) < first_visible_line.row() {
             first_visible_line.row()

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

@@ -1,11 +1,10 @@
 use crate::Vim;
 use editor::{
-    display_map::ToDisplayPoint,
-    scroll::{ScrollAmount, VERTICAL_SCROLL_MARGIN},
-    DisplayPoint, Editor,
+    display_map::ToDisplayPoint, scroll::ScrollAmount, DisplayPoint, Editor, EditorSettings,
 };
 use gpui::{actions, ViewContext};
 use language::Bias;
+use settings::Settings;
 use workspace::Workspace;
 
 actions!(
@@ -77,6 +76,7 @@ fn scroll_editor(
         };
 
         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| {
@@ -88,8 +88,8 @@ fn scroll_editor(
                     let new_row = top.row() + selection.head().row() - old_top.row();
                     head = map.clip_point(DisplayPoint::new(new_row, head.column()), Bias::Left)
                 }
-                let min_row = top.row() + VERTICAL_SCROLL_MARGIN as u32;
-                let max_row = top.row() + visible_rows - VERTICAL_SCROLL_MARGIN as u32 - 1;
+                let min_row = top.row() + vertical_scroll_margin as u32;
+                let max_row = top.row() + 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)

crates/vim/src/test/neovim_backed_test_context.rs 🔗

@@ -1,4 +1,4 @@
-use editor::{scroll::VERTICAL_SCROLL_MARGIN, test::editor_test_context::ContextHandle};
+use editor::test::editor_test_context::ContextHandle;
 use gpui::{px, size, Context};
 use indoc::indoc;
 use settings::SettingsStore;
@@ -155,9 +155,7 @@ impl NeovimBackedTestContext {
 
     pub async fn set_scroll_height(&mut self, rows: u32) {
         // match Zed's scrolling behavior
-        self.neovim
-            .set_option(&format!("scrolloff={}", VERTICAL_SCROLL_MARGIN))
-            .await;
+        self.neovim.set_option(&format!("scrolloff={}", 3)).await;
         // +2 to account for the vim command UI at the bottom.
         self.neovim.set_option(&format!("lines={}", rows + 2)).await;
         let (line_height, visible_line_count) = self.editor(|editor, cx| {