Fix cursor position when navigating to a multibuffer's first excerpt (#25723)

Cole Miller and Max created

This PR fixes an unexpected cursor position when jumping to the
beginning of the project diff editor's first excerpt if that excerpt
starts with a deleted region. Previously, the cursor would end up in the
*following* region in this situation; now it ends up at the start of the
deleted region, as happens already for excerpts that are not the first.

Release Notes:

- N/A

---------

Co-authored-by: Max <max@zed.dev>

Change summary

crates/git_ui/src/project_diff.rs             | 96 +++++++++++++++++++++
crates/multi_buffer/src/multi_buffer.rs       |  5 -
crates/multi_buffer/src/multi_buffer_tests.rs | 86 ++++++++++++++++++
3 files changed, 182 insertions(+), 5 deletions(-)

Detailed changes

crates/git_ui/src/project_diff.rs πŸ”—

@@ -935,6 +935,8 @@ impl Render for ProjectDiffToolbar {
 
 #[cfg(test)]
 mod tests {
+    use std::path::Path;
+
     use collections::HashMap;
     use editor::test::editor_test_context::assert_state_with_diff;
     use git::status::{StatusCode, TrackedStatus};
@@ -1021,4 +1023,98 @@ mod tests {
         let text = String::from_utf8(fs.read_file_sync("/project/foo").unwrap()).unwrap();
         assert_eq!(text, "foo\n");
     }
+
+    #[gpui::test]
+    async fn test_scroll_to_beginning_with_deletion(cx: &mut TestAppContext) {
+        init_test(cx);
+
+        let fs = FakeFs::new(cx.executor());
+        fs.insert_tree(
+            path!("/project"),
+            json!({
+                ".git": {},
+                "bar": "BAR\n",
+                "foo": "FOO\n",
+            }),
+        )
+        .await;
+        let project = Project::test(fs.clone(), [path!("/project").as_ref()], cx).await;
+        let (workspace, cx) =
+            cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx));
+        let diff = cx.new_window_entity(|window, cx| {
+            ProjectDiff::new(project.clone(), workspace, window, cx)
+        });
+        cx.run_until_parked();
+
+        fs.set_head_for_repo(
+            path!("/project/.git").as_ref(),
+            &[
+                ("bar".into(), "bar\n".into()),
+                ("foo".into(), "foo\n".into()),
+            ],
+        );
+        fs.with_git_state(path!("/project/.git").as_ref(), true, |state| {
+            state.statuses = HashMap::from_iter([
+                (
+                    "bar".into(),
+                    TrackedStatus {
+                        index_status: StatusCode::Unmodified,
+                        worktree_status: StatusCode::Modified,
+                    }
+                    .into(),
+                ),
+                (
+                    "foo".into(),
+                    TrackedStatus {
+                        index_status: StatusCode::Unmodified,
+                        worktree_status: StatusCode::Modified,
+                    }
+                    .into(),
+                ),
+            ]);
+        });
+        cx.run_until_parked();
+
+        let editor = cx.update_window_entity(&diff, |diff, window, cx| {
+            diff.scroll_to_path(
+                PathKey::namespaced(TRACKED_NAMESPACE, Path::new("foo").into()),
+                window,
+                cx,
+            );
+            diff.editor.clone()
+        });
+        assert_state_with_diff(
+            &editor,
+            cx,
+            &"
+                - bar
+                + BAR
+
+                - Λ‡foo
+                + FOO
+            "
+            .unindent(),
+        );
+
+        let editor = cx.update_window_entity(&diff, |diff, window, cx| {
+            diff.scroll_to_path(
+                PathKey::namespaced(TRACKED_NAMESPACE, Path::new("bar").into()),
+                window,
+                cx,
+            );
+            diff.editor.clone()
+        });
+        assert_state_with_diff(
+            &editor,
+            cx,
+            &"
+                - Λ‡bar
+                + BAR
+
+                - foo
+                + FOO
+            "
+            .unindent(),
+        );
+    }
 }

crates/multi_buffer/src/multi_buffer.rs πŸ”—

