diff --git a/crates/multi_buffer/src/multi_buffer_tests.rs b/crates/multi_buffer/src/multi_buffer_tests.rs index 8792f95e39c966fbc0af08db33cd692ed6fe5c37..455bef14ec8c07532be2c91da4e4927ef9be2ebd 100644 --- a/crates/multi_buffer/src/multi_buffer_tests.rs +++ b/crates/multi_buffer/src/multi_buffer_tests.rs @@ -5041,3 +5041,86 @@ fn test_range_to_buffer_ranges_with_range_bounds(cx: &mut App) { assert_eq!(ranges_unbounded_trailing[0].2, te_excerpt_1_id); assert_eq!(ranges_unbounded_trailing[1].2, te_excerpt_2_id); } + +#[gpui::test] +fn test_cannot_seek_backward_after_excerpt_replacement(cx: &mut TestAppContext) { + let buffer_b_text: String = (0..50).map(|i| format!("line_b {i}\n")).collect(); + let buffer_b = cx.new(|cx| Buffer::local(buffer_b_text, cx)); + + let buffer_c_text: String = (0..10).map(|i| format!("line_c {i}\n")).collect(); + let buffer_c = cx.new(|cx| Buffer::local(buffer_c_text, cx)); + + let buffer_d_text: String = (0..10).map(|i| format!("line_d {i}\n")).collect(); + let buffer_d = cx.new(|cx| Buffer::local(buffer_d_text, cx)); + + let path_b = PathKey::with_sort_prefix(0, rel_path("bbb.rs").into_arc()); + let path_c = PathKey::with_sort_prefix(0, rel_path("ddd.rs").into_arc()); + let path_d = PathKey::with_sort_prefix(0, rel_path("ccc.rs").into_arc()); + + let multibuffer = cx.new(|_| MultiBuffer::new(Capability::ReadWrite)); + + multibuffer.update(cx, |multibuffer, cx| { + multibuffer.set_excerpts_for_path( + path_b.clone(), + buffer_b.clone(), + vec![ + Point::row_range(0..3), + Point::row_range(15..18), + Point::row_range(30..33), + ], + 0, + cx, + ); + }); + + multibuffer.update(cx, |multibuffer, cx| { + multibuffer.set_excerpts_for_path( + path_c.clone(), + buffer_c.clone(), + vec![Point::row_range(0..3)], + 0, + cx, + ); + }); + + 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 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 anchor_b2 = Anchor::in_buffer(e_b2_id, e_b2.range.context.start); + let anchor_b3 = Anchor::in_buffer(e_b3_id, e_b3.range.context.start); + (anchor_b2, anchor_b3) + }); + + multibuffer.update(cx, |multibuffer, cx| { + multibuffer.set_excerpts_for_path( + path_b.clone(), + buffer_b.clone(), + vec![Point::row_range(0..3), Point::row_range(28..36)], + 0, + cx, + ); + }); + + multibuffer.update(cx, |multibuffer, cx| { + multibuffer.set_excerpts_for_path( + path_d.clone(), + buffer_d.clone(), + vec![Point::row_range(0..3)], + 0, + cx, + ); + }); + + multibuffer.read_with(cx, |multibuffer, cx| { + let snapshot = multibuffer.snapshot(cx); + snapshot.summaries_for_anchors::(&[anchor_in_e_b2, anchor_in_e_b3]); + }); +} diff --git a/crates/multi_buffer/src/path_key.rs b/crates/multi_buffer/src/path_key.rs index 8c20f211f61990b3775d231dc37a80352d7b9b98..62ef32a8625741df7882ed6327e353e4a717c402 100644 --- a/crates/multi_buffer/src/path_key.rs +++ b/crates/multi_buffer/src/path_key.rs @@ -316,8 +316,7 @@ impl MultiBuffer { let snapshot = self.snapshot(cx); let mut next_excerpt_id = - // todo(lw): is this right? What if we remove the last excerpt, then we might reallocate with a wrong mapping? - if let Some(last_entry) = self.snapshot.borrow().excerpt_ids.last() { + if let Some(last_entry) = self.snapshot.get_mut().excerpt_ids.last() { last_entry.id.0 + 1 } else { 1 @@ -347,7 +346,11 @@ impl MultiBuffer { }; let new = new_iter.peek(); + // Try to merge the next new range or existing excerpt into the last + // queued insert. if let Some((last_id, last)) = to_insert.last_mut() { + // Next new range overlaps the last queued insert: absorb it by + // extending the insert's end. if let Some(new) = new && last.context.end >= new.context.start { @@ -356,6 +359,9 @@ impl MultiBuffer { new_iter.next(); continue; } + // Next existing excerpt overlaps the last queued insert: absorb + // it by extending the insert's end, and record the existing + // excerpt as replaced so anchors in it resolve to the new one. if let Some((existing_id, existing_range)) = &existing && last.context.end >= existing_range.start { @@ -372,63 +378,90 @@ impl MultiBuffer { match (new, existing) { (None, None) => break, + + // No more new ranges; remove the remaining existing excerpt. (None, Some((existing_id, _))) => { existing_iter.next(); to_remove.push(existing_id); - continue; } + + // No more existing excerpts; queue the new range for insertion. (Some(_), None) => { added_a_new_excerpt = true; let new_id = next_excerpt_id(); excerpt_ids.push(new_id); to_insert.push((new_id, new_iter.next().unwrap())); - continue; } - (Some(new), Some((_, existing_range))) => { - if existing_range.end < new.context.start { - let existing_id = existing_iter.next().unwrap(); - to_remove.push(existing_id); - continue; - } else if existing_range.start > new.context.end { - let new_id = next_excerpt_id(); - excerpt_ids.push(new_id); - to_insert.push((new_id, new_iter.next().unwrap())); - continue; - } + // Existing excerpt ends before the new range starts, so it + // has no corresponding new range and must be removed. Flush + // pending inserts and advance `insert_after` past it so that + // future inserts receive locators *after* this excerpt's + // locator, preserving forward ordering. + (Some(new), Some((_, existing_range))) + if existing_range.end < new.context.start => + { + self.insert_excerpts_with_ids_after( + insert_after, + buffer.clone(), + mem::take(&mut to_insert), + cx, + ); + insert_after = existing_iter.next().unwrap(); + to_remove.push(insert_after); + } + // New range ends before the existing excerpt starts, so the + // new range has no corresponding existing excerpt. Queue it + // for insertion at the current `insert_after` position + // (before the existing excerpt), which is the correct + // spatial ordering. + (Some(new), Some((_, existing_range))) + if existing_range.start > new.context.end => + { + let new_id = next_excerpt_id(); + excerpt_ids.push(new_id); + to_insert.push((new_id, new_iter.next().unwrap())); + } + // Exact match: keep the existing excerpt in place, flush + // any pending inserts before it, and use it as the new + // `insert_after` anchor. + (Some(new), Some((_, existing_range))) if existing_range.start == new.context.start - && existing_range.end == new.context.end - { - self.insert_excerpts_with_ids_after( - insert_after, - buffer.clone(), - mem::take(&mut to_insert), - cx, - ); - insert_after = existing_iter.next().unwrap(); - excerpt_ids.push(insert_after); - new_iter.next(); - } else { - let existing_id = existing_iter.next().unwrap(); - let new_id = next_excerpt_id(); - self.snapshot - .get_mut() - .replaced_excerpts - .insert(existing_id, new_id); - to_remove.push(existing_id); - let mut range = new_iter.next().unwrap(); - range.context.start = range.context.start.min(existing_range.start); - range.context.end = range.context.end.max(existing_range.end); - excerpt_ids.push(new_id); - to_insert.push((new_id, range)); - } + && existing_range.end == new.context.end => + { + self.insert_excerpts_with_ids_after( + insert_after, + buffer.clone(), + mem::take(&mut to_insert), + cx, + ); + insert_after = existing_iter.next().unwrap(); + excerpt_ids.push(insert_after); + new_iter.next(); + } + + // Partial overlap: replace the existing excerpt with a new + // one whose range is the union of both, and record the + // replacement so that anchors in the old excerpt resolve to + // the new one. + (Some(_), Some((_, existing_range))) => { + let existing_id = existing_iter.next().unwrap(); + let new_id = next_excerpt_id(); + self.snapshot + .get_mut() + .replaced_excerpts + .insert(existing_id, new_id); + to_remove.push(existing_id); + let mut range = new_iter.next().unwrap(); + range.context.start = range.context.start.min(existing_range.start); + range.context.end = range.context.end.max(existing_range.end); + excerpt_ids.push(new_id); + to_insert.push((new_id, range)); } }; } self.insert_excerpts_with_ids_after(insert_after, buffer, to_insert, cx); - // todo(lw): There is a logic bug somewhere that causes the to_remove vector to be not ordered correctly - to_remove.sort_by_cached_key(|&id| snapshot.excerpt_locator_for_id(id)); self.remove_excerpts(to_remove, cx); if excerpt_ids.is_empty() { @@ -437,10 +470,8 @@ impl MultiBuffer { for excerpt_id in &excerpt_ids { self.paths_by_excerpt.insert(*excerpt_id, path.clone()); } - let snapshot = &*self.snapshot.get_mut(); - let mut excerpt_ids: Vec<_> = excerpt_ids.iter().dedup().cloned().collect(); - excerpt_ids.sort_by_cached_key(|&id| snapshot.excerpt_locator_for_id(id)); - self.excerpts_by_path.insert(path, excerpt_ids); + self.excerpts_by_path + .insert(path, excerpt_ids.iter().dedup().cloned().collect()); } (excerpt_ids, added_a_new_excerpt)