Merge pull request #26 from zed-industries/stable-file-finder-results

Max Brunsfeld created

Improve file finder behavior when opening large directories like `~` or `/`

Change summary

zed/src/file_finder.rs    | 85 +++++++++++++++++++++++++++++++++++++++-
zed/src/util.rs           | 37 +++++++++++++++++
zed/src/worktree/fuzzy.rs | 56 ++++++++++++++++----------
3 files changed, 153 insertions(+), 25 deletions(-)

Detailed changes

zed/src/file_finder.rs 🔗

@@ -30,6 +30,8 @@ pub struct FileFinder {
     query_buffer: ViewHandle<BufferView>,
     search_count: usize,
     latest_search_id: usize,
+    latest_search_did_cancel: bool,
+    latest_search_query: String,
     matches: Vec<PathMatch>,
     include_root_name: bool,
     selected: Option<Arc<Path>>,
@@ -293,6 +295,8 @@ impl FileFinder {
             query_buffer,
             search_count: 0,
             latest_search_id: 0,
+            latest_search_did_cancel: false,
+            latest_search_query: String::new(),
             matches: Vec::new(),
             include_root_name: false,
             selected: None,
@@ -395,10 +399,11 @@ impl FileFinder {
                 false,
                 false,
                 100,
-                cancel_flag,
+                cancel_flag.clone(),
                 pool,
             );
-            (search_id, include_root_name, matches)
+            let did_cancel = cancel_flag.load(atomic::Ordering::Relaxed);
+            (search_id, include_root_name, did_cancel, query, matches)
         });
 
         ctx.spawn(task, Self::update_matches).detach();
@@ -406,12 +411,24 @@ impl FileFinder {
 
     fn update_matches(
         &mut self,
-        (search_id, include_root_name, matches): (usize, bool, Vec<PathMatch>),
+        (search_id, include_root_name, did_cancel, query, matches): (
+            usize,
+            bool,
+            bool,
+            String,
+            Vec<PathMatch>,
+        ),
         ctx: &mut ViewContext<Self>,
     ) {
         if search_id >= self.latest_search_id {
             self.latest_search_id = search_id;
-            self.matches = matches;
+            if self.latest_search_did_cancel && query == self.latest_search_query {
+                util::extend_sorted(&mut self.matches, matches.into_iter(), 100, |a, b| b.cmp(a));
+            } else {
+                self.matches = matches;
+            }
+            self.latest_search_query = query;
+            self.latest_search_did_cancel = did_cancel;
             self.include_root_name = include_root_name;
             self.list_state.scroll_to(self.selected_index());
             ctx.notify();
@@ -432,9 +449,11 @@ mod tests {
     use super::*;
     use crate::{
         editor, settings,
+        test::temp_tree,
         workspace::{Workspace, WorkspaceView},
     };
     use gpui::App;
+    use serde_json::json;
     use smol::fs;
     use tempdir::TempDir;
 
@@ -508,4 +527,62 @@ mod tests {
             });
         });
     }
+
+    #[test]
+    fn test_matching_cancellation() {
+        App::test_async((), |mut app| async move {
+            let tmp_dir = temp_tree(json!({
+                "hello": "",
+                "goodbye": "",
+                "halogen-light": "",
+                "happiness": "",
+                "height": "",
+                "hi": "",
+                "hiccup": "",
+            }));
+            let settings = settings::channel(&app.font_cache()).unwrap().1;
+            let workspace = app.add_model(|ctx| Workspace::new(vec![tmp_dir.path().into()], ctx));
+            app.read(|ctx| workspace.read(ctx).worktree_scans_complete(ctx))
+                .await;
+            let (_, finder) =
+                app.add_window(|ctx| FileFinder::new(settings, workspace.clone(), ctx));
+
+            let query = "hi".to_string();
+            finder.update(&mut app, |f, ctx| f.spawn_search(query.clone(), ctx));
+            finder.condition(&app, |f, _| f.matches.len() == 5).await;
+
+            finder.update(&mut app, |finder, ctx| {
+                let matches = finder.matches.clone();
+
+                // Simulate a search being cancelled after the time limit,
+                // returning only a subset of the matches that would have been found.
+                finder.spawn_search(query.clone(), ctx);
+                finder.update_matches(
+                    (
+                        finder.latest_search_id,
+                        true,
+                        true, // did-cancel
+                        query.clone(),
+                        vec![matches[1].clone(), matches[3].clone()],
+                    ),
+                    ctx,
+                );
+
+                // Simulate another cancellation.
+                finder.spawn_search(query.clone(), ctx);
+                finder.update_matches(
+                    (
+                        finder.latest_search_id,
+                        true,
+                        true, // did-cancel
+                        query.clone(),
+                        vec![matches[0].clone(), matches[2].clone(), matches[3].clone()],
+                    ),
+                    ctx,
+                );
+
+                assert_eq!(finder.matches, matches[0..4])
+            });
+        });
+    }
 }

zed/src/util.rs 🔗

@@ -7,6 +7,29 @@ pub fn post_inc(value: &mut usize) -> usize {
     prev
 }
 
