Make multibuffer anchor's buffer_id optional

Max Brunsfeld , Antonio Scandurra , and Nathan Sobo created

Avoid using 0 for a buffer id on Anchor::min and max

Co-Authored-By: Antonio Scandurra <me@as-cii.com>
Co-Authored-By: Nathan Sobo <nathan@zed.dev>

Change summary

crates/editor/src/editor.rs              | 67 +++++++++++++++----------
crates/editor/src/multi_buffer.rs        | 48 +++++++++--------
crates/editor/src/multi_buffer/anchor.rs | 16 +++---
3 files changed, 72 insertions(+), 59 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -1911,10 +1911,15 @@ impl Editor {
         };
 
         let position = self.newest_anchor_selection().head();
-        let (buffer, buffer_position) = self
+        let (buffer, buffer_position) = if let Some(output) = self
             .buffer
             .read(cx)
-            .text_anchor_for_position(position.clone(), cx);
+            .text_anchor_for_position(position.clone(), cx)
+        {
+            output
+        } else {
+            return;
+        };
 
         let query = Self::completion_query(&self.buffer.read(cx).read(cx), position.clone());
         let completions = project.update(cx, |project, cx| {
@@ -2012,7 +2017,7 @@ impl Editor {
 
         let selections = self.local_selections::<usize>(cx);
         let newest_selection = self.newest_anchor_selection();
-        if newest_selection.start.buffer_id != buffer_handle.id() {
+        if newest_selection.start.buffer_id != Some(buffer_handle.id()) {
             return None;
         }
 
@@ -2221,30 +2226,30 @@ impl Editor {
         }))
     }
 
-    fn refresh_code_actions(&mut self, cx: &mut ViewContext<Self>) {
-        if let Some(project) = self.project.as_ref() {
-            let new_cursor_position = self.newest_anchor_selection().head();
-            let (buffer, head) = self
-                .buffer
-                .read(cx)
-                .text_anchor_for_position(new_cursor_position, cx);
-            let actions = project.update(cx, |project, cx| project.code_actions(&buffer, head, cx));
-            self.code_actions_task = Some(cx.spawn_weak(|this, mut cx| async move {
-                let actions = actions.await;
-                if let Some(this) = this.upgrade(&cx) {
-                    this.update(&mut cx, |this, cx| {
-                        this.available_code_actions = actions.log_err().and_then(|actions| {
-                            if actions.is_empty() {
-                                None
-                            } else {
-                                Some((buffer, actions.into()))
-                            }
-                        });
-                        cx.notify();
-                    })
-                }
-            }));
-        }
+    fn refresh_code_actions(&mut self, cx: &mut ViewContext<Self>) -> Option<()> {
+        let project = self.project.as_ref()?;
+        let new_cursor_position = self.newest_anchor_selection().head();
+        let (buffer, head) = self
+            .buffer
+            .read(cx)
+            .text_anchor_for_position(new_cursor_position, cx)?;
+        let actions = project.update(cx, |project, cx| project.code_actions(&buffer, head, cx));
+        self.code_actions_task = Some(cx.spawn_weak(|this, mut cx| async move {
+            let actions = actions.await;
+            if let Some(this) = this.upgrade(&cx) {
+                this.update(&mut cx, |this, cx| {
+                    this.available_code_actions = actions.log_err().and_then(|actions| {
+                        if actions.is_empty() {
+                            None
+                        } else {
+                            Some((buffer, actions.into()))
+                        }
+                    });
+                    cx.notify();
+                })
+            }
+        }));
+        None
     }
 
     pub fn render_code_actions_indicator(&self, cx: &mut ViewContext<Self>) -> Option<ElementBox> {
@@ -4007,7 +4012,13 @@ impl Editor {
         let editor = editor_handle.read(cx);
         let buffer = editor.buffer.read(cx);
         let head = editor.newest_selection::<usize>(&buffer.read(cx)).head();
-        let (buffer, head) = editor.buffer.read(cx).text_anchor_for_position(head, cx);
+        let (buffer, head) =
+            if let Some(text_anchor) = editor.buffer.read(cx).text_anchor_for_position(head, cx) {
+                text_anchor
+            } else {
+                return;
+            };
+
         let definitions = workspace
             .project()
             .update(cx, |project, cx| project.definition(&buffer, head, cx));

crates/editor/src/multi_buffer.rs 🔗

@@ -667,12 +667,12 @@ impl MultiBuffer {
         for (excerpt_id, range_count) in excerpt_ids.into_iter().zip(range_counts.into_iter()) {
             anchor_ranges.extend(ranges.by_ref().take(range_count).map(|range| {
                 let start = Anchor {
-                    buffer_id,
+                    buffer_id: Some(buffer_id),
                     excerpt_id: excerpt_id.clone(),
                     text_anchor: buffer_snapshot.anchor_after(range.start),
                 };
                 let end = Anchor {
-                    buffer_id,
+                    buffer_id: Some(buffer_id),
                     excerpt_id: excerpt_id.clone(),
                     text_anchor: buffer_snapshot.anchor_after(range.end),
                 };
@@ -913,13 +913,13 @@ impl MultiBuffer {
         &'a self,
         position: T,
         cx: &AppContext,
-    ) -> (ModelHandle<Buffer>, language::Anchor) {
+    ) -> Option<(ModelHandle<Buffer>, language::Anchor)> {
         let snapshot = self.read(cx);
         let anchor = snapshot.anchor_before(position);
-        (
-            self.buffers.borrow()[&anchor.buffer_id].buffer.clone(),
+        Some((
+            self.buffers.borrow()[&anchor.buffer_id?].buffer.clone(),
             anchor.text_anchor,
-        )
+        ))
     }
 
     fn on_buffer_event(
@@ -988,12 +988,14 @@ impl MultiBuffer {
 
         let snapshot = self.snapshot(cx);
         let anchor = snapshot.anchor_before(position);
-        let buffer = self.buffers.borrow()[&anchor.buffer_id].buffer.clone();
-        buffer
-            .read(cx)
-            .completion_triggers()
-            .iter()
-            .any(|string| string == text)
+        anchor.buffer_id.map_or(false, |buffer_id| {
+            let buffer = self.buffers.borrow()[&buffer_id].buffer.clone();
+            buffer
+                .read(cx)
+                .completion_triggers()
+                .iter()
+                .any(|string| string == text)
+        })
     }
 
     pub fn language<'a>(&self, cx: &'a AppContext) -> Option<&'a Arc<Language>> {
@@ -1704,7 +1706,7 @@ impl MultiBufferSnapshot {
 
         let mut position = D::from_text_summary(&cursor.start().text);
         if let Some(excerpt) = cursor.item() {
-            if excerpt.id == anchor.excerpt_id && excerpt.buffer_id == anchor.buffer_id {
+            if excerpt.id == anchor.excerpt_id && Some(excerpt.buffer_id) == anchor.buffer_id {
                 let excerpt_buffer_start = excerpt.range.start.summary::<D>(&excerpt.buffer);
                 let excerpt_buffer_end = excerpt.range.end.summary::<D>(&excerpt.buffer);
                 let buffer_position = cmp::min(
@@ -1753,7 +1755,7 @@ impl MultiBufferSnapshot {
 
             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 {
+                if excerpt.id == *excerpt_id && Some(excerpt.buffer_id) == buffer_id {
                     let excerpt_buffer_start = excerpt.range.start.summary::<D>(&excerpt.buffer);
                     let excerpt_buffer_end = excerpt.range.end.summary::<D>(&excerpt.buffer);
                     summaries.extend(
@@ -1846,7 +1848,7 @@ impl MultiBufferSnapshot {
                             text_anchor = excerpt.range.end.clone();
                         }
                         Anchor {
-                            buffer_id: excerpt.buffer_id,
+                            buffer_id: Some(excerpt.buffer_id),
                             excerpt_id: excerpt.id.clone(),
                             text_anchor,
                         }
@@ -1863,7 +1865,7 @@ impl MultiBufferSnapshot {
                             text_anchor = excerpt.range.start.clone();
                         }
                         Anchor {
-                            buffer_id: excerpt.buffer_id,
+                            buffer_id: Some(excerpt.buffer_id),
                             excerpt_id: excerpt.id.clone(),
                             text_anchor,
                         }
@@ -1893,7 +1895,7 @@ impl MultiBufferSnapshot {
         let offset = position.to_offset(self);
         if let Some(excerpt) = self.as_singleton() {
             return Anchor {
-                buffer_id: excerpt.buffer_id,
+                buffer_id: Some(excerpt.buffer_id),
                 excerpt_id: excerpt.id.clone(),
                 text_anchor: excerpt.buffer.anchor_at(offset, bias),
             };
@@ -1915,7 +1917,7 @@ impl MultiBufferSnapshot {
             let text_anchor =
                 excerpt.clip_anchor(excerpt.buffer.anchor_at(buffer_start + overshoot, bias));
             Anchor {
-                buffer_id: excerpt.buffer_id,
+                buffer_id: Some(excerpt.buffer_id),
                 excerpt_id: excerpt.id.clone(),
                 text_anchor,
             }
@@ -1934,7 +1936,7 @@ impl MultiBufferSnapshot {
                 let text_anchor = excerpt.clip_anchor(text_anchor);
                 drop(cursor);
                 return Anchor {
-                    buffer_id: excerpt.buffer_id,
+                    buffer_id: Some(excerpt.buffer_id),
                     excerpt_id,
                     text_anchor,
                 };
@@ -1949,7 +1951,7 @@ impl MultiBufferSnapshot {
         } else if let Some((buffer_id, buffer_snapshot)) =
             self.buffer_snapshot_for_excerpt(&anchor.excerpt_id)
         {
-            anchor.buffer_id == buffer_id && buffer_snapshot.can_resolve(&anchor.text_anchor)
+            anchor.buffer_id == Some(buffer_id) && buffer_snapshot.can_resolve(&anchor.text_anchor)
         } else {
             false
         }
@@ -2215,12 +2217,12 @@ impl MultiBufferSnapshot {
                     .flat_map(move |(replica_id, selections)| {
                         selections.map(move |selection| {
                             let mut start = Anchor {
-                                buffer_id: excerpt.buffer_id,
+                                buffer_id: Some(excerpt.buffer_id),
                                 excerpt_id: excerpt.id.clone(),
                                 text_anchor: selection.start.clone(),
                             };
                             let mut end = Anchor {
-                                buffer_id: excerpt.buffer_id,
+                                buffer_id: Some(excerpt.buffer_id),
                                 excerpt_id: excerpt.id.clone(),
                                 text_anchor: selection.end.clone(),
                             };
@@ -2460,7 +2462,7 @@ impl Excerpt {
     }
 
     fn contains(&self, anchor: &Anchor) -> bool {
-        self.buffer_id == anchor.buffer_id
+        Some(self.buffer_id) == anchor.buffer_id
             && self
                 .range
                 .start

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

@@ -9,7 +9,7 @@ use text::{rope::TextDimension, Point};
 
 #[derive(Clone, Eq, PartialEq, Debug, Hash)]
 pub struct Anchor {
-    pub(crate) buffer_id: usize,
+    pub(crate) buffer_id: Option<usize>,
     pub(crate) excerpt_id: ExcerptId,
     pub(crate) text_anchor: text::Anchor,
 }
@@ -17,7 +17,7 @@ pub struct Anchor {
 impl Anchor {
     pub fn min() -> Self {
         Self {
-            buffer_id: 0,
+            buffer_id: None,
             excerpt_id: ExcerptId::min(),
             text_anchor: text::Anchor::min(),
         }
@@ -25,7 +25,7 @@ impl Anchor {
 
     pub fn max() -> Self {
         Self {
-            buffer_id: 0,
+            buffer_id: None,
             excerpt_id: ExcerptId::max(),
             text_anchor: text::Anchor::max(),
         }
@@ -46,11 +46,11 @@ impl Anchor {
                 // 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 {
+                if self.buffer_id == Some(buffer_id) && other.buffer_id == Some(buffer_id) {
                     self.text_anchor.cmp(&other.text_anchor, buffer_snapshot)
-                } else if self.buffer_id == buffer_id {
+                } else if self.buffer_id == Some(buffer_id) {
                     Ok(Ordering::Greater)
-                } else if other.buffer_id == buffer_id {
+                } else if other.buffer_id == Some(buffer_id) {
                     Ok(Ordering::Less)
                 } else {
                     Ok(Ordering::Equal)
@@ -68,7 +68,7 @@ impl Anchor {
             if let Some((buffer_id, buffer_snapshot)) =
                 snapshot.buffer_snapshot_for_excerpt(&self.excerpt_id)
             {
-                if self.buffer_id == buffer_id {
+                if self.buffer_id == Some(buffer_id) {
                     return Self {
                         buffer_id: self.buffer_id,
                         excerpt_id: self.excerpt_id.clone(),
@@ -85,7 +85,7 @@ impl Anchor {
             if let Some((buffer_id, buffer_snapshot)) =
                 snapshot.buffer_snapshot_for_excerpt(&self.excerpt_id)
             {
-                if self.buffer_id == buffer_id {
+                if self.buffer_id == Some(buffer_id) {
                     return Self {
                         buffer_id: self.buffer_id,
                         excerpt_id: self.excerpt_id.clone(),