Ensure muiltibuffer anchors are contained within their excerpt ranges

Max Brunsfeld , Nathan Sobo , and Antonio Scandurra created

Co-Authored-By: Nathan Sobo <nathan@zed.dev>
Co-Authored-By: Antonio Scandurra <me@as-cii.com>

Change summary

crates/diagnostics/src/diagnostics.rs    | 162 ++++++++++++++-----------
crates/editor/src/multi_buffer.rs        |  38 +++++
crates/editor/src/multi_buffer/anchor.rs |   7 -
3 files changed, 127 insertions(+), 80 deletions(-)

Detailed changes

crates/diagnostics/src/diagnostics.rs 🔗

@@ -92,81 +92,99 @@ impl workspace::Item for ProjectDiagnostics {
 
                     this.update(&mut cx, |this, cx| {
                         let mut blocks = Vec::new();
-                        this.excerpts.update(cx, |excerpts, excerpts_cx| {
-                            for group in snapshot.diagnostic_groups::<Point>() {
-                                let excerpt_start = cmp::min(
-                                    group.primary.range.start.row,
-                                    group
-                                        .supporting
-                                        .first()
-                                        .map_or(u32::MAX, |entry| entry.range.start.row),
-                                );
-                                let excerpt_end = cmp::max(
-                                    group.primary.range.end.row,
-                                    group
-                                        .supporting
-                                        .last()
-                                        .map_or(0, |entry| entry.range.end.row),
-                                );
-
-                                let primary_diagnostic = group.primary.diagnostic;
-                                let excerpt_id = excerpts.push_excerpt(
-                                    ExcerptProperties {
-                                        buffer: &buffer,
-                                        range: Point::new(excerpt_start, 0)
-                                            ..Point::new(
-                                                excerpt_end,
-                                                snapshot.line_len(excerpt_end),
+                        let excerpts_snapshot =
+                            this.excerpts.update(cx, |excerpts, excerpts_cx| {
+                                for group in snapshot.diagnostic_groups::<Point>() {
+                                    let excerpt_start = cmp::min(
+                                        group.primary.range.start.row,
+                                        group
+                                            .supporting
+                                            .first()
+                                            .map_or(u32::MAX, |entry| entry.range.start.row),
+                                    );
+                                    let excerpt_end = cmp::max(
+                                        group.primary.range.end.row,
+                                        group
+                                            .supporting
+                                            .last()
+                                            .map_or(0, |entry| entry.range.end.row),
+                                    );
+
+                                    let primary_diagnostic = group.primary.diagnostic;
+                                    let excerpt_id = excerpts.push_excerpt(
+                                        ExcerptProperties {
+                                            buffer: &buffer,
+                                            range: Point::new(excerpt_start, 0)
+                                                ..Point::new(
+                                                    excerpt_end,
+                                                    snapshot.line_len(excerpt_end),
+                                                ),
+                                            header_height: primary_diagnostic
+                                                .message
+                                                .matches('\n')
+                                                .count()
+                                                as u8
+                                                + 1,
+                                            render_header: Some(Arc::new({
+                                                let settings = settings.clone();
+
+                                                move |_| {
+                                                    let editor_style =
+                                                        &settings.borrow().theme.editor;
+                                                    let mut text_style = editor_style.text.clone();
+                                                    text_style.color = diagnostic_style(
+                                                        primary_diagnostic.severity,
+                                                        true,
+                                                        &editor_style,
+                                                    )
+                                                    .text;
+
+                                                    Text::new(
+                                                        primary_diagnostic.message.clone(),
+                                                        text_style,
+                                                    )
+                                                    .boxed()
+                                                }
+                                            })),
+                                        },
+                                        excerpts_cx,
+                                    );
+
+                                    for entry in group.supporting {
+                                        let buffer_anchor =
+                                            snapshot.anchor_before(entry.range.start);
+                                        blocks.push(BlockProperties {
+                                            position: (excerpt_id.clone(), buffer_anchor),
+                                            height: entry.diagnostic.message.matches('\n').count()
+                                                as u8
+                                                + 1,
+                                            render: diagnostic_block_renderer(
+                                                entry.diagnostic,
+                                                true,
+                                                build_settings.clone(),
                                             ),
-                                        header_height: primary_diagnostic
-                                            .message
-                                            .matches('\n')
-                                            .count()
-                                            as u8
-                                            + 1,
-                                        render_header: Some(Arc::new({
-                                            let settings = settings.clone();
-
-                                            move |_| {
-                                                let editor_style = &settings.borrow().theme.editor;
-                                                let mut text_style = editor_style.text.clone();
-                                                text_style.color = diagnostic_style(
-                                                    primary_diagnostic.severity,
-                                                    true,
-                                                    &editor_style,
-                                                )
-                                                .text;
-
-                                                Text::new(
-                                                    primary_diagnostic.message.clone(),
-                                                    text_style,
-                                                )
-                                                .boxed()
-                                            }
-                                        })),
-                                    },
-                                    excerpts_cx,
-                                );
-
-                                for entry in group.supporting {
-                                    let buffer_anchor = snapshot.anchor_before(entry.range.start);
-                                    blocks.push(BlockProperties {
-                                        position: Anchor::new(excerpt_id.clone(), buffer_anchor),
-                                        height: entry.diagnostic.message.matches('\n').count()
-                                            as u8
-                                            + 1,
-                                        render: diagnostic_block_renderer(
-                                            entry.diagnostic,
-                                            true,
-                                            build_settings.clone(),
-                                        ),
-                                        disposition: BlockDisposition::Below,
-                                    });
+                                            disposition: BlockDisposition::Below,
+                                        });
+                                    }
                                 }
-                            }
-                        });
+
+                                excerpts.snapshot(excerpts_cx)
+                            });
+
                         this.editor.update(cx, |editor, cx| {
-                            editor.insert_blocks(blocks, cx);
+                            editor.insert_blocks(
+                                blocks.into_iter().map(|block| {
+                                    let (excerpt_id, text_anchor) = block.position;
+                                    BlockProperties {
+                                        position: excerpts_snapshot
+                                            .anchor_in_excerpt(excerpt_id, text_anchor),
+                                        height: block.height,
+                                        render: block.render,
+                                        disposition: block.disposition,
+                                    }
+                                }),
+                                cx,
+                            );
                         });
                     })
                 }

crates/editor/src/multi_buffer.rs 🔗

@@ -1360,9 +1360,11 @@ impl MultiBufferSnapshot {
             }
 
             let buffer_start = excerpt.range.start.to_offset(&excerpt.buffer);
+            let text_anchor =
+                excerpt.clip_anchor(excerpt.buffer.anchor_at(buffer_start + overshoot, bias));
             Anchor {
                 excerpt_id: excerpt.id.clone(),
-                text_anchor: excerpt.buffer.anchor_at(buffer_start + overshoot, bias),
+                text_anchor,
             }
         } else if offset == 0 && bias == Bias::Left {
             Anchor::min()
@@ -1371,6 +1373,22 @@ impl MultiBufferSnapshot {
         }
     }
 
+    pub fn anchor_in_excerpt(&self, excerpt_id: ExcerptId, text_anchor: text::Anchor) -> Anchor {
+        let mut cursor = self.excerpts.cursor::<Option<&ExcerptId>>();
+        cursor.seek(&Some(&excerpt_id), Bias::Left, &());
+        if let Some(excerpt) = cursor.item() {
+            if excerpt.id == excerpt_id {
+                let text_anchor = excerpt.clip_anchor(text_anchor);
+                drop(cursor);
+                return Anchor {
+                    excerpt_id,
+                    text_anchor,
+                };
+            }
+        }
+        panic!("excerpt not found");
+    }
+
     pub fn parse_count(&self) -> usize {
         self.parse_count
     }
@@ -1688,6 +1706,24 @@ impl Excerpt {
             footer_height,
         }
     }
+
+    fn clip_anchor(&self, text_anchor: text::Anchor) -> text::Anchor {
+        if text_anchor
+            .cmp(&self.range.start, &self.buffer)
+            .unwrap()
+            .is_lt()
+        {
+            self.range.start.clone()
+        } else if text_anchor
+            .cmp(&self.range.end, &self.buffer)
+            .unwrap()
+            .is_gt()
+        {
+            self.range.end.clone()
+        } else {
+            text_anchor
+        }
+    }
 }
 
 impl fmt::Debug for Excerpt {

crates/editor/src/multi_buffer/anchor.rs 🔗

@@ -14,13 +14,6 @@ pub struct Anchor {
 }
 
 impl Anchor {
-    pub fn new(excerpt_id: ExcerptId, text_anchor: text::Anchor) -> Self {
-        Self {
-            excerpt_id,
-            text_anchor,
-        }
-    }
-
     pub fn min() -> Self {
         Self {
             excerpt_id: ExcerptId::min(),