Get selections rendering again when local selections are owned by Editor

Nathan Sobo created

Change summary

Cargo.lock                               |   1 
crates/editor/Cargo.toml                 |   1 
crates/editor/src/editor.rs              | 129 ++++++++++++++++++-------
crates/editor/src/element.rs             |  49 ++++-----
crates/editor/src/multi_buffer.rs        |  68 ++++++++-----
crates/editor/src/multi_buffer/anchor.rs |  17 +-
crates/language/src/buffer.rs            |  22 ---
7 files changed, 169 insertions(+), 118 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -1531,6 +1531,7 @@ dependencies = [
  "ctor",
  "env_logger",
  "gpui",
+ "itertools",
  "language",
  "lazy_static",
  "log",

crates/editor/Cargo.toml 🔗

@@ -28,6 +28,7 @@ util = { path = "../util" }
 workspace = { path = "../workspace" }
 aho-corasick = "0.7"
 anyhow = "1.0"
+itertools = "0.10"
 lazy_static = "1.4"
 log = "0.4"
 parking_lot = "0.11"

crates/editor/src/editor.rs 🔗

@@ -3006,61 +3006,90 @@ impl Editor {
         }
     }
 
-    pub fn all_selections_in_range<'a, D>(
+    pub fn visible_selections<'a>(
         &'a self,
-        range: Range<DisplayPoint>,
+        display_rows: Range<u32>,
         cx: &'a mut MutableAppContext,
-    ) -> HashMap<ReplicaId, Vec<Selection<D>>>
-    where
-        D: TextDimension + Ord + Sub<D, Output = D>,
-    {
-        let mut result = HashMap::new();
-
+    ) -> HashMap<ReplicaId, Vec<Selection<DisplayPoint>>> {
         let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx));
         let buffer = &display_map.buffer_snapshot;
-        let range = range.start.to_offset(&display_map, Bias::Left)
-            ..range.end.to_offset(&display_map, Bias::Left);
 
-        let anchor_range = buffer.anchor_before(range.start)..buffer.anchor_after(range.end);
+        let start = if display_rows.start == 0 {
+            Anchor::min()
+        } else {
+            buffer.anchor_before(
+                DisplayPoint::new(display_rows.start, 0).to_offset(&display_map, Bias::Left),
+            )
+        };
+        let end = if display_rows.end > display_map.max_point().row() {
+            Anchor::max()
+        } else {
+            buffer.anchor_before(
+                DisplayPoint::new(display_rows.end, 0).to_offset(&display_map, Bias::Right),
+            )
+        };
+
+        dbg!(&start, &end);
+        dbg!(&self.selections);
+
         let start_ix = match self
             .selections
-            .binary_search_by(|probe| probe.end.cmp(&anchor_range.start, &buffer).unwrap())
+            .binary_search_by(|probe| probe.end.cmp(&start, &buffer).unwrap())
         {
             Ok(ix) | Err(ix) => ix,
         };
         let end_ix = match self
             .selections
-            .binary_search_by(|probe| probe.start.cmp(&anchor_range.end, &buffer).unwrap())
+            .binary_search_by(|probe| probe.start.cmp(&end, &buffer).unwrap())
         {
-            Ok(ix) | Err(ix) => ix,
+            Ok(ix) => ix + 1,
+            Err(ix) => ix,
         };
 
-        let selections = &self.selections[start_ix..end_ix];
-        let mut summaries = buffer
-            .summaries_for_anchors::<D, _>(selections.iter().flat_map(|s| [&s.start, &s.end]))
-            .into_iter();
+        dbg!(start_ix, end_ix);
+
+        fn display_selection(
+            selection: &Selection<Anchor>,
+            display_map: &DisplaySnapshot,
+        ) -> Selection<DisplayPoint> {
+            Selection {
+                id: selection.id,
+                start: selection.start.to_display_point(&display_map),
+                end: selection.end.to_display_point(&display_map),
+                reversed: selection.reversed,
+                goal: selection.goal,
+            }
+        }
+
+        let mut result = HashMap::new();
 
         result.insert(
             self.replica_id(cx),
-            selections
+            self.selections[start_ix..end_ix]
                 .iter()
-                .map(|s| Selection {
-                    id: s.id,
-                    start: summaries.next().unwrap(),
-                    end: summaries.next().unwrap(),
-                    reversed: s.reversed,
-                    goal: s.goal,
-                })
+                .chain(
+                    self.pending_selection
+                        .as_ref()
+                        .map(|pending| &pending.selection),
+                )
+                .map(|s| display_selection(s, &display_map))
                 .collect(),
         );
 
         for (replica_id, selections) in display_map
             .buffer_snapshot
