Avoid ending the pending selection until updating selections

Antonio Scandurra and Max Brunsfeld created

Co-Authored-By: Max Brunsfeld <max@zed.dev>

Change summary

crates/buffer/src/anchor.rs    |  47 +++++-------
crates/buffer/src/lib.rs       |  30 ++++---
crates/buffer/src/selection.rs |  30 ++++---
crates/editor/src/lib.rs       | 131 ++++++++++++++++++++++-------------
crates/language/src/lib.rs     |   9 +
crates/workspace/src/items.rs  |  26 ++++---
6 files changed, 159 insertions(+), 114 deletions(-)

Detailed changes

crates/buffer/src/anchor.rs 🔗

@@ -113,6 +113,14 @@ impl Anchor {
             buffer.anchor_after(self)
         }
     }
+
+    pub fn summary<'a, D, C>(&self, content: C) -> D
+    where
+        D: TextDimension<'a>,
+        C: Into<Content<'a>>,
+    {
+        content.into().summary_for_anchor(self)
+    }
 }
 
 impl<T> AnchorMap<T> {
@@ -124,24 +132,15 @@ impl<T> AnchorMap<T> {
         self.entries.len()
     }
 
-    pub fn offsets<'a>(
-        &'a self,
-        content: impl Into<Content<'a>> + 'a,
-    ) -> impl Iterator<Item = (usize, &'a T)> + 'a {
-        let content = content.into();
-        content
-            .summaries_for_anchors(self)
-            .map(move |(sum, value)| (sum.bytes, value))
-    }
-
-    pub fn points<'a>(
-        &'a self,
-        content: impl Into<Content<'a>> + 'a,
-    ) -> impl Iterator<Item = (Point, &'a T)> + 'a {
+    pub fn iter<'a, D, C>(&'a self, content: C) -> impl Iterator<Item = (D, &'a T)> + 'a
+    where
+        D: 'a + TextDimension<'a>,
+        C: 'a + Into<Content<'a>>,
+    {
         let content = content.into();
         content
             .summaries_for_anchors(self)
-            .map(move |(sum, value)| (sum.lines, value))
+            .map(move |(sum, value)| (sum, value))
     }
 }
 
@@ -154,18 +153,12 @@ impl AnchorSet {
         self.0.len()
     }
 
-    pub fn offsets<'a>(
-        &'a self,
-        content: impl Into<Content<'a>> + 'a,
-    ) -> impl Iterator<Item = usize> + 'a {
-        self.0.offsets(content).map(|(offset, _)| offset)
-    }
-
-    pub fn points<'a>(
-        &'a self,
-        content: impl Into<Content<'a>> + 'a,
-    ) -> impl Iterator<Item = Point> + 'a {
-        self.0.points(content).map(|(point, _)| point)
+    pub fn iter<'a, D, C>(&'a self, content: C) -> impl Iterator<Item = D> + 'a
+    where
+        D: 'a + TextDimension<'a>,
+        C: 'a + Into<Content<'a>>,
+    {
+        self.0.iter(content).map(|(position, _)| position)
     }
 }
 

crates/buffer/src/lib.rs 🔗

@@ -1719,7 +1719,10 @@ impl<'a> Content<'a> {
         result
     }
 
