Fix excerpt jumps using selections, not the match data (#20491)

Kirill Bulatov and Bennet created

Fixes
https://github.com/zed-industries/zed/pull/20469#issuecomment-2466805325

Release Notes:

- N/A

---------

Co-authored-by: Bennet <bennet@zed.dev>

Change summary

crates/editor/src/editor.rs  | 105 ++++++++++++++++++++++++++---------
crates/editor/src/element.rs | 111 ++++++++++++++++++++++++-------------
2 files changed, 149 insertions(+), 67 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -1748,6 +1748,15 @@ pub(crate) struct FocusedBlock {
     focus_handle: WeakFocusHandle,
 }
 
+#[derive(Clone)]
+struct JumpData {
+    excerpt_id: ExcerptId,
+    position: Point,
+    anchor: text::Anchor,
+    path: Option<project::ProjectPath>,
+    line_offset_from_top: u32,
+}
+
 impl Editor {
     pub fn single_line(cx: &mut ViewContext<Self>) -> Self {
         let buffer = cx.new_model(|cx| Buffer::local("", cx));
@@ -12584,49 +12593,85 @@ impl Editor {
     }
 
     pub fn open_excerpts_in_split(&mut self, _: &OpenExcerptsSplit, cx: &mut ViewContext<Self>) {
-        self.open_excerpts_common(true, cx)
+        self.open_excerpts_common(None, true, cx)
     }
 
     pub fn open_excerpts(&mut self, _: &OpenExcerpts, cx: &mut ViewContext<Self>) {
-        self.open_excerpts_common(false, cx)
+        self.open_excerpts_common(None, false, cx)
     }
 
-    fn open_excerpts_common(&mut self, split: bool, cx: &mut ViewContext<Self>) {
-        let selections = self.selections.all::<usize>(cx);
-        let buffer = self.buffer.read(cx);
-        if buffer.is_singleton() {
+    fn open_excerpts_common(
+        &mut self,
+        jump_data: Option<JumpData>,
+        split: bool,
+        cx: &mut ViewContext<Self>,
+    ) {
+        let Some(workspace) = self.workspace() else {
             cx.propagate();
             return;
-        }
+        };
 
-        let Some(workspace) = self.workspace() else {
+        if self.buffer.read(cx).is_singleton() {
             cx.propagate();
             return;
-        };
+        }
 
         let mut new_selections_by_buffer = HashMap::default();
-        for selection in selections {
-            for (mut buffer_handle, mut range, _) in
-                buffer.range_to_buffer_ranges(selection.range(), cx)
-            {
-                // When editing branch buffers, jump to the corresponding location
-                // in their base buffer.
-                let buffer = buffer_handle.read(cx);
-                if let Some(base_buffer) = buffer.diff_base_buffer() {
-                    range = buffer.range_to_version(range, &base_buffer.read(cx).version());
-                    buffer_handle = base_buffer;
+        match &jump_data {
+            Some(jump_data) => {
+                let multi_buffer_snapshot = self.buffer.read(cx).snapshot(cx);
+                if let Some(buffer) = multi_buffer_snapshot
+                    .buffer_id_for_excerpt(jump_data.excerpt_id)
+                    .and_then(|buffer_id| self.buffer.read(cx).buffer(buffer_id))
+                {
+                    let buffer_snapshot = buffer.read(cx).snapshot();
+                    let jump_to_point = if buffer_snapshot.can_resolve(&jump_data.anchor) {
+                        language::ToPoint::to_point(&jump_data.anchor, &buffer_snapshot)
+                    } else {
+                        buffer_snapshot.clip_point(jump_data.position, Bias::Left)
+                    };
+                    let jump_to_offset = buffer_snapshot.point_to_offset(jump_to_point);
+                    new_selections_by_buffer.insert(
+                        buffer,
+                        (
+                            vec![jump_to_offset..jump_to_offset],
+                            Some(jump_data.line_offset_from_top),
+                        ),
+                    );
                 }
+            }
+            None => {
+                let selections = self.selections.all::<usize>(cx);
+                let buffer = self.buffer.read(cx);
+                for selection in selections {
+                    for (mut buffer_handle, mut range, _) in
+                        buffer.range_to_buffer_ranges(selection.range(), cx)
+                    {
+                        // When editing branch buffers, jump to the corresponding location
+                        // in their base buffer.
+                        let buffer = buffer_handle.read(cx);
+                        if let Some(base_buffer) = buffer.diff_base_buffer() {
+                            range = buffer.range_to_version(range, &base_buffer.read(cx).version());
+                            buffer_handle = base_buffer;
+                        }
 
-                if selection.reversed {
-                    mem::swap(&mut range.start, &mut range.end);
+                        if selection.reversed {
+                            mem::swap(&mut range.start, &mut range.end);
+                        }
+                        new_selections_by_buffer
+                            .entry(buffer_handle)
+                            .or_insert((Vec::new(), None))
+                            .0
+                            .push(range)
+                    }
                 }
-                new_selections_by_buffer
-                    .entry(buffer_handle)
-                    .or_insert(Vec::new())
-                    .push(range)
             }
         }
 
+        if new_selections_by_buffer.is_empty() {
+            return;
+        }
+
         // We defer the pane interaction because we ourselves are a workspace item
         // and activating a new item causes the pane to call a method on us reentrantly,
         // which panics if we're on the stack.
@@ -12638,13 +12683,19 @@ impl Editor {
                     workspace.active_pane().clone()
                 };
 
-                for (buffer, ranges) in new_selections_by_buffer {
+                for (buffer, (ranges, scroll_offset)) in new_selections_by_buffer {
                     let editor =
                         workspace.open_project_item::<Self>(pane.clone(), buffer, true, true, cx);
                     editor.update(cx, |editor, cx| {
-                        editor.change_selections(Some(Autoscroll::newest()), cx, |s| {
+                        let autoscroll = match scroll_offset {
+                            Some(scroll_offset) => Autoscroll::top_relative(scroll_offset as usize),
+                            None => Autoscroll::newest(),
+                        };
+                        let nav_history = editor.nav_history.take();
+                        editor.change_selections(Some(autoscroll), cx, |s| {
                             s.select_ranges(ranges);
                         });
+                        editor.nav_history = nav_history;
                     });
                 }
             })

crates/editor/src/element.rs 🔗

@@ -19,8 +19,8 @@ use crate::{
     BlockId, CodeActionsMenu, CursorShape, CustomBlockId, DisplayPoint, DisplayRow,
     DocumentHighlightRead, DocumentHighlightWrite, Editor, EditorMode, EditorSettings,
     EditorSnapshot, EditorStyle, ExpandExcerpts, FocusedBlock, GutterDimensions, HalfPageDown,
-    HalfPageUp, HandleInput, HoveredCursor, HoveredHunk, LineDown, LineUp, OpenExcerpts, PageDown,
-    PageUp, Point, RowExt, RowRangeExt, SelectPhase, Selection, SoftWrap, ToPoint,
+    HalfPageUp, HandleInput, HoveredCursor, HoveredHunk, JumpData, LineDown, LineUp, OpenExcerpts,
+    PageDown, PageUp, Point, RowExt, RowRangeExt, SelectPhase, Selection, SoftWrap, ToPoint,
     CURSORS_VISIBLE_FOR, FILE_HEADER_HEIGHT, GIT_BLAME_MAX_AUTHOR_CHARS_DISPLAYED, MAX_LINE_LEN,
     MULTI_BUFFER_EXCERPT_HEADER_HEIGHT,
 };
@@ -2058,6 +2058,7 @@ impl EditorElement {
         block: &Block,
         available_width: AvailableSpace,
         block_id: BlockId,
+        block_row_start: DisplayRow,
         snapshot: &EditorSnapshot,
         text_x: Pixels,
         rows: &Range<DisplayRow>,
@@ -2129,14 +2130,9 @@ impl EditorElement {
                 next_excerpt,
                 show_excerpt_controls,
                 starts_new_buffer,
+                height,
                 ..
             } => {
-                #[derive(Clone)]
-                struct JumpData {
-                    position: Point,
-                    path: Option<ProjectPath>,
-                }
-
                 let icon_offset = gutter_dimensions.width
                     - (gutter_dimensions.left_padding + gutter_dimensions.margin);
 
@@ -2175,9 +2171,28 @@ 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.0 + *height + offset_from_excerpt_start
+                                - snapshot
+                                    .scroll_anchor
+                                    .scroll_position(&snapshot.display_snapshot)
+                                    .y as u32;
                         JumpData {
+                            excerpt_id: next_excerpt.id,
+                            anchor: jump_anchor,
                             position: language::ToPoint::to_point(&jump_anchor, buffer),
                             path: jump_path,
+                            line_offset_from_top,
                         }
                     };
 
@@ -2245,6 +2260,7 @@ impl EditorElement {
                                         .on_click(cx.listener_for(&self.editor, {
                                             move |editor, e: &ClickEvent, cx| {
                                                 editor.open_excerpts_common(
+                                                    Some(jump_data.clone()),
                                                     e.down.modifiers.secondary(),
                                                     cx,
                                                 );
@@ -2289,39 +2305,51 @@ impl EditorElement {
                                         }),
                                 )
                                 .cursor_pointer()
-                                .on_click(cx.listener_for(&self.editor, {
-                                    move |editor, e: &ClickEvent, cx| {
-                                        cx.stop_propagation();
-                                        editor
-                                            .open_excerpts_common(e.down.modifiers.secondary(), cx);
+                                .on_click({
+                                    let jump_data = jump_data.clone();
+                                    cx.listener_for(&self.editor, {
+                                        let jump_data = jump_data.clone();
+                                        move |editor, e: &ClickEvent, cx| {
+                                            cx.stop_propagation();
+                                            editor.open_excerpts_common(
+                                                Some(jump_data.clone()),
+                                                e.down.modifiers.secondary(),
+                                                cx,
+                                            );
+                                        }
+                                    })
+                                })
+                                .tooltip({
+                                    let jump_data = jump_data.clone();
+                                    move |cx| {
+                                        let jump_message = format!(
+                                            "Jump to {}:L{}",
+                                            match &jump_data.path {
+                                                Some(project_path) =>
+                                                    project_path.path.display().to_string(),
+                                                None => {
+                                                    let editor = editor.read(cx);
+                                                    editor
+                                                        .file_at(jump_data.position, cx)
+                                                        .map(|file| {
+                                                            file.full_path(cx).display().to_string()
+                                                        })
+                                                        .or_else(|| {
+                                                            Some(
+                                                                editor
+                                                                    .tab_description(0, cx)?
+                                                                    .to_string(),
+                                                            )
+                                                        })
+                                                        .unwrap_or_else(|| {
+                                                            "Unknown buffer".to_string()
+                                                        })
+                                                }
+                                            },
+                                            jump_data.position.row + 1
+                                        );
+                                        Tooltip::for_action(jump_message, &OpenExcerpts, cx)
                                     }
-                                }))
-                                .tooltip(move |cx| {
-                                    let jump_message = format!(
-                                        "Jump to {}:L{}",
-                                        match &jump_data.path {
-                                            Some(project_path) =>
-                                                project_path.path.display().to_string(),
-                                            None => {
-                                                let editor = editor.read(cx);
-                                                editor
-                                                    .file_at(jump_data.position, cx)
-                                                    .map(|file| {
-                                                        file.full_path(cx).display().to_string()
-                                                    })
-                                                    .or_else(|| {
-                                                        Some(
-                                                            editor
-                                                                .tab_description(0, cx)?
-                                                                .to_string(),
-                                                        )
-                                                    })
-                                                    .unwrap_or_else(|| "Unknown buffer".to_string())
-                                            }
-                                        },
-                                        jump_data.position.row + 1
-                                    );
-                                    Tooltip::for_action(jump_message, &OpenExcerpts, cx)
                                 })
                                 .child(
                                     h_flex()
@@ -2456,6 +2484,7 @@ impl EditorElement {
                 block,
                 AvailableSpace::MinContent,
                 block_id,
+                row,
                 snapshot,
                 text_x,
                 &rows,
@@ -2501,6 +2530,7 @@ impl EditorElement {
                 block,
                 width.into(),
                 block_id,
+                row,
                 snapshot,
                 text_x,
                 &rows,
@@ -2547,6 +2577,7 @@ impl EditorElement {
                             &block,
                             width,
                             focused_block.id,
+                            rows.end,
                             snapshot,
                             text_x,
                             &rows,