fix: align view after jumplist_picker (#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.

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
+        view.ensure_cursor_in_view_center(doc, scrolloff);
-        if view.is_cursor_in_view(doc, 0) {
-            view.ensure_cursor_in_view(doc, scrolloff);
-        } else {
-            align_view(doc, view, Align::Center)
-        }
     };
 }
 
@@ -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,14 +189,14 @@ impl View {
 
         let inner_area = self.inner_area(doc);
         let last_line = (self.offset.row + inner_area.height as usize).saturating_sub(1);
+        let last_col = self.offset.col + inner_area.width.saturating_sub(1) as usize;
 
+        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 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))
@@ -207,20 +216,42 @@ impl View {
             } else {
                 self.offset.col
             };
-        if row == self.offset.row && col == self.offset.col {
+            (row, col)
-            None
+        };
+        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 {
-            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()
     }