editor: Preserve scroll position when jumping from multibuffer (#9921)

Piotr Osiewicz created

This is a best-effort attempt, as the target offset from the top is just
an estimate; furthermore, this does not account for things like project
search header (which adds a bit of vertical offset by itself and is
removed once we jump into a buffer), but it still should improve the
situation quite a bit.

Fixes: #5296

Release Notes:

- Improved target selection when jumping from multibuffer; final
position in the buffer should more closely match the original position
of the cursor in the multibuffer.

Change summary

crates/editor/src/editor.rs            | 15 ++++++++++-----
crates/editor/src/element.rs           | 27 +++++++++++++++++++++++++--
crates/editor/src/scroll/autoscroll.rs |  9 +++++++++
3 files changed, 44 insertions(+), 7 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -7670,7 +7670,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::focused()), cx, |s| {
                                 s.select_ranges([range]);
                             });
                         } else {
@@ -7690,7 +7690,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::focused()),
                                         cx,
                                         |s| {
                                             s.select_ranges([range]);
@@ -9504,6 +9504,7 @@ impl Editor {
         path: ProjectPath,
         position: Point,
         anchor: language::Anchor,
+        offset_from_top: u32,
         cx: &mut ViewContext<Self>,
     ) {
         let workspace = self.workspace();
@@ -9531,9 +9532,13 @@ impl Editor {
                 };
 
                 let nav_history = editor.nav_history.take();
-                editor.change_selections(Some(Autoscroll::newest()), cx, |s| {
-                    s.select_ranges([cursor..cursor]);
-                });
+                editor.change_selections(
+                    Some(Autoscroll::top_relative(offset_from_top as usize)),
+                    cx,
+                    |s| {
+                        s.select_ranges([cursor..cursor]);
+                    },
+                );
                 editor.nav_history = nav_history;
 
                 anyhow::Ok(())

crates/editor/src/element.rs 🔗

@@ -1406,6 +1406,7 @@ impl EditorElement {
         let render_block = |block: &TransformBlock,
                             available_space: Size<AvailableSpace>,
                             block_id: usize,
+                            block_row_start: u32,
                             cx: &mut ElementContext| {
             let mut element = match block {
                 TransformBlock::Custom(block) => {
@@ -1440,6 +1441,7 @@ impl EditorElement {
                     buffer,
                     range,
                     starts_new_buffer,
+                    height,
                     ..
                 } => {
                     let include_root = self
@@ -1455,6 +1457,7 @@ impl EditorElement {
                         position: Point,
                         anchor: text::Anchor,
                         path: ProjectPath,
+                        line_offset_from_top: u32,
                     }
 
                     let jump_data = project::File::from_dyn(buffer.file()).map(|file| {
@@ -1466,12 +1469,29 @@ impl EditorElement {
                             .primary
                             .as_ref()
                             .map_or(range.context.start, |primary| primary.start);
+
+                        let excerpt_start = range.context.start;
                         let jump_position = language::ToPoint::to_point(&jump_anchor, buffer);
+                        let offset_from_excerpt_start = if jump_anchor == excerpt_start {
+                            0
+                        } else {
+                            let excerpt_start_row =
+                                language::ToPoint::to_point(&jump_anchor, buffer).row;
+                            jump_position.row - excerpt_start_row
+                        };
+
+                        let line_offset_from_top =
+                            block_row_start + *height as u32 + offset_from_excerpt_start
+                                - snapshot
+                                    .scroll_anchor
+                                    .scroll_position(&snapshot.display_snapshot)
+                                    .y as u32;
 
                         JumpData {
                             position: jump_position,
                             anchor: jump_anchor,
                             path: jump_path,
+                            line_offset_from_top,
                         }
                     });
 
@@ -1541,6 +1561,7 @@ impl EditorElement {
                                                         jump_data.path.clone(),
                                                         jump_data.position,
                                                         jump_data.anchor,
+                                                        jump_data.line_offset_from_top,
                                                         cx,
                                                     );
                                                 }
@@ -1599,6 +1620,7 @@ impl EditorElement {
                                                             path.clone(),
                                                             jump_data.position,
                                                             jump_data.anchor,
+                                                            jump_data.line_offset_from_top,
                                                             cx,
                                                         );
                                                     }
@@ -1631,6 +1653,7 @@ impl EditorElement {
                                             path.clone(),
                                             jump_data.position,
                                             jump_data.anchor,
+                                            jump_data.line_offset_from_top,
                                             cx,
                                         );
                                     }
@@ -1663,7 +1686,7 @@ impl EditorElement {
                 AvailableSpace::MinContent,
                 AvailableSpace::Definite(block.height() as f32 * line_height),
             );
-            let (element, element_size) = render_block(block, available_space, block_id, cx);
+            let (element, element_size) = render_block(block, available_space, block_id, row, cx);
             block_id += 1;
             fixed_block_max_width = fixed_block_max_width.max(element_size.width + em_width);
             blocks.push(BlockLayout {
@@ -1691,7 +1714,7 @@ impl EditorElement {
                 AvailableSpace::Definite(width),
                 AvailableSpace::Definite(block.height() as f32 * line_height),
             );
-            let (element, _) = render_block(block, available_space, block_id, cx);
+            let (element, _) = render_block(block, available_space, block_id, row, cx);
             block_id += 1;
             blocks.push(BlockLayout {
                 row,

crates/editor/src/scroll/autoscroll.rs 🔗

@@ -32,6 +32,10 @@ impl Autoscroll {
     pub fn focused() -> Self {
         Self::Strategy(AutoscrollStrategy::Focused)
     }
+    /// Scrolls so that the newest cursor is roughly an n-th line from the top.
+    pub fn top_relative(n: usize) -> Self {
+        Self::Strategy(AutoscrollStrategy::TopRelative(n))
+    }
 }
 
 #[derive(PartialEq, Eq, Default, Clone, Copy)]
@@ -43,6 +47,7 @@ pub enum AutoscrollStrategy {
     Focused,
     Top,
     Bottom,
+    TopRelative(usize),
 }
 
 impl AutoscrollStrategy {
@@ -178,6 +183,10 @@ impl Editor {
                 scroll_position.y = (target_bottom - visible_lines).max(0.0);
                 self.set_scroll_position_internal(scroll_position, local, true, cx);
             }
+            AutoscrollStrategy::TopRelative(lines) => {
+                scroll_position.y = target_top - lines as f32;
+                self.set_scroll_position_internal(scroll_position, local, true, cx);
+            }
         }
 
         self.scroll_manager.last_autoscroll = Some((