Avoid scrolling when clicking near editor top/bottom

Also makes go-to navigation autoscroll guarantee some amount of
margin regardless of user cursor scroll margin

assets/settings/default.json

@@ -157,7 +157,7 @@
     // Whether to show fold buttons in the gutter.
     "folds": true
   },
-  // The number of lines to keep above/below the cursor when scrolling.
+  // The number of lines to keep above/below the cursor when moving the cursor.
   "vertical_scroll_margin": 3,
   "relative_line_numbers": false,
   // When to populate a new search's query based on the text under the cursor.

crates/editor/src/editor.rs

@@ -2129,20 +2129,17 @@ impl Editor {
         let start;
         let end;
         let mode;
-        let auto_scroll;
         match click_count {
             1 => {
                 start = buffer.anchor_before(position.to_point(&display_map));
                 end = start;
                 mode = SelectMode::Character;
-                auto_scroll = true;
             }
             2 => {
                 let range = movement::surrounding_word(&display_map, position);
                 start = buffer.anchor_before(range.start.to_point(&display_map));
                 end = buffer.anchor_before(range.end.to_point(&display_map));
                 mode = SelectMode::Word(start..end);
-                auto_scroll = true;
             }
             3 => {
                 let position = display_map
@@ -2156,17 +2153,15 @@ impl Editor {
                 start = buffer.anchor_before(line_start);
                 end = buffer.anchor_before(next_line_start);
                 mode = SelectMode::Line(start..end);
-                auto_scroll = true;
             }
             _ => {
                 start = buffer.anchor_before(0);
                 end = buffer.anchor_before(buffer.len());
                 mode = SelectMode::All;
-                auto_scroll = false;
             }
         }
 
-        self.change_selections(auto_scroll.then(|| Autoscroll::newest()), cx, |s| {
+        self.change_selections(None, cx, |s| {
             if !add {
                 s.clear_disjoint();
             } else if click_count > 1 {
@@ -6872,7 +6867,7 @@ impl Editor {
         self.select_next_match_internal(
             &display_map,
             action.replace_newest,
-            Some(Autoscroll::newest()),
+            Some(Autoscroll::newest_margin()),
             cx,
         )?;
         Ok(())
@@ -6924,7 +6919,7 @@ impl Editor {
 
                 if let Some(next_selected_range) = next_selected_range {
                     self.unfold_ranges([next_selected_range.clone()], false, true, cx);
-                    self.change_selections(Some(Autoscroll::newest()), cx, |s| {
+                    self.change_selections(Some(Autoscroll::newest_margin()), cx, |s| {
                         if action.replace_newest {
                             s.delete(s.newest_anchor().id);
                         }
@@ -7005,7 +7000,7 @@ impl Editor {
                     true,
                     cx,
                 );
-                self.change_selections(Some(Autoscroll::newest()), cx, |s| {
+                self.change_selections(Some(Autoscroll::newest_margin()), cx, |s| {
                     s.select(selections);
                 });
             } else if let Some(selected_text) = selected_text {
@@ -7444,7 +7439,7 @@ impl Editor {
             if let Some(popover) = self.hover_state.diagnostic_popover.as_ref() {
                 let (group_id, jump_to) = popover.activation_info();
                 if self.activate_diagnostics(group_id, cx) {
-                    self.change_selections(Some(Autoscroll::fit()), cx, |s| {
+                    self.change_selections(Some(Autoscroll::fit_margin()), cx, |s| {
                         let mut new_selection = s.newest_anchor().clone();
                         new_selection.collapse_to(jump_to, SelectionGoal::None);
                         s.select_anchors(vec![new_selection.clone()]);
@@ -7491,7 +7486,7 @@ impl Editor {
 
             if let Some((primary_range, group_id)) = group {
                 if self.activate_diagnostics(group_id, cx) {
-                    self.change_selections(Some(Autoscroll::fit()), cx, |s| {
+                    self.change_selections(Some(Autoscroll::fit_margin()), cx, |s| {
                         s.select(vec![Selection {
                             id: selection.id,
                             start: primary_range.start,
@@ -7598,7 +7593,7 @@ impl Editor {
             .dedup();
 
         if let Some(hunk) = hunks.next() {
-            self.change_selections(Some(Autoscroll::fit()), cx, |s| {
+            self.change_selections(Some(Autoscroll::fit_margin()), cx, |s| {
                 let row = hunk.start_display_row();
                 let point = DisplayPoint::new(row, 0);
                 s.select_display_ranges([point..point]);
@@ -7750,7 +7745,7 @@ impl Editor {
                         let range = target.range.to_offset(target.buffer.read(cx));
                         let range = editor.range_for_match(&range);
                         if Some(&target.buffer) == editor.buffer.read(cx).as_singleton().as_ref() {
-                            editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
+                            editor.change_selections(Some(Autoscroll::fit_margin()), cx, |s| {
                                 s.select_ranges([range]);
                             });
                         } else {
@@ -7770,7 +7765,7 @@ impl Editor {
                                     // to avoid creating a history entry at the previous cursor location.
                                     pane.update(cx, |pane, _| pane.disable_history());
                                     target_editor.change_selections(
-                                        Some(Autoscroll::fit()),
+                                        Some(Autoscroll::fit_margin()),
                                         cx,
                                         |s| {
                                             s.select_ranges([range]);
@@ -7979,7 +7974,7 @@ impl Editor {
                     let range = editor.range_for_match(&range);
 
                     if Some(&target.buffer) == editor.buffer().read(cx).as_singleton().as_ref() {
-                        editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
+                        editor.change_selections(Some(Autoscroll::fit_margin()), cx, |s| {
                             s.select_ranges([range]);
                         });
                     } else {
@@ -7993,9 +7988,13 @@ impl Editor {
                                     )
                                 });
                             target_editor.update(cx, |target_editor, cx| {
-                                target_editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
+                                target_editor.change_selections(
-                                    s.select_ranges([range]);
+                                    Some(Autoscroll::fit_margin()),
-                                })
+                                    cx,
+                                    |s| {
+                                        s.select_ranges([range]);
+                                    },
+                                )
                             })
                         })
                     }
@@ -8242,7 +8241,7 @@ impl Editor {
                             }),
                             disposition: BlockDisposition::Below,
                         }],
-                        Some(Autoscroll::fit()),
+                        Some(Autoscroll::fit_margin()),
                         cx,
                     )[0];
                     this.pending_rename = Some(RenameState {
@@ -8321,7 +8320,7 @@ impl Editor {
 
         self.remove_blocks(
             [rename.block_id].into_iter().collect(),
-            Some(Autoscroll::fit()),
+            Some(Autoscroll::fit_margin()),
             cx,
         );
         self.clear_highlights::<Rename>(cx);
@@ -8682,7 +8681,7 @@ impl Editor {
             self.display_map.update(cx, |map, cx| map.fold(ranges, cx));
 
             if auto_scroll {
-                self.request_autoscroll(Autoscroll::fit(), cx);
+                self.request_autoscroll(Autoscroll::fit_margin(), cx);
             }
 
             cx.notify();
@@ -8701,7 +8700,7 @@ impl Editor {
             self.display_map
                 .update(cx, |map, cx| map.unfold(ranges, inclusive, cx));
             if auto_scroll {
-                self.request_autoscroll(Autoscroll::fit(), cx);
+                self.request_autoscroll(Autoscroll::fit_margin(), cx);
             }
 
             cx.notify();
@@ -9528,7 +9527,7 @@ impl Editor {
                 for (buffer, ranges) in new_selections_by_buffer.into_iter() {
                     let editor = workspace.open_project_item::<Self>(pane.clone(), buffer, cx);
                     editor.update(cx, |editor, cx| {
-                        editor.change_selections(Some(Autoscroll::newest()), cx, |s| {
+                        editor.change_selections(Some(Autoscroll::newest_margin()), cx, |s| {
                             s.select_ranges(ranges);
                         });
                     });
@@ -9571,7 +9570,7 @@ impl Editor {
                 };
 
                 let nav_history = editor.nav_history.take();
-                editor.change_selections(Some(Autoscroll::newest()), cx, |s| {
+                editor.change_selections(Some(Autoscroll::newest_margin()), cx, |s| {
                     s.select_ranges([cursor..cursor]);
                 });
                 editor.nav_history = nav_history;

crates/editor/src/scroll/autoscroll.rs

@@ -27,11 +27,25 @@ impl Autoscroll {
         Self::Strategy(AutoscrollStrategy::Center)
     }
 
-    /// scrolls so the neweset cursor is near the top
+    /// scrolls so the newest cursor is near the top
     /// (offset by vertical_scroll_margin)
     pub fn focused() -> Self {
         Self::Strategy(AutoscrollStrategy::Focused)
     }
+
+    /// scrolls the minimal amount to (try) and fit all cursors onscreen
+    /// while guaranteeing an amount of margin from the top and bottom
+    /// of the screen, regardless of user configured scroll margin
+    pub fn fit_margin() -> Self {
+        Self::Strategy(AutoscrollStrategy::FitMargin)
+    }
+
+    /// scrolls the minimal amount to fit the newest cursor onscreen
+    /// while guaranteeing an amount of margin from the top and bottom
+    /// of the screen, regardless of user configured scroll margin
+    pub fn newest_margin() -> Self {
+        Self::Strategy(AutoscrollStrategy::NewestMargin)
+    }
 }
 
 #[derive(PartialEq, Eq, Default, Clone, Copy)]
@@ -41,6 +55,8 @@ pub enum AutoscrollStrategy {
     #[default]
     Center,
     Focused,
+    FitMargin,
+    NewestMargin,
     Top,
     Bottom,
 }
@@ -119,7 +135,7 @@ impl Editor {
         let margin = if matches!(self.mode, EditorMode::AutoHeight { .. }) {
             0.
         } else {
-            ((visible_lines - (target_bottom - target_top)) / 2.0).floor()
+            ((visible_lines - (target_bottom - target_top)) / 6.0).floor()
         };
 
         let strategy = match autoscroll {
@@ -142,8 +158,20 @@ impl Editor {
         };
 
         match strategy {
-            AutoscrollStrategy::Fit | AutoscrollStrategy::Newest => {
+            AutoscrollStrategy::Fit
-                let margin = margin.min(self.scroll_manager.vertical_scroll_margin);
+            | AutoscrollStrategy::Newest
+            | AutoscrollStrategy::FitMargin
+            | AutoscrollStrategy::NewestMargin => {
+                let margin = if matches!(
+                    strategy,
+                    AutoscrollStrategy::FitMargin | AutoscrollStrategy::NewestMargin
+                ) && !matches!(self.mode, EditorMode::AutoHeight { .. })
+                {
+                    margin.max(self.scroll_manager.vertical_scroll_margin)
+                } else {
+                    margin.min(self.scroll_manager.vertical_scroll_margin)
+                };
+
                 let target_top = (target_top - margin).max(0.0);
                 let target_bottom = target_bottom + margin;
                 let start_row = scroll_position.y;