@@ -4535,7 +4535,6 @@ impl MultiBufferSnapshot {
                     base_text_byte_range,
                     ..
                 }) => {
-                    let mut in_deleted_hunk = false;
                     if let Some(diff_base_anchor) = &anchor.diff_base_anchor {
                         if let Some(base_text) =
                             self.diffs.get(buffer_id).and_then(|diff| diff.base_text())
@@ -4550,7 +4549,6 @@ impl MultiBufferSnapshot {
                                             base_text_byte_range.start..base_text_offset,
                                         );
                                     position.add_assign(&position_in_hunk);
-                                    in_deleted_hunk = true;
                                 } else if at_transform_end {
                                     diff_transforms.next(&());
                                     continue;
@@ -4558,9 +4556,6 @@ impl MultiBufferSnapshot {
                             }
                         }
                     }
-                    if !in_deleted_hunk {
-                        position = diff_transforms.end(&()).1 .0;
-                    }
                 }
                 _ => {
                     if at_transform_end && anchor.diff_base_anchor.is_some() {

crates/multi_buffer/src/multi_buffer_tests.rs πŸ”—

@@ -3009,6 +3009,92 @@ async fn test_enclosing_indent(cx: &mut TestAppContext) {
     );
 }
 
+#[gpui::test]
+fn test_summaries_for_anchors(cx: &mut TestAppContext) {
+    let base_text_1 = indoc!(
+        "
+        bar
+        "
+    );
+    let text_1 = indoc!(
+        "
+        BAR
+        "
+    );
+    let base_text_2 = indoc!(
+        "
+        foo
+        "
+    );
+    let text_2 = indoc!(
+        "
+        FOO
+        "
+    );
+
+    let buffer_1 = cx.new(|cx| Buffer::local(text_1, cx));
+    let buffer_2 = cx.new(|cx| Buffer::local(text_2, cx));
+    let diff_1 = cx.new(|cx| BufferDiff::new_with_base_text(base_text_1, &buffer_1, cx));
+    let diff_2 = cx.new(|cx| BufferDiff::new_with_base_text(base_text_2, &buffer_2, cx));
+    cx.run_until_parked();
+
+    let mut ids = vec![];
+    let multibuffer = cx.new(|cx| {
+        let mut multibuffer = MultiBuffer::new(Capability::ReadWrite);
+        multibuffer.set_all_diff_hunks_expanded(cx);
+        ids.extend(multibuffer.push_excerpts(
+            buffer_1.clone(),
+            [ExcerptRange {
+                context: text::Anchor::MIN..text::Anchor::MAX,
+                primary: None,
+            }],
+            cx,
+        ));
+        ids.extend(multibuffer.push_excerpts(
+            buffer_2.clone(),
+            [ExcerptRange {
+                context: text::Anchor::MIN..text::Anchor::MAX,
+                primary: None,
+            }],
+            cx,
+        ));
+        multibuffer.add_diff(diff_1.clone(), cx);
+        multibuffer.add_diff(diff_2.clone(), cx);
+        multibuffer
+    });
+
+    let (mut snapshot, mut subscription) = multibuffer.update(cx, |multibuffer, cx| {
+        (multibuffer.snapshot(cx), multibuffer.subscribe())
+    });
+
+    assert_new_snapshot(
+        &multibuffer,
+        &mut snapshot,
+        &mut subscription,
+        cx,
+        indoc!(
+            "
+            - bar
+            + BAR
+
+            - foo
+            + FOO
+            "
+        ),
+    );
+
+    let id_1 = buffer_1.read_with(cx, |buffer, _| buffer.remote_id());
+    let id_2 = buffer_2.read_with(cx, |buffer, _| buffer.remote_id());
+
+    let anchor_1 = Anchor::in_buffer(ids[0], id_1, text::Anchor::MIN);
+    let point_1 = snapshot.summaries_for_anchors::<Point, _>([&anchor_1])[0];
+    assert_eq!(point_1, Point::new(0, 0));
+
+    let anchor_2 = Anchor::in_buffer(ids[1], id_2, text::Anchor::MIN);
+    let point_2 = snapshot.summaries_for_anchors::<Point, _>([&anchor_2])[0];
+    assert_eq!(point_2, Point::new(3, 0));
+}
+
 fn format_diff(
     text: &str,
     row_infos: &Vec<RowInfo>,