From 0459b1d30387361d35a0e0600304d28acdf6ef4a Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Tue, 8 Apr 2025 18:01:40 -0400 Subject: [PATCH] Fix panic when a file in a path-based multibuffer excerpt is renamed (#28364) Closes #ISSUE Release Notes: - Fixed a panic that could occur when paths changed in the project diff. Co-authored-by: Conrad Irwin --- 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(-) diff --git a/crates/multi_buffer/src/multi_buffer.rs b/crates/multi_buffer/src/multi_buffer.rs index 8d3363b55c..cda6161406 100644 --- a/crates/multi_buffer/src/multi_buffer.rs +++ b/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 diff --git a/crates/multi_buffer/src/multi_buffer_tests.rs b/crates/multi_buffer/src/multi_buffer_tests.rs index c3e8a69652..0c71cfabb1 100644 --- a/crates/multi_buffer/src/multi_buffer_tests.rs +++ b/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!( diff --git a/crates/text/src/text.rs b/crates/text/src/text.rs index d042ebf6d0..62130e0510 100644 --- a/crates/text/src/text.rs +++ b/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,