In file finder, handle single-file worktrees & multiple matches w/ same rel path

Max Brunsfeld created

Change summary

zed/src/file_finder.rs    | 287 +++++++++++++++++++++++++---------------
zed/src/worktree.rs       |  49 +++---
zed/src/worktree/fuzzy.rs |   8 
3 files changed, 212 insertions(+), 132 deletions(-)

Detailed changes

zed/src/file_finder.rs 🔗

@@ -33,8 +33,7 @@ pub struct FileFinder {
     latest_search_did_cancel: bool,
     latest_search_query: String,
     matches: Vec<PathMatch>,
-    include_root_name: bool,
-    selected: Option<Arc<Path>>,
+    selected: Option<(usize, Arc<Path>)>,
     cancel_flag: Arc<AtomicBool>,
     list_state: UniformListState,
 }
@@ -147,101 +146,110 @@ impl FileFinder {
         index: usize,
         app: &AppContext,
     ) -> Option<ElementBox> {
-        let tree_id = path_match.tree_id;
-
-        self.worktree(tree_id, app).map(|tree| {
-            let prefix = if self.include_root_name {
-                tree.root_name()
-            } else {
-                ""
-            };
-            let path = path_match.path.clone();
-            let path_string = path_match.path.to_string_lossy();
-            let file_name = path_match
-                .path
-                .file_name()
-                .unwrap_or_default()
-                .to_string_lossy();
-
-            let path_positions = path_match.positions.clone();
-            let file_name_start =
-                prefix.len() + path_string.chars().count() - file_name.chars().count();
-            let mut file_name_positions = Vec::new();
-            file_name_positions.extend(path_positions.iter().filter_map(|pos| {
-                if pos >= &file_name_start {
-                    Some(pos - file_name_start)
-                } else {
-                    None
-                }
-            }));
-
-            let settings = smol::block_on(self.settings.read());
-            let highlight_color = ColorU::from_u32(0x304ee2ff);
-            let bold = *Properties::new().weight(Weight::BOLD);
-
-            let mut full_path = prefix.to_string();
-            full_path.push_str(&path_string);
-
-            let mut container = Container::new(
-                Flex::row()
-                    .with_child(
-                        Container::new(
-                            LineBox::new(
-                                settings.ui_font_family,
-                                settings.ui_font_size,
-                                Svg::new("icons/file-16.svg").boxed(),
+        self.labels_for_match(path_match, app).map(
+            |(file_name, file_name_positions, full_path, full_path_positions)| {
+                let settings = smol::block_on(self.settings.read());
+                let highlight_color = ColorU::from_u32(0x304ee2ff);
+                let bold = *Properties::new().weight(Weight::BOLD);
+                let mut container = Container::new(
+                    Flex::row()
+                        .with_child(
+                            Container::new(
+                                LineBox::new(
+                                    settings.ui_font_family,
+                                    settings.ui_font_size,
+                                    Svg::new("icons/file-16.svg").boxed(),
+                                )
+                                .boxed(),
                             )
+                            .with_padding_right(6.0)
                             .boxed(),
                         )
-                        .with_padding_right(6.0)
-                        .boxed(),
-                    )
-                    .with_child(
-                        Expanded::new(
-                            1.0,
-                            Flex::column()
-                                .with_child(
-                                    Label::new(
-                                        file_name.to_string(),
-                                        settings.ui_font_family,
-                                        settings.ui_font_size,
+                        .with_child(
+                            Expanded::new(
+                                1.0,
+                                Flex::column()
+                                    .with_child(
+                                        Label::new(
+                                            file_name.to_string(),
+                                            settings.ui_font_family,
+                                            settings.ui_font_size,
+                                        )
+                                        .with_highlights(highlight_color, bold, file_name_positions)
+                                        .boxed(),
                                     )
-                                    .with_highlights(highlight_color, bold, file_name_positions)
-                                    .boxed(),
-                                )
-                                .with_child(
-                                    Label::new(
-                                        full_path,
-                                        settings.ui_font_family,
-                                        settings.ui_font_size,
+                                    .with_child(
+                                        Label::new(
+                                            full_path,
+                                            settings.ui_font_family,
+                                            settings.ui_font_size,
+                                        )
+                                        .with_highlights(highlight_color, bold, full_path_positions)
+                                        .boxed(),
                                     )
-                                    .with_highlights(highlight_color, bold, path_positions)
                                     .boxed(),
-                                )
-                                .boxed(),
+                            )
+                            .boxed(),
                         )
                         .boxed(),
-                    )
-                    .boxed(),
-            )
-            .with_uniform_padding(6.0);
+                )
+                .with_uniform_padding(6.0);
 
-            let selected_index = self.selected_index();
-            if index == selected_index || index < self.matches.len() - 1 {
-                container =
-                    container.with_border(Border::bottom(1.0, ColorU::from_u32(0xdbdbdcff)));
-            }
+                let selected_index = self.selected_index();
+                if index == selected_index || index < self.matches.len() - 1 {
+                    container =
+                        container.with_border(Border::bottom(1.0, ColorU::from_u32(0xdbdbdcff)));
+                }
 
-            if index == selected_index {
-                container = container.with_background_color(ColorU::from_u32(0xdbdbdcff));
-            }
+                if index == selected_index {
+                    container = container.with_background_color(ColorU::from_u32(0xdbdbdcff));
+                }
+
+                let entry = (path_match.tree_id, path_match.path.clone());
+                EventHandler::new(container.boxed())
+                    .on_mouse_down(move |ctx| {
+                        ctx.dispatch_action("file_finder:select", entry.clone());
+                        true
+                    })
+                    .named("match")
+            },
+        )
+    }
+
+    fn labels_for_match(
+        &self,
+        path_match: &PathMatch,
+        app: &AppContext,
+    ) -> Option<(String, Vec<usize>, String, Vec<usize>)> {
+        self.worktree(path_match.tree_id, app).map(|tree| {
+            let prefix = if path_match.include_root_name {
+                tree.root_name()
+            } else {
+                ""
+            };
 
-            EventHandler::new(container.boxed())
-                .on_mouse_down(move |ctx| {
-                    ctx.dispatch_action("file_finder:select", (tree_id, path.clone()));
-                    true
+            let path_string = path_match.path.to_string_lossy();
+            let full_path = [prefix, path_string.as_ref()].join("");
+            let path_positions = path_match.positions.clone();
+
+            let file_name = path_match.path.file_name().map_or_else(
+                || prefix.to_string(),
+                |file_name| file_name.to_string_lossy().to_string(),
+            );
+            let file_name_start =
+                prefix.chars().count() + path_string.chars().count() - file_name.chars().count();
+            let file_name_positions = path_positions
+                .iter()
+                .filter_map(|pos| {
+                    if pos >= &file_name_start {
+                        Some(pos - file_name_start)
+                    } else {
+                        None
+                    }
                 })
-                .named("match")
+                .collect();
+
+            (file_name, file_name_positions, full_path, path_positions)
         })
     }
 
@@ -298,7 +306,6 @@ impl FileFinder {
             latest_search_did_cancel: false,
             latest_search_query: String::new(),
             matches: Vec::new(),
-            include_root_name: false,
             selected: None,
             cancel_flag: Arc::new(AtomicBool::new(false)),
             list_state: UniformListState::new(),
@@ -335,7 +342,9 @@ impl FileFinder {
     fn selected_index(&self) -> usize {
         if let Some(selected) = self.selected.as_ref() {
             for (ix, path_match) in self.matches.iter().enumerate() {
-                if path_match.path.as_ref() == selected.as_ref() {
+                if (path_match.tree_id, path_match.path.as_ref())
+                    == (selected.0, selected.1.as_ref())
+                {
                     return ix;
                 }
             }
@@ -347,7 +356,8 @@ impl FileFinder {
         let mut selected_index = self.selected_index();
         if selected_index > 0 {
             selected_index -= 1;
-            self.selected = Some(self.matches[selected_index].path.clone());
+            let mat = &self.matches[selected_index];
+            self.selected = Some((mat.tree_id, mat.path.clone()));
         }
         self.list_state.scroll_to(selected_index);
         ctx.notify();
@@ -357,7 +367,8 @@ impl FileFinder {
         let mut selected_index = self.selected_index();
         if selected_index + 1 < self.matches.len() {
             selected_index += 1;
-            self.selected = Some(self.matches[selected_index].path.clone());
+            let mat = &self.matches[selected_index];
+            self.selected = Some((mat.tree_id, mat.path.clone()));
         }
         self.list_state.scroll_to(selected_index);
         ctx.notify();
@@ -403,7 +414,7 @@ impl FileFinder {
                 pool,
             );
             let did_cancel = cancel_flag.load(atomic::Ordering::Relaxed);
-            (search_id, include_root_name, did_cancel, query, matches)
+            (search_id, did_cancel, query, matches)
         });
 
         ctx.spawn(task, Self::update_matches).detach();
@@ -411,13 +422,7 @@ impl FileFinder {
 
     fn update_matches(
         &mut self,
-        (search_id, include_root_name, did_cancel, query, matches): (
-            usize,
-            bool,
-            bool,
-            String,
-            Vec<PathMatch>,
-        ),
+        (search_id, did_cancel, query, matches): (usize, bool, String, Vec<PathMatch>),
         ctx: &mut ViewContext<Self>,
     ) {
         if search_id >= self.latest_search_id {
@@ -429,7 +434,6 @@ impl FileFinder {
             }
             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();
         }
@@ -454,20 +458,16 @@ mod tests {
     };
     use gpui::App;
     use serde_json::json;
-    use smol::fs;
+    use std::fs;
     use tempdir::TempDir;
 
     #[test]
     fn test_matching_paths() {
         App::test_async((), |mut app| async move {
             let tmp_dir = TempDir::new("example").unwrap();
-            fs::create_dir(tmp_dir.path().join("a")).await.unwrap();
-            fs::write(tmp_dir.path().join("a/banana"), "banana")
-                .await
-                .unwrap();
-            fs::write(tmp_dir.path().join("a/bandana"), "bandana")
-                .await
-                .unwrap();
+            fs::create_dir(tmp_dir.path().join("a")).unwrap();
+            fs::write(tmp_dir.path().join("a/banana"), "banana").unwrap();
+            fs::write(tmp_dir.path().join("a/bandana"), "bandana").unwrap();
             app.update(|ctx| {
                 super::init(ctx);
                 editor::init(ctx);
@@ -560,7 +560,6 @@ mod tests {
                 finder.update_matches(
                     (
                         finder.latest_search_id,
-                        true,
                         true, // did-cancel
                         query.clone(),
                         vec![matches[1].clone(), matches[3].clone()],
@@ -573,7 +572,6 @@ mod tests {
                 finder.update_matches(
                     (
                         finder.latest_search_id,
-                        true,
                         true, // did-cancel
                         query.clone(),
                         vec![matches[0].clone(), matches[2].clone(), matches[3].clone()],
@@ -585,4 +583,77 @@ mod tests {
             });
         });
     }
+
+    #[test]
+    fn test_single_file_worktrees() {
+        App::test_async((), |mut app| async move {
+            let temp_dir = TempDir::new("test-single-file-worktrees").unwrap();
+            let dir_path = temp_dir.path().join("the-parent-dir");
+            let file_path = dir_path.join("the-file");
+            fs::create_dir(&dir_path).unwrap();
+            fs::write(&file_path, "").unwrap();
+
+            let settings = settings::channel(&app.font_cache()).unwrap().1;
+            let workspace = app.add_model(|ctx| Workspace::new(vec![file_path], ctx));
+            app.read(|ctx| workspace.read(ctx).worktree_scans_complete(ctx))
+                .await;
+            let (_, finder) =
+                app.add_window(|ctx| FileFinder::new(settings, workspace.clone(), ctx));
+
+            // Even though there is only one worktree, that worktree's filename
+            // is included in the matching, because the worktree is a single file.
+            finder.update(&mut app, |f, ctx| f.spawn_search("thf".into(), ctx));
+            finder.condition(&app, |f, _| f.matches.len() == 1).await;
+
+            app.read(|ctx| {
+                let finder = finder.read(ctx);
+                let (file_name, file_name_positions, full_path, full_path_positions) =
+                    finder.labels_for_match(&finder.matches[0], ctx).unwrap();
+
+                assert_eq!(file_name, "the-file");
+                assert_eq!(file_name_positions, &[0, 1, 4]);
+                assert_eq!(full_path, "the-file");
+                assert_eq!(full_path_positions, &[0, 1, 4]);
+            });
+
+            // Since the worktree root is a file, searching for its name followed by a slash does
+            // not match anything.
+            finder.update(&mut app, |f, ctx| f.spawn_search("thf/".into(), ctx));
+            finder.condition(&app, |f, _| f.matches.len() == 0).await;
+        });
+    }
+
+    #[test]
+    fn test_multiple_matches_with_same_relative_path() {
+        App::test_async((), |mut app| async move {
+            let tmp_dir = temp_tree(json!({
+                "dir1": { "a.txt": "" },
+                "dir2": { "a.txt": "" }
+            }));
+            let settings = settings::channel(&app.font_cache()).unwrap().1;
+            let workspace = app.add_model(|ctx| {
+                Workspace::new(
+                    vec![tmp_dir.path().join("dir1"), tmp_dir.path().join("dir2")],
+                    ctx,
+                )
+            });
+            app.read(|ctx| workspace.read(ctx).worktree_scans_complete(ctx))
+                .await;
+            let (_, finder) =
+                app.add_window(|ctx| FileFinder::new(settings, workspace.clone(), ctx));
+
+            // Run a search that matches two files with the same relative path.
+            finder.update(&mut app, |f, ctx| f.spawn_search("a.t".into(), ctx));
+            finder.condition(&app, |f, _| f.matches.len() == 2).await;
+
+            // Can switch between different matches with the same relative path.
+            finder.update(&mut app, |f, ctx| {
+                assert_eq!(f.selected_index(), 0);
+                f.select_next(&(), ctx);
+                assert_eq!(f.selected_index(), 1);
+                f.select_prev(&(), ctx);
+                assert_eq!(f.selected_index(), 0);
+            });
+        });
+    }
 }

zed/src/worktree.rs 🔗

@@ -68,16 +68,13 @@ struct FileHandleState {
 impl Worktree {
     pub fn new(path: impl Into<Arc<Path>>, ctx: &mut ModelContext<Self>) -> Self {
         let abs_path = path.into();
-        let root_name = abs_path
-            .file_name()
-            .map_or(String::new(), |n| n.to_string_lossy().to_string() + "/");
         let (scan_state_tx, scan_state_rx) = smol::channel::unbounded();
         let id = ctx.model_id();
         let snapshot = Snapshot {
             id,
             scan_id: 0,
             abs_path,
-            root_name,
+            root_name: Default::default(),
             ignores: Default::default(),
             entries: Default::default(),
         };
@@ -269,8 +266,8 @@ impl Snapshot {
         self.entry_for_path("").unwrap()
     }
 
-    /// Returns the filename of the snapshot's root directory,
-    /// with a trailing slash.
+    /// Returns the filename of the snapshot's root, plus a trailing slash if the snapshot's root is
+    /// a directory.
     pub fn root_name(&self) -> &str {
         &self.root_name
     }
@@ -481,6 +478,10 @@ impl Entry {
     fn is_dir(&self) -> bool {
         matches!(self.kind, EntryKind::Dir | EntryKind::PendingDir)
     }
+
+    fn is_file(&self) -> bool {
+        matches!(self.kind, EntryKind::File(_))
+    }
 }
 
 impl sum_tree::Item for Entry {
@@ -489,7 +490,7 @@ impl sum_tree::Item for Entry {
     fn summary(&self) -> Self::Summary {
         let file_count;
         let visible_file_count;
-        if matches!(self.kind, EntryKind::File(_)) {
+        if self.is_file() {
             file_count = 1;
             if self.is_ignored {
                 visible_file_count = 0;
@@ -631,14 +632,8 @@ impl BackgroundScanner {
         notify: Sender<ScanState>,
         worktree_id: usize,
     ) -> Self {
-        let root_char_bag = snapshot
-            .lock()
-            .root_name
-            .chars()
-            .map(|c| c.to_ascii_lowercase())
-            .collect();
         let mut scanner = Self {
-            root_char_bag,
+            root_char_bag: Default::default(),
             snapshot,
             notify,
             handles,
@@ -699,7 +694,7 @@ impl BackgroundScanner {
         });
     }
 
-    fn scan_dirs(&self) -> io::Result<()> {
+    fn scan_dirs(&mut self) -> io::Result<()> {
         self.snapshot.lock().scan_id += 1;
 
         let path: Arc<Path> = Arc::from(Path::new(""));
@@ -707,19 +702,29 @@ impl BackgroundScanner {
         let metadata = fs::metadata(&abs_path)?;
         let inode = metadata.ino();
         let is_symlink = fs::symlink_metadata(&abs_path)?.file_type().is_symlink();
+        let is_dir = metadata.file_type().is_dir();
 
-        if metadata.file_type().is_dir() {
-            let dir_entry = Entry {
+        // After determining whether the root entry is a file or a directory, populate the
+        // snapshot's "root name", which will be used for the purpose of fuzzy matching.
+        let mut root_name = abs_path
+            .file_name()
+            .map_or(String::new(), |f| f.to_string_lossy().to_string());
+        if is_dir {
+            root_name.push('/');
+        }
+        self.root_char_bag = root_name.chars().map(|c| c.to_ascii_lowercase()).collect();
+        self.snapshot.lock().root_name = root_name;
+
+        if is_dir {
+            self.snapshot.lock().insert_entry(Entry {
                 kind: EntryKind::PendingDir,
                 path: path.clone(),
                 inode,
                 is_symlink,
                 is_ignored: false,
-            };
-            self.snapshot.lock().insert_entry(dir_entry);
+            });
 
             let (tx, rx) = crossbeam_channel::unbounded();
-
             tx.send(ScanJob {
                 abs_path: abs_path.to_path_buf(),
                 path,
@@ -1541,7 +1546,7 @@ mod tests {
             scanner.snapshot().check_invariants();
 
             let (notify_tx, _notify_rx) = smol::channel::unbounded();
-            let new_scanner = BackgroundScanner::new(
+            let mut new_scanner = BackgroundScanner::new(
                 Arc::new(Mutex::new(Snapshot {
                     id: 0,
                     scan_id: 0,
@@ -1711,7 +1716,7 @@ mod tests {
             let mut files = self.files(0);
             let mut visible_files = self.visible_files(0);
             for entry in self.entries.cursor::<(), ()>() {
-                if matches!(entry.kind, EntryKind::File(_)) {
+                if entry.is_file() {
                     assert_eq!(files.next().unwrap().inode(), entry.inode);
                     if !entry.is_ignored {
                         assert_eq!(visible_files.next().unwrap().inode(), entry.inode);

zed/src/worktree/fuzzy.rs 🔗

@@ -24,6 +24,7 @@ pub struct PathMatch {
     pub positions: Vec<usize>,
     pub tree_id: usize,
     pub path: Arc<Path>,
+    pub include_root_name: bool,
 }
 
 impl PartialEq for PathMatch {
@@ -84,7 +85,7 @@ where
 
     pool.scoped(|scope| {
         for (segment_idx, results) in segment_results.iter_mut().enumerate() {
-            let trees = snapshots.clone();
+            let snapshots = snapshots.clone();
             let cancel_flag = &cancel_flag;
             scope.execute(move || {
                 let segment_start = segment_idx * segment_size;
@@ -99,12 +100,14 @@ where
                 let mut best_position_matrix = Vec::new();
 
                 let mut tree_start = 0;
-                for snapshot in trees {
+                for snapshot in snapshots {
                     let tree_end = if include_ignored {
                         tree_start + snapshot.file_count()
                     } else {
                         tree_start + snapshot.visible_file_count()
                     };
+
+                    let include_root_name = include_root_name || snapshot.root_entry().is_file();
                     if tree_start < segment_end && segment_start < tree_end {
                         let start = max(tree_start, segment_start) - tree_start;
                         let end = min(tree_end, segment_end) - tree_start;
@@ -246,6 +249,7 @@ fn match_single_tree_paths<'a>(
                 path: candidate.path.clone(),
                 score,
                 positions: match_positions.clone(),
+                include_root_name,
             };
             if let Err(i) = results.binary_search_by(|m| mat.cmp(&m)) {
                 if results.len() < max_results {