Merge pull request #328 from zed-industries/fix-multibuffer-anchors

Randomize test multibuffer anchors and fix resulting issues
This commit is contained in:
Antonio Scandurra 2022-01-11 17:42:53 +01:00 committed by GitHub
commit 58e45dd9be
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 156 additions and 100 deletions

View file

@ -1104,7 +1104,7 @@ impl Editor {
T: ToOffset,
{
let buffer = self.buffer.read(cx).snapshot(cx);
let mut selections = ranges
let selections = ranges
.into_iter()
.map(|range| {
let mut start = range.start.to_offset(&buffer);
@ -1124,7 +1124,6 @@ impl Editor {
}
})
.collect::<Vec<_>>();
selections.sort_unstable_by_key(|s| s.start);
self.update_selections(selections, autoscroll, cx);
}
@ -1649,7 +1648,6 @@ impl Editor {
edit_ranges.push(edit_start..edit_end);
}
new_cursors.sort_unstable_by(|a, b| a.1.cmp(&b.1, &buffer).unwrap());
let buffer = self.buffer.update(cx, |buffer, cx| {
buffer.edit(edit_ranges, "", cx);
buffer.snapshot(cx)
@ -2648,7 +2646,6 @@ impl Editor {
reversed: false,
goal: SelectionGoal::None,
});
selections.sort_unstable_by_key(|s| s.start);
self.update_selections(selections, Some(Autoscroll::Newest), cx);
} else {
select_next_state.done = true;
@ -2801,7 +2798,7 @@ impl Editor {
let mut stack = mem::take(&mut self.select_larger_syntax_node_stack);
let mut selected_larger_node = false;
let mut new_selections = old_selections
let new_selections = old_selections
.iter()
.map(|selection| {
let old_range = selection.start..selection.end;
@ -2830,7 +2827,6 @@ impl Editor {
if selected_larger_node {
stack.push(old_selections);
new_selections.sort_unstable_by_key(|selection| selection.start);
self.update_selections(new_selections, Some(Autoscroll::Fit), cx);
}
self.select_larger_syntax_node_stack = stack;
@ -3225,6 +3221,8 @@ impl Editor {
) where
T: ToOffset + ToPoint + Ord + std::marker::Copy + std::fmt::Debug,
{
selections.sort_unstable_by_key(|s| s.start);
// Merge overlapping selections.
let buffer = self.buffer.read(cx).snapshot(cx);
let mut i = 1;
@ -3298,35 +3296,45 @@ impl Editor {
/// was no longer present. The keys of the map are selection ids. The values are
/// the id of the new excerpt where the head of the selection has been moved.
pub fn refresh_selections(&mut self, cx: &mut ViewContext<Self>) -> HashMap<usize, ExcerptId> {
let anchors_with_status = self.buffer.update(cx, |buffer, cx| {
let snapshot = buffer.read(cx);
snapshot.refresh_anchors(
self.selections
.iter()
.flat_map(|selection| [&selection.start, &selection.end]),
)
});
let snapshot = self.buffer.read(cx).read(cx);
let anchors_with_status = snapshot.refresh_anchors(
self.selections
.iter()
.flat_map(|selection| [&selection.start, &selection.end]),
);
let offsets =
snapshot.summaries_for_anchors::<usize, _>(anchors_with_status.iter().map(|a| &a.0));
let offsets = offsets.chunks(2);
let statuses = anchors_with_status.chunks(2).map(|a| (a[0].1, a[1].1));
let mut selections_with_lost_position = HashMap::default();
self.selections = self
let new_selections = self
.selections
.iter()
.cloned()
.zip(anchors_with_status.chunks(2))
.map(|(mut selection, anchors)| {
selection.start = anchors[0].0.clone();
selection.end = anchors[1].0.clone();
let kept_head_position = if selection.reversed {
anchors[0].1
.zip(offsets)
.zip(statuses)
.map(|((selection, offsets), (kept_start, kept_end))| {
let kept_head = if selection.reversed {
kept_start
} else {
anchors[1].1
kept_end
};
if !kept_head_position {
if !kept_head {
selections_with_lost_position
.insert(selection.id, selection.head().excerpt_id.clone());
}
selection
Selection {
id: selection.id,
start: offsets[0],
end: offsets[1],
reversed: selection.reversed,
goal: selection.goal,
}
})
.collect();
drop(snapshot);
self.update_selections(new_selections, Some(Autoscroll::Fit), cx);
selections_with_lost_position
}

View file

@ -1342,6 +1342,58 @@ impl MultiBufferSnapshot {
position
}
pub fn summaries_for_anchors<'a, D, I>(&'a self, anchors: I) -> Vec<D>
where
D: TextDimension + Ord + Sub<D, Output = D>,
I: 'a + IntoIterator<Item = &'a Anchor>,
{
let mut anchors = anchors.into_iter().peekable();
let mut cursor = self.excerpts.cursor::<ExcerptSummary>();
let mut summaries = Vec::new();
while let Some(anchor) = anchors.peek() {
let excerpt_id = &anchor.excerpt_id;
let buffer_id = anchor.buffer_id;
let excerpt_anchors = iter::from_fn(|| {
let anchor = anchors.peek()?;
if anchor.excerpt_id == *excerpt_id && anchor.buffer_id == buffer_id {
Some(&anchors.next().unwrap().text_anchor)
} else {
None
}
});
cursor.seek_forward(&Some(excerpt_id), Bias::Left, &());
if cursor.item().is_none() {
cursor.next(&());
}
let position = D::from_text_summary(&cursor.start().text);
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);
summaries.extend(
excerpt
.buffer
.summaries_for_anchors::<D, _>(excerpt_anchors)
.map(move |summary| {
let mut position = position.clone();
let excerpt_buffer_start = excerpt_buffer_start.clone();
if summary > excerpt_buffer_start {
position.add_assign(&(summary - excerpt_buffer_start));
}
position
}),
);
continue;
}
}
summaries.extend(excerpt_anchors.map(|_| position.clone()));
}
summaries
}
pub fn refresh_anchors<'a, I>(&'a self, anchors: I) -> Vec<(Anchor, bool)>
where
I: 'a + IntoIterator<Item = &'a Anchor>,
@ -1423,58 +1475,6 @@ impl MultiBufferSnapshot {
result
}
pub fn summaries_for_anchors<'a, D, I>(&'a self, anchors: I) -> Vec<D>
where
D: TextDimension + Ord + Sub<D, Output = D>,
I: 'a + IntoIterator<Item = &'a Anchor>,
{
let mut anchors = anchors.into_iter().peekable();
let mut cursor = self.excerpts.cursor::<ExcerptSummary>();
let mut summaries = Vec::new();
while let Some(anchor) = anchors.peek() {
let excerpt_id = &anchor.excerpt_id;
let buffer_id = anchor.buffer_id;
let excerpt_anchors = iter::from_fn(|| {
let anchor = anchors.peek()?;
if anchor.excerpt_id == *excerpt_id {
Some(&anchors.next().unwrap().text_anchor)
} else {
None
}
});
cursor.seek_forward(&Some(excerpt_id), Bias::Left, &());
if cursor.item().is_none() {
cursor.next(&());
}
let position = D::from_text_summary(&cursor.start().text);
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);
summaries.extend(
excerpt
.buffer
.summaries_for_anchors::<D, _>(excerpt_anchors)
.map(move |summary| {
let mut position = position.clone();
let excerpt_buffer_start = excerpt_buffer_start.clone();
if summary > excerpt_buffer_start {
position.add_assign(&(summary - excerpt_buffer_start));
}
position
}),
);
continue;
}
}
summaries.extend(excerpt_anchors.map(|_| position.clone()));
}
summaries
}
pub fn anchor_before<T: ToOffset>(&self, position: T) -> Anchor {
self.anchor_at(position, Bias::Left)
}
@ -1685,12 +1685,12 @@ impl MultiBufferSnapshot {
fn buffer_snapshot_for_excerpt<'a>(
&'a self,
excerpt_id: &'a ExcerptId,
) -> Option<&'a BufferSnapshot> {
) -> Option<(usize, &'a BufferSnapshot)> {
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 {
return Some(&excerpt.buffer);
return Some((excerpt.buffer_id, &excerpt.buffer));
}
}
None
@ -2637,7 +2637,7 @@ mod tests {
}
#[gpui::test(iterations = 100)]
fn test_random_excerpts(cx: &mut MutableAppContext, mut rng: StdRng) {
fn test_random_multibuffer(cx: &mut MutableAppContext, mut rng: StdRng) {
let operations = env::var("OPERATIONS")
.map(|i| i.parse().expect("invalid `OPERATIONS` variable"))
.unwrap_or(10);
@ -2646,6 +2646,7 @@ mod tests {
let multibuffer = cx.add_model(|_| MultiBuffer::new(0));
let mut excerpt_ids = Vec::new();
let mut expected_excerpts = Vec::<(ModelHandle<Buffer>, Range<text::Anchor>)>::new();
let mut anchors = Vec::new();
let mut old_versions = Vec::new();
for _ in 0..operations {
@ -2678,6 +2679,24 @@ mod tests {
multibuffer.remove_excerpts(&ids_to_remove, cx)
});
}
30..=39 if !expected_excerpts.is_empty() => {
let multibuffer = multibuffer.read(cx).read(cx);
let offset =
multibuffer.clip_offset(rng.gen_range(0..=multibuffer.len()), Bias::Left);
let bias = if rng.gen() { Bias::Left } else { Bias::Right };
log::info!("Creating anchor at {} with bias {:?}", offset, bias);
anchors.push(multibuffer.anchor_at(offset, bias));
anchors.sort_by(|a, b| a.cmp(&b, &multibuffer).unwrap());
}
40..=44 if !anchors.is_empty() => {
let multibuffer = multibuffer.read(cx).read(cx);
anchors = multibuffer
.refresh_anchors(&anchors)
.into_iter()
.map(|a| a.0)
.collect();
anchors.sort_by(|a, b| a.cmp(&b, &multibuffer).unwrap());
}
_ => {
let buffer_handle = if buffers.is_empty() || rng.gen_bool(0.4) {
let base_text = RandomCharIter::new(&mut rng).take(10).collect::<String>();
@ -2958,6 +2977,17 @@ mod tests {
);
}
// Anchor resolution
for (anchor, resolved_offset) in anchors
.iter()
.zip(snapshot.summaries_for_anchors::<usize, _>(&anchors))
{
assert_eq!(
snapshot.summary_for_anchor::<usize>(anchor),
resolved_offset
);
}
for _ in 0..10 {
let end_ix = text_rope.clip_offset(rng.gen_range(0..=text_rope.len()), Bias::Right);
assert_eq!(

View file

@ -1,5 +1,5 @@
use super::{ExcerptId, MultiBufferSnapshot, ToOffset, ToPoint};
use anyhow::{anyhow, Result};
use anyhow::Result;
use std::{
cmp::Ordering,
ops::{Range, Sub},
@ -40,27 +40,41 @@ impl Anchor {
if excerpt_id_cmp.is_eq() {
if self.excerpt_id == ExcerptId::min() || self.excerpt_id == ExcerptId::max() {
Ok(Ordering::Equal)
} else if let Some((buffer_id, buffer_snapshot)) =
snapshot.buffer_snapshot_for_excerpt(&self.excerpt_id)
{
// Even though the anchor refers to a valid excerpt the underlying buffer might have
// changed. In that case, treat the anchor as if it were at the start of that
// excerpt.
if self.buffer_id == buffer_id && other.buffer_id == buffer_id {
self.text_anchor.cmp(&other.text_anchor, buffer_snapshot)
} else if self.buffer_id == buffer_id {
Ok(Ordering::Greater)
} else if other.buffer_id == buffer_id {
Ok(Ordering::Less)
} else {
Ok(Ordering::Equal)
}
} else {
self.text_anchor.cmp(
&other.text_anchor,
snapshot
.buffer_snapshot_for_excerpt(&self.excerpt_id)
.ok_or_else(|| anyhow!("excerpt {:?} not found", self.excerpt_id))?,
)
Ok(Ordering::Equal)
}
} else {
return Ok(excerpt_id_cmp);
Ok(excerpt_id_cmp)
}
}
pub fn bias_left(&self, snapshot: &MultiBufferSnapshot) -> Anchor {
if self.text_anchor.bias != Bias::Left {
if let Some(buffer_snapshot) = snapshot.buffer_snapshot_for_excerpt(&self.excerpt_id) {
return Self {
buffer_id: self.buffer_id,
excerpt_id: self.excerpt_id.clone(),
text_anchor: self.text_anchor.bias_left(buffer_snapshot),
};
if let Some((buffer_id, buffer_snapshot)) =
snapshot.buffer_snapshot_for_excerpt(&self.excerpt_id)
{
if self.buffer_id == buffer_id {
return Self {
buffer_id: self.buffer_id,
excerpt_id: self.excerpt_id.clone(),
text_anchor: self.text_anchor.bias_left(buffer_snapshot),
};
}
}
}
self.clone()
@ -68,12 +82,16 @@ impl Anchor {
pub fn bias_right(&self, snapshot: &MultiBufferSnapshot) -> Anchor {
if self.text_anchor.bias != Bias::Right {
if let Some(buffer_snapshot) = snapshot.buffer_snapshot_for_excerpt(&self.excerpt_id) {
return Self {
buffer_id: self.buffer_id,
excerpt_id: self.excerpt_id.clone(),
text_anchor: self.text_anchor.bias_right(buffer_snapshot),
};
if let Some((buffer_id, buffer_snapshot)) =
snapshot.buffer_snapshot_for_excerpt(&self.excerpt_id)
{
if self.buffer_id == buffer_id {
return Self {
buffer_id: self.buffer_id,
excerpt_id: self.excerpt_id.clone(),
text_anchor: self.text_anchor.bias_right(buffer_snapshot),
};
}
}
}
self.clone()