Fix panic when a file in a path-based multibuffer excerpt is renamed (#28364)

Cole Miller and Conrad Irwin created 8 months ago

Closes #ISSUE

Release Notes:

- Fixed a panic that could occur when paths changed in the project diff.

Co-authored-by: Conrad Irwin <[e-mail]>

Change summary

crates/multi_buffer/src/multi_buffer.rs       | 42 +++++-----
crates/multi_buffer/src/multi_buffer_tests.rs | 82 +++++++++++++++++++++
crates/text/src/text.rs                       |  1 
3 files changed, 105 insertions(+), 20 deletions(-)

Detailed changes

crates/multi_buffer/src/multi_buffer.rs

@@ -1718,21 +1718,25 @@ impl MultiBuffer {
                 (None, None) => break,
                 (None, Some(_)) => {
                     let existing_id = existing_iter.next().unwrap();
-                    let locator = snapshot.excerpt_locator_for_id(existing_id);
-                    let existing_excerpt = excerpts_cursor.item().unwrap();
-                    excerpts_cursor.seek_forward(&Some(locator), Bias::Left, &());
-                    let existing_end = existing_excerpt
-                        .range
-                        .context
-                        .end
-                        .to_point(&buffer_snapshot);
                     if let Some((new_id, last)) = to_insert.last() {
-                        if existing_end <= last.context.end {
-                            self.snapshot
-                                .borrow_mut()
-                                .replaced_excerpts
-                                .insert(existing_id, *new_id);
-                        }
+                        let locator = snapshot.excerpt_locator_for_id(existing_id);
+                        excerpts_cursor.seek_forward(&Some(locator), Bias::Left, &());
+                        if let Some(existing_excerpt) = excerpts_cursor
+                            .item()
+                            .filter(|e| e.buffer_id == buffer_snapshot.remote_id())
+                        {
+                            let existing_end = existing_excerpt
+                                .range
+                                .context
+                                .end
+                                .to_point(&buffer_snapshot);
+                            if existing_end <= last.context.end {
+                                self.snapshot
+                                    .borrow_mut()
+                                    .replaced_excerpts
+                                    .insert(existing_id, *new_id);
+                            }
+                        };
                     }
                     to_remove.push(existing_id);
                     continue;
@@ -1745,16 +1749,14 @@ impl MultiBuffer {
             };
             let locator = snapshot.excerpt_locator_for_id(*existing);
             excerpts_cursor.seek_forward(&Some(locator), Bias::Left, &());
-            let Some(existing_excerpt) = excerpts_cursor.item() else {
+            let Some(existing_excerpt) = excerpts_cursor
+                .item()
+                .filter(|e| e.buffer_id == buffer_snapshot.remote_id())
+            else {
                 to_remove.push(existing_iter.next().unwrap());
                 to_insert.push((next_excerpt_id(), new_iter.next().unwrap()));
                 continue;
             };
-            if existing_excerpt.buffer_id != buffer_snapshot.remote_id() {
-                to_remove.push(existing_iter.next().unwrap());
-                to_insert.push((next_excerpt_id(), new_iter.next().unwrap()));
-                continue;
-            }
 
             let existing_start = existing_excerpt
                 .range

crates/multi_buffer/src/multi_buffer_tests.rs

@@ -1798,6 +1798,88 @@ fn test_set_excerpts_for_buffer(cx: &mut TestAppContext) {
     });
 }
 
+#[gpui::test]
+fn test_set_excerpts_for_buffer_rename(cx: &mut TestAppContext) {
+    let buf1 = cx.new(|cx| {
+        Buffer::local(
+            indoc! {
+            "zero
+            one
+            two
+            three
+            four
+            five
+            six
+            seven
+            ",
+            },
+            cx,
+        )
+    });
+    let path: PathKey = PathKey::namespaced(0, Path::new("/").into());
+    let buf2 = cx.new(|cx| {
+        Buffer::local(
+            indoc! {
+            "000
+            111
+            222
+            333
+            "
+            },
+            cx,
+        )
+    });
+
+    let multibuffer = cx.new(|_| MultiBuffer::new(Capability::ReadWrite));
+    multibuffer.update(cx, |multibuffer, cx| {
+        multibuffer.set_excerpts_for_path(
+            path.clone(),
+            buf1.clone(),
+            vec![Point::row_range(1..1), Point::row_range(4..5)],
+            1,
+            cx,
+        );
+    });
+
+    assert_excerpts_match(
+        &multibuffer,
+        cx,
+        indoc! {
+        "-----
+        zero
+        one
+        two
+        -----
+        three
+        four
+        five
+        six
+        "
+        },
+    );
+
+    multibuffer.update(cx, |multibuffer, cx| {
+        multibuffer.set_excerpts_for_path(
+            path.clone(),
+            buf2.clone(),
+            vec![Point::row_range(0..1)],
+            2,
+            cx,
+        );
+    });
+
+    assert_excerpts_match(
+        &multibuffer,
+        cx,
+        indoc! {"-----
+                000
+                111
+                222
+                333
+                "},
+    );
+}
+
 #[gpui::test]
 fn test_diff_hunks_with_multiple_excerpts(cx: &mut TestAppContext) {
     let base_text_1 = indoc!(

crates/text/src/text.rs

@@ -2231,6 +2231,7 @@ impl BufferSnapshot {
         } else if *anchor == Anchor::MAX {
             self.visible_text.len()
         } else {
+            debug_assert!(anchor.buffer_id == Some(self.remote_id));
             let anchor_key = InsertionFragmentKey {
                 timestamp: anchor.timestamp,
                 split_offset: anchor.offset,