diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index dbc827e7f9..2678428821 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -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::>(); - 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) -> HashMap { - 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::(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 } diff --git a/crates/editor/src/multi_buffer.rs b/crates/editor/src/multi_buffer.rs index 28d935d2ec..5adbc8c9a1 100644 --- a/crates/editor/src/multi_buffer.rs +++ b/crates/editor/src/multi_buffer.rs @@ -1342,6 +1342,58 @@ impl MultiBufferSnapshot { position } + pub fn summaries_for_anchors<'a, D, I>(&'a self, anchors: I) -> Vec + where + D: TextDimension + Ord + Sub, + I: 'a + IntoIterator, + { + let mut anchors = anchors.into_iter().peekable(); + let mut cursor = self.excerpts.cursor::(); + 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::(&excerpt.buffer); + summaries.extend( + excerpt + .buffer + .summaries_for_anchors::(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, @@ -1423,58 +1475,6 @@ impl MultiBufferSnapshot { result } - pub fn summaries_for_anchors<'a, D, I>(&'a self, anchors: I) -> Vec - where - D: TextDimension + Ord + Sub, - I: 'a + IntoIterator, - { - let mut anchors = anchors.into_iter().peekable(); - let mut cursor = self.excerpts.cursor::(); - 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::(&excerpt.buffer); - summaries.extend( - excerpt - .buffer - .summaries_for_anchors::(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(&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::>(); 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, Range)>::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::(); @@ -2958,6 +2977,17 @@ mod tests { ); } + // Anchor resolution + for (anchor, resolved_offset) in anchors + .iter() + .zip(snapshot.summaries_for_anchors::(&anchors)) + { + assert_eq!( + snapshot.summary_for_anchor::(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!( diff --git a/crates/editor/src/multi_buffer/anchor.rs b/crates/editor/src/multi_buffer/anchor.rs index 2e1e1a9246..e2b6d2f2ad 100644 --- a/crates/editor/src/multi_buffer/anchor.rs +++ b/crates/editor/src/multi_buffer/anchor.rs @@ -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()