add path key index to anchor

Cole Miller created

Change summary

crates/multi_buffer/src/anchor.rs       | 55 +++++++++++++-----
crates/multi_buffer/src/multi_buffer.rs | 34 +++++++---
crates/multi_buffer/src/path_key.rs     | 81 +++++++++++++++++++-------
3 files changed, 121 insertions(+), 49 deletions(-)

Detailed changes

crates/multi_buffer/src/anchor.rs 🔗

@@ -1,4 +1,6 @@
-use crate::{ExcerptSummary, MultiBufferDimension, MultiBufferOffset, MultiBufferOffsetUtf16};
+use crate::{
+    ExcerptSummary, MultiBufferDimension, MultiBufferOffset, MultiBufferOffsetUtf16, PathKeyIndex,
+};
 
 use super::{MultiBufferSnapshot, ToOffset, ToPoint};
 use language::Point;
@@ -30,6 +32,9 @@ pub enum Anchor {
         /// the offset.
         bias: Bias,
         buffer_id: BufferId,
+        /// Refers to the path key that the buffer had when this anchor was created,
+        /// so that ordering is stable when the path key for a buffer changes
+        path: PathKeyIndex,
         /// When present, indicates this anchor points into deleted text within an
         /// expanded diff hunk. The anchor references a position in the diff base
         /// (original) text rather than the current buffer text. This is used when
@@ -97,11 +102,12 @@ impl Anchor {
         self
     }
 
-    pub fn text(text_anchor: text::Anchor) -> Self {
+    pub fn text(path: PathKeyIndex, text_anchor: text::Anchor) -> Self {
         let Some(buffer_id) = text_anchor.buffer_id else {
             panic!("text_anchor must have a buffer_id");
         };
         Self::Text {
+            path,
             diff_base_anchor: None,
             timestamp: text_anchor.timestamp(),
             buffer_id,
@@ -110,8 +116,8 @@ impl Anchor {
         }
     }
 
-    pub fn range_in_buffer(range: Range<text::Anchor>) -> Range<Self> {
-        Self::text(range.start)..Self::text(range.end)
+    pub fn range_in_buffer(path: PathKeyIndex, range: Range<text::Anchor>) -> Range<Self> {
+        Self::text(path, range.start)..Self::text(path, range.end)
     }
 
     pub fn min() -> Self {
@@ -131,25 +137,35 @@ impl Anchor {
     }
 
     pub fn cmp(&self, other: &Anchor, snapshot: &MultiBufferSnapshot) -> Ordering {
-        let (self_text_anchor, other_text_anchor) = match (self, other) {
+        let (self_text_anchor, self_path, other_text_anchor, other_path) = match (self, other) {
             (Anchor::Min, Anchor::Min) => return Ordering::Equal,
             (Anchor::Max, Anchor::Max) => return Ordering::Equal,
             (Anchor::Min, _) => return Ordering::Less,
             (Anchor::Max, _) => return Ordering::Greater,
             (_, Anchor::Max) => return Ordering::Less,
             (_, Anchor::Min) => return Ordering::Greater,
-            (Anchor::Text { .. }, Anchor::Text { .. }) => {
-                (self.text_anchor().unwrap(), other.text_anchor().unwrap())
-            }
+            (
+                Anchor::Text {
+                    path: self_path, ..
+                },
+                Anchor::Text {
+                    path: other_path, ..
+                },
+            ) => (
+                self.text_anchor().unwrap(),
+                self_path,
+                other.text_anchor().unwrap(),
+                other_path,
+            ),
         };
         let self_buffer_id = self_text_anchor.buffer_id.unwrap();
         let other_buffer_id = other_text_anchor.buffer_id.unwrap();
 
-        let Some(self_path_key) = snapshot.path_keys.get(&self_buffer_id) else {
-            panic!("path key was never set for buffer_id")
+        let Some(self_path_key) = snapshot.path_keys_by_index.get(&self_path) else {
+            panic!("anchor's path was never added to multibuffer")
         };
-        let Some(other_path_key) = snapshot.path_keys.get(&other_buffer_id) else {
-            panic!("path key was never set for buffer_id")
+        let Some(other_path_key) = snapshot.path_keys_by_index.get(&other_path) else {
+            panic!("anchor's path was never added to multibuffer")
         };
 
         if self_path_key.cmp(other_path_key) != Ordering::Equal {
@@ -158,6 +174,7 @@ impl Anchor {
 
         // in the case that you removed the buffer contianing self,
         // and added the buffer containing other with the same path key
+        // (ordering is arbitrary but consistent)
         if self_buffer_id != other_buffer_id {
             return self_buffer_id.cmp(&other_buffer_id);
         }
@@ -208,7 +225,10 @@ impl Anchor {
             Anchor::Min => *self,
             Anchor::Max => snapshot.anchor_before(snapshot.max_point()),
             Anchor::Text {
-                bias, buffer_id, ..
+                path,
+                bias,
+                buffer_id,
+                ..
             } => {
                 if *bias == Bias::Left {
                     return *self;
@@ -217,7 +237,7 @@ impl Anchor {
                     return *self;
                 };
                 let text_anchor = self.text_anchor().unwrap().bias_left(&buffer);
-                let ret = Self::text(text_anchor);
+                let ret = Self::text(*path, text_anchor);
                 if let Some(diff_base_anchor) = self.diff_base_anchor() {
                     if let Some(diff) = snapshot.diffs.get(&buffer_id)
                         && diff_base_anchor.is_valid(&diff.base_text())
@@ -238,7 +258,10 @@ impl Anchor {
             Anchor::Min => *self,
             Anchor::Max => snapshot.anchor_after(Point::zero()),
             Anchor::Text {
-                bias, buffer_id, ..
+                path,
+                bias,
+                buffer_id,
+                ..
             } => {
                 if *bias == Bias::Right {
                     return *self;
@@ -247,7 +270,7 @@ impl Anchor {
                     return *self;
                 };
                 let text_anchor = self.text_anchor().unwrap().bias_right(&buffer);
-                let ret = Self::text(text_anchor);
+                let ret = Self::text(*path, text_anchor);
                 if let Some(diff_base_anchor) = self.diff_base_anchor() {
                     if let Some(diff) = snapshot.diffs.get(&buffer_id)
                         && diff_base_anchor.is_valid(&diff.base_text())

crates/multi_buffer/src/multi_buffer.rs 🔗

@@ -89,6 +89,9 @@ pub struct MultiBuffer {
     buffer_changed_since_sync: Rc<Cell<bool>>,
 }
 
+#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+pub struct PathKeyIndex(u64);
+
 pub struct ExcerptInfo {
     path_key: PathKey,
     buffer_id: BufferId,
@@ -610,7 +613,8 @@ impl DiffState {
 #[derive(Clone, Default)]
 pub struct MultiBufferSnapshot {
     excerpts: SumTree<Excerpt>,
-    path_keys: TreeMap<BufferId, PathKey>,
+    path_keys_by_buffer: TreeMap<BufferId, PathKey>,
+    path_keys_by_index: TreeMap<PathKeyIndex, PathKey>,
     diffs: TreeMap<BufferId, DiffStateSnapshot>,
     diff_transforms: SumTree<DiffTransform>,
     non_text_state_update_count: usize,
@@ -796,6 +800,10 @@ impl ExcerptSummary {
             count: 0,
         }
     }
+
+    pub fn len(&self) -> ExcerptOffset {
+        ExcerptDimension(self.text.len)
+    }
 }
 
 #[derive(Debug, Clone)]
@@ -1727,7 +1735,7 @@ impl MultiBuffer {
             show_deleted_hunks: _,
             use_extended_diff_range: _,
             show_headers: _,
-            path_keys: _,
+            path_keys_by_buffer: _,
         } = self.snapshot.get_mut();
         let start = ExcerptDimension(MultiBufferOffset::ZERO);
         let prev_len = ExcerptDimension(excerpts.summary().text.len);
@@ -1764,7 +1772,7 @@ impl MultiBuffer {
     ) -> Vec<ExcerptRange<text::Anchor>> {
         let mut excerpts = Vec::new();
         let snapshot = self.read(cx);
-        let Some(path_key) = snapshot.path_keys.get(&buffer_id).cloned() else {
+        let Some(path_key) = snapshot.path_keys_by_buffer.get(&buffer_id).cloned() else {
             return excerpts;
         };
 
@@ -1790,7 +1798,7 @@ impl MultiBuffer {
             .cursor::<Dimensions<ExcerptPoint, OutputDimension<Point>>>(());
         diff_transforms.next();
         let mut result = Vec::new();
-        let Some(path_key) = snapshot.path_keys.get(&buffer_id) else {
+        let Some(path_key) = snapshot.path_keys_by_buffer.get(&buffer_id) else {
             return result;
         };
 
@@ -6598,11 +6606,19 @@ impl MultiBufferSnapshot {
     }
 
     pub fn path_for_buffer(&self, buffer_id: BufferId) -> Option<&PathKey> {
-        self.path_keys.get(&buffer_id)
+        self.path_keys_by_buffer.get(&buffer_id)
     }
 
     pub fn buffer_for_id(&self, id: BufferId) -> Option<&BufferSnapshot> {
-        self.buffer_for_path(self.path_keys.get(&id)?)
+        self.buffer_for_path(self.path_keys_by_buffer.get(&id)?)
+    }
+
+    pub fn path_for_anchor(&self, anchor: Anchor) -> Option<PathKey> {
+        match anchor {
+            Anchor::Min => Some(self.excerpts.first()?.path_key.clone()),
+            Anchor::Max => Some(self.excerpts.last()?.path_key.clone()),
+            Anchor::Text { path, .. } => self.path_keys_by_index.get(&path).cloned(),
+        }
     }
 
     pub fn range_for_excerpt(&self, excerpt_id: ExcerptId) -> Option<Range<Point>> {
@@ -6836,10 +6852,6 @@ impl MultiBufferSnapshot {
         }
         excerpt_edits
     }
-
-    fn path_key_for_anchor(&self, anchor: Anchor) -> Option<PathKey> {
-        todo!()
-    }
 }
 
 #[cfg(any(test, feature = "test-support"))]
@@ -7689,7 +7701,7 @@ where
     }
 }
 
-#[derive(Copy, Clone, PartialOrd, Ord, Eq, PartialEq, Debug)]
+#[derive(Copy, Clone, PartialOrd, Ord, Eq, PartialEq, Debug, Default)]
 struct ExcerptDimension<T>(T);
 
 impl<T: PartialEq> PartialEq<T> for ExcerptDimension<T> {

crates/multi_buffer/src/path_key.rs 🔗

@@ -16,8 +16,9 @@ use util::rel_path::RelPath;
 use ztracing::instrument;
 
 use crate::{
-    Anchor, BufferState, DiffChangeKind, Event, Excerpt, ExcerptRange, ExcerptSummary,
-    ExpandExcerptDirection, MultiBuffer, MultiBufferOffset, build_excerpt_ranges,
+    Anchor, BufferState, DiffChangeKind, Event, Excerpt, ExcerptOffset, ExcerptRange,
+    ExcerptSummary, ExpandExcerptDirection, MultiBuffer, MultiBufferOffset, PathKeyIndex,
+    build_excerpt_ranges,
 };
 
 #[derive(PartialEq, Eq, Ord, PartialOrd, Clone, Hash, Debug)]
@@ -210,33 +211,38 @@ impl MultiBuffer {
         let mut cursor = snapshot.excerpts.cursor::<ExcerptSummary>(());
         let mut sorted_anchors = sorted_anchors.into_iter().peekable();
         while let Some(anchor) = sorted_anchors.next() {
-            let Some(buffer) = self.buffer_for_anchor(anchor, cx) else {
+            let Some(path) = snapshot.path_for_anchor(anchor) else {
                 continue;
             };
-            let buffer_snapshot = buffer.read(cx).snapshot();
-            let Some(path) = snapshot.path_key_for_anchor(anchor) else {
+            let Some(buffer) = self.buffer_for_path(&path, cx) else {
                 continue;
             };
+            let buffer_snapshot = buffer.read(cx).snapshot();
 
             let mut expanded_ranges = Vec::new();
-            cursor.seek(&path, Bias::Left);
-            while let Some(anchor) = sorted_anchors.next() {
+            // Move to the first excerpt for this path
+            cursor.seek_forward(&path, Bias::Left);
+            while let Some(anchor) = sorted_anchors.peek().copied()
+                && snapshot.path_for_anchor(anchor).as_ref() == Some(&path)
+            {
+                sorted_anchors.next();
                 let Some(target) = snapshot.anchor_seek_target(anchor) else {
                     continue;
                 };
-                // Add ranges for non-expanded excerpts before the next expanded excerpt (or after the last one)
+                // Move to the next excerpt to be expanded, and push unchanged ranges for intervening excerpts
                 expanded_ranges.extend(
                     cursor
                         .slice(&target, Bias::Left)
                         .iter()
                         .map(|excerpt| excerpt.range.clone()),
                 );
-                if snapshot.path_key_for_anchor(anchor) != Some(path.clone()) {
-                    break;
-                }
                 let Some(excerpt) = cursor.item() else {
                     continue;
                 };
+                if excerpt.path_key != path {
+                    continue;
+                }
+                // Expand the range for this excerpt
                 let mut context = excerpt.range.context.to_point(&buffer_snapshot);
                 match direction {
                     ExpandExcerptDirection::Up => {
@@ -261,7 +267,17 @@ impl MultiBuffer {
                     context,
                     primary: excerpt.range.primary.clone(),
                 });
+                cursor.next();
+            }
+
+            // Add unchanged ranges for this path after the last expanded excerpt
+            while let Some(excerpt) = cursor.item()
+                && excerpt.path_key == path
+            {
+                expanded_ranges.push(excerpt.range.clone());
+                cursor.next();
             }
+
             let mut merged_ranges: Vec<ExcerptRange<text::Anchor>> = Vec::new();
             for range in expanded_ranges {
                 if let Some(last_range) = merged_ranges.last_mut()
@@ -312,9 +328,9 @@ impl MultiBuffer {
             if let Some(old_path_key) = self
                 .snapshot(cx)
                 .path_for_buffer(buffer_snapshot.remote_id())
-                && old_path_key != path_key
+                && old_path_key != &path_key
             {
-                self.remove_excerpts_for_path(old_path_key, cx);
+                self.remove_excerpts_for_path(old_path_key.clone(), cx);
             }
 
             return false;
@@ -345,14 +361,31 @@ impl MultiBuffer {
         let mut snapshot = self.snapshot.get_mut();
         let mut cursor = snapshot
             .excerpts
-            .cursor::<Dimensions<PathKey, MultiBufferOffset>>(());
+            .cursor::<Dimensions<PathKey, ExcerptOffset>>(());
         let mut new_excerpts = SumTree::new(());
 
         let to_insert = to_insert.iter().peekable();
         let mut patch = Patch::empty();
         let mut added_new_excerpt = false;
 
-        let old_path_key = snapshot.path_keys.insert_or_replace(buffer_id, path_key);
+        let path_key_index = snapshot
+            .path_keys_by_index
+            .iter()
+            // todo!() perf? (but ExcerptIdMapping was doing this)
+            .find(|(_, existing_path)| existing_path == &&path_key)
+            .map(|(index, _)| *index)
+            .unwrap_or_else(|| {
+                let index = snapshot
+                    .path_keys_by_index
+                    .last()
+                    .map(|(index, _)| PathKeyIndex(index.0 + 1))
+                    .unwrap_or(PathKeyIndex(0));
+                snapshot.path_keys_by_index.insert(index, path_key.clone());
+                index
+            });
+        let old_path_key = snapshot
+            .path_keys_by_buffer
+            .insert_or_replace(buffer_id, path_key);
         // handle the case where the buffer's path key has changed by
         // removing any old excerpts for the buffer
         if let Some(old_path_key) = old_path_key
@@ -364,7 +397,7 @@ impl MultiBuffer {
             let after = cursor.position.1;
             patch.push(Edit {
                 old: before..after,
-                new: new_excerpts.summary().text.len..new_excerpts.summary().text.len,
+                new: new_excerpts.summary().len()..new_excerpts.summary().len(),
             });
         }
 
@@ -381,7 +414,7 @@ impl MultiBuffer {
             let after = cursor.position.1;
             patch.push(Edit {
                 old: before..after,
-                new: new_excerpts.summary().text.len..new_excerpts.summary().text.len,
+                new: new_excerpts.summary().len()..new_excerpts.summary().len(),
             });
         }
 
@@ -393,7 +426,7 @@ impl MultiBuffer {
             let Some(next_excerpt) = to_insert.peek() else {
                 break;
             };
-            if &excerpt.range == next_excerpt {
+            if &&excerpt.range == next_excerpt {
                 new_excerpts.push(excerpt.clone(), ());
                 to_insert.next();
                 cursor.next();
@@ -407,17 +440,20 @@ impl MultiBuffer {
                 .cmp(&next_excerpt.context.start, &buffer_snapshot)
                 .is_le()
             {
+                // remove old excerpt
                 let before = cursor.position.1;
                 cursor.next();
                 let after = cursor.position.1;
                 patch.push(Edit {
                     old: before..after,
-                    new: new_excerpts.summary().text.len..new_excerpts.summary().text.len,
+                    new: new_excerpts.summary().len()..new_excerpts.summary().len(),
                 });
             } else {
+                // insert new excerpt
                 let before = new_excerpts.summary().text.len;
                 let next_excerpt = to_insert.next().unwrap();
                 added_new_excerpt = true;
+                let before = new_excerpts.summary().len();
                 new_excerpts.push(
                     Excerpt::new(
                         path_key.clone(),
@@ -427,9 +463,10 @@ impl MultiBuffer {
                     ),
                     (),
                 );
+                let after = new_excerpts.summary().len();
                 patch.push(Edit {
                     old: cursor.position.1..cursor.position.1,
-                    new: new_excerpts.summary().text.len..new_excerpts.summary().text.len,
+                    new: before..after,
                 });
             }
         }
@@ -440,7 +477,7 @@ impl MultiBuffer {
         let after = cursor.position.1;
         patch.push(Edit {
             old: before..after,
-            new: new_excerpts.summary().text.len..new_excerpts.summary().text.len,
+            new: new_excerpts.summary().len()..new_excerpts.summary().len(),
         });
 
         // handle the case where the buffer's path key has changed by
@@ -454,7 +491,7 @@ impl MultiBuffer {
             let after = cursor.position.1;
             patch.push(Edit {
                 old: before..after,
-                new: new_excerpts.summary().text.len..new_excerpts.summary().text.len,
+                new: new_excerpts.summary().len()..new_excerpts.summary().len(),
             });
         }