From fab55486f524d129ebdd661bd23782eaf9fd802d Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 14 Mar 2024 16:40:48 +0100 Subject: [PATCH] Fix project search filtering on projects with multiple worktrees (#9337) Fixes #9285 Release Notes: - Fixed a bug that broke search filtering when searching a project with multiple worktrees ([#9285](https://github.com/zed-industries/zed/issues/9285)). --------- Co-authored-by: Thorsten --- crates/project/src/project.rs | 52 ++++--- crates/project/src/project_tests.rs | 169 +++++++++++++++++----- crates/project_panel/src/project_panel.rs | 24 ++- crates/search/src/project_search.rs | 40 +---- 4 files changed, 195 insertions(+), 90 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index d18c14fa6c..563599c753 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -6166,6 +6166,7 @@ impl Project { Some(tree.snapshot()) }) .collect::>(); + let include_root = snapshots.len() > 1; let background = cx.background_executor().clone(); let path_count: usize = snapshots @@ -6199,8 +6200,18 @@ impl Project { }); if is_ignored && !query.include_ignored() { return None; - } else if let Some(path) = snapshot.file().map(|file| file.path()) { - Some((path.clone(), (buffer, snapshot))) + } else if let Some(file) = snapshot.file() { + let matched_path = if include_root { + query.file_matches(Some(&file.full_path(cx))) + } else { + query.file_matches(Some(file.path())) + }; + + if matched_path { + Some((file.path().clone(), (buffer, snapshot))) + } else { + None + } } else { unnamed_files.push(buffer); None @@ -6215,6 +6226,7 @@ impl Project { self.fs.clone(), workers, query.clone(), + include_root, path_count, snapshots, matching_paths_tx, @@ -6251,21 +6263,15 @@ impl Project { while let Some((entry, buffer_index)) = buffers_rx.next().await { let buffer_matches = if let Some((_, snapshot)) = entry.as_ref() { - if query.file_matches( - snapshot.file().map(|file| file.path().as_ref()), - ) { - query - .search(snapshot, None) - .await - .iter() - .map(|range| { - snapshot.anchor_before(range.start) - ..snapshot.anchor_after(range.end) - }) - .collect() - } else { - Vec::new() - } + query + .search(snapshot, None) + .await + .iter() + .map(|range| { + snapshot.anchor_before(range.start) + ..snapshot.anchor_after(range.end) + }) + .collect() } else { Vec::new() }; @@ -6337,6 +6343,7 @@ impl Project { fs: Arc, workers: usize, query: SearchQuery, + include_root: bool, path_count: usize, snapshots: Vec, matching_paths_tx: Sender, @@ -6405,7 +6412,16 @@ impl Project { if unnamed_buffers.contains_key(&entry.path) { continue; } - let matches = if query.file_matches(Some(&entry.path)) { + + let matched_path = if include_root { + let mut full_path = PathBuf::from(snapshot.root_name()); + full_path.push(&entry.path); + query.file_matches(Some(&full_path)) + } else { + query.file_matches(Some(&entry.path)) + }; + + let matches = if matched_path { abs_path.clear(); abs_path.push(&snapshot.abs_path()); abs_path.push(&entry.path); diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index 1a6bf5d64f..b9aeb750ad 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -3758,8 +3758,8 @@ async fn test_search(cx: &mut gpui::TestAppContext) { .await .unwrap(), HashMap::from_iter([ - ("two.rs".to_string(), vec![6..9]), - ("three.rs".to_string(), vec![37..40]) + ("dir/two.rs".to_string(), vec![6..9]), + ("dir/three.rs".to_string(), vec![37..40]) ]) ); @@ -3783,9 +3783,9 @@ async fn test_search(cx: &mut gpui::TestAppContext) { .await .unwrap(), HashMap::from_iter([ - ("two.rs".to_string(), vec![6..9]), - ("three.rs".to_string(), vec![37..40]), - ("four.rs".to_string(), vec![25..28, 36..39]) + ("dir/two.rs".to_string(), vec![6..9]), + ("dir/three.rs".to_string(), vec![37..40]), + ("dir/four.rs".to_string(), vec![25..28, 36..39]) ]) ); } @@ -3846,8 +3846,8 @@ async fn test_search_with_inclusions(cx: &mut gpui::TestAppContext) { .await .unwrap(), HashMap::from_iter([ - ("one.rs".to_string(), vec![8..12]), - ("two.rs".to_string(), vec![8..12]), + ("dir/one.rs".to_string(), vec![8..12]), + ("dir/two.rs".to_string(), vec![8..12]), ]), "Rust only search should give only Rust files" ); @@ -3871,8 +3871,8 @@ async fn test_search_with_inclusions(cx: &mut gpui::TestAppContext) { .await .unwrap(), HashMap::from_iter([ - ("one.ts".to_string(), vec![14..18]), - ("two.ts".to_string(), vec![14..18]), + ("dir/one.ts".to_string(), vec![14..18]), + ("dir/two.ts".to_string(), vec![14..18]), ]), "TypeScript only search should give only TypeScript files, even if other inclusions don't match anything" ); @@ -3897,10 +3897,10 @@ async fn test_search_with_inclusions(cx: &mut gpui::TestAppContext) { .await .unwrap(), HashMap::from_iter([ - ("one.rs".to_string(), vec![8..12]), - ("one.ts".to_string(), vec![14..18]), - ("two.rs".to_string(), vec![8..12]), - ("two.ts".to_string(), vec![14..18]), + ("dir/one.rs".to_string(), vec![8..12]), + ("dir/one.ts".to_string(), vec![14..18]), + ("dir/two.rs".to_string(), vec![8..12]), + ("dir/two.ts".to_string(), vec![14..18]), ]), "Rust and typescript search should give both Rust and TypeScript files, even if other inclusions don't match anything" ); @@ -3942,10 +3942,10 @@ async fn test_search_with_exclusions(cx: &mut gpui::TestAppContext) { .await .unwrap(), HashMap::from_iter([ - ("one.rs".to_string(), vec![8..12]), - ("one.ts".to_string(), vec![14..18]), - ("two.rs".to_string(), vec![8..12]), - ("two.ts".to_string(), vec![14..18]), + ("dir/one.rs".to_string(), vec![8..12]), + ("dir/one.ts".to_string(), vec![14..18]), + ("dir/two.rs".to_string(), vec![8..12]), + ("dir/two.ts".to_string(), vec![14..18]), ]), "If no exclusions match, all files should be returned" ); @@ -3967,8 +3967,8 @@ async fn test_search_with_exclusions(cx: &mut gpui::TestAppContext) { .await .unwrap(), HashMap::from_iter([ - ("one.ts".to_string(), vec![14..18]), - ("two.ts".to_string(), vec![14..18]), + ("dir/one.ts".to_string(), vec![14..18]), + ("dir/two.ts".to_string(), vec![14..18]), ]), "Rust exclusion search should give only TypeScript files" ); @@ -3992,8 +3992,8 @@ async fn test_search_with_exclusions(cx: &mut gpui::TestAppContext) { .await .unwrap(), HashMap::from_iter([ - ("one.rs".to_string(), vec![8..12]), - ("two.rs".to_string(), vec![8..12]), + ("dir/one.rs".to_string(), vec![8..12]), + ("dir/two.rs".to_string(), vec![8..12]), ]), "TypeScript exclusion search should give only Rust files, even if other exclusions don't match anything" ); @@ -4128,13 +4128,105 @@ async fn test_search_with_exclusions_and_inclusions(cx: &mut gpui::TestAppContex .await .unwrap(), HashMap::from_iter([ - ("one.ts".to_string(), vec![14..18]), - ("two.ts".to_string(), vec![14..18]), + ("dir/one.ts".to_string(), vec![14..18]), + ("dir/two.ts".to_string(), vec![14..18]), ]), "Non-intersecting TypeScript inclusions and Rust exclusions should return TypeScript files" ); } +#[gpui::test] +async fn test_search_multiple_worktrees_with_inclusions(cx: &mut gpui::TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + "/worktree-a", + json!({ + "haystack.rs": r#"// NEEDLE"#, + "haystack.ts": r#"// NEEDLE"#, + }), + ) + .await; + fs.insert_tree( + "/worktree-b", + json!({ + "haystack.rs": r#"// NEEDLE"#, + "haystack.ts": r#"// NEEDLE"#, + }), + ) + .await; + + let project = Project::test( + fs.clone(), + ["/worktree-a".as_ref(), "/worktree-b".as_ref()], + cx, + ) + .await; + + assert_eq!( + search( + &project, + SearchQuery::text( + "NEEDLE", + false, + true, + false, + vec![PathMatcher::new("worktree-a/*.rs").unwrap()], + Vec::new() + ) + .unwrap(), + cx + ) + .await + .unwrap(), + HashMap::from_iter([("worktree-a/haystack.rs".to_string(), vec![3..9])]), + "should only return results from included worktree" + ); + assert_eq!( + search( + &project, + SearchQuery::text( + "NEEDLE", + false, + true, + false, + vec![PathMatcher::new("worktree-b/*.rs").unwrap()], + Vec::new() + ) + .unwrap(), + cx + ) + .await + .unwrap(), + HashMap::from_iter([("worktree-b/haystack.rs".to_string(), vec![3..9])]), + "should only return results from included worktree" + ); + + assert_eq!( + search( + &project, + SearchQuery::text( + "NEEDLE", + false, + true, + false, + vec![PathMatcher::new("*.ts").unwrap()], + Vec::new() + ) + .unwrap(), + cx + ) + .await + .unwrap(), + HashMap::from_iter([ + ("worktree-a/haystack.ts".to_string(), vec![3..9]), + ("worktree-b/haystack.ts".to_string(), vec![3..9]) + ]), + "should return results from both worktrees" + ); +} + #[gpui::test] async fn test_search_in_gitignored_dirs(cx: &mut gpui::TestAppContext) { init_test(cx); @@ -4173,7 +4265,7 @@ async fn test_search_in_gitignored_dirs(cx: &mut gpui::TestAppContext) { ) .await .unwrap(), - HashMap::from_iter([("package.json".to_string(), vec![8..11])]), + HashMap::from_iter([("dir/package.json".to_string(), vec![8..11])]), "Only one non-ignored file should have the query" ); @@ -4186,15 +4278,21 @@ async fn test_search_in_gitignored_dirs(cx: &mut gpui::TestAppContext) { .await .unwrap(), HashMap::from_iter([ - ("package.json".to_string(), vec![8..11]), - ("target/index.txt".to_string(), vec![6..9]), + ("dir/package.json".to_string(), vec![8..11]), + ("dir/target/index.txt".to_string(), vec![6..9]), ( - "node_modules/prettier/package.json".to_string(), + "dir/node_modules/prettier/package.json".to_string(), vec![9..12] ), - ("node_modules/prettier/index.ts".to_string(), vec![15..18]), - ("node_modules/eslint/index.ts".to_string(), vec![13..16]), - ("node_modules/eslint/package.json".to_string(), vec![8..11]), + ( + "dir/node_modules/prettier/index.ts".to_string(), + vec![15..18] + ), + ("dir/node_modules/eslint/index.ts".to_string(), vec![13..16]), + ( + "dir/node_modules/eslint/package.json".to_string(), + vec![8..11] + ), ]), "Unrestricted search with ignored directories should find every file with the query" ); @@ -4216,7 +4314,7 @@ async fn test_search_in_gitignored_dirs(cx: &mut gpui::TestAppContext) { .await .unwrap(), HashMap::from_iter([( - "node_modules/prettier/package.json".to_string(), + "dir/node_modules/prettier/package.json".to_string(), vec![9..12] )]), "With search including ignored prettier directory and excluding TS files, only one file should be found" @@ -4313,8 +4411,13 @@ async fn search( Ok(result .into_iter() .map(|(buffer, ranges)| { - buffer.update(cx, |buffer, _| { - let path = buffer.file().unwrap().path().to_string_lossy().to_string(); + buffer.update(cx, |buffer, cx| { + let path = buffer + .file() + .unwrap() + .full_path(cx) + .to_string_lossy() + .to_string(); let ranges = ranges .into_iter() .map(|range| range.to_offset(buffer)) diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 76b51a8ef1..cc18378717 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -23,7 +23,13 @@ use project::{ }; use project_panel_settings::{ProjectPanelDockPosition, ProjectPanelSettings}; use serde::{Deserialize, Serialize}; -use std::{cmp::Ordering, ffi::OsStr, ops::Range, path::Path, sync::Arc}; +use std::{ + cmp::Ordering, + ffi::OsStr, + ops::Range, + path::{Path, PathBuf}, + sync::Arc, +}; use theme::ThemeSettings; use ui::{prelude::*, v_flex, ContextMenu, Icon, KeyBinding, Label, ListItem}; use unicase::UniCase; @@ -991,12 +997,22 @@ impl ProjectPanel { _: &NewSearchInDirectory, cx: &mut ViewContext, ) { - if let Some((_, entry)) = self.selected_entry(cx) { + if let Some((worktree, entry)) = self.selected_entry(cx) { if entry.is_dir() { - let entry = entry.clone(); + let include_root = self.project.read(cx).visible_worktrees(cx).count() > 1; + let dir_path = if include_root { + let mut full_path = PathBuf::from(worktree.root_name()); + full_path.push(&entry.path); + Arc::from(full_path) + } else { + entry.path.clone() + }; + self.workspace .update(cx, |workspace, cx| { - search::ProjectSearchView::new_search_in_directory(workspace, &entry, cx); + search::ProjectSearchView::new_search_in_directory( + workspace, &dir_path, cx, + ); }) .ok(); } diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index 5729740958..7944d6ba3f 100644 --- a/crates/search/src/project_search.rs +++ b/crates/search/src/project_search.rs @@ -23,7 +23,7 @@ use gpui::{ use menu::Confirm; use project::{ search::{SearchInputs, SearchQuery}, - Entry, Project, + Project, }; use semantic_index::{SemanticIndex, SemanticIndexStatus}; @@ -34,7 +34,7 @@ use std::{ any::{Any, TypeId}, mem, ops::{Not, Range}, - path::PathBuf, + path::{Path, PathBuf}, time::{Duration, Instant}, }; use theme::ThemeSettings; @@ -990,13 +990,10 @@ impl ProjectSearchView { pub fn new_search_in_directory( workspace: &mut Workspace, - dir_entry: &Entry, + dir_path: &Path, cx: &mut ViewContext, ) { - if !dir_entry.is_dir() { - return; - } - let Some(filter_str) = dir_entry.path.to_str() else { + let Some(filter_str) = dir_path.to_str() else { return; }; @@ -2806,33 +2803,6 @@ pub mod tests { }) .unwrap(); - let one_file_entry = cx.update(|cx| { - workspace - .read(cx) - .project() - .read(cx) - .entry_for_path(&(worktree_id, "a/one.rs").into(), cx) - .expect("no entry for /a/one.rs file") - }); - assert!(one_file_entry.is_file()); - window - .update(cx, |workspace, cx| { - ProjectSearchView::new_search_in_directory(workspace, &one_file_entry, cx) - }) - .unwrap(); - let active_search_entry = cx.read(|cx| { - workspace - .read(cx) - .active_pane() - .read(cx) - .active_item() - .and_then(|item| item.downcast::()) - }); - assert!( - active_search_entry.is_none(), - "Expected no search panel to be active for file entry" - ); - let a_dir_entry = cx.update(|cx| { workspace .read(cx) @@ -2844,7 +2814,7 @@ pub mod tests { assert!(a_dir_entry.is_dir()); window .update(cx, |workspace, cx| { - ProjectSearchView::new_search_in_directory(workspace, &a_dir_entry, cx) + ProjectSearchView::new_search_in_directory(workspace, &a_dir_entry.path, cx) }) .unwrap();