-            .remote_selections_in_range(range)
+            .remote_selections_in_range(start..end)
         {
-            result.insert(replica_id, selections.collect());
+            result.insert(
+                replica_id,
+                selections
+                    .map(|s| display_selection(&s, &display_map))
+                    .collect(),
+            );
         }
 
+        dbg!(&result);
+
         result
     }
 
@@ -3137,6 +3166,31 @@ impl Editor {
         }
     }
 
+    fn resolve_selections<
+        'a,
+        D: TextDimension + Ord + Sub<D, Output = D>,
+        I: 'a + Iterator<Item = &'a Selection<Anchor>>,
+    >(
+        &self,
+        selections: I,
+        buffer: &MultiBufferSnapshot,
+    ) -> impl 'a + Iterator<Item = Selection<D>> {
+        use itertools::Itertools as _;
+
+        let (to_map, to_summarize) = selections.tee();
+        let mut summaries = buffer
+            .summaries_for_anchors::<D, _>(to_summarize.flat_map(|s| [&s.start, &s.end]))
+            .into_iter();
+
+        to_map.map(move |s| Selection {
+            id: s.id,
+            start: summaries.next().unwrap(),
+            end: summaries.next().unwrap(),
+            reversed: s.reversed,
+            goal: s.goal,
+        })
+    }
+
     fn selection_count<'a>(&self) -> usize {
         let mut count = self.selections.len();
         if self.pending_selection.is_some() {
@@ -3729,8 +3783,6 @@ pub fn diagnostic_style(
 
 #[cfg(test)]
 mod tests {
-    use std::mem;
-
     use super::*;
     use language::LanguageConfig;
     use text::Point;
@@ -3786,6 +3838,7 @@ mod tests {
             view.update_selection(DisplayPoint::new(0, 0), 0, Vector2F::zero(), cx);
         });
 
+        eprintln!(">>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>");
         assert_eq!(
             editor.update(cx, |view, cx| view.selection_ranges(cx)),
             [
@@ -3984,6 +4037,8 @@ mod tests {
         });
 
         view.update(cx, |view, cx| {
+            dbg!(&view.selections);
+
             assert_eq!(
                 view.selection_ranges(cx),
                 &[DisplayPoint::new(0, 0)..DisplayPoint::new(0, 0)]
@@ -5691,16 +5746,16 @@ mod tests {
 
     impl Editor {
         fn selection_ranges(&self, cx: &mut MutableAppContext) -> Vec<Range<DisplayPoint>> {
-            let snapshot = self.display_map.update(cx, |map, cx| map.snapshot(cx));
-            self.selections
-                .iter()
+            self.visible_selections(0..self.max_point(cx).row() + 1, cx)
+                .get(&self.replica_id(cx))
+                .unwrap()
+                .into_iter()
                 .map(|s| {
-                    let mut range =
-                        s.start.to_display_point(&snapshot)..s.end.to_display_point(&snapshot);
                     if s.reversed {
-                        mem::swap(&mut range.start, &mut range.end);
+                        s.end..s.start
+                    } else {
+                        s.start..s.end
                     }
-                    range
                 })
                 .collect()
         }

crates/editor/src/element.rs 🔗

@@ -733,37 +733,28 @@ impl Element for EditorElement {
         let scroll_top = scroll_position.y() * line_height;
         let end_row = ((scroll_top + size.y()) / line_height).ceil() as u32 + 1; // Add 1 to ensure selections bleed off screen
 
-        let selections = HashMap::new();
-        let active_rows = BTreeMap::new();
+        let mut active_rows = BTreeMap::new();
         let mut highlighted_row = None;
-        self.update_view(cx.app, |view, cx| {
+        let selections = self.update_view(cx.app, |view, cx| {
             highlighted_row = view.highlighted_row();
-
-            // TODO: Get this working with editors owning their own selections
-
-            // for selection_set_id in view.active_selection_sets(cx).collect::<Vec<_>>() {
-            //     let replica_selections = view.intersecting_selections(
-            //         selection_set_id,
-            //         DisplayPoint::new(start_row, 0)..DisplayPoint::new(end_row, 0),
-            //         cx,
-            //     );
-            //     for selection in &replica_selections {
-            //         if selection_set_id == view.selection_set_id {
-            //             let is_empty = selection.start == selection.end;
-            //             let selection_start = snapshot.prev_row_boundary(selection.start).0;
-            //             let selection_end = snapshot.next_row_boundary(selection.end).0;
-            //             for row in cmp::max(selection_start.row(), start_row)
-            //                 ..=cmp::min(selection_end.row(), end_row)
-            //             {
-            //                 let contains_non_empty_selection =
-            //                     active_rows.entry(row).or_insert(!is_empty);
-            //                 *contains_non_empty_selection |= !is_empty;
-            //             }
-            //         }
-            //     }
-
-            //     selections.insert(selection_set_id.replica_id, replica_selections);
-            // }
+            let selections = view.visible_selections(start_row..end_row, cx);
+            for (replica_id, selections) in &selections {
+                if *replica_id == view.replica_id(cx) {
+                    for selection in selections {
+                        let is_empty = selection.start == selection.end;
+                        let selection_start = snapshot.prev_row_boundary(selection.start).0;
+                        let selection_end = snapshot.next_row_boundary(selection.end).0;
+                        for row in cmp::max(selection_start.row(), start_row)
+                            ..=cmp::min(selection_end.row(), end_row)
+                        {
+                            let contains_non_empty_selection =
+                                active_rows.entry(row).or_insert(!is_empty);
+                            *contains_non_empty_selection |= !is_empty;
+                        }
+                    }
+                }
+            }
+            selections
         });
 
         let line_number_layouts = self.layout_rows(start_row..end_row, &active_rows, &snapshot, cx);

crates/editor/src/multi_buffer.rs 🔗

@@ -7,8 +7,8 @@ use clock::ReplicaId;
 use collections::HashMap;
 use gpui::{AppContext, Entity, ModelContext, ModelHandle, MutableAppContext, Task};
 use language::{
-    Buffer, BufferChunks, BufferSnapshot, Chunk, DiagnosticEntry, Event, File, Language, Selection,
-    ToOffset as _, ToPoint as _, TransactionId,
+    Buffer, BufferChunks, BufferSnapshot, Chunk, DiagnosticEntry, Event, File, FromAnchor,
+    Language, Selection, ToOffset as _, ToPoint as _, TransactionId,
 };
 use std::{
     cell::{Ref, RefCell},
@@ -805,19 +805,18 @@ impl MultiBufferSnapshot {
         let mut summaries = Vec::new();
         while let Some(anchor) = anchors.peek() {
             let excerpt_id = &anchor.excerpt_id;
-            cursor.seek(&Some(excerpt_id), Bias::Left, &());
-            if let Some(excerpt) = cursor.item() {
-                let excerpt_exists = excerpt.id == *excerpt_id;
-                let excerpt_anchors = std::iter::from_fn(|| {
-                    let anchor = anchors.peek()?;
-                    if anchor.excerpt_id == *excerpt_id {
-                        Some(&anchors.next().unwrap().text_anchor)
-                    } else {
-                        None
-                    }
-                });
+            let excerpt_anchors = std::iter::from_fn(|| {
+                let anchor = anchors.peek()?;
+                if anchor.excerpt_id == *excerpt_id {
+                    Some(&anchors.next().unwrap().text_anchor)
+                } else {
+                    None
+                }
+            });
 
-                if excerpt_exists {
+            cursor.seek_forward(&Some(excerpt_id), Bias::Left, &());
+            if let Some(excerpt) = cursor.item() {
+                if excerpt.id == *excerpt_id {
                     let mut excerpt_start = D::from_text_summary(&cursor.start().text);
                     excerpt_start.add_summary(&excerpt.header_summary(), &());
                     let excerpt_buffer_start = excerpt.range.start.summary::<D>(&excerpt.buffer);
@@ -834,12 +833,12 @@ impl MultiBufferSnapshot {
                                 excerpt_start
                             }),
                     );
-                } else {
-                    excerpt_anchors.for_each(drop);
+                    continue;
                 }
-            } else {
-                break;
             }
+
+            let summary = D::from_text_summary(&cursor.start().text);
+            summaries.extend(excerpt_anchors.map(|_| summary.clone()));
         }
 
         summaries
@@ -935,17 +934,34 @@ impl MultiBufferSnapshot {
         None
     }
 
-    pub fn remote_selections_in_range<'a, I, O>(
+    pub fn remote_selections_in_range<'a>(
         &'a self,
-        range: Range<I>,
-    ) -> impl 'a + Iterator<Item = (ReplicaId, impl 'a + Iterator<Item = Selection<O>>)>
-    where
-        I: ToOffset,
-        O: TextDimension,
-    {
+        range: Range<Anchor>,
+    ) -> impl 'a + Iterator<Item = (ReplicaId, impl 'a + Iterator<Item = Selection<Anchor>>)> {
+        // TODO
+        let excerpt_id = self.excerpts.first().unwrap().id.clone();
         self.as_singleton()
             .unwrap()
-            .remote_selections_in_range(range.start.to_offset(self)..range.end.to_offset(self))
+            .remote_selections_in_range(range.start.text_anchor..range.end.text_anchor)
+            .map(move |(replica_id, selections)| {
+                let excerpt_id = excerpt_id.clone();
+                (
+                    replica_id,
+                    selections.map(move |s| Selection {
+                        id: s.id,
+                        start: Anchor {
+                            excerpt_id: excerpt_id.clone(),
+                            text_anchor: s.start.clone(),
+                        },
+                        end: Anchor {
+                            excerpt_id: excerpt_id.clone(),
+                            text_anchor: s.end.clone(),
+                        },
+                        reversed: s.reversed,
+                        goal: s.goal,
+                    }),
+                )
+            })
     }
 }
 

crates/editor/src/multi_buffer/anchor.rs 🔗

@@ -31,15 +31,16 @@ impl Anchor {
     pub fn cmp<'a>(&self, other: &Anchor, snapshot: &MultiBufferSnapshot) -> Result<Ordering> {
         let excerpt_id_cmp = self.excerpt_id.cmp(&other.excerpt_id);
         if excerpt_id_cmp.is_eq() {
-            if self.excerpt_id == ExcerptId::max() {
-                return Ok(Ordering::Equal);
+            if self.excerpt_id == ExcerptId::min() || self.excerpt_id == ExcerptId::max() {
+                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))?,
+                )
             }
-            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))?,
-            )
         } else {
             return Ok(excerpt_id_cmp);
         }

crates/language/src/buffer.rs 🔗

@@ -1648,15 +1648,11 @@ impl BufferSnapshot {
             .min_by_key(|(open_range, close_range)| close_range.end - open_range.start)
     }
 
-    pub fn remote_selections_in_range<'a, I, O>(
+    pub fn remote_selections_in_range<'a>(
         &'a self,
-        range: Range<I>,
-    ) -> impl 'a + Iterator<Item = (ReplicaId, impl 'a + Iterator<Item = Selection<O>>)>
-    where
-        I: ToOffset,
-        O: TextDimension,
+        range: Range<Anchor>,
+    ) -> impl 'a + Iterator<Item = (ReplicaId, impl 'a + Iterator<Item = &'a Selection<Anchor>>)>
     {
-        let range = self.anchor_before(range.start)..self.anchor_after(range.end);
         self.remote_selections
             .iter()
             .map(move |(replica_id, selections)| {
@@ -1671,17 +1667,7 @@ impl BufferSnapshot {
                     Ok(ix) | Err(ix) => ix,
                 };
 
-                let selections = &selections[start_ix..end_ix];
-                let mut summaries =
-                    self.summaries_for_anchors(selections.iter().flat_map(|s| [&s.start, &s.end]));
-                let resolved = selections.iter().map(move |s| Selection {
-                    id: s.id,
-                    start: summaries.next().unwrap(),
-                    end: summaries.next().unwrap(),
-                    reversed: s.reversed,
-                    goal: s.goal,
-                });
-                (*replica_id, resolved)
+                (*replica_id, selections[start_ix..end_ix].iter())
             })
     }