multi_buffer: Remove redundant buffer id field (#43459)

It is easy for us to get the two fields out of sync causing weird problems, there is no reason to have both here so. Release Notes: - N/A *or* Added/Fixed/Improved ... Co-authored by: Antonio Scandurra <antonio@zed.dev>
2025-11-25 17:13:16 +01:00
parent ab80ef1845
commit fafe1afa61
39 changed files with 378 additions and 306 deletions
@@ -7,12 +7,9 @@ use std::{
    ops::{AddAssign, Range, Sub},
 };
 use sum_tree::Bias;
-use text::BufferId;

 #[derive(Clone, Copy, Eq, PartialEq, Hash)]
 pub struct Anchor {
-    /// Invariant: If buffer id is `None`, excerpt id must be `ExcerptId::min()` or `ExcerptId::max()`.
-    pub buffer_id: Option<BufferId>,
    pub excerpt_id: ExcerptId,
    pub text_anchor: text::Anchor,
    pub diff_base_anchor: Option<text::Anchor>,
@@ -20,15 +17,14 @@ pub struct Anchor {

 impl std::fmt::Debug for Anchor {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        if *self == Self::min() {
-            return f.write_str("Anchor::MIN");
+        if self.is_min() {
+            return write!(f, "Anchor::min({:?})", self.text_anchor.buffer_id);
        }
-        if *self == Self::max() {
-            return f.write_str("Anchor::MAX");
+        if self.is_max() {
+            return write!(f, "Anchor::max({:?})", self.text_anchor.buffer_id);
        }

        f.debug_struct("Anchor")
-            .field("buffer_id", &self.buffer_id)
            .field("excerpt_id", &self.excerpt_id)
            .field("text_anchor", &self.text_anchor)
            .field("diff_base_anchor", &self.diff_base_anchor)
@@ -44,35 +40,20 @@ impl Anchor {
        }
    }

-    pub fn in_buffer(
-        excerpt_id: ExcerptId,
-        buffer_id: BufferId,
-        text_anchor: text::Anchor,
-    ) -> Self {
-        debug_assert!(
-            text_anchor.buffer_id.is_none_or(|id| id == buffer_id),
-            "buffer id does not match the one in the text anchor: {buffer_id:?} {text_anchor:?}",
-        );
+    pub fn in_buffer(excerpt_id: ExcerptId, text_anchor: text::Anchor) -> Self {
        Self {
-            buffer_id: Some(buffer_id),
            excerpt_id,
            text_anchor,
            diff_base_anchor: None,
        }
    }

-    pub fn range_in_buffer(
-        excerpt_id: ExcerptId,
-        buffer_id: BufferId,
-        range: Range<text::Anchor>,
-    ) -> Range<Self> {
-        Self::in_buffer(excerpt_id, buffer_id, range.start)
-            ..Self::in_buffer(excerpt_id, buffer_id, range.end)
+    pub fn range_in_buffer(excerpt_id: ExcerptId, range: Range<text::Anchor>) -> Range<Self> {
+        Self::in_buffer(excerpt_id, range.start)..Self::in_buffer(excerpt_id, range.end)
    }

    pub fn min() -> Self {
        Self {
-            buffer_id: None,
            excerpt_id: ExcerptId::min(),
            text_anchor: text::Anchor::MIN,
            diff_base_anchor: None,
@@ -81,13 +62,24 @@ impl Anchor {

    pub fn max() -> Self {
        Self {
-            buffer_id: None,
            excerpt_id: ExcerptId::max(),
            text_anchor: text::Anchor::MAX,
            diff_base_anchor: None,
        }
    }

+    pub fn is_min(&self) -> bool {
+        self.excerpt_id == ExcerptId::min()
+            && self.text_anchor.is_min()
+            && self.diff_base_anchor.is_none()
+    }
+
+    pub fn is_max(&self) -> bool {
+        self.excerpt_id == ExcerptId::max()
+            && self.text_anchor.is_max()
+            && self.diff_base_anchor.is_none()
+    }
+
    pub fn cmp(&self, other: &Anchor, snapshot: &MultiBufferSnapshot) -> Ordering {
        if self == other {
            return Ordering::Equal;
@@ -101,8 +93,8 @@ impl Anchor {
            return excerpt_id_cmp;
        }
        if self_excerpt_id == ExcerptId::max()
-            && self.text_anchor == text::Anchor::MAX
-            && self.text_anchor == text::Anchor::MAX
+            && self.text_anchor.is_max()
+            && self.text_anchor.is_max()
            && self.diff_base_anchor.is_none()
            && other.diff_base_anchor.is_none()
        {
@@ -147,7 +139,6 @@ impl Anchor {
            && let Some(excerpt) = snapshot.excerpt(self.excerpt_id)
        {
            return Self {
-                buffer_id: Some(excerpt.buffer_id),
                excerpt_id: excerpt.id,
                text_anchor: self.text_anchor.bias_left(&excerpt.buffer),
                diff_base_anchor: self.diff_base_anchor.map(|a| {
@@ -171,7 +162,6 @@ impl Anchor {
            && let Some(excerpt) = snapshot.excerpt(self.excerpt_id)
        {
            return Self {
-                buffer_id: Some(excerpt.buffer_id),
                excerpt_id: excerpt.id,
                text_anchor: self.text_anchor.bias_right(&excerpt.buffer),
                diff_base_anchor: self.diff_base_anchor.map(|a| {
@@ -202,8 +192,8 @@ impl Anchor {
    }

    pub fn is_valid(&self, snapshot: &MultiBufferSnapshot) -> bool {
-        if *self == Anchor::min() || self.excerpt_id == ExcerptId::max() {
-            !snapshot.is_empty()
+        if self.is_min() || self.is_max() {
+            true
        } else if let Some(excerpt) = snapshot.excerpt(self.excerpt_id) {
            (self.text_anchor == excerpt.range.context.start
                || self.text_anchor == excerpt.range.context.end
@@ -158,12 +158,13 @@ impl MultiBufferDiffHunk {

    pub fn is_created_file(&self) -> bool {
        self.diff_base_byte_range == (BufferOffset(0)..BufferOffset(0))
-            && self.buffer_range == (text::Anchor::MIN..text::Anchor::MAX)
+            && self.buffer_range.start.is_min()
+            && self.buffer_range.end.is_max()
    }

    pub fn multi_buffer_range(&self) -> Range<Anchor> {
-        let start = Anchor::in_buffer(self.excerpt_id, self.buffer_id, self.buffer_range.start);
-        let end = Anchor::in_buffer(self.excerpt_id, self.buffer_id, self.buffer_range.end);
+        let start = Anchor::in_buffer(self.excerpt_id, self.buffer_range.start);
+        let end = Anchor::in_buffer(self.excerpt_id, self.buffer_range.end);
        start..end
    }
 }
@@ -1028,9 +1029,12 @@ impl MultiBuffer {
            },
        );
        this.singleton = true;
+        let buffer_id = buffer.read(cx).remote_id();
        this.push_excerpts(
            buffer,
-            [ExcerptRange::new(text::Anchor::MIN..text::Anchor::MAX)],
+            [ExcerptRange::new(text::Anchor::min_max_range_for_buffer(
+                buffer_id,
+            ))],
            cx,
        );
        this
@@ -1912,7 +1916,7 @@ impl MultiBuffer {
    }

    pub fn buffer_for_anchor(&self, anchor: Anchor, cx: &App) -> Option<Entity<Buffer>> {
-        if let Some(buffer_id) = anchor.buffer_id {
+        if let Some(buffer_id) = anchor.text_anchor.buffer_id {
            self.buffer(buffer_id)
        } else {
            let (_, buffer, _) = self.excerpt_containing(anchor, cx)?;
@@ -1975,7 +1979,7 @@ impl MultiBuffer {

        found.map(|(point, excerpt_id)| {
            let text_anchor = snapshot.anchor_after(point);
-            Anchor::in_buffer(excerpt_id, snapshot.remote_id(), text_anchor)
+            Anchor::in_buffer(excerpt_id, text_anchor)
        })
    }

@@ -1990,7 +1994,7 @@ impl MultiBuffer {
            if range.context.start.cmp(&anchor, &snapshot).is_le()
                && range.context.end.cmp(&anchor, &snapshot).is_ge()
            {
-                return Some(Anchor::in_buffer(excerpt_id, snapshot.remote_id(), anchor));
+                return Some(Anchor::in_buffer(excerpt_id, anchor));
            }
        }

@@ -2112,7 +2116,7 @@ impl MultiBuffer {
        let mut error = None;
        let mut futures = Vec::new();
        for anchor in anchors {
-            if let Some(buffer_id) = anchor.buffer_id {
+            if let Some(buffer_id) = anchor.text_anchor.buffer_id {
                if let Some(buffer) = self.buffers.get(&buffer_id) {
                    buffer.buffer.update(cx, |buffer, _| {
                        futures.push(buffer.wait_for_anchors([anchor.text_anchor]))
@@ -2143,7 +2147,11 @@ impl MultiBuffer {
    ) -> Option<(Entity<Buffer>, language::Anchor)> {
        let snapshot = self.read(cx);
        let anchor = snapshot.anchor_before(position);
-        let buffer = self.buffers.get(&anchor.buffer_id?)?.buffer.clone();
+        let buffer = self
+            .buffers
+            .get(&anchor.text_anchor.buffer_id?)?
+            .buffer
+            .clone();
        Some((buffer, anchor.text_anchor))
    }

@@ -2205,7 +2213,7 @@ impl MultiBuffer {
            .get(&buffer_id)
            .is_none_or(|old_diff| !new_diff.base_texts_eq(old_diff));

-        snapshot.diffs.insert(buffer_id, new_diff);
+        snapshot.diffs.insert_or_replace(buffer_id, new_diff);

        let mut excerpt_edits = Vec::new();
        for locator in &buffer_state.excerpts {
@@ -2402,7 +2410,11 @@ impl MultiBuffer {

    pub fn add_diff(&mut self, diff: Entity<BufferDiff>, cx: &mut Context<Self>) {
        let buffer_id = diff.read(cx).buffer_id;
-        self.buffer_diff_changed(diff.clone(), text::Anchor::MIN..text::Anchor::MAX, cx);
+        self.buffer_diff_changed(
+            diff.clone(),
+            text::Anchor::min_max_range_for_buffer(buffer_id),
+            cx,
+        );
        self.diffs.insert(buffer_id, DiffState::new(diff, cx));
    }

@@ -2500,16 +2512,8 @@ impl MultiBuffer {
                if last_hunk_row.is_some_and(|row| row >= diff_hunk.row_range.start) {
                    continue;
                }
-                let start = Anchor::in_buffer(
-                    diff_hunk.excerpt_id,
-                    diff_hunk.buffer_id,
-                    diff_hunk.buffer_range.start,
-                );
-                let end = Anchor::in_buffer(
-                    diff_hunk.excerpt_id,
-                    diff_hunk.buffer_id,
-                    diff_hunk.buffer_range.end,
-                );
+                let start = Anchor::in_buffer(diff_hunk.excerpt_id, diff_hunk.buffer_range.start);
+                let end = Anchor::in_buffer(diff_hunk.excerpt_id, diff_hunk.buffer_range.end);
                let start = snapshot.excerpt_offset_for_anchor(&start);
                let end = snapshot.excerpt_offset_for_anchor(&end);
                last_hunk_row = Some(diff_hunk.row_range.start);
@@ -3945,9 +3949,7 @@ impl MultiBufferSnapshot {
                if hunk_end >= current_position {
                    continue;
                }
-                let start =
-                    Anchor::in_buffer(excerpt.id, excerpt.buffer_id, hunk.buffer_range.start)
-                        .to_point(self);
+                let start = Anchor::in_buffer(excerpt.id, hunk.buffer_range.start).to_point(self);
                return Some(MultiBufferRow(start.row));
            }
        }
@@ -3964,8 +3966,7 @@ impl MultiBufferSnapshot {
            let Some(hunk) = hunks.next() else {
                continue;
            };
-            let start = Anchor::in_buffer(excerpt.id, excerpt.buffer_id, hunk.buffer_range.start)
-                .to_point(self);
+            let start = Anchor::in_buffer(excerpt.id, hunk.buffer_range.start).to_point(self);
            return Some(MultiBufferRow(start.row));
        }
    }
@@ -4955,7 +4956,7 @@ impl MultiBufferSnapshot {
                        {
                            text_anchor = excerpt.range.context.end;
                        }
-                        Anchor::in_buffer(excerpt.id, excerpt.buffer_id, text_anchor)
+                        Anchor::in_buffer(excerpt.id, text_anchor)
                    } else if let Some(excerpt) = prev_excerpt {
                        let mut text_anchor = excerpt
                            .range
@@ -4968,7 +4969,7 @@ impl MultiBufferSnapshot {
                        {
                            text_anchor = excerpt.range.context.start;
                        }
-                        Anchor::in_buffer(excerpt.id, excerpt.buffer_id, text_anchor)
+                        Anchor::in_buffer(excerpt.id, text_anchor)
                    } else if anchor.text_anchor.bias == Bias::Left {
                        Anchor::min()
                    } else {
@@ -5050,7 +5051,7 @@ impl MultiBufferSnapshot {
            let buffer_start = excerpt.range.context.start.to_offset(&excerpt.buffer);
            let text_anchor =
                excerpt.clip_anchor(excerpt.buffer.anchor_at(buffer_start + overshoot, bias));
-            let anchor = Anchor::in_buffer(excerpt.id, excerpt.buffer_id, text_anchor);
+            let anchor = Anchor::in_buffer(excerpt.id, text_anchor);
            match diff_base_anchor {
                Some(diff_base_anchor) => anchor.with_diff_base_anchor(diff_base_anchor),
                None => anchor,
@@ -5066,7 +5067,11 @@ impl MultiBufferSnapshot {
    /// Wraps the [`text::Anchor`] in a [`multi_buffer::Anchor`] if this multi-buffer is a singleton.
    pub fn as_singleton_anchor(&self, text_anchor: text::Anchor) -> Option<Anchor> {
        let (excerpt, buffer, _) = self.as_singleton()?;
-        Some(Anchor::in_buffer(*excerpt, buffer, text_anchor))
+        if text_anchor.buffer_id.is_none_or(|id| id == buffer) {
+            Some(Anchor::in_buffer(*excerpt, text_anchor))
+        } else {
+            None
+        }
    }

    /// Returns an anchor for the given excerpt and text anchor,
@@ -5099,12 +5104,8 @@ impl MultiBufferSnapshot {
        match text_anchor.buffer_id {
            Some(buffer_id) if buffer_id == excerpt.buffer_id => (),
            Some(_) => return None,
-            None if text_anchor == text::Anchor::MAX || text_anchor == text::Anchor::MIN => {
-                return Some(Anchor::in_buffer(
-                    excerpt.id,
-                    excerpt.buffer_id,
-                    text_anchor,
-                ));
+            None if text_anchor.is_max() || text_anchor.is_min() => {
+                return Some(Anchor::in_buffer(excerpt.id, text_anchor));
            }
            None => return None,
        }
@@ -5116,11 +5117,7 @@ impl MultiBufferSnapshot {
            return None;
        }

-        Some(Anchor::in_buffer(
-            excerpt.id,
-            excerpt.buffer_id,
-            text_anchor,
-        ))
+        Some(Anchor::in_buffer(excerpt.id, text_anchor))
    }

    pub fn context_range_for_excerpt(&self, excerpt_id: ExcerptId) -> Option<Range<text::Anchor>> {
@@ -5128,7 +5125,7 @@ impl MultiBufferSnapshot {
    }

    pub fn can_resolve(&self, anchor: &Anchor) -> bool {
-        if *anchor == Anchor::min() || anchor.excerpt_id == ExcerptId::max() {
+        if anchor.is_min() || anchor.is_max() {
            // todo(lw): should be `!self.is_empty()`
            true
        } else if let Some(excerpt) = self.excerpt(anchor.excerpt_id) {
@@ -5998,7 +5995,7 @@ impl MultiBufferSnapshot {
            ..
        } = self.excerpt(anchor.excerpt_id)?;
        if cfg!(debug_assertions) {
-            match anchor.buffer_id {
+            match anchor.text_anchor.buffer_id {
                // we clearly are hitting this according to sentry, but in what situations can this occur?
                Some(anchor_buffer_id) => {
                    assert_eq!(
@@ -6006,7 +6003,7 @@ impl MultiBufferSnapshot {
                        "anchor {anchor:?} does not match with resolved excerpt {excerpt:?}"
                    )
                }
-                None => assert_eq!(anchor, Anchor::max()),
+                None => assert!(anchor.is_max()),
            }
        };
        Some((
@@ -6019,19 +6016,18 @@ impl MultiBufferSnapshot {
                        depth: item.depth,
                        source_range_for_text: Anchor::range_in_buffer(
                            excerpt_id,
-                            buffer_id,
                            item.source_range_for_text,
                        ),
-                        range: Anchor::range_in_buffer(excerpt_id, buffer_id, item.range),
+                        range: Anchor::range_in_buffer(excerpt_id, item.range),
                        text: item.text,
                        highlight_ranges: item.highlight_ranges,
                        name_ranges: item.name_ranges,
-                        body_range: item.body_range.map(|body_range| {
-                            Anchor::range_in_buffer(excerpt_id, buffer_id, body_range)
-                        }),
-                        annotation_range: item.annotation_range.map(|body_range| {
-                            Anchor::range_in_buffer(excerpt_id, buffer_id, body_range)
-                        }),
+                        body_range: item
+                            .body_range
+                            .map(|body_range| Anchor::range_in_buffer(excerpt_id, body_range)),
+                        annotation_range: item
+                            .annotation_range
+                            .map(|body_range| Anchor::range_in_buffer(excerpt_id, body_range)),
                    })
                })
                .collect(),
@@ -6180,7 +6176,7 @@ impl MultiBufferSnapshot {
    }

    pub fn buffer_id_for_anchor(&self, anchor: Anchor) -> Option<BufferId> {
-        if let Some(id) = anchor.buffer_id {
+        if let Some(id) = anchor.text_anchor.buffer_id {
            return Some(id);
        }
        let excerpt = self.excerpt_containing(anchor..anchor)?;
@@ -6212,10 +6208,8 @@ impl MultiBufferSnapshot {
                    .selections_in_range(query_range, include_local)
                    .flat_map(move |(replica_id, line_mode, cursor_shape, selections)| {
                        selections.map(move |selection| {
-                            let mut start =
-                                Anchor::in_buffer(excerpt.id, excerpt.buffer_id, selection.start);
-                            let mut end =
-                                Anchor::in_buffer(excerpt.id, excerpt.buffer_id, selection.end);
+                            let mut start = Anchor::in_buffer(excerpt.id, selection.start);
+                            let mut end = Anchor::in_buffer(excerpt.id, selection.end);
                            if range.start.cmp(&start, self).is_gt() {
                                start = range.start;
                            }
@@ -6687,7 +6681,8 @@ impl Excerpt {
    }

    fn contains(&self, anchor: &Anchor) -> bool {
-        (anchor.buffer_id == None || anchor.buffer_id == Some(self.buffer_id))
+        (anchor.text_anchor.buffer_id == None
+            || anchor.text_anchor.buffer_id == Some(self.buffer_id))
            && self
                .range
                .context
@@ -6723,19 +6718,11 @@ impl<'a> MultiBufferExcerpt<'a> {
    }

    pub fn start_anchor(&self) -> Anchor {
-        Anchor::in_buffer(
-            self.excerpt.id,
-            self.excerpt.buffer_id,
-            self.excerpt.range.context.start,
-        )
+        Anchor::in_buffer(self.excerpt.id, self.excerpt.range.context.start)
    }

    pub fn end_anchor(&self) -> Anchor {
-        Anchor::in_buffer(
-            self.excerpt.id,
-            self.excerpt.buffer_id,
-            self.excerpt.range.context.end,
-        )
+        Anchor::in_buffer(self.excerpt.id, self.excerpt.range.context.end)
    }

    pub fn buffer(&self) -> &'a BufferSnapshot {
@@ -3401,14 +3401,11 @@ fn test_summaries_for_anchors(cx: &mut TestAppContext) {
        ),
    );

-    let id_1 = buffer_1.read_with(cx, |buffer, _| buffer.remote_id());
-    let id_2 = buffer_2.read_with(cx, |buffer, _| buffer.remote_id());
-
-    let anchor_1 = Anchor::in_buffer(ids[0], id_1, text::Anchor::MIN);
+    let anchor_1 = Anchor::in_buffer(ids[0], text::Anchor::MIN);
    let point_1 = snapshot.summaries_for_anchors::<Point, _>([&anchor_1])[0];
    assert_eq!(point_1, Point::new(0, 0));

-    let anchor_2 = Anchor::in_buffer(ids[1], id_2, text::Anchor::MIN);
+    let anchor_2 = Anchor::in_buffer(ids[1], text::Anchor::MIN);
    let point_2 = snapshot.summaries_for_anchors::<Point, _>([&anchor_2])[0];
    assert_eq!(point_2, Point::new(3, 0));
 }
@@ -56,11 +56,7 @@ impl MultiBuffer {
        let excerpt_id = self.excerpts_by_path.get(path)?.first()?;
        let snapshot = self.read(cx);
        let excerpt = snapshot.excerpt(*excerpt_id)?;
-        Some(Anchor::in_buffer(
-            excerpt.id,
-            excerpt.buffer_id,
-            excerpt.range.context.start,
-        ))
+        Some(Anchor::in_buffer(excerpt.id, excerpt.range.context.start))
    }

    pub fn excerpt_paths(&self) -> impl Iterator<Item = &PathKey> {
@@ -263,7 +259,6 @@ impl MultiBuffer {
            for range in ranges.by_ref().take(range_count) {
                let range = Anchor::range_in_buffer(
                    excerpt_id,
-                    buffer_snapshot.remote_id(),
                    buffer_snapshot.anchor_before(&range.primary.start)
                        ..buffer_snapshot.anchor_after(&range.primary.end),
                );