Merge pull request #378 from zed-industries/multibuffer-anchors

Antonio Scandurra created 4 years ago

Fix errors when anchors escape an excerpt's buffer boundaries

Change summary

crates/editor/src/multi_buffer.rs | 58 ++++++++++++++++++++++++++++----
crates/text/src/anchor.rs         |  8 ++++
2 files changed, 58 insertions(+), 8 deletions(-)

Detailed changes

crates/editor/src/multi_buffer.rs

@@ -1390,7 +1390,11 @@ impl MultiBufferSnapshot {
         if let Some(excerpt) = cursor.item() {
             if excerpt.id == anchor.excerpt_id && excerpt.buffer_id == anchor.buffer_id {
                 let excerpt_buffer_start = excerpt.range.start.summary::<D>(&excerpt.buffer);
-                let buffer_position = anchor.text_anchor.summary::<D>(&excerpt.buffer);
+                let excerpt_buffer_end = excerpt.range.end.summary::<D>(&excerpt.buffer);
+                let buffer_position = cmp::min(
+                    excerpt_buffer_end,
+                    anchor.text_anchor.summary::<D>(&excerpt.buffer),
+                );
                 if buffer_position > excerpt_buffer_start {
                     position.add_assign(&(buffer_position - excerpt_buffer_start));
                 }
@@ -1428,11 +1432,13 @@ impl MultiBufferSnapshot {
             if let Some(excerpt) = cursor.item() {
                 if excerpt.id == *excerpt_id && excerpt.buffer_id == buffer_id {
                     let excerpt_buffer_start = excerpt.range.start.summary::<D>(&excerpt.buffer);
+                    let excerpt_buffer_end = excerpt.range.end.summary::<D>(&excerpt.buffer);
                     summaries.extend(
                         excerpt
                             .buffer
                             .summaries_for_anchors::<D, _>(excerpt_anchors)
                             .map(move |summary| {
+                                let summary = cmp::min(excerpt_buffer_end.clone(), summary);
                                 let mut position = position.clone();
                                 let excerpt_buffer_start = excerpt_buffer_start.clone();
                                 if summary > excerpt_buffer_start {
@@ -1486,7 +1492,7 @@ impl MultiBufferSnapshot {
                 // If the old excerpt still exists at this location, then leave
                 // the anchor unchanged.
                 else if next_excerpt.map_or(false, |excerpt| {
-                    excerpt.id == *old_excerpt_id && excerpt.buffer_id == anchor.buffer_id
+                    excerpt.id == *old_excerpt_id && excerpt.contains(&anchor)
                 }) {
                     kept_position = true;
                 }
@@ -1505,20 +1511,38 @@ impl MultiBufferSnapshot {
                 // then report that the anchor has lost its position.
                 if !kept_position {
                     anchor = if let Some(excerpt) = next_excerpt {
+                        let mut text_anchor = excerpt
+                            .range
+                            .start
+                            .bias(anchor.text_anchor.bias, &excerpt.buffer);
+                        if text_anchor
+                            .cmp(&excerpt.range.end, &excerpt.buffer)
+                            .unwrap()
+                            .is_gt()
+                        {
+                            text_anchor = excerpt.range.end.clone();
+                        }
                         Anchor {
                             buffer_id: excerpt.buffer_id,
                             excerpt_id: excerpt.id.clone(),
-                            text_anchor: excerpt
-                                .buffer
-                                .anchor_at(&excerpt.range.start, anchor.text_anchor.bias),
+                            text_anchor,
                         }
                     } else if let Some(excerpt) = prev_excerpt {
+                        let mut text_anchor = excerpt
+                            .range
+                            .end
+                            .bias(anchor.text_anchor.bias, &excerpt.buffer);
+                        if text_anchor
+                            .cmp(&excerpt.range.start, &excerpt.buffer)
+                            .unwrap()
+                            .is_lt()
+                        {
+                            text_anchor = excerpt.range.start.clone();
+                        }
                         Anchor {
                             buffer_id: excerpt.buffer_id,
                             excerpt_id: excerpt.id.clone(),
-                            text_anchor: excerpt
-                                .buffer
-                                .anchor_at(&excerpt.range.end, anchor.text_anchor.bias),
+                            text_anchor,
                         }
                     } else if anchor.text_anchor.bias == Bias::Left {
                         Anchor::min()
@@ -2790,11 +2814,28 @@ mod tests {
                 }
                 40..=44 if !anchors.is_empty() => {
                     let multibuffer = multibuffer.read(cx).read(cx);
+
                     anchors = multibuffer
                         .refresh_anchors(&anchors)
                         .into_iter()
                         .map(|a| a.1)
                         .collect();
+
+                    // Ensure the newly-refreshed anchors point to a valid excerpt and don't
+                    // overshoot its boundaries.
+                    let mut cursor = multibuffer.excerpts.cursor::<Option<&ExcerptId>>();
+                    for anchor in &anchors {
+                        if anchor.excerpt_id == ExcerptId::min()
+                            || anchor.excerpt_id == ExcerptId::max()
+                        {
+                            continue;
+                        }
+
+                        cursor.seek_forward(&Some(&anchor.excerpt_id), Bias::Left, &());
+                        let excerpt = cursor.item().unwrap();
+                        assert_eq!(excerpt.id, anchor.excerpt_id);
+                        assert!(excerpt.contains(anchor));
+                    }
                 }
                 _ => {
                     let buffer_handle = if buffers.is_empty() || rng.gen_bool(0.4) {
@@ -3081,6 +3122,7 @@ mod tests {
                 .iter()
                 .zip(snapshot.summaries_for_anchors::<usize, _>(&anchors))
             {
+                assert!(resolved_offset <= snapshot.len());
                 assert_eq!(
                     snapshot.summary_for_anchor::<usize>(anchor),
                     resolved_offset

crates/text/src/anchor.rs

@@ -42,6 +42,14 @@ impl Anchor {
             .then_with(|| self.bias.cmp(&other.bias)))
     }
 
+    pub fn bias(&self, bias: Bias, buffer: &BufferSnapshot) -> Anchor {
+        if bias == Bias::Left {
+            self.bias_left(buffer)
+        } else {
+            self.bias_right(buffer)
+        }
+    }
+
     pub fn bias_left(&self, buffer: &BufferSnapshot) -> Anchor {
         if self.bias == Bias::Left {
             self.clone()