From d9326ec29324deb4a617b999f74867fdb1b0b4a7 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Wed, 4 Mar 2026 09:45:26 -0500 Subject: [PATCH] fix more multibuffer tests --- crates/multi_buffer/src/multi_buffer.rs | 6 +- crates/multi_buffer/src/multi_buffer_tests.rs | 304 ++++++------------ crates/multi_buffer/src/path_key.rs | 12 +- 3 files changed, 105 insertions(+), 217 deletions(-) diff --git a/crates/multi_buffer/src/multi_buffer.rs b/crates/multi_buffer/src/multi_buffer.rs index 889c8e9d2e221f47888203249e13b305f448dc22..4b84dc5dda0918b9be01b2b6532dba2d62e35ac5 100644 --- a/crates/multi_buffer/src/multi_buffer.rs +++ b/crates/multi_buffer/src/multi_buffer.rs @@ -106,7 +106,7 @@ pub enum Event { BufferUpdated { buffer: Entity, path_key: PathKey, - ranges: Vec>, + ranges: Vec>, }, BuffersRemoved { removed_buffer_ids: Vec, @@ -5367,10 +5367,10 @@ impl MultiBufferSnapshot { } } - pub fn excerpts(&self) -> impl Iterator)> { + pub fn excerpts(&self) -> impl Iterator { self.excerpts .iter() - .map(|excerpt| (excerpt.buffer_snapshot(self), excerpt.range.clone())) + .map(|excerpt| (excerpt.buffer_snapshot(self), excerpt.info())) } fn cursor<'a, MBD, BD>(&'a self) -> MultiBufferCursor<'a, MBD, BD> diff --git a/crates/multi_buffer/src/multi_buffer_tests.rs b/crates/multi_buffer/src/multi_buffer_tests.rs index b5446349de83cdbd9095901d8b8d72ef1c08a7da..7a3eaa6956b81853b4ad9fd226806e8f841e23f3 100644 --- a/crates/multi_buffer/src/multi_buffer_tests.rs +++ b/crates/multi_buffer/src/multi_buffer_tests.rs @@ -739,12 +739,27 @@ fn test_excerpt_events(cx: &mut App) { cx.subscribe( &leader_multibuffer, move |follower, _, event, cx| match event.clone() { - Event::ExcerptsAdded { + Event::BufferUpdated { buffer, - predecessor, - excerpts, - } => follower.insert_excerpts_with_ids_after(predecessor, buffer, excerpts, cx), - Event::BuffersRemoved { ids, .. } => follower.remove_excerpts(ids, cx), + path_key, + ranges, + } => { + let buffer_snapshot = buffer.read(cx).snapshot(); + follower.set_merged_excerpt_ranges_for_path( + path_key, + buffer, + &buffer_snapshot, + ranges, + cx, + ); + } + Event::BuffersRemoved { + removed_buffer_ids, .. + } => { + for id in removed_buffer_ids { + follower.remove_excerpts_for_buffer(id, cx); + } + } Event::Edited { .. } => { *follower_edit_event_count.write() += 1; } @@ -859,8 +874,12 @@ fn test_expand_excerpts(cx: &mut App) { multibuffer.update(cx, |multibuffer, cx| { let line_zero = multibuffer.snapshot(cx).anchor_before(Point::new(0, 0)); - multibuffer.expand_excerpts( - multibuffer.excerpt_ids(), + + multibuffer.expand_excerpts_with_paths( + multibuffer + .snapshot(cx) + .excerpts() + .map(|(_, info)| Anchor::in_buffer(info.path_key_index, info.range.context.end)), 1, ExpandExcerptDirection::UpAndDown, cx, @@ -1243,153 +1262,6 @@ fn test_multibuffer_anchors(cx: &mut App) { ); } -#[gpui::test] -fn test_resolving_anchors_after_replacing_their_excerpts(cx: &mut App) { - let buffer_1 = cx.new(|cx| Buffer::local("abcd", cx)); - let buffer_2 = cx.new(|cx| Buffer::local("ABCDEFGHIJKLMNOP", cx)); - let multibuffer = cx.new(|_| MultiBuffer::new(Capability::ReadWrite)); - - // Create an insertion id in buffer 1 that doesn't exist in buffer 2. - // Add an excerpt from buffer 1 that spans this new insertion. - buffer_1.update(cx, |buffer, cx| buffer.edit([(4..4, "123")], None, cx)); - let excerpt_id_1 = multibuffer.update(cx, |multibuffer, cx| { - let buffer_1_snapshot = buffer_1.read(cx).snapshot(); - multibuffer.set_excerpt_ranges_for_path( - PathKey::sorted(0), - buffer_1, - &buffer_1_snapshot, - vec![ExcerptRange::new((0..7).to_point(&buffer_1_snapshot))], - cx, - ); - multibuffer.excerpt_ids().into_iter().next().unwrap() - }); - - let snapshot_1 = multibuffer.read(cx).snapshot(cx); - assert_eq!(snapshot_1.text(), "abcd123"); - - // Replace the buffer 1 excerpt with new excerpts from buffer 2. - let (excerpt_id_2, _excerpt_id_3) = multibuffer.update(cx, |multibuffer, cx| { - multibuffer.remove_excerpts_for_path(PathKey::sorted(0), cx); - let snapshot_2 = buffer_2.read(cx).snapshot(); - multibuffer.set_excerpt_ranges_for_path( - PathKey::sorted(1), - buffer_2.clone(), - &buffer_2.read(cx).snapshot(), - vec![ - ExcerptRange::new((0..4).to_point(&snapshot_2)), - ExcerptRange::new((6..10).to_point(&snapshot_2)), - ExcerptRange::new((12..16).to_point(&snapshot_2)), - ], - cx, - ); - let mut ids = multibuffer - .excerpts_for_buffer(buffer_2.read(cx).remote_id(), cx) - .into_iter() - .map(|(id, _)| id); - (ids.next().unwrap(), ids.next().unwrap()) - }); - let snapshot_2 = multibuffer.read(cx).snapshot(cx); - assert_eq!(snapshot_2.text(), "ABCD\nGHIJ\nMNOP"); - - // The old excerpt id doesn't get reused. - assert_ne!(excerpt_id_2, excerpt_id_1); - - // Resolve some anchors from the previous snapshot in the new snapshot. - // The current excerpts are from a different buffer, so we don't attempt to - // resolve the old text anchor in the new buffer. - assert_eq!( - snapshot_2.summary_for_anchor::( - &snapshot_1.anchor_before(MultiBufferOffset(2)) - ), - MultiBufferOffset(0) - ); - assert_eq!( - snapshot_2.summaries_for_anchors::(&[ - snapshot_1.anchor_before(MultiBufferOffset(2)), - snapshot_1.anchor_after(MultiBufferOffset(3)) - ]), - vec![MultiBufferOffset(0), MultiBufferOffset(0)] - ); - - // Refresh anchors from the old snapshot. The return value indicates that both - // anchors lost their original excerpt. - let refresh = snapshot_2.refresh_anchors(&[ - snapshot_1.anchor_before(MultiBufferOffset(2)), - snapshot_1.anchor_after(MultiBufferOffset(3)), - ]); - assert_eq!( - refresh, - &[ - (0, snapshot_2.anchor_before(MultiBufferOffset(0)), false), - (1, snapshot_2.anchor_after(MultiBufferOffset(0)), false), - ] - ); - - // Replace the middle excerpt with a smaller excerpt in buffer 2, - // that intersects the old excerpt. - multibuffer.update(cx, |multibuffer, cx| { - let snapshot_2 = buffer_2.read(cx).snapshot(); - multibuffer.set_excerpt_ranges_for_path( - PathKey::sorted(1), - buffer_2.clone(), - &buffer_2.read(cx).snapshot(), - vec![ - ExcerptRange::new((0..4).to_point(&snapshot_2)), - ExcerptRange::new((12..16).to_point(&snapshot_2)), - ], - cx, - ); - multibuffer.set_excerpt_ranges_for_path( - PathKey::sorted(1), - buffer_2.clone(), - &buffer_2.read(cx).snapshot(), - vec![ - ExcerptRange::new((0..4).to_point(&snapshot_2)), - ExcerptRange::new((5..8).to_point(&snapshot_2)), - ExcerptRange::new((12..16).to_point(&snapshot_2)), - ], - cx, - ); - }); - - let snapshot_3 = multibuffer.read(cx).snapshot(cx); - assert_eq!(snapshot_3.text(), "ABCD\nFGH\nMNOP"); - - // Resolve some anchors from the previous snapshot in the new snapshot. - // The third anchor can't be resolved, since its excerpt has been removed, - // so it resolves to the same position as its predecessor. - let anchors = [ - snapshot_2.anchor_before(MultiBufferOffset(0)), - snapshot_2.anchor_after(MultiBufferOffset(2)), - snapshot_2.anchor_after(MultiBufferOffset(6)), - snapshot_2.anchor_after(MultiBufferOffset(14)), - ]; - assert_eq!( - snapshot_3.summaries_for_anchors::(&anchors), - &[ - MultiBufferOffset(0), - MultiBufferOffset(2), - MultiBufferOffset(9), - MultiBufferOffset(13) - ] - ); - - let new_anchors = snapshot_3.refresh_anchors(&anchors); - assert_eq!( - new_anchors.iter().map(|a| (a.0, a.2)).collect::>(), - &[(0, true), (1, true), (2, true), (3, true)] - ); - assert_eq!( - snapshot_3.summaries_for_anchors::(new_anchors.iter().map(|a| &a.1)), - &[ - MultiBufferOffset(0), - MultiBufferOffset(2), - MultiBufferOffset(7), - MultiBufferOffset(13) - ] - ); -} - #[gpui::test] async fn test_basic_diff_hunks(cx: &mut TestAppContext) { let text = indoc!( @@ -4498,7 +4370,7 @@ fn assert_position_translation(snapshot: &MultiBufferSnapshot) { if let Some((buffer, offset)) = snapshot.point_to_buffer_offset(snapshot.max_point()) { assert!(offset.0 <= buffer.len()); } - if let Some((buffer, point, _)) = snapshot.point_to_buffer_point(snapshot.max_point()) { + if let Some((buffer, point)) = snapshot.point_to_buffer_point(snapshot.max_point()) { assert!(point <= buffer.max_point()); } } @@ -4969,38 +4841,40 @@ fn test_excerpts_containment_functions(cx: &mut App) { let multibuffer = cx.new(|_| MultiBuffer::new(Capability::ReadWrite)); - let (excerpt_1_id, excerpt_2_id, excerpt_3_id) = multibuffer.update(cx, |multibuffer, cx| { - multibuffer.set_excerpts_for_path( - PathKey::sorted(0), - buffer_1.clone(), - [Point::new(0, 0)..Point::new(1, 3)], - 0, - cx, - ); + let (excerpt_1_info, excerpt_2_info, excerpt_3_info) = + multibuffer.update(cx, |multibuffer, cx| { + multibuffer.set_excerpts_for_path( + PathKey::sorted(0), + buffer_1.clone(), + [Point::new(0, 0)..Point::new(1, 3)], + 0, + cx, + ); - multibuffer.set_excerpts_for_path( - PathKey::sorted(1), - buffer_2.clone(), - [Point::new(0, 0)..Point::new(1, 3)], - 0, - cx, - ); + multibuffer.set_excerpts_for_path( + PathKey::sorted(1), + buffer_2.clone(), + [Point::new(0, 0)..Point::new(1, 3)], + 0, + cx, + ); - multibuffer.set_excerpts_for_path( - PathKey::sorted(2), - buffer_3.clone(), - [Point::new(0, 0)..Point::new(0, 3)], - 0, - cx, - ); + multibuffer.set_excerpts_for_path( + PathKey::sorted(2), + buffer_3.clone(), + [Point::new(0, 0)..Point::new(0, 3)], + 0, + cx, + ); - let mut ids = multibuffer.excerpt_ids().into_iter(); - ( - ids.next().unwrap(), - ids.next().unwrap(), - ids.next().unwrap(), - ) - }); + let snapshot = multibuffer.snapshot(cx); + let mut infos = snapshot.excerpts().map(|(_, info)| info); + ( + infos.next().unwrap(), + infos.next().unwrap(), + infos.next().unwrap(), + ) + }); let snapshot = multibuffer.read(cx).snapshot(cx); @@ -5018,24 +4892,24 @@ fn test_excerpts_containment_functions(cx: &mut App) { let excerpts: Vec<_> = snapshot.excerpts_for_range(p00..p00).collect(); assert_eq!(excerpts.len(), 1); - assert_eq!(excerpts[0].id, excerpt_1_id); + assert_eq!(excerpts[0].info(), excerpt_1_info); // Cursor at very end of excerpt 3 let excerpts: Vec<_> = snapshot.excerpts_for_range(p43..p43).collect(); assert_eq!(excerpts.len(), 1); - assert_eq!(excerpts[0].id, excerpt_3_id); + assert_eq!(excerpts[0].info(), excerpt_3_info); let excerpts: Vec<_> = snapshot.excerpts_for_range(p00..p23).collect(); assert_eq!(excerpts.len(), 2); - assert_eq!(excerpts[0].id, excerpt_1_id); - assert_eq!(excerpts[1].id, excerpt_2_id); + assert_eq!(excerpts[0].info(), excerpt_1_info); + assert_eq!(excerpts[1].info(), excerpt_2_info); // This range represent an selection with end-point just inside excerpt_2 // Today we only expand the first excerpt, but another interpretation that // we could consider is expanding both here let excerpts: Vec<_> = snapshot.excerpts_for_range(p10..p20).collect(); assert_eq!(excerpts.len(), 1); - assert_eq!(excerpts[0].id, excerpt_1_id); + assert_eq!(excerpts[0].info(), excerpt_1_info); //// Test that `excerpts_for_range` and `excerpt_containing` agree for all single offsets (cursor positions) for offset in 0..=snapshot.len().0 { @@ -5054,8 +4928,8 @@ fn test_excerpts_containment_functions(cx: &mut App) { ); assert_eq!( - excerpts_for_range[0].id, - excerpt_containing.unwrap().id(), + excerpts_for_range[0].info(), + excerpt_containing.unwrap().excerpt.info(), "excerpts_for_range and excerpt_containing should agree for offset {offset}", ); } @@ -5065,7 +4939,7 @@ fn test_excerpts_containment_functions(cx: &mut App) { // Ranges intersecting a single-excerpt let containing = snapshot.excerpt_containing(p00..p13); assert!(containing.is_some()); - assert_eq!(containing.unwrap().id(), excerpt_1_id); + assert_eq!(containing.unwrap().excerpt.info(), excerpt_1_info); // Ranges intersecting multiple excerpts (should return None) let containing = snapshot.excerpt_containing(p20..p40); @@ -5150,7 +5024,7 @@ fn test_range_to_buffer_ranges_with_range_bounds(cx: &mut App) { let buffer_empty = cx.new(|cx| Buffer::local("", cx)); let multibuffer_trailing_empty = cx.new(|_| MultiBuffer::new(Capability::ReadWrite)); - let (te_excerpt_1_id, te_excerpt_2_id) = + let (te_excerpt_1_info, te_excerpt_2_info) = multibuffer_trailing_empty.update(cx, |multibuffer, cx| { multibuffer.set_excerpts_for_path( PathKey::sorted(0), @@ -5168,8 +5042,9 @@ fn test_range_to_buffer_ranges_with_range_bounds(cx: &mut App) { cx, ); - let excerpt_ids = multibuffer.excerpt_ids(); - (excerpt_ids[0], excerpt_ids[1]) + let snapshot = multibuffer.snapshot(cx); + let mut infos = snapshot.excerpts().map(|(_, info)| info); + (infos.next().unwrap(), infos.next().unwrap()) }); let snapshot_trailing = multibuffer_trailing_empty.read(cx).snapshot(cx); @@ -5177,32 +5052,40 @@ fn test_range_to_buffer_ranges_with_range_bounds(cx: &mut App) { let max_point = snapshot_trailing.max_point(); - let ranges_half_open_max = snapshot_trailing.range_to_buffer_ranges(Point::zero()..max_point); + let ranges_half_open_max = + snapshot_trailing.range_to_buffer_ranges_with_context(Point::zero()..max_point); assert_eq!( ranges_half_open_max.len(), 1, "Half-open range to max_point should EXCLUDE trailing empty excerpt at max_point" ); - assert_eq!(ranges_half_open_max[0].2, te_excerpt_1_id); + assert_eq!(ranges_half_open_max[0].2, te_excerpt_1_info.range.context); - let ranges_inclusive_max = snapshot_trailing.range_to_buffer_ranges(Point::zero()..=max_point); + let ranges_inclusive_max = + snapshot_trailing.range_to_buffer_ranges_with_context(Point::zero()..=max_point); assert_eq!( ranges_inclusive_max.len(), 2, "Inclusive range to max_point should INCLUDE trailing empty excerpt" ); - assert_eq!(ranges_inclusive_max[0].2, te_excerpt_1_id); - assert_eq!(ranges_inclusive_max[1].2, te_excerpt_2_id); + assert_eq!(ranges_inclusive_max[0].2, te_excerpt_1_info.range.context); + assert_eq!(ranges_inclusive_max[1].2, te_excerpt_2_info.range.context); let ranges_unbounded_trailing = snapshot_trailing - .range_to_buffer_ranges((Bound::Included(Point::zero()), Bound::Unbounded)); + .range_to_buffer_ranges_with_context((Bound::Included(Point::zero()), Bound::Unbounded)); assert_eq!( ranges_unbounded_trailing.len(), 2, "Unbounded end should include trailing empty excerpt" ); - assert_eq!(ranges_unbounded_trailing[0].2, te_excerpt_1_id); - assert_eq!(ranges_unbounded_trailing[1].2, te_excerpt_2_id); + assert_eq!( + ranges_unbounded_trailing[0].2, + te_excerpt_1_info.range.context + ); + assert_eq!( + ranges_unbounded_trailing[1].2, + te_excerpt_2_info.range.context + ); } #[gpui::test] @@ -5248,17 +5131,14 @@ fn test_cannot_seek_backward_after_excerpt_replacement(cx: &mut TestAppContext) let (anchor_in_e_b2, anchor_in_e_b3) = multibuffer.read_with(cx, |multibuffer, cx| { let snapshot = multibuffer.snapshot(cx); - let excerpt_ids: Vec = snapshot.excerpts().map(|(id, _, _)| id).collect(); - assert_eq!(excerpt_ids.len(), 4, "expected 4 excerpts (3×B + 1×C)"); - - let e_b2_id = excerpt_ids[1]; - let e_b3_id = excerpt_ids[2]; + let excerpt_infos: Vec = snapshot.excerpts().map(|(_, info)| info).collect(); + assert_eq!(excerpt_infos.len(), 4, "expected 4 excerpts (3×B + 1×C)"); - let e_b2 = snapshot.excerpt(e_b2_id).expect("E_B2 should exist"); - let e_b3 = snapshot.excerpt(e_b3_id).expect("E_B3 should exist"); + let e_b2_info = excerpt_infos[1].clone(); + let e_b3_info = excerpt_infos[2].clone(); - let anchor_b2 = Anchor::text(e_b2_id, e_b2.range.context.start); - let anchor_b3 = Anchor::text(e_b3_id, e_b3.range.context.start); + let anchor_b2 = Anchor::in_buffer(e_b2_info.path_key_index, e_b2_info.range.context.start); + let anchor_b3 = Anchor::in_buffer(e_b3_info.path_key_index, e_b3_info.range.context.start); (anchor_b2, anchor_b3) }); diff --git a/crates/multi_buffer/src/path_key.rs b/crates/multi_buffer/src/path_key.rs index b433efc00d0e8e8deda9df07882f0d2027f78787..b5a0d83851a05796904f629d56abdae7e0b5b000 100644 --- a/crates/multi_buffer/src/path_key.rs +++ b/crates/multi_buffer/src/path_key.rs @@ -5,7 +5,7 @@ use itertools::Itertools; use language::{Buffer, BufferSnapshot}; use rope::Point; use sum_tree::{Dimensions, SumTree}; -use text::{Bias, Edit, OffsetRangeExt, Patch}; +use text::{Bias, BufferId, Edit, OffsetRangeExt, Patch}; use util::rel_path::RelPath; use ztracing::instrument; @@ -468,13 +468,21 @@ impl MultiBuffer { cx.emit(Event::BufferUpdated { buffer, path_key: path_key.clone(), - ranges: to_insert.map(|range| range.context.clone()).collect(), + ranges: to_insert, }); cx.notify(); (added_new_excerpt, path_key_index) } + pub fn remove_excerpts_for_buffer(&mut self, buffer: BufferId, cx: &mut Context) { + let snapshot = self.sync_mut(cx); + let Some(path) = snapshot.path_for_buffer(buffer).cloned() else { + return; + }; + self.remove_excerpts_for_path(path, cx); + } + pub fn remove_excerpts_for_path(&mut self, path: PathKey, cx: &mut Context) { assert_eq!(self.history.transaction_depth(), 0); self.sync_mut(cx);