-    fn summary_for_anchor(&self, anchor: &Anchor) -> TextSummary {
+    fn summary_for_anchor<D>(&self, anchor: &Anchor) -> D
+    where
+        D: TextDimension<'a>,
+    {
         let cx = Some(anchor.version.clone());
         let mut cursor = self.fragments.cursor::<(VersionedFullOffset, usize)>();
         cursor.seek(
@@ -1735,16 +1738,19 @@ impl<'a> Content<'a> {
         self.text_summary_for_range(0..cursor.start().1 + overshoot)
     }
 
-    fn text_summary_for_range(&self, range: Range<usize>) -> TextSummary {
+    fn text_summary_for_range<D>(&self, range: Range<usize>) -> D
+    where
+        D: TextDimension<'a>,
+    {
         self.visible_text.cursor(range.start).summary(range.end)
     }
 
-    fn summaries_for_anchors<T>(
-        &self,
-        map: &'a AnchorMap<T>,
-    ) -> impl Iterator<Item = (TextSummary, &'a T)> {
+    fn summaries_for_anchors<D, T>(&self, map: &'a AnchorMap<T>) -> impl Iterator<Item = (D, &'a T)>
+    where
+        D: TextDimension<'a>,
+    {
         let cx = Some(map.version.clone());
-        let mut summary = TextSummary::default();
+        let mut summary = D::default();
         let mut rope_cursor = self.visible_text.cursor(0);
         let mut cursor = self.fragments.cursor::<(VersionedFullOffset, usize)>();
         map.entries.iter().map(move |((offset, bias), value)| {
@@ -1754,7 +1760,7 @@ impl<'a> Content<'a> {
             } else {
                 0
             };
-            summary += rope_cursor.summary::<TextSummary>(cursor.start().1 + overshoot);
+            summary.add_assign(&rope_cursor.summary(cursor.start().1 + overshoot));
             (summary.clone(), value)
         })
     }
@@ -1928,7 +1934,7 @@ impl<'a> Content<'a> {
 
     fn point_for_offset(&self, offset: usize) -> Result<Point> {
         if offset <= self.len() {
-            Ok(self.text_summary_for_range(0..offset).lines)
+            Ok(self.text_summary_for_range(0..offset))
         } else {
             Err(anyhow!("offset out of bounds"))
         }
@@ -2316,13 +2322,13 @@ impl ToOffset for usize {
 
 impl ToOffset for Anchor {
     fn to_offset<'a>(&self, content: impl Into<Content<'a>>) -> usize {
-        content.into().summary_for_anchor(self).bytes
+        content.into().summary_for_anchor(self)
     }
 }
 
 impl<'a> ToOffset for &'a Anchor {
     fn to_offset<'b>(&self, content: impl Into<Content<'b>>) -> usize {
-        content.into().summary_for_anchor(self).bytes
+        content.into().summary_for_anchor(self)
     }
 }
 
@@ -2332,7 +2338,7 @@ pub trait ToPoint {
 
 impl ToPoint for Anchor {
     fn to_point<'a>(&self, content: impl Into<Content<'a>>) -> Point {
-        content.into().summary_for_anchor(self).lines
+        content.into().summary_for_anchor(self)
     }
 }
 

crates/buffer/src/selection.rs 🔗

@@ -36,18 +36,28 @@ pub struct SelectionState {
     pub goal: SelectionGoal,
 }
 
-impl<T: ToOffset + ToPoint + Copy + Ord> Selection<T> {
-    pub fn is_empty(&self) -> bool {
-        self.start == self.end
+impl<T: Clone> Selection<T> {
+    pub fn head(&self) -> T {
+        if self.reversed {
+            self.start.clone()
+        } else {
+            self.end.clone()
+        }
     }
 
-    pub fn head(&self) -> T {
+    pub fn tail(&self) -> T {
         if self.reversed {
-            self.start
+            self.end.clone()
         } else {
-            self.end
+            self.start.clone()
         }
     }
+}
+
+impl<T: ToOffset + ToPoint + Copy + Ord> Selection<T> {
+    pub fn is_empty(&self) -> bool {
+        self.start == self.end
+    }
 
     pub fn set_head(&mut self, head: T) {
         if head.cmp(&self.tail()) < Ordering::Equal {
@@ -65,14 +75,6 @@ impl<T: ToOffset + ToPoint + Copy + Ord> Selection<T> {
         }
     }
 
-    pub fn tail(&self) -> T {
-        if self.reversed {
-            self.end
-        } else {
-            self.start
-        }
-    }
-
     pub fn point_range(&self, buffer: &Buffer) -> Range<Point> {
         let start = self.start.to_point(buffer);
         let end = self.end.to_point(buffer);

crates/editor/src/lib.rs 🔗

@@ -21,7 +21,8 @@ use smallvec::SmallVec;
 use smol::Timer;
 use std::{
     cell::RefCell,
-    cmp, iter, mem,
+    cmp::{self, Ordering},
+    iter, mem,
     ops::{Range, RangeInclusive},
     rc::Rc,
     sync::Arc,
@@ -293,7 +294,7 @@ pub struct Editor {
     buffer: ModelHandle<Buffer>,
     display_map: ModelHandle<DisplayMap>,
     selection_set_id: SelectionSetId,
-    pending_selection: Option<Selection<Point>>,
+    pending_selection: Option<Selection<Anchor>>,
     next_selection_id: usize,
     add_selections_state: Option<AddSelectionsState>,
     autoclose_stack: Vec<BracketPairState>,
@@ -618,10 +619,11 @@ impl Editor {
         }
 
         let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx));
-        let cursor = position.to_buffer_point(&display_map, Bias::Left);
+        let buffer = self.buffer.read(cx);
+        let cursor = buffer.anchor_before(position.to_buffer_point(&display_map, Bias::Left));
         let selection = Selection {
             id: post_inc(&mut self.next_selection_id),
-            start: cursor,
+            start: cursor.clone(),
             end: cursor,
             reversed: false,
             goal: SelectionGoal::None,
@@ -642,9 +644,22 @@ impl Editor {
         cx: &mut ViewContext<Self>,
     ) {
         let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx));
-        let cursor = position.to_buffer_point(&display_map, Bias::Left);
-        if let Some(selection) = self.pending_selection.as_mut() {
-            selection.set_head(cursor);
+        if let Some(pending_selection) = self.pending_selection.as_mut() {
+            let buffer = self.buffer.read(cx);
+            let cursor = buffer.anchor_before(position.to_buffer_point(&display_map, Bias::Left));
+            if cursor.cmp(&pending_selection.tail(), buffer).unwrap() < Ordering::Equal {
+                if !pending_selection.reversed {
+                    pending_selection.end = pending_selection.start.clone();
+                    pending_selection.reversed = true;
+                }
+                pending_selection.start = cursor;
+            } else {
+                if pending_selection.reversed {
+                    pending_selection.start = pending_selection.end.clone();
+                    pending_selection.reversed = false;
+                }
+                pending_selection.end = cursor;
+            }
         } else {
             log::error!("update_selection dispatched with no pending selection");
             return;
@@ -655,12 +670,8 @@ impl Editor {
     }
 
     fn end_selection(&mut self, cx: &mut ViewContext<Self>) {
-        if let Some(selection) = self.pending_selection.take() {
-            let mut selections = self.selections::<Point>(cx).collect::<Vec<_>>();
-            let ix = self.selection_insertion_index(&selections, selection.start);
-            selections.insert(ix, selection);
-            self.update_selections(selections, false, cx);
-        }
+        let selections = self.selections::<Point>(cx).collect::<Vec<_>>();
+        self.update_selections(selections, false, cx);
     }
 
     pub fn is_selecting(&self) -> bool {
@@ -669,13 +680,27 @@ impl Editor {
 
     pub fn cancel(&mut self, _: &Cancel, cx: &mut ViewContext<Self>) {
         if let Some(pending_selection) = self.pending_selection.take() {
+            let buffer = self.buffer.read(cx);
+            let pending_selection = Selection {
+                id: pending_selection.id,
+                start: pending_selection.start.to_point(buffer),
+                end: pending_selection.end.to_point(buffer),
+                reversed: pending_selection.reversed,
+                goal: pending_selection.goal,
+            };
             if self.selections::<Point>(cx).next().is_none() {
                 self.update_selections(vec![pending_selection], true, cx);
             }
         } else {
-            let selection_count = self.selection_count(cx);
             let selections = self.selections::<Point>(cx);
-            let mut oldest_selection = selections.min_by_key(|s| s.id).unwrap().clone();
+            let mut selection_count = 0;
+            let mut oldest_selection = selections
+                .min_by_key(|s| {
+                    selection_count += 1;
+                    s.id
+                })
+                .unwrap()
+                .clone();
             if selection_count == 1 {
                 oldest_selection.start = oldest_selection.head().clone();
                 oldest_selection.end = oldest_selection.head().clone();
@@ -970,7 +995,6 @@ impl Editor {
     }
 
     fn skip_autoclose_end(&mut self, text: &str, cx: &mut ViewContext<Self>) -> bool {
-        let old_selection_count = self.selection_count(cx);
         let old_selections = self.selections::<usize>(cx).collect::<Vec<_>>();
         let autoclose_pair_state = if let Some(autoclose_pair_state) = self.autoclose_stack.last() {
             autoclose_pair_state
@@ -981,7 +1005,7 @@ impl Editor {
             return false;
         }
 
-        debug_assert_eq!(old_selection_count, autoclose_pair_state.ranges.len());
+        debug_assert_eq!(old_selections.len(), autoclose_pair_state.ranges.len());
 
         let buffer = self.buffer.read(cx);
         if old_selections
@@ -2220,21 +2244,6 @@ impl Editor {
             .map(|(set_id, _)| *set_id)
     }
 
-    pub fn last_selection(&self, cx: &AppContext) -> Selection<Point> {
-        if let Some(pending_selection) = self.pending_selection.as_ref() {
-            pending_selection.clone()
-        } else {
-            let buffer = self.buffer.read(cx);
-            let last_selection = buffer
-                .selection_set(self.selection_set_id)
-                .unwrap()
-                .selections::<Point, _>(buffer)
-                .max_by_key(|s| s.id)
-                .unwrap();
-            last_selection
-        }
-    }
-
     pub fn selections_in_range<'a>(
         &'a self,
         set_id: SelectionSetId,
@@ -2251,10 +2260,14 @@ impl Editor {
         let start = range.start.to_buffer_point(&display_map, Bias::Left);
         let start_index = self.selection_insertion_index(&selections, start);
         let pending_selection = if set_id.replica_id == self.buffer.read(cx).replica_id() {
-            self.pending_selection.as_ref().and_then(|s| {
-                let selection_range = s.display_range(&display_map);
-                if selection_range.start <= range.end || selection_range.end <= range.end {
-                    Some(selection_range)
+            self.pending_selection.as_ref().and_then(|pending| {
+                let mut selection_start = pending.start.to_display_point(&display_map, Bias::Left);
+                let mut selection_end = pending.end.to_display_point(&display_map, Bias::Left);
+                if pending.reversed {
+                    mem::swap(&mut selection_start, &mut selection_end);
+                }
+                if selection_start <= range.end || selection_end <= range.end {
+                    Some(selection_start..selection_end)
                 } else {
                     None
                 }
@@ -2283,25 +2296,46 @@ impl Editor {
         }
     }
 
-    fn selections<'a, D>(
-        &mut self,
-        cx: &'a mut ViewContext<Self>,
-    ) -> impl 'a + Iterator<Item = Selection<D>>
+    pub fn selections<'a, D>(&self, cx: &'a AppContext) -> impl 'a + Iterator<Item = Selection<D>>
     where
-        D: 'a + TextDimension<'a>,
+        D: 'a + TextDimension<'a> + Ord,
     {
-        self.end_selection(cx);
         let buffer = self.buffer.read(cx);
-        buffer
+        let mut selections = buffer
             .selection_set(self.selection_set_id)
             .unwrap()
             .selections::<D, _>(buffer)
-    }
+            .peekable();
+        let mut pending_selection = self.pending_selection.clone().map(|selection| Selection {
+            id: selection.id,
+            start: selection.start.summary::<D, _>(buffer),
+            end: selection.end.summary::<D, _>(buffer),
+            reversed: selection.reversed,
+            goal: selection.goal,
+        });
+        iter::from_fn(move || {
+            if let Some(pending) = pending_selection.as_mut() {
+                while let Some(next_selection) = selections.peek() {
+                    if pending.start <= next_selection.end && pending.end >= next_selection.start {
+                        let next_selection = selections.next().unwrap();
+                        if next_selection.start < pending.start {
+                            pending.start = next_selection.start;
+                        }
+                        if next_selection.end > pending.end {
+                            pending.end = next_selection.end;
+                        }
+                    } else if next_selection.end < pending.start {
+                        return selections.next();
+                    } else {
+                        break;
+                    }
+                }
 
-    fn selection_count(&mut self, cx: &mut ViewContext<Self>) -> usize {
-        self.end_selection(cx);
-        let buffer = self.buffer.read(cx);
-        buffer.selection_set(self.selection_set_id).unwrap().len()
+                pending_selection.take()
+            } else {
+                selections.next()
+            }
+        })
     }
 
     fn update_selections<T>(
@@ -2329,6 +2363,7 @@ impl Editor {
             }
         }
 
+        self.pending_selection = None;
         self.add_selections_state = None;
         self.select_larger_syntax_node_stack.clear();
         while let Some(autoclose_pair_state) = self.autoclose_stack.last() {

crates/language/src/lib.rs 🔗

@@ -830,9 +830,14 @@ impl Buffer {
             for request in autoindent_requests {
                 let old_to_new_rows = request
                     .edited
-                    .points(&request.before_edit)
+                    .iter::<Point, _>(&request.before_edit)
                     .map(|point| point.row)
-                    .zip(request.edited.points(&snapshot).map(|point| point.row))
+                    .zip(
+                        request
+                            .edited
+                            .iter::<Point, _>(&snapshot)
+                            .map(|point| point.row),
+                    )
                     .collect::<BTreeMap<u32, u32>>();
 
                 let mut old_suggestions = HashMap::<u32, u32>::default();

crates/workspace/src/items.rs 🔗

@@ -1,7 +1,7 @@
 use super::{Item, ItemView};
 use crate::{status_bar::StatusItemView, Settings};
 use anyhow::Result;
-use buffer::{Point, ToOffset};
+use buffer::{Point, Selection, ToPoint};
 use editor::{Editor, EditorSettings, Event};
 use gpui::{
     elements::*, fonts::TextStyle, AppContext, Entity, ModelHandle, RenderContext, Subscription,
@@ -181,17 +181,21 @@ impl CursorPosition {
 
     fn update_position(&mut self, editor: ViewHandle<Editor>, cx: &mut ViewContext<Self>) {
         let editor = editor.read(cx);
-
-        let last_selection = editor.last_selection(cx);
-        self.position = Some(last_selection.head());
-        if last_selection.is_empty() {
-            self.selected_count = 0;
-        } else {
-            let buffer = editor.buffer().read(cx);
-            let start = last_selection.start.to_offset(buffer);
-            let end = last_selection.end.to_offset(buffer);
-            self.selected_count = end - start;
+        let buffer = editor.buffer().read(cx);
+
+        self.selected_count = 0;
+        let mut last_selection: Option<Selection<usize>> = None;
+        for selection in editor.selections::<usize>(cx) {
+            self.selected_count += selection.end - selection.start;
+            if last_selection
+                .as_ref()
+                .map_or(true, |last_selection| selection.id > last_selection.id)
+            {
+                last_selection = Some(selection);
+            }
         }
+        self.position = last_selection.map(|s| s.head().to_point(buffer));
+
         cx.notify();
     }
 }