fix: align view after jumplist_picker (helix-editor#3743)

* Add `View::ensure_cursor_in_view_center` to adjust view after searching and jumping

Also `offset_coodrs_to_in_view` was refactored to reduce duplicated position calculations.

* Fix a wrong offset calculation in `offset_coords_to_in_view_center`

It ignored `scrolloff` if `centering` is false.

Author: shnarazk

helix-term/src/commands.rs

@@ -1667,12 +1667,7 @@ fn search_impl(
         };
 
         doc.set_selection(view.id, selection);
-        // TODO: is_cursor_in_view does the same calculation as ensure_cursor_in_view
-        if view.is_cursor_in_view(doc, 0) {
-            view.ensure_cursor_in_view(doc, scrolloff);
-        } else {
-            align_view(doc, view, Align::Center)
-        }
+        view.ensure_cursor_in_view_center(doc, scrolloff);
     };
 }
 
@@ -2434,8 +2429,10 @@ fn jumplist_picker(cx: &mut Context) {
         (),
         |cx, meta, action| {
             cx.editor.switch(meta.id, action);
+            let config = cx.editor.config();
             let (view, doc) = current!(cx.editor);
             doc.set_selection(view.id, meta.selection.clone());
+            view.ensure_cursor_in_view_center(doc, config.scrolloff);
         },
         |editor, meta| {
             let doc = &editor.documents.get(&meta.id)?;
@@ -4205,6 +4202,7 @@ fn match_brackets(cx: &mut Context) {
 
 fn jump_forward(cx: &mut Context) {
     let count = cx.count();
+    let config = cx.editor.config();
     let view = view_mut!(cx.editor);
     let doc_id = view.doc;
 
@@ -4218,12 +4216,13 @@ fn jump_forward(cx: &mut Context) {
         }
 
         doc.set_selection(view.id, selection);
-        align_view(doc, view, Align::Center);
+        view.ensure_cursor_in_view_center(doc, config.scrolloff);
     };
 }
 
 fn jump_backward(cx: &mut Context) {
     let count = cx.count();
+    let config = cx.editor.config();
     let (view, doc) = current!(cx.editor);
     let doc_id = doc.id();
 
@@ -4237,7 +4236,7 @@ fn jump_backward(cx: &mut Context) {
         }
 
         doc.set_selection(view.id, selection);
-        align_view(doc, view, Align::Center);
+        view.ensure_cursor_in_view_center(doc, config.scrolloff);
     };
 }
 

helix-view/src/view.rs

@@ -1,4 +1,4 @@
-use crate::{editor::GutterType, graphics::Rect, Document, DocumentId, ViewId};
+use crate::{align_view, editor::GutterType, graphics::Rect, Align, Document, DocumentId, ViewId};
 use helix_core::{
     pos_at_visual_coords, visual_coords_at_pos, Position, RopeSlice, Selection, Transaction,
 };
@@ -169,6 +169,15 @@ impl View {
         &self,
         doc: &Document,
         scrolloff: usize,
+    ) -> Option<(usize, usize)> {
+        self.offset_coords_to_in_view_center(doc, scrolloff, false)
+    }
+
+    pub fn offset_coords_to_in_view_center(
+        &self,
+        doc: &Document,
+        scrolloff: usize,
+        centering: bool,
     ) -> Option<(usize, usize)> {
         let cursor = doc
             .selection(self.id)
@@ -180,47 +189,69 @@ impl View {
 
         let inner_area = self.inner_area(doc);
         let last_line = (self.offset.row + inner_area.height as usize).saturating_sub(1);
-
-        // - 1 so we have at least one gap in the middle.
-        // a height of 6 with padding of 3 on each side will keep shifting the view back and forth
-        // as we type
-        let scrolloff = scrolloff.min(inner_area.height.saturating_sub(1) as usize / 2);
-
         let last_col = self.offset.col + inner_area.width.saturating_sub(1) as usize;
 
-        let row = if line > last_line.saturating_sub(scrolloff) {
-            // scroll down
-            self.offset.row + line - (last_line.saturating_sub(scrolloff))
-        } else if line < self.offset.row + scrolloff {
-            // scroll up
-            line.saturating_sub(scrolloff)
-        } else {
-            self.offset.row
+        let new_offset = |scrolloff: usize| {
+            // - 1 so we have at least one gap in the middle.
+            // a height of 6 with padding of 3 on each side will keep shifting the view back and forth
+            // as we type
+            let scrolloff = scrolloff.min(inner_area.height.saturating_sub(1) as usize / 2);
+
+            let row = if line > last_line.saturating_sub(scrolloff) {
+                // scroll down
+                self.offset.row + line - (last_line.saturating_sub(scrolloff))
+            } else if line < self.offset.row + scrolloff {
+                // scroll up
+                line.saturating_sub(scrolloff)
+            } else {
+                self.offset.row
+            };
+
+            let col = if col > last_col.saturating_sub(scrolloff) {
+                // scroll right
+                self.offset.col + col - (last_col.saturating_sub(scrolloff))
+            } else if col < self.offset.col + scrolloff {
+                // scroll left
+                col.saturating_sub(scrolloff)
+            } else {
+                self.offset.col
+            };
+            (row, col)
         };
-
-        let col = if col > last_col.saturating_sub(scrolloff) {
-            // scroll right
-            self.offset.col + col - (last_col.saturating_sub(scrolloff))
-        } else if col < self.offset.col + scrolloff {
-            // scroll left
-            col.saturating_sub(scrolloff)
+        let current_offset = (self.offset.row, self.offset.col);
+        if centering {
+            // return None if cursor is out of view
+            let offset = new_offset(0);
+            (offset == current_offset).then(|| {
+                if scrolloff == 0 {
+                    offset
+                } else {
+                    new_offset(scrolloff)
+                }
+            })
         } else {
-            self.offset.col
-        };
-        if row == self.offset.row && col == self.offset.col {
-            None
-        } else {
-            Some((row, col))
+            // return None if cursor is in (view - scrolloff)
+            let offset = new_offset(scrolloff);
+            (offset != current_offset).then(|| offset) // TODO: use 'then_some' when 1.62 <= MSRV
         }
     }
 
     pub fn ensure_cursor_in_view(&mut self, doc: &Document, scrolloff: usize) {
-        if let Some((row, col)) = self.offset_coords_to_in_view(doc, scrolloff) {
+        if let Some((row, col)) = self.offset_coords_to_in_view_center(doc, scrolloff, false) {
             self.offset.row = row;
             self.offset.col = col;
         }
     }
 
+    pub fn ensure_cursor_in_view_center(&mut self, doc: &Document, scrolloff: usize) {
+        if let Some((row, col)) = self.offset_coords_to_in_view_center(doc, scrolloff, true) {
+            self.offset.row = row;
+            self.offset.col = col;
+        } else {
+            align_view(doc, self, Align::Center);
+        }
+    }
+
     pub fn is_cursor_in_view(&mut self, doc: &Document, scrolloff: usize) -> bool {
         self.offset_coords_to_in_view(doc, scrolloff).is_none()
     }