From a284e7140c8f76ca1fd769dfed8cc0b8a41821b8 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Sat, 29 Jan 2022 10:10:53 +0100 Subject: [PATCH] Always return valid locations when refreshing anchors Specifically, with this commit: - We will now refresh the anchor if it escapes the boundaries of the excerpt by using the `Excerpt::contains` method. This was not the case before, as we were just checking if the excerpt id and buffer id of the anchors matched the ones stored on the excerpt. - We fixed a bug that was causing the anchor to be outside of the excerpt when resetting it to one of the excerpt's endpoints after we couldn't keep its position. This would happen because we were using `anchor_at`, which resolved the anchor to an offset first and then converted it back into an anchor with the given bias, which is a lossy operation. We now use `Anchor::bias` to achieve the same goal: note that this could still lead to the anchor escaping the excerpt's boundary when the bias doesn't match the endpoint's bias, so we take extra care to avoid that and `min`/`max` the newly-produced anchor with the other endpoint. --- crates/editor/src/multi_buffer.rs | 48 ++++++++++++++++++++++++++----- crates/text/src/anchor.rs | 8 ++++++ 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/crates/editor/src/multi_buffer.rs b/crates/editor/src/multi_buffer.rs index 17d7019a8d..f454106479 100644 --- a/crates/editor/src/multi_buffer.rs +++ b/crates/editor/src/multi_buffer.rs @@ -1468,7 +1468,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; } @@ -1487,20 +1487,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() @@ -2763,11 +2781,27 @@ 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 anchors point to a valid excerpt after refreshing them. + let mut cursor = multibuffer.excerpts.cursor::>(); + 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) { diff --git a/crates/text/src/anchor.rs b/crates/text/src/anchor.rs index 738de690b4..5a36bf0401 100644 --- a/crates/text/src/anchor.rs +++ b/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()