Add/remove excerpts in BlockMap randomized tests and fix resulting errors

Antonio Scandurra , Nathan Sobo , and Max Brunsfeld created

Co-Authored-By: Nathan Sobo <nathan@zed.dev>
Co-Authored-By: Max Brunsfeld <max@zed.dev>

Change summary

crates/editor/src/display_map/block_map.rs | 133 ++++++++++++++++-----
crates/editor/src/display_map/fold_map.rs  |   1 
crates/editor/src/display_map/wrap_map.rs  |   4 
crates/editor/src/multi_buffer.rs          | 146 ++++++++++++++++-------
4 files changed, 207 insertions(+), 77 deletions(-)

Detailed changes

crates/editor/src/display_map/block_map.rs 🔗

@@ -95,6 +95,7 @@ pub enum TransformBlock {
         buffer: BufferSnapshot,
         range: Range<text::Anchor>,
         height: u8,
+        starts_new_buffer: bool,
     },
 }
 
@@ -182,9 +183,18 @@ impl BlockMap {
         BlockMapWriter(self)
     }
 
-    fn sync(&self, wrap_snapshot: &WrapSnapshot, edits: Vec<WrapEdit>) {
+    fn sync(&self, wrap_snapshot: &WrapSnapshot, mut edits: Vec<WrapEdit>) {
         if edits.is_empty() {
-            return;
+            // Handle removing the last excerpt or inserting the first excerpt when the excerpt is
+            // empty.
+            if wrap_snapshot.max_point().is_zero() {
+                edits.push(WrapEdit {
+                    old: 0..1,
+                    new: 0..1,
+                });
+            } else {
+                return;
+            }
         }
 
         let buffer = wrap_snapshot.buffer_snapshot();
@@ -273,13 +283,12 @@ impl BlockMap {
             // Find the blocks within this edited region.
             let new_buffer_start =
                 wrap_snapshot.to_point(WrapPoint::new(new_start.0, 0), Bias::Left);
-            let start_anchor = buffer.anchor_before(new_buffer_start);
-            let start_bound = Bound::Included(start_anchor.clone());
+            let start_bound = Bound::Included(new_buffer_start);
             let start_block_ix = match self.blocks[last_block_ix..].binary_search_by(|probe| {
                 probe
                     .position
-                    .cmp(&start_anchor, &buffer)
-                    .unwrap()
+                    .to_point(&buffer)
+                    .cmp(&new_buffer_start)
                     .then(Ordering::Greater)
             }) {
                 Ok(ix) | Err(ix) => last_block_ix + ix,
@@ -292,13 +301,12 @@ impl BlockMap {
             } else {
                 let new_buffer_end =
                     wrap_snapshot.to_point(WrapPoint::new(new_end.0, 0), Bias::Left);
-                let end_anchor = buffer.anchor_before(new_buffer_end);
-                end_bound = Bound::Excluded(end_anchor.clone());
+                end_bound = Bound::Excluded(new_buffer_end);
                 match self.blocks[start_block_ix..].binary_search_by(|probe| {
                     probe
                         .position
-                        .cmp(&end_anchor, &buffer)
-                        .unwrap()
+                        .to_point(&buffer)
+                        .cmp(&new_buffer_end)
                         .then(Ordering::Greater)
                 }) {
                     Ok(ix) | Err(ix) => start_block_ix + ix,
@@ -334,6 +342,7 @@ impl BlockMap {
                                 buffer: excerpt_boundary.buffer,
                                 range: excerpt_boundary.range,
                                 height: self.excerpt_header_height,
+                                starts_new_buffer: excerpt_boundary.starts_new_buffer,
                             },
                         )
                     }),
@@ -1285,9 +1294,9 @@ mod tests {
                 }
                 _ => {
                     buffer.update(cx, |buffer, cx| {
-                        let edit_count = rng.gen_range(1..=5);
+                        let mutation_count = rng.gen_range(1..=5);
                         let subscription = buffer.subscribe();
-                        buffer.randomly_edit(&mut rng, edit_count, cx);
+                        buffer.randomly_mutate(&mut rng, mutation_count, cx);
                         buffer_snapshot = buffer.snapshot(cx);
                         buffer_edits.extend(subscription.consume());
                         log::info!("buffer text: {:?}", buffer_snapshot.text());
@@ -1319,7 +1328,14 @@ mod tests {
                     }
                 };
                 let row = wraps_snapshot.from_point(position, Bias::Left).row();
-                (row, block.disposition, Some(*id), block.height)
+                (
+                    row,
+                    ExpectedBlock::Custom {
+                        disposition: block.disposition,
+                        id: *id,
+                        height: block.height,
+                    },
+                )
             }));
             expected_blocks.extend(buffer_snapshot.excerpt_boundaries_in_range(0..).map(
                 |boundary| {
@@ -1327,15 +1343,15 @@ mod tests {
                         wraps_snapshot.from_point(Point::new(boundary.row, 0), Bias::Left);
                     (
                         position.row(),
-                        BlockDisposition::Above,
-                        None,
-                        excerpt_header_height,
+                        ExpectedBlock::ExcerptHeader {
+                            height: excerpt_header_height,
+                            starts_new_buffer: boundary.starts_new_buffer,
+                        },
                     )
                 },
             ));
-            expected_blocks
-                .sort_unstable_by_key(|(row, disposition, id, _)| (*row, *disposition, *id));
-            let mut sorted_blocks_iter = expected_blocks.iter().peekable();
+            expected_blocks.sort_unstable();
+            let mut sorted_blocks_iter = expected_blocks.into_iter().peekable();
 
             let input_buffer_rows = buffer_snapshot.buffer_rows(0).collect::<Vec<_>>();
             let mut expected_buffer_rows = Vec::new();
@@ -1352,16 +1368,17 @@ mod tests {
                     .to_point(WrapPoint::new(row, 0), Bias::Left)
                     .row as usize];
 
-                while let Some((block_row, disposition, id, height)) = sorted_blocks_iter.peek() {
-                    if *block_row == row && *disposition == BlockDisposition::Above {
+                while let Some((block_row, block)) = sorted_blocks_iter.peek() {
+                    if *block_row == row && block.disposition() == BlockDisposition::Above {
+                        let (_, block) = sorted_blocks_iter.next().unwrap();
+                        let height = block.height() as usize;
                         expected_block_positions
-                            .push((expected_text.matches('\n').count() as u32, *id));
-                        let text = "\n".repeat(*height as usize);
+                            .push((expected_text.matches('\n').count() as u32, block));
+                        let text = "\n".repeat(height);
                         expected_text.push_str(&text);
-                        for _ in 0..*height {
+                        for _ in 0..height {
                             expected_buffer_rows.push(None);
                         }
-                        sorted_blocks_iter.next();
                     } else {
                         break;
                     }
@@ -1371,16 +1388,17 @@ mod tests {
                 expected_buffer_rows.push(if soft_wrapped { None } else { buffer_row });
                 expected_text.push_str(input_line);
 
-                while let Some((block_row, disposition, id, height)) = sorted_blocks_iter.peek() {
-                    if *block_row == row && *disposition == BlockDisposition::Below {
+                while let Some((block_row, block)) = sorted_blocks_iter.peek() {
+                    if *block_row == row && block.disposition() == BlockDisposition::Below {
+                        let (_, block) = sorted_blocks_iter.next().unwrap();
+                        let height = block.height() as usize;
                         expected_block_positions
-                            .push((expected_text.matches('\n').count() as u32 + 1, *id));
-                        let text = "\n".repeat(*height as usize);
+                            .push((expected_text.matches('\n').count() as u32 + 1, block));
+                        let text = "\n".repeat(height);
                         expected_text.push_str(&text);
-                        for _ in 0..*height {
+                        for _ in 0..height {
                             expected_buffer_rows.push(None);
                         }
-                        sorted_blocks_iter.next();
                     } else {
                         break;
                     }
@@ -1392,7 +1410,7 @@ mod tests {
             for start_row in 0..expected_row_count {
                 let expected_text = expected_lines[start_row..].join("\n");
                 let actual_text = blocks_snapshot
-                    .chunks(start_row as u32..expected_row_count as u32, false)
+                    .chunks(start_row as u32..blocks_snapshot.max_point().row + 1, false)
                     .map(|chunk| chunk.text)
                     .collect::<String>();
                 assert_eq!(
@@ -1411,7 +1429,7 @@ mod tests {
             assert_eq!(
                 blocks_snapshot
                     .blocks_in_range(0..(expected_row_count as u32))
-                    .map(|(row, block)| { (row, block.as_custom().map(|b| b.id)) })
+                    .map(|(row, block)| (row, block.clone().into()))
                     .collect::<Vec<_>>(),
                 expected_block_positions
             );
@@ -1490,6 +1508,55 @@ mod tests {
                 }
             }
         }
+
+        #[derive(Debug, Eq, PartialEq, Ord, PartialOrd)]
+        enum ExpectedBlock {
+            ExcerptHeader {
+                height: u8,
+                starts_new_buffer: bool,
+            },
+            Custom {
+                disposition: BlockDisposition,
+                id: BlockId,
+                height: u8,
+            },
+        }
+
+        impl ExpectedBlock {
+            fn height(&self) -> u8 {
+                match self {
+                    ExpectedBlock::ExcerptHeader { height, .. } => *height,
+                    ExpectedBlock::Custom { height, .. } => *height,
+                }
+            }
+
+            fn disposition(&self) -> BlockDisposition {
+                match self {
+                    ExpectedBlock::ExcerptHeader { .. } => BlockDisposition::Above,
+                    ExpectedBlock::Custom { disposition, .. } => *disposition,
+                }
+            }
+        }
+
+        impl From<TransformBlock> for ExpectedBlock {
+            fn from(block: TransformBlock) -> Self {
+                match block {
+                    TransformBlock::Custom(block) => ExpectedBlock::Custom {
+                        id: block.id,
+                        disposition: block.disposition,
+                        height: block.height,
+                    },
+                    TransformBlock::ExcerptHeader {
+                        height,
+                        starts_new_buffer,
+                        ..
+                    } => ExpectedBlock::ExcerptHeader {
+                        height,
+                        starts_new_buffer,
+                    },
+                }
+            }
+        }
     }
 
     impl TransformBlock {

crates/editor/src/display_map/fold_map.rs 🔗

@@ -268,6 +268,7 @@ impl FoldMap {
             let mut buffer = self.buffer.lock();
             if buffer.parse_count() != new_buffer.parse_count()
                 || buffer.diagnostics_update_count() != new_buffer.diagnostics_update_count()
+                || buffer.excerpt_update_count() != new_buffer.excerpt_update_count()
             {
                 self.version.fetch_add(1, SeqCst);
             }

crates/editor/src/display_map/wrap_map.rs 🔗

@@ -954,6 +954,10 @@ impl WrapPoint {
     pub fn column_mut(&mut self) -> &mut u32 {
         &mut self.0.column
     }
+
+    pub fn is_zero(&self) -> bool {
+        self.0.is_zero()
+    }
 }
 
 impl sum_tree::Summary for TransformSummary {

crates/editor/src/multi_buffer.rs 🔗

@@ -93,6 +93,7 @@ pub struct MultiBufferSnapshot {
     excerpts: SumTree<Excerpt>,
     parse_count: usize,
     diagnostics_update_count: usize,
+    excerpt_update_count: usize,
     is_dirty: bool,
     has_conflict: bool,
 }
@@ -196,54 +197,13 @@ impl MultiBuffer {
 
     #[cfg(any(test, feature = "test-support"))]
     pub fn build_random(
-        mut rng: &mut impl rand::Rng,
+        rng: &mut impl rand::Rng,
         cx: &mut gpui::MutableAppContext,
     ) -> ModelHandle<Self> {
-        use rand::prelude::*;
-        use std::env;
-        use text::RandomCharIter;
-
-        let max_excerpts = env::var("MAX_EXCERPTS")
-            .map(|i| i.parse().expect("invalid `MAX_EXCERPTS` variable"))
-            .unwrap_or(5);
-        let excerpts = rng.gen_range(1..=max_excerpts);
-
         cx.add_model(|cx| {
             let mut multibuffer = MultiBuffer::new(0);
-            let mut buffers = Vec::new();
-            for _ in 0..excerpts {
-                let buffer_handle = if rng.gen() || buffers.is_empty() {
-                    let text = RandomCharIter::new(&mut rng).take(10).collect::<String>();
-                    buffers.push(cx.add_model(|cx| Buffer::new(0, text, cx)));
-                    let buffer = buffers.last().unwrap();
-                    log::info!(
-                        "Creating new buffer {} with text: {:?}",
-                        buffer.id(),
-                        buffer.read(cx).text()
-                    );
-                    buffers.last().unwrap()
-                } else {
-                    buffers.choose(rng).unwrap()
-                };
-
-                let buffer = buffer_handle.read(cx);
-                let end_ix = buffer.clip_offset(rng.gen_range(0..=buffer.len()), Bias::Right);
-                let start_ix = buffer.clip_offset(rng.gen_range(0..=end_ix), Bias::Left);
-                log::info!(
-                    "Inserting excerpt from buffer {} and range {:?}: {:?}",
-                    buffer_handle.id(),
-                    start_ix..end_ix,
-                    &buffer.text()[start_ix..end_ix]
-                );
-
-                multibuffer.push_excerpt(
-                    ExcerptProperties {
-                        buffer: buffer_handle,
-                        range: start_ix..end_ix,
-                    },
-                    cx,
-                );
-            }
+            let mutation_count = rng.gen_range(1..=5);
+            multibuffer.randomly_edit_excerpts(rng, mutation_count, cx);
             multibuffer
         })
     }
@@ -315,6 +275,10 @@ impl MultiBuffer {
         S: ToOffset,
         T: Into<String>,
     {
+        if self.buffers.borrow().is_empty() {
+            return;
+        }
+
         if let Some(buffer) = self.as_singleton() {
             let snapshot = self.read(cx);
             let ranges = ranges_iter
@@ -701,6 +665,7 @@ impl MultiBuffer {
         new_excerpts.push_tree(cursor.suffix(&()), &());
         drop(cursor);
         snapshot.excerpts = new_excerpts;
+        snapshot.excerpt_update_count += 1;
 
         self.subscriptions.publish_mut([Edit {
             old: edit_start..edit_start,
@@ -814,6 +779,7 @@ impl MultiBuffer {
         new_excerpts.push_tree(cursor.suffix(&()), &());
         drop(cursor);
         snapshot.excerpts = new_excerpts;
+        snapshot.excerpt_update_count += 1;
         self.subscriptions.publish_mut(edits);
         cx.notify();
     }
@@ -1052,6 +1018,94 @@ impl MultiBuffer {
 
         self.edit(old_ranges.iter().cloned(), new_text.as_str(), cx);
     }
+
+    pub fn randomly_edit_excerpts(
+        &mut self,
+        rng: &mut impl rand::Rng,
+        mutation_count: usize,
+        cx: &mut ModelContext<Self>,
+    ) {
+        use rand::prelude::*;
+        use std::env;
+        use text::RandomCharIter;
+
+        let max_excerpts = env::var("MAX_EXCERPTS")
+            .map(|i| i.parse().expect("invalid `MAX_EXCERPTS` variable"))
+            .unwrap_or(5);
+
+        let mut buffers = Vec::new();
+        for _ in 0..mutation_count {
+            let excerpt_ids = self
+                .buffers
+                .borrow()
+                .values()
+                .flat_map(|b| &b.excerpts)
+                .cloned()
+                .collect::<Vec<_>>();
+            if excerpt_ids.len() == 0 || (rng.gen() && excerpt_ids.len() < max_excerpts) {
+                let buffer_handle = if rng.gen() || self.buffers.borrow().is_empty() {
+                    let text = RandomCharIter::new(&mut *rng).take(10).collect::<String>();
+                    buffers.push(cx.add_model(|cx| Buffer::new(0, text, cx)));
+                    let buffer = buffers.last().unwrap();
+                    log::info!(
+                        "Creating new buffer {} with text: {:?}",
+                        buffer.id(),
+                        buffer.read(cx).text()
+                    );
+                    buffers.last().unwrap().clone()
+                } else {
+                    self.buffers
+                        .borrow()
+                        .values()
+                        .choose(rng)
+                        .unwrap()
+                        .buffer
+                        .clone()
+                };
+
+                let buffer = buffer_handle.read(cx);
+                let end_ix = buffer.clip_offset(rng.gen_range(0..=buffer.len()), Bias::Right);
+                let start_ix = buffer.clip_offset(rng.gen_range(0..=end_ix), Bias::Left);
+                log::info!(
+                    "Inserting excerpt from buffer {} and range {:?}: {:?}",
+                    buffer_handle.id(),
+                    start_ix..end_ix,
+                    &buffer.text()[start_ix..end_ix]
+                );
+
+                let excerpt_id = self.push_excerpt(
+                    ExcerptProperties {
+                        buffer: &buffer_handle,
+                        range: start_ix..end_ix,
+                    },
+                    cx,
+                );
+                log::info!("Inserted with id: {:?}", excerpt_id);
+            } else {
+                let remove_count = rng.gen_range(1..=excerpt_ids.len());
+                let mut excerpts_to_remove = excerpt_ids
+                    .choose_multiple(rng, remove_count)
+                    .cloned()
+                    .collect::<Vec<_>>();
+                excerpts_to_remove.sort();
+                log::info!("Removing excerpts {:?}", excerpts_to_remove);
+                self.remove_excerpts(&excerpts_to_remove, cx);
+            }
+        }
+    }
+
+    pub fn randomly_mutate(
+        &mut self,
+        rng: &mut impl rand::Rng,
+        mutation_count: usize,
+        cx: &mut ModelContext<Self>,
+    ) {
+        if rng.gen_bool(0.7) || self.singleton {
+            self.randomly_edit(rng, mutation_count, cx);
+        } else {
+            self.randomly_edit_excerpts(rng, mutation_count, cx);
+        }
+    }
 }
 
 impl Entity for MultiBuffer {
@@ -1883,6 +1937,10 @@ impl MultiBufferSnapshot {
         self.diagnostics_update_count
     }
 
+    pub fn excerpt_update_count(&self) -> usize {
+        self.excerpt_update_count
+    }
+
     pub fn language(&self) -> Option<&Arc<Language>> {
         self.excerpts
             .iter()