Use new multibuffer excerpts in project search (#27893)

Kirill Bulatov and Conrad Irwin created

Follow-up of https://github.com/zed-industries/zed/pull/27876
Closes https://github.com/zed-industries/zed/issues/13513

Release Notes:

- Improved multi buffer excerpts to merge when expanded

---------

Co-authored-by: Conrad Irwin <conrad.irwin@gmail.com>

Change summary

Cargo.lock                                    |   1 
crates/multi_buffer/Cargo.toml                |   1 
crates/multi_buffer/src/multi_buffer.rs       | 228 +++++++-------------
crates/multi_buffer/src/multi_buffer_tests.rs |  52 ++-
crates/project/src/search.rs                  |   1 
crates/search/src/project_search.rs           |  78 ++++---
6 files changed, 154 insertions(+), 207 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -8665,7 +8665,6 @@ dependencies = [
  "collections",
  "ctor",
  "env_logger 0.11.8",
- "futures 0.3.31",
  "gpui",
  "indoc",
  "itertools 0.14.0",

crates/multi_buffer/Cargo.toml 🔗

@@ -28,7 +28,6 @@ collections.workspace = true
 ctor.workspace = true
 buffer_diff.workspace = true
 env_logger.workspace = true
-futures.workspace = true
 gpui.workspace = true
 itertools.workspace = true
 language.workspace = true

crates/multi_buffer/src/multi_buffer.rs 🔗

@@ -13,7 +13,6 @@ use buffer_diff::{
 };
 use clock::ReplicaId;
 use collections::{BTreeMap, Bound, HashMap, HashSet};
-use futures::{SinkExt, channel::mpsc};
 use gpui::{App, AppContext as _, Context, Entity, EntityId, EventEmitter, Task};
 use itertools::Itertools;
 use language::{
@@ -1569,32 +1568,70 @@ impl MultiBuffer {
         cx: &mut Context<Self>,
     ) -> (Vec<Range<Anchor>>, bool) {
         let buffer_snapshot = buffer.update(cx, |buffer, _| buffer.snapshot());
+        let excerpt_ranges = build_excerpt_ranges(ranges, context_line_count, &buffer_snapshot);
 
-        let excerpt_ranges = ranges.into_iter().map(|range| {
-            let start_row = range.start.row.saturating_sub(context_line_count);
-            let start = Point::new(start_row, 0);
-            let end_row = (range.end.row + context_line_count).min(buffer_snapshot.max_point().row);
-            let end = Point::new(end_row, buffer_snapshot.line_len(end_row));
-            ExcerptRange {
-                context: start..end,
-                primary: range,
-            }
-        });
+        let (new, counts) = Self::merge_excerpt_ranges(&excerpt_ranges);
+        self.set_excerpt_ranges_for_path(
+            path,
+            buffer,
+            excerpt_ranges,
+            buffer_snapshot,
+            new,
+            counts,
+            cx,
+        )
+    }
 
-        self.set_excerpt_ranges_for_path(path, buffer, excerpt_ranges.collect(), cx)
+    pub fn set_anchored_excerpts_for_path(
+        &self,
+        buffer: Entity<Buffer>,
+        ranges: Vec<Range<text::Anchor>>,
+        context_line_count: u32,
+        cx: &mut Context<Self>,
+    ) -> Task<Vec<Range<Anchor>>> {
+        let buffer_snapshot = buffer.read(cx).snapshot();
+        let path_key = PathKey::for_buffer(&buffer, cx);
+        cx.spawn(async move |multi_buffer, cx| {
+            let snapshot = buffer_snapshot.clone();
+            let (excerpt_ranges, new, counts) = cx
+                .background_spawn(async move {
+                    let ranges = ranges.into_iter().map(|range| range.to_point(&snapshot));
+                    let excerpt_ranges =
+                        build_excerpt_ranges(ranges, context_line_count, &snapshot);
+                    let (new, counts) = Self::merge_excerpt_ranges(&excerpt_ranges);
+                    (excerpt_ranges, new, counts)
+                })
+                .await;
+
+            multi_buffer
+                .update(cx, move |multi_buffer, cx| {
+                    let (ranges, _) = multi_buffer.set_excerpt_ranges_for_path(
+                        path_key,
+                        buffer,
+                        excerpt_ranges,
+                        buffer_snapshot,
+                        new,
+                        counts,
+                        cx,
+                    );
+                    ranges
+                })
+                .ok()
+                .unwrap_or_default()
+        })
     }
 
     /// Sets excerpts, returns `true` if at least one new excerpt was added.
-    pub fn set_excerpt_ranges_for_path(
+    fn set_excerpt_ranges_for_path(
         &mut self,
         path: PathKey,
         buffer: Entity<Buffer>,
         ranges: Vec<ExcerptRange<Point>>,
+        buffer_snapshot: BufferSnapshot,
+        new: Vec<ExcerptRange<Point>>,
+        counts: Vec<usize>,
         cx: &mut Context<Self>,
     ) -> (Vec<Range<Anchor>>, bool) {
-        let buffer_snapshot = buffer.update(cx, |buffer, _| buffer.snapshot());
-
-        let (new, counts) = self.merge_excerpt_ranges(&ranges);
         let (excerpt_ids, added_a_new_excerpt) =
             self.update_path_excerpts(path, buffer, &buffer_snapshot, new, cx);
 
@@ -1614,9 +1651,8 @@ impl MultiBuffer {
         (result, added_a_new_excerpt)
     }
 
-    fn merge_excerpt_ranges(
-        &self,
-        expanded_ranges: &Vec<ExcerptRange<Point>>,
+    fn merge_excerpt_ranges<'a>(
+        expanded_ranges: impl IntoIterator<Item = &'a ExcerptRange<Point>> + 'a,
     ) -> (Vec<ExcerptRange<Point>>, Vec<usize>) {
         let mut merged_ranges: Vec<ExcerptRange<Point>> = Vec::new();
         let mut counts: Vec<usize> = Vec::new();
@@ -1683,7 +1719,9 @@ impl MultiBuffer {
                 (None, Some(_)) => {
                     let existing_id = existing_iter.next().unwrap();
                     let locator = snapshot.excerpt_locator_for_id(existing_id);
-                    let existing_excerpt = excerpts_cursor.item().unwrap();
+                    let Some(existing_excerpt) = excerpts_cursor.item() else {
+                        break;
+                    };
                     excerpts_cursor.seek_forward(&Some(locator), Bias::Left, &());
                     let existing_end = existing_excerpt
                         .range
@@ -1799,92 +1837,6 @@ impl MultiBuffer {
         }
     }
 
-    pub fn push_multiple_excerpts_with_context_lines(
-        &self,
-        buffers_with_ranges: Vec<(Entity<Buffer>, Vec<Range<text::Anchor>>)>,
-        context_line_count: u32,
-        cx: &mut Context<Self>,
-    ) -> Task<Vec<Range<Anchor>>> {
-        use futures::StreamExt;
-
-        let (excerpt_ranges_tx, mut excerpt_ranges_rx) = mpsc::channel(256);
-
-        let mut buffer_ids = Vec::with_capacity(buffers_with_ranges.len());
-
-        for (buffer, ranges) in buffers_with_ranges {
-            let (buffer_id, buffer_snapshot) =
-                buffer.update(cx, |buffer, _| (buffer.remote_id(), buffer.snapshot()));
-
-            buffer_ids.push(buffer_id);
-
-            cx.background_spawn({
-                let mut excerpt_ranges_tx = excerpt_ranges_tx.clone();
-
-                async move {
-                    let (excerpt_ranges, counts) =
-                        build_excerpt_ranges(&buffer_snapshot, &ranges, context_line_count);
-                    excerpt_ranges_tx
-                        .send((buffer_id, buffer.clone(), ranges, excerpt_ranges, counts))
-                        .await
-                        .ok();
-                }
-            })
-            .detach()
-        }
-
-        cx.spawn(async move |this, cx| {
-            let mut results_by_buffer_id = HashMap::default();
-            while let Some((buffer_id, buffer, ranges, excerpt_ranges, range_counts)) =
-                excerpt_ranges_rx.next().await
-            {
-                results_by_buffer_id
-                    .insert(buffer_id, (buffer, ranges, excerpt_ranges, range_counts));
-            }
-
-            let mut multi_buffer_ranges = Vec::default();
-            'outer: for buffer_id in buffer_ids {
-                let Some((buffer, ranges, excerpt_ranges, range_counts)) =
-                    results_by_buffer_id.remove(&buffer_id)
-                else {
-                    continue;
-                };
-
-                let mut ranges = ranges.into_iter();
-                let mut range_counts = range_counts.into_iter();
-                for excerpt_ranges in excerpt_ranges.chunks(100) {
-                    let excerpt_ids = match this.update(cx, |this, cx| {
-                        this.push_excerpts(buffer.clone(), excerpt_ranges.iter().cloned(), cx)
-                    }) {
-                        Ok(excerpt_ids) => excerpt_ids,
-                        Err(_) => continue 'outer,
-                    };
-
-                    for (excerpt_id, range_count) in
-                        excerpt_ids.into_iter().zip(range_counts.by_ref())
-                    {
-                        for range in ranges.by_ref().take(range_count) {
-                            let start = Anchor {
-                                buffer_id: Some(buffer_id),
-                                excerpt_id,
-                                text_anchor: range.start,
-                                diff_base_anchor: None,
-                            };
-                            let end = Anchor {
-                                buffer_id: Some(buffer_id),
-                                excerpt_id,
-                                text_anchor: range.end,
-                                diff_base_anchor: None,
-                            };
-                            multi_buffer_ranges.push(start..end);
-                        }
-                    }
-                }
-            }
-
-            multi_buffer_ranges
-        })
-    }
-
     pub fn insert_excerpts_after<O>(
         &mut self,
         prev_excerpt_id: ExcerptId,
@@ -3452,6 +3404,26 @@ impl MultiBuffer {
     }
 }
 
+fn build_excerpt_ranges(
+    ranges: impl IntoIterator<Item = Range<Point>>,
+    context_line_count: u32,
+    buffer_snapshot: &BufferSnapshot,
+) -> Vec<ExcerptRange<Point>> {
+    ranges
+        .into_iter()
+        .map(|range| {
+            let start_row = range.start.row.saturating_sub(context_line_count);
+            let start = Point::new(start_row, 0);
+            let end_row = (range.end.row + context_line_count).min(buffer_snapshot.max_point().row);
+            let end = Point::new(end_row, buffer_snapshot.line_len(end_row));
+            ExcerptRange {
+                context: start..end,
+                primary: range,
+            }
+        })
+        .collect()
+}
+
 #[cfg(any(test, feature = "test-support"))]
 impl MultiBuffer {
     pub fn build_simple(text: &str, cx: &mut gpui::App) -> Entity<Self> {
@@ -7772,47 +7744,3 @@ impl From<ExcerptId> for EntityId {
         EntityId::from(id.0 as u64)
     }
 }
-
-pub fn build_excerpt_ranges<T>(
-    buffer: &BufferSnapshot,
-    ranges: &[Range<T>],
-    context_line_count: u32,
-) -> (Vec<ExcerptRange<Point>>, Vec<usize>)
-where
-    T: text::ToPoint,
-{
-    let max_point = buffer.max_point();
-    let mut range_counts = Vec::new();
-    let mut excerpt_ranges = Vec::new();
-    let mut range_iter = ranges
-        .iter()
-        .map(|range| range.start.to_point(buffer)..range.end.to_point(buffer))
-        .peekable();
-    while let Some(range) = range_iter.next() {
-        let excerpt_start = Point::new(range.start.row.saturating_sub(context_line_count), 0);
-        let row = (range.end.row + context_line_count).min(max_point.row);
-        let mut excerpt_end = Point::new(row, buffer.line_len(row));
-
-        let mut ranges_in_excerpt = 1;
-
-        while let Some(next_range) = range_iter.peek() {
-            if next_range.start.row <= excerpt_end.row + context_line_count {
-                let row = (next_range.end.row + context_line_count).min(max_point.row);
-                excerpt_end = Point::new(row, buffer.line_len(row));
-
-                ranges_in_excerpt += 1;
-                range_iter.next();
-            } else {
-                break;
-            }
-        }
-
-        excerpt_ranges.push(ExcerptRange {
-            context: excerpt_start..excerpt_end,
-            primary: range,
-        });
-        range_counts.push(ranges_in_excerpt);
-    }
-
-    (excerpt_ranges, range_counts)
-}

crates/multi_buffer/src/multi_buffer_tests.rs 🔗

@@ -781,7 +781,7 @@ fn test_expand_excerpts(cx: &mut App) {
 }
 
 #[gpui::test(iterations = 100)]
-async fn test_push_multiple_excerpts_with_context_lines(cx: &mut TestAppContext) {
+async fn test_set_anchored_excerpts_for_path(cx: &mut TestAppContext) {
     let buffer_1 = cx.new(|cx| Buffer::local(sample_text(20, 3, 'a'), cx));
     let buffer_2 = cx.new(|cx| Buffer::local(sample_text(15, 4, 'a'), cx));
     let snapshot_1 = buffer_1.update(cx, |buffer, _| buffer.snapshot());
@@ -797,15 +797,39 @@ async fn test_push_multiple_excerpts_with_context_lines(cx: &mut TestAppContext)
     ];
 
     let multibuffer = cx.new(|_| MultiBuffer::new(Capability::ReadWrite));
-    let anchor_ranges = multibuffer
+    let anchor_ranges_1 = multibuffer
         .update(cx, |multibuffer, cx| {
-            multibuffer.push_multiple_excerpts_with_context_lines(
-                vec![(buffer_1.clone(), ranges_1), (buffer_2.clone(), ranges_2)],
-                2,
-                cx,
-            )
+            multibuffer.set_anchored_excerpts_for_path(buffer_1.clone(), ranges_1, 2, cx)
         })
         .await;
+    let snapshot_1 = multibuffer.update(cx, |multibuffer, cx| multibuffer.snapshot(cx));
+    assert_eq!(
+        anchor_ranges_1
+            .iter()
+            .map(|range| range.to_point(&snapshot_1))
+            .collect::<Vec<_>>(),
+        vec![
+            Point::new(2, 2)..Point::new(3, 2),
+            Point::new(6, 1)..Point::new(6, 3),
+            Point::new(11, 0)..Point::new(11, 0),
+        ]
+    );
+    let anchor_ranges_2 = multibuffer
+        .update(cx, |multibuffer, cx| {
+            multibuffer.set_anchored_excerpts_for_path(buffer_2.clone(), ranges_2, 2, cx)
+        })
+        .await;
+    let snapshot_2 = multibuffer.update(cx, |multibuffer, cx| multibuffer.snapshot(cx));
+    assert_eq!(
+        anchor_ranges_2
+            .iter()
+            .map(|range| range.to_point(&snapshot_2))
+            .collect::<Vec<_>>(),
+        vec![
+            Point::new(16, 1)..Point::new(17, 1),
+            Point::new(22, 0)..Point::new(22, 2)
+        ]
+    );
 
     let snapshot = multibuffer.update(cx, |multibuffer, cx| multibuffer.snapshot(cx));
     assert_eq!(
@@ -841,20 +865,6 @@ async fn test_push_multiple_excerpts_with_context_lines(cx: &mut TestAppContext)
             "mmmm",   //
         )
     );
-
-    assert_eq!(
-        anchor_ranges
-            .iter()
-            .map(|range| range.to_point(&snapshot))
-            .collect::<Vec<_>>(),
-        vec![
-            Point::new(2, 2)..Point::new(3, 2),
-            Point::new(6, 1)..Point::new(6, 3),
-            Point::new(11, 0)..Point::new(11, 0),
-            Point::new(16, 1)..Point::new(17, 1),
-            Point::new(22, 0)..Point::new(22, 2)
-        ]
-    );
 }
 
 #[gpui::test]

crates/project/src/search.rs 🔗

@@ -15,6 +15,7 @@ use std::{
 use text::Anchor;
 use util::paths::PathMatcher;
 
+#[derive(Debug)]
 pub enum SearchResult {
     Buffer {
         buffer: Entity<Buffer>,

crates/search/src/project_search.rs 🔗

@@ -9,7 +9,7 @@ use editor::{
     Anchor, Editor, EditorElement, EditorEvent, EditorSettings, EditorStyle, MAX_TAB_TITLE_LEN,
     MultiBuffer, actions::SelectAll, items::active_match_index, scroll::Autoscroll,
 };
-use futures::StreamExt;
+use futures::{StreamExt, stream::FuturesOrdered};
 use gpui::{
     Action, AnyElement, AnyView, App, Axis, Context, Entity, EntityId, EventEmitter, FocusHandle,
     Focusable, Global, Hsla, InteractiveElement, IntoElement, KeyContext, ParentElement, Point,
@@ -260,16 +260,18 @@ impl ProjectSearch {
         self.search_id += 1;
         self.active_query = Some(query);
         self.match_ranges.clear();
-        self.pending_search = Some(cx.spawn(async move |this, cx| {
+        self.pending_search = Some(cx.spawn(async move |project_search, cx| {
             let mut matches = pin!(search.ready_chunks(1024));
-            let this = this.upgrade()?;
-            this.update(cx, |this, cx| {
-                this.match_ranges.clear();
-                this.excerpts.update(cx, |this, cx| this.clear(cx));
-                this.no_results = Some(true);
-                this.limit_reached = false;
-            })
-            .ok()?;
+            project_search
+                .update(cx, |project_search, cx| {
+                    project_search.match_ranges.clear();
+                    project_search
+                        .excerpts
+                        .update(cx, |excerpts, cx| excerpts.clear(cx));
+                    project_search.no_results = Some(true);
+                    project_search.limit_reached = false;
+                })
+                .ok()?;
 
             let mut limit_reached = false;
             while let Some(results) = matches.next().await {
@@ -285,35 +287,43 @@ impl ProjectSearch {
                     }
                 }
 
-                let match_ranges = this
-                    .update(cx, |this, cx| {
-                        this.excerpts.update(cx, |excerpts, cx| {
-                            excerpts.push_multiple_excerpts_with_context_lines(
-                                buffers_with_ranges,
-                                editor::DEFAULT_MULTIBUFFER_CONTEXT,
-                                cx,
-                            )
-                        })
+                let excerpts = project_search
+                    .update(cx, |project_search, _| project_search.excerpts.clone())
+                    .ok()?;
+                let mut new_ranges = excerpts
+                    .update(cx, |excerpts, cx| {
+                        buffers_with_ranges
+                            .into_iter()
+                            .map(|(buffer, ranges)| {
+                                excerpts.set_anchored_excerpts_for_path(
+                                    buffer,
+                                    ranges,
+                                    editor::DEFAULT_MULTIBUFFER_CONTEXT,
+                                    cx,
+                                )
+                            })
+                            .collect::<FuturesOrdered<_>>()
                     })
-                    .ok()?
-                    .await;
+                    .ok()?;
+                while let Some(new_ranges) = new_ranges.next().await {
+                    project_search
+                        .update(cx, |project_search, _| {
+                            project_search.match_ranges.extend(new_ranges);
+                        })
+                        .ok()?;
+                }
+            }
 
-                this.update(cx, |this, cx| {
-                    this.match_ranges.extend(match_ranges);
+            project_search
+                .update(cx, |project_search, cx| {
+                    if !project_search.match_ranges.is_empty() {
+                        project_search.no_results = Some(false);
+                    }
+                    project_search.limit_reached = limit_reached;
+                    project_search.pending_search.take();
                     cx.notify();
                 })
                 .ok()?;
-            }
-
-            this.update(cx, |this, cx| {
-                if !this.match_ranges.is_empty() {
-                    this.no_results = Some(false);
-                }
-                this.limit_reached = limit_reached;
-                this.pending_search.take();
-                cx.notify();
-            })
-            .ok()?;
 
             None
         }));