search: Fix sort order not being maintained in presence of open buffers (#44135) (cherry-pick to preview) (#44138)

zed-zippy[bot] , Piotr Osiewicz , Smit Barmase , and Smit created 1 month ago

Cherry-pick of #44135 to preview

----
In project search UI code we were seeing an issue where "Go to next
match" would act up and behave weirdly. It would not wrap at times.
Stuff would be weird, yo. It turned out that match ranges reported by
core project search were sometimes out of sync with the state of the
multi-buffer. As in, the sort order of
`search::ProjectSearch::match_ranges` would not match up with
multi-buffer's sort order. This is ~because multi-buffers maintain their
own sort order.

What happened within project search is that we were skipping straight
from stage 1 (filtering paths) to stage 3 via an internal channel and in
the process we've dropped the channel used to maintain result sorting.
This made is so that, given 2 files to scan:
- project/file1.rs <- not open, has to go through stage2 (FS scan)
- project/file2.rs <- open, goes straight from stage1 (path filtering)
  to stage3 (finding all matches) We would report matches for
  project/file2.rs first, because we would notice that there's an
  existing language::Buffer for it. However, we should wait for
  project/file1.rs status to be reported first before we kick off
  project/file2.rs

The fix is to use the sorting channel instead of an internal one, as
that keeps the sorting worker "in the loop" about the state of the
world.

Closes #43672

Co-authored-by: Smit Barmase <[e-mail]>

Release Notes:

- Fixed "Select next match" in project search results misbehaving when
some of the buffers within the search results were open before search
was ran.
- Fixed project search results being scrolled to the last file active
prior to running the search.

---------

Co-authored-by: Smit Barmase <[e-mail]>
Co-authored-by: Smit <[e-mail]>

Co-authored-by: Piotr Osiewicz <[e-mail]>
Co-authored-by: Smit Barmase <[e-mail]>
Co-authored-by: Smit <[e-mail]>

Change summary

crates/project/src/project_search.rs | 29 ++++++-----------------------
1 file changed, 6 insertions(+), 23 deletions(-)

Detailed changes

crates/project/src/project_search.rs

@@ -93,9 +93,6 @@ enum FindSearchCandidates {
         /// based on disk contents of a buffer. This step is not performed for buffers we already have in memory.
         confirm_contents_will_match_tx: Sender<MatchingEntry>,
         confirm_contents_will_match_rx: Receiver<MatchingEntry>,
-        /// Of those that contain at least one match (or are already in memory), look for rest of matches (and figure out their ranges).
-        /// But wait - first, we need to go back to the main thread to open a buffer (& create an entity for it).
-        get_buffer_for_full_scan_tx: Sender<ProjectPath>,
     },
     Remote,
     OpenBuffersOnly,
@@ -226,7 +223,7 @@ impl Search {
                             .boxed_local(),
                             cx.background_spawn(Self::maintain_sorted_search_results(
                                 sorted_search_results_rx,
-                                get_buffer_for_full_scan_tx.clone(),
+                                get_buffer_for_full_scan_tx,
                                 self.limit,
                             ))
                             .boxed_local(),
@@ -234,7 +231,6 @@ impl Search {
                         (
                             FindSearchCandidates::Local {
                                 fs,
-                                get_buffer_for_full_scan_tx,
                                 confirm_contents_will_match_tx,
                                 confirm_contents_will_match_rx,
                                 input_paths_rx,
@@ -593,7 +589,6 @@ impl Worker<'_> {
             input_paths_rx,
             confirm_contents_will_match_rx,
             mut confirm_contents_will_match_tx,
-            mut get_buffer_for_full_scan_tx,
             fs,
         ) = match self.candidates {
             FindSearchCandidates::Local {
@@ -601,21 +596,15 @@ impl Worker<'_> {
                 input_paths_rx,
                 confirm_contents_will_match_rx,
                 confirm_contents_will_match_tx,
-                get_buffer_for_full_scan_tx,
             } => (
                 input_paths_rx,
                 confirm_contents_will_match_rx,
                 confirm_contents_will_match_tx,
-                get_buffer_for_full_scan_tx,
                 Some(fs),
             ),
-            FindSearchCandidates::Remote | FindSearchCandidates::OpenBuffersOnly => (
-                unbounded().1,
-                unbounded().1,
-                unbounded().0,
-                unbounded().0,
-                None,
-            ),
+            FindSearchCandidates::Remote | FindSearchCandidates::OpenBuffersOnly => {
+                (unbounded().1, unbounded().1, unbounded().0, None)
+            }
         };
         // WorkerA: grabs a request for "find all matches in file/a" <- takes 5 minutes
         // right after: WorkerB: grabs a request for "find all matches in file/b" <- takes 5 seconds
@@ -629,7 +618,6 @@ impl Worker<'_> {
                 open_entries: &self.open_buffers,
                 fs: fs.as_deref(),
                 confirm_contents_will_match_tx: &confirm_contents_will_match_tx,
-                get_buffer_for_full_scan_tx: &get_buffer_for_full_scan_tx,
             };
             // Whenever we notice that some step of a pipeline is closed, we don't want to close subsequent
             // steps straight away. Another worker might be about to produce a value that will
@@ -645,10 +633,7 @@ impl Worker<'_> {
                 find_first_match = find_first_match.next() => {
                     if let Some(buffer_with_at_least_one_match) = find_first_match {
                         handler.handle_find_first_match(buffer_with_at_least_one_match).await;
-                    } else {
-                        get_buffer_for_full_scan_tx = bounded(1).0;
                     }
-
                 },
                 scan_path = scan_path.next() => {
                     if let Some(path_to_scan) = scan_path {
@@ -673,7 +658,6 @@ struct RequestHandler<'worker> {
     fs: Option<&'worker dyn Fs>,
     open_entries: &'worker HashSet<ProjectEntryId>,
     confirm_contents_will_match_tx: &'worker Sender<MatchingEntry>,
-    get_buffer_for_full_scan_tx: &'worker Sender<ProjectPath>,
 }
 
 impl RequestHandler<'_> {
@@ -729,9 +713,8 @@ impl RequestHandler<'_> {
         _ = maybe!(async move {
             let InputPath {
                 entry,
-
                 snapshot,
-                should_scan_tx,
+                mut should_scan_tx,
             } = req;
 
             if entry.is_fifo || !entry.is_file() {
@@ -754,7 +737,7 @@ impl RequestHandler<'_> {
             if self.open_entries.contains(&entry.id) {
                 // The buffer is already in memory and that's the version we want to scan;
                 // hence skip the dilly-dally and look for all matches straight away.
-                self.get_buffer_for_full_scan_tx
+                should_scan_tx
                     .send(ProjectPath {
                         worktree_id: snapshot.id(),
                         path: entry.path.clone(),