+/// Extend a sorted vector with a sorted sequence of items, maintaining the vector's sort order and
+/// enforcing a maximum length. Sort the items according to the given callback. Before calling this,
+/// both `vec` and `new_items` should already be sorted according to the `cmp` comparator.
+pub fn extend_sorted<T, I, F>(vec: &mut Vec<T>, new_items: I, limit: usize, mut cmp: F)
+where
+    I: IntoIterator<Item = T>,
+    F: FnMut(&T, &T) -> Ordering,
+{
+    let mut start_index = 0;
+    for new_item in new_items {
+        if let Err(i) = vec[start_index..].binary_search_by(|m| cmp(m, &new_item)) {
+            let index = start_index + i;
+            if vec.len() < limit {
+                vec.insert(index, new_item);
+            } else if index < vec.len() {
+                vec.pop();
+                vec.insert(index, new_item);
+            }
+            start_index = index;
+        }
+    }
+}
+
 pub fn find_insertion_index<'a, F, T, E>(slice: &'a [T], mut f: F) -> Result<usize, E>
 where
     F: FnMut(&'a T) -> Result<Ordering, E>,
@@ -69,4 +92,18 @@ mod tests {
             Ok(1)
         );
     }
+
+    #[test]
+    fn test_extend_sorted() {
+        let mut vec = vec![];
+
+        extend_sorted(&mut vec, vec![21, 17, 13, 8, 1, 0], 5, |a, b| b.cmp(a));
+        assert_eq!(vec, &[21, 17, 13, 8, 1]);
+
+        extend_sorted(&mut vec, vec![101, 19, 17, 8, 2], 8, |a, b| b.cmp(a));
+        assert_eq!(vec, &[101, 21, 19, 17, 13, 8, 2, 1]);
+
+        extend_sorted(&mut vec, vec![1000, 19, 17, 9, 5], 8, |a, b| b.cmp(a));
+        assert_eq!(vec, &[1000, 101, 21, 19, 17, 13, 9, 8]);
+    }
 }

zed/src/worktree/fuzzy.rs 🔗

@@ -1,8 +1,8 @@
 use super::{char_bag::CharBag, EntryKind, Snapshot};
+use crate::util;
 use gpui::scoped_pool;
 use std::{
-    cmp::{max, min, Ordering, Reverse},
-    collections::BinaryHeap,
+    cmp::{max, min, Ordering},
     path::Path,
     sync::atomic::{self, AtomicBool},
     sync::Arc,
@@ -36,13 +36,17 @@ impl Eq for PathMatch {}
 
 impl PartialOrd for PathMatch {
     fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
-        self.score.partial_cmp(&other.score)
+        Some(self.cmp(other))
     }
 }
 
 impl Ord for PathMatch {
     fn cmp(&self, other: &Self) -> Ordering {
-        self.partial_cmp(other).unwrap_or(Ordering::Equal)
+        self.score
+            .partial_cmp(&other.score)
+            .unwrap_or(Ordering::Equal)
+            .then_with(|| self.tree_id.cmp(&other.tree_id))
+            .then_with(|| Arc::as_ptr(&self.path).cmp(&Arc::as_ptr(&other.path)))
     }
 }
 
@@ -74,7 +78,9 @@ where
     };
 
     let segment_size = (path_count + cpus - 1) / cpus;
-    let mut segment_results = (0..cpus).map(|_| BinaryHeap::new()).collect::<Vec<_>>();
+    let mut segment_results = (0..cpus)
+        .map(|_| Vec::with_capacity(max_results))
+        .collect::<Vec<_>>();
 
     pool.scoped(|scope| {
         for (segment_idx, results) in segment_results.iter_mut().enumerate() {
@@ -145,13 +151,14 @@ where
         }
     });
 
-    let mut results = segment_results
-        .into_iter()
-        .flatten()
-        .map(|r| r.0)
-        .collect::<Vec<_>>();
-    results.sort_unstable_by(|a, b| b.score.partial_cmp(&a.score).unwrap());
-    results.truncate(max_results);
+    let mut results = Vec::new();
+    for segment_result in segment_results {
+        if results.is_empty() {
+            results = segment_result;
+        } else {
+            util::extend_sorted(&mut results, segment_result, max_results, |a, b| b.cmp(&a));
+        }
+    }
     results
 }
 
@@ -163,7 +170,7 @@ fn match_single_tree_paths<'a>(
     lowercase_query: &[char],
     query_chars: CharBag,
     smart_case: bool,
-    results: &mut BinaryHeap<Reverse<PathMatch>>,
+    results: &mut Vec<PathMatch>,
     max_results: usize,
     min_score: &mut f64,
     match_positions: &mut Vec<usize>,
@@ -234,14 +241,22 @@ fn match_single_tree_paths<'a>(
         );
 
         if score > 0.0 {
-            results.push(Reverse(PathMatch {
+            let mat = PathMatch {
                 tree_id: snapshot.id,
                 path: candidate.path.clone(),
                 score,
                 positions: match_positions.clone(),
-            }));
-            if results.len() == max_results {
-                *min_score = results.peek().unwrap().0.score;
+            };
+            if let Err(i) = results.binary_search_by(|m| mat.cmp(&m)) {
+                if results.len() < max_results {
+                    results.insert(i, mat);
+                } else if i < results.len() {
+                    results.pop();
+                    results.insert(i, mat);
+                }
+                if results.len() == max_results {
+                    *min_score = results.last().unwrap().score;
+                }
             }
         }
     }
@@ -560,7 +575,7 @@ mod tests {
         last_positions.resize(query.len(), 0);
 
         let cancel_flag = AtomicBool::new(false);
-        let mut results = BinaryHeap::new();
+        let mut results = Vec::new();
         match_single_tree_paths(
             &Snapshot {
                 id: 0,
@@ -588,15 +603,14 @@ mod tests {
 
         results
             .into_iter()
-            .rev()
             .map(|result| {
                 (
                     paths
                         .iter()
                         .copied()
-                        .find(|p| result.0.path.as_ref() == Path::new(p))
+                        .find(|p| result.path.as_ref() == Path::new(p))
                         .unwrap(),
-                    result.0.positions,
+                    result.positions,
                 )
             })
             .collect()