From a59b75c83998dd4a28a7e58744fe349b91724978 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 27 Apr 2021 14:02:55 -0700 Subject: [PATCH] Keep results stable when using file-finder while scanning files --- zed/src/file_finder.rs | 85 +++++++++++++++++++++++++++++++++++++-- zed/src/util.rs | 37 +++++++++++++++++ zed/src/worktree/fuzzy.rs | 10 +++-- 3 files changed, 125 insertions(+), 7 deletions(-) diff --git a/zed/src/file_finder.rs b/zed/src/file_finder.rs index d6ec299593b4cbce237369304a8a028731042794..6a7c7280c1bd2b94db6b44ef262f4aa041ebd739 100644 --- a/zed/src/file_finder.rs +++ b/zed/src/file_finder.rs @@ -30,6 +30,8 @@ pub struct FileFinder { query_buffer: ViewHandle, search_count: usize, latest_search_id: usize, + latest_search_did_cancel: bool, + latest_search_query: String, matches: Vec, include_root_name: bool, selected: Option>, @@ -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), + (search_id, include_root_name, did_cancel, query, matches): ( + usize, + bool, + bool, + String, + Vec, + ), ctx: &mut ViewContext, ) { if search_id >= self.latest_search_id { self.latest_search_id = search_id; - self.matches = matches; + if did_cancel && 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_did_cancel = did_cancel; + self.latest_search_query = query; + } 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]) + }); + }); + } } diff --git a/zed/src/util.rs b/zed/src/util.rs index 70f2b1a6d6485445862042fe749ca6d69714b23d..09ce2f87e155c30c996f75f5b1a8a62127ff9c4f 100644 --- a/zed/src/util.rs +++ b/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(vec: &mut Vec, new_items: I, limit: usize, mut cmp: F) +where + I: IntoIterator, + 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 where F: FnMut(&'a T) -> Result, @@ -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]); + } } diff --git a/zed/src/worktree/fuzzy.rs b/zed/src/worktree/fuzzy.rs index 10a1cace3196de0a5f6392b0839efcd86ab5deb5..3831084e0ac863280d755d90cc62de754568fa8f 100644 --- a/zed/src/worktree/fuzzy.rs +++ b/zed/src/worktree/fuzzy.rs @@ -36,13 +36,17 @@ impl Eq for PathMatch {} impl PartialOrd for PathMatch { fn partial_cmp(&self, other: &Self) -> Option { - 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))) } } @@ -150,7 +154,7 @@ where .flatten() .map(|r| r.0) .collect::>(); - results.sort_unstable_by(|a, b| b.score.partial_cmp(&a.score).unwrap()); + results.sort_unstable_by(|a, b| b.cmp(&a)); results.truncate(max_results); results }