diff --git a/Cargo.lock b/Cargo.lock index 80840bf3d3c631f6469f18811f1f3b79979003e4..43658ef42a0d0e459e834cccbb36fbc658c2930f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -15116,6 +15116,7 @@ dependencies = [ "editor", "futures 0.3.31", "gpui", + "itertools 0.14.0", "language", "lsp", "menu", diff --git a/crates/agent/src/tools/find_path_tool.rs b/crates/agent/src/tools/find_path_tool.rs index 41954ee31b2a4529e75541e78eff278a521307da..70d7b29f75d4da984c4acda13dcdbfe7bc69fbbc 100644 --- a/crates/agent/src/tools/find_path_tool.rs +++ b/crates/agent/src/tools/find_path_tool.rs @@ -177,7 +177,7 @@ fn search_paths(glob: &str, project: Entity, cx: &mut App) -> Task matcher, diff --git a/crates/assistant_slash_commands/src/diagnostics_command.rs b/crates/assistant_slash_commands/src/diagnostics_command.rs index 3a9c33061575d385652b685dcca70ee87c6cac35..3b3e3f7b895d50b36c3981bf4ee442b09bfdf33f 100644 --- a/crates/assistant_slash_commands/src/diagnostics_command.rs +++ b/crates/assistant_slash_commands/src/diagnostics_command.rs @@ -233,18 +233,11 @@ fn collect_diagnostics( options: Options, cx: &mut App, ) -> Task>> { - let error_source = if let Some(path_matcher) = &options.path_matcher { - debug_assert_eq!(path_matcher.sources().len(), 1); - Some(path_matcher.sources().first().cloned().unwrap_or_default()) - } else { - None - }; - let path_style = project.read(cx).path_style(cx); let glob_is_exact_file_match = if let Some(path) = options .path_matcher .as_ref() - .and_then(|pm| pm.sources().first()) + .and_then(|pm| pm.sources().next()) { project .read(cx) @@ -266,6 +259,13 @@ fn collect_diagnostics( .collect(); cx.spawn(async move |cx| { + let error_source = if let Some(path_matcher) = &options.path_matcher { + debug_assert_eq!(path_matcher.sources().count(), 1); + Some(path_matcher.sources().next().unwrap_or_default()) + } else { + None + }; + let mut output = SlashCommandOutput::default(); if let Some(error_source) = error_source.as_ref() { @@ -277,7 +277,7 @@ fn collect_diagnostics( let mut project_summary = DiagnosticSummary::default(); for (project_path, path, summary) in diagnostic_summaries { if let Some(path_matcher) = &options.path_matcher - && !path_matcher.is_match(&path.as_std_path()) + && !path_matcher.is_match(&path) { continue; } diff --git a/crates/prettier/src/prettier.rs b/crates/prettier/src/prettier.rs index b9c40e814c4caf760123cf460e2eed7298f9e951..381fdc2b2b35be53a0f07878c83cadd2862d06bf 100644 --- a/crates/prettier/src/prettier.rs +++ b/crates/prettier/src/prettier.rs @@ -12,7 +12,10 @@ use std::{ path::{Path, PathBuf}, sync::Arc, }; -use util::paths::{PathMatcher, PathStyle}; +use util::{ + paths::{PathMatcher, PathStyle}, + rel_path::RelPath, +}; #[derive(Debug, Clone)] pub enum Prettier { @@ -119,20 +122,32 @@ impl Prettier { None } }).any(|workspace_definition| { - workspace_definition == subproject_path.to_string_lossy() || PathMatcher::new(&[workspace_definition], PathStyle::local()).ok().is_some_and(|path_matcher| path_matcher.is_match(subproject_path)) + workspace_definition == subproject_path.to_string_lossy() || PathMatcher::new(&[workspace_definition], PathStyle::local()).ok().is_some_and( + |path_matcher| RelPath::new(subproject_path, PathStyle::local()).is_ok_and(|path| path_matcher.is_match(path))) }) { - anyhow::ensure!(has_prettier_in_node_modules(fs, &path_to_check).await?, "Path {path_to_check:?} is the workspace root for project in {closest_package_json_path:?}, but it has no prettier installed"); - log::info!("Found prettier path {path_to_check:?} in the workspace root for project in {closest_package_json_path:?}"); + anyhow::ensure!(has_prettier_in_node_modules(fs, &path_to_check).await?, + "Path {path_to_check:?} is the workspace root for project in \ + {closest_package_json_path:?}, but it has no prettier installed" + ); + log::info!( + "Found prettier path {path_to_check:?} in the workspace \ + root for project in {closest_package_json_path:?}" + ); return Ok(ControlFlow::Continue(Some(path_to_check))); } else { - log::warn!("Skipping path {path_to_check:?} workspace root with workspaces {workspaces:?} that have no prettier installed"); + log::warn!( + "Skipping path {path_to_check:?} workspace root with \ + workspaces {workspaces:?} that have no prettier installed" + ); } } Some(unknown) => log::error!( - "Failed to parse workspaces for {path_to_check:?} from package.json, got {unknown:?}. Skipping." + "Failed to parse workspaces for {path_to_check:?} from package.json, \ + got {unknown:?}. Skipping." ), None => log::warn!( - "Skipping path {path_to_check:?} that has no prettier dependency and no workspaces section in its package.json" + "Skipping path {path_to_check:?} that has no prettier \ + dependency and no workspaces section in its package.json" ), } } @@ -221,7 +236,12 @@ impl Prettier { ) .ok() .is_some_and( - |path_matcher| path_matcher.is_match(subproject_path), + |path_matcher| { + RelPath::new(subproject_path, PathStyle::local()) + .is_ok_and(|rel_path| { + path_matcher.is_match(rel_path) + }) + }, ) }) { diff --git a/crates/project/src/project_search.rs b/crates/project/src/project_search.rs index fa8279e4506c6bbcd44856756b6d6521af809281..d3e24b47b3eab20391dd390c9b6a21b3fc2a1981 100644 --- a/crates/project/src/project_search.rs +++ b/crates/project/src/project_search.rs @@ -23,7 +23,7 @@ use smol::{ }; use text::BufferId; -use util::{ResultExt, maybe, paths::compare_rel_paths}; +use util::{ResultExt, maybe, paths::compare_rel_paths, rel_path::RelPath}; use worktree::{Entry, ProjectEntryId, Snapshot, Worktree, WorktreeSettings}; use crate::{ @@ -267,10 +267,6 @@ impl Search { .spawn(async move |cx| { let _ = maybe!(async move { let response = request.await?; - log::error!( - "Received {} match candidates for a project search", - response.buffer_ids.len() - ); for buffer_id in response.buffer_ids { let buffer_id = BufferId::new(buffer_id)?; let buffer = buffer_store @@ -547,7 +543,7 @@ impl Search { .filter(|buffer| { let b = buffer.read(cx); if let Some(file) = b.file() { - if !search_query.match_path(file.path().as_std_path()) { + if !search_query.match_path(file.path()) { return false; } if !search_query.include_ignored() @@ -744,11 +740,11 @@ impl RequestHandler<'_> { if self.query.filters_path() { let matched_path = if self.query.match_full_paths() { - let mut full_path = snapshot.root_name().as_std_path().to_owned(); - full_path.push(entry.path.as_std_path()); + let mut full_path = snapshot.root_name().to_owned(); + full_path.push(&entry.path); self.query.match_path(&full_path) } else { - self.query.match_path(entry.path.as_std_path()) + self.query.match_path(&entry.path) }; if !matched_path { return Ok(()); @@ -815,7 +811,6 @@ impl PathInclusionMatcher { query .files_to_include() .sources() - .iter() .flat_map(|glob| Some(wax::Glob::new(glob).ok()?.partition().0)), ); } @@ -842,10 +837,10 @@ impl PathInclusionMatcher { } let as_abs_path = LazyCell::new(move || snapshot.absolutize(&entry.path)); - let entry_path = entry.path.as_std_path(); + let entry_path = &entry.path; // 3. Check Exclusions (Pruning) // If the current path is a child of an excluded path, we stop. - let is_excluded = self.path_is_definitely_excluded(entry_path, snapshot); + let is_excluded = self.path_is_definitely_excluded(&entry_path, snapshot); if is_excluded { return false; @@ -865,10 +860,12 @@ impl PathInclusionMatcher { as_abs_path.starts_with(prefix), ) } else { - ( - prefix.starts_with(entry_path), - entry_path.starts_with(prefix), - ) + RelPath::new(prefix, snapshot.path_style()).map_or((false, false), |prefix| { + ( + prefix.starts_with(entry_path), + entry_path.starts_with(&prefix), + ) + }) }; // Logic: @@ -879,10 +876,10 @@ impl PathInclusionMatcher { is_included } - fn path_is_definitely_excluded(&self, path: &Path, snapshot: &Snapshot) -> bool { - if !self.query.files_to_exclude().sources().is_empty() { + fn path_is_definitely_excluded(&self, path: &RelPath, snapshot: &Snapshot) -> bool { + if !self.query.files_to_exclude().sources().next().is_none() { let mut path = if self.query.match_full_paths() { - let mut full_path = snapshot.root_name().as_std_path().to_owned(); + let mut full_path = snapshot.root_name().to_owned(); full_path.push(path); full_path } else { diff --git a/crates/project/src/search.rs b/crates/project/src/search.rs index 9460540729aab4ab75022594fddeeb01df7919e5..bb37ba2111db459b808434d58fa9a9c4d973c0b1 100644 --- a/crates/project/src/search.rs +++ b/crates/project/src/search.rs @@ -3,17 +3,20 @@ use anyhow::Result; use client::proto; use fancy_regex::{Captures, Regex, RegexBuilder}; use gpui::Entity; +use itertools::Itertools as _; use language::{Buffer, BufferSnapshot, CharKind}; use smol::future::yield_now; use std::{ borrow::Cow, io::{BufRead, BufReader, Read}, ops::Range, - path::Path, sync::{Arc, LazyLock}, }; use text::Anchor; -use util::paths::{PathMatcher, PathStyle}; +use util::{ + paths::{PathMatcher, PathStyle}, + rel_path::RelPath, +}; #[derive(Debug)] pub enum SearchResult { @@ -306,16 +309,16 @@ impl SearchQuery { } pub fn to_proto(&self) -> proto::SearchQuery { - let files_to_include = self.files_to_include().sources().to_vec(); - let files_to_exclude = self.files_to_exclude().sources().to_vec(); + let mut files_to_include = self.files_to_include().sources(); + let mut files_to_exclude = self.files_to_exclude().sources(); proto::SearchQuery { query: self.as_str().to_string(), regex: self.is_regex(), whole_word: self.whole_word(), case_sensitive: self.case_sensitive(), include_ignored: self.include_ignored(), - files_to_include: files_to_include.clone(), - files_to_exclude: files_to_exclude.clone(), + files_to_include: files_to_include.clone().map(ToOwned::to_owned).collect(), + files_to_exclude: files_to_exclude.clone().map(ToOwned::to_owned).collect(), match_full_paths: self.match_full_paths(), // Populate legacy fields for backwards compatibility files_to_include_legacy: files_to_include.join(","), @@ -551,8 +554,8 @@ impl SearchQuery { } pub fn filters_path(&self) -> bool { - !(self.files_to_exclude().sources().is_empty() - && self.files_to_include().sources().is_empty()) + !(self.files_to_exclude().sources().next().is_none() + && self.files_to_include().sources().next().is_none()) } pub fn match_full_paths(&self) -> bool { @@ -561,12 +564,12 @@ impl SearchQuery { /// Check match full paths to determine whether you're required to pass a fully qualified /// project path (starts with a project root). - pub fn match_path(&self, file_path: &Path) -> bool { - let mut path = file_path.to_path_buf(); + pub fn match_path(&self, file_path: &RelPath) -> bool { + let mut path = file_path.to_rel_path_buf(); loop { if self.files_to_exclude().is_match(&path) { return false; - } else if self.files_to_include().sources().is_empty() + } else if self.files_to_include().sources().next().is_none() || self.files_to_include().is_match(&path) { return true; @@ -608,14 +611,14 @@ mod tests { "~/dir/another_dir/", "./dir/file", "dir/[a-z].txt", - "../dir/filé", ] { let path_matcher = PathMatcher::new(&[valid_path.to_owned()], PathStyle::local()) .unwrap_or_else(|e| { panic!("Valid path {valid_path} should be accepted, but got: {e}") }); assert!( - path_matcher.is_match(valid_path), + path_matcher + .is_match(&RelPath::new(valid_path.as_ref(), PathStyle::local()).unwrap()), "Path matcher for valid path {valid_path} should match itself" ) } diff --git a/crates/search/Cargo.toml b/crates/search/Cargo.toml index 291257e74258359356203955cec3a0a6d065b3fa..ac07e43fb0317bbe4e11fde98fc3ccbe886baf9d 100644 --- a/crates/search/Cargo.toml +++ b/crates/search/Cargo.toml @@ -42,6 +42,7 @@ util.workspace = true util_macros.workspace = true workspace.workspace = true zed_actions.workspace = true +itertools.workspace = true [dev-dependencies] client = { workspace = true, features = ["test-support"] } diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index 911e9ec34e2dcd93d76fcd3365539fbe02173064..abc36a1667fc053b0833ef989006bb6b083b66da 100644 --- a/crates/search/src/project_search.rs +++ b/crates/search/src/project_search.rs @@ -21,6 +21,7 @@ use gpui::{ Global, Hsla, InteractiveElement, IntoElement, KeyContext, ParentElement, Point, Render, SharedString, Styled, Subscription, Task, UpdateGlobal, WeakEntity, Window, actions, div, }; +use itertools::Itertools; use language::{Buffer, Language}; use menu::Confirm; use project::{ diff --git a/crates/util/src/paths.rs b/crates/util/src/paths.rs index 0834039e0e59ff4149614ad863bd7a07b4a2efd7..f8e3e557152a24a6be8bb4cdad3a86d2256a764e 100644 --- a/crates/util/src/paths.rs +++ b/crates/util/src/paths.rs @@ -1,5 +1,5 @@ use anyhow::Context; -use globset::{Glob, GlobSet, GlobSetBuilder}; +use globset::{GlobBuilder, GlobSet, GlobSetBuilder}; use itertools::Itertools; use regex::Regex; use serde::{Deserialize, Serialize}; @@ -16,6 +16,7 @@ use std::{ sync::LazyLock, }; +use crate::rel_path::RelPathBuf; use crate::{rel_path::RelPath, shell::ShellKind}; static HOME_DIR: OnceLock = OnceLock::new(); @@ -346,10 +347,21 @@ impl PathStyle { } } + pub fn separators_ch(&self) -> &'static [char] { + match self { + PathStyle::Posix => &['/'], + PathStyle::Windows => &['\\', '/'], + } + } + pub fn is_windows(&self) -> bool { *self == PathStyle::Windows } + pub fn is_posix(&self) -> bool { + *self == PathStyle::Posix + } + pub fn join(self, left: impl AsRef, right: impl AsRef) -> Option { let right = right.as_ref().to_str()?; if is_absolute(right, self) { @@ -769,17 +781,11 @@ impl PathWithPosition { #[derive(Clone, Debug)] pub struct PathMatcher { - sources: Vec, + sources: Vec<(String, RelPathBuf, /*trailing separator*/ bool)>, glob: GlobSet, path_style: PathStyle, } -// impl std::fmt::Display for PathMatcher { -// fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { -// self.sources.fmt(f) -// } -// } - impl PartialEq for PathMatcher { fn eq(&self, other: &Self) -> bool { self.sources.eq(&other.sources) @@ -795,9 +801,25 @@ impl PathMatcher { ) -> Result { let globs = globs .into_iter() - .map(|as_str| Glob::new(as_str.as_ref())) + .map(|as_str| { + GlobBuilder::new(as_str.as_ref()) + .backslash_escape(path_style.is_posix()) + .build() + }) .collect::, _>>()?; - let sources = globs.iter().map(|glob| glob.glob().to_owned()).collect(); + let sources = globs + .iter() + .filter_map(|glob| { + let glob = glob.glob(); + Some(( + glob.to_string(), + RelPath::new(&glob.as_ref(), path_style) + .ok() + .map(std::borrow::Cow::into_owned)?, + glob.ends_with(path_style.separators_ch()), + )) + }) + .collect(); let mut glob_builder = GlobSetBuilder::new(); for single_glob in globs { glob_builder.add(single_glob); @@ -810,27 +832,24 @@ impl PathMatcher { }) } - pub fn sources(&self) -> &[String] { - &self.sources + pub fn sources(&self) -> impl Iterator + Clone { + self.sources.iter().map(|(source, ..)| source.as_str()) } - pub fn is_match>(&self, other: P) -> bool { - let other_path = other.as_ref(); - self.sources.iter().any(|source| { - let as_bytes = other_path.as_os_str().as_encoded_bytes(); - as_bytes.starts_with(source.as_bytes()) || as_bytes.ends_with(source.as_bytes()) - }) || self.glob.is_match(other_path) - || self.check_with_end_separator(other_path) - } + pub fn is_match>(&self, other: P) -> bool { + if self.sources.iter().any(|(_, source, _)| { + other.as_ref().starts_with(source) || other.as_ref().ends_with(source) + }) { + return true; + } + let other_path = other.as_ref().display(self.path_style); - fn check_with_end_separator(&self, path: &Path) -> bool { - let path_str = path.to_string_lossy(); - let separator = self.path_style.primary_separator(); - if path_str.ends_with(separator) { - false - } else { - self.glob.is_match(path_str.to_string() + separator) + if self.glob.is_match(&*other_path) { + return true; } + + self.glob + .is_match(other_path.into_owned() + self.path_style.primary_separator()) } } @@ -2068,42 +2087,41 @@ mod tests { } #[perf] - fn edge_of_glob() { - let path = Path::new("/work/node_modules"); - let path_matcher = - PathMatcher::new(&["**/node_modules/**".to_owned()], PathStyle::Posix).unwrap(); - assert!( - path_matcher.is_match(path), - "Path matcher should match {path:?}" - ); - } - - #[perf] - fn file_in_dirs() { - let path = Path::new("/work/.env"); - let path_matcher = PathMatcher::new(&["**/.env".to_owned()], PathStyle::Posix).unwrap(); - assert!( - path_matcher.is_match(path), - "Path matcher should match {path:?}" - ); - let path = Path::new("/work/package.json"); - assert!( - !path_matcher.is_match(path), - "Path matcher should not match {path:?}" - ); - } - - #[perf] - fn project_search() { - let path = Path::new("/Users/someonetoignore/work/zed/zed.dev/node_modules"); - let path_matcher = - PathMatcher::new(&["**/node_modules/**".to_owned()], PathStyle::Posix).unwrap(); - assert!( - path_matcher.is_match(path), - "Path matcher should match {path:?}" - ); - } - + // fn edge_of_glob() { + // let path = Path::new("/work/node_modules"); + // let path_matcher = + // PathMatcher::new(&["**/node_modules/**".to_owned()], PathStyle::Posix).unwrap(); + // assert!( + // path_matcher.is_match(path), + // "Path matcher should match {path:?}" + // ); + // } + + // #[perf] + // fn file_in_dirs() { + // let path = Path::new("/work/.env"); + // let path_matcher = PathMatcher::new(&["**/.env".to_owned()], PathStyle::Posix).unwrap(); + // assert!( + // path_matcher.is_match(path), + // "Path matcher should match {path:?}" + // ); + // let path = Path::new("/work/package.json"); + // assert!( + // !path_matcher.is_match(path), + // "Path matcher should not match {path:?}" + // ); + // } + + // #[perf] + // fn project_search() { + // let path = Path::new("/Users/someonetoignore/work/zed/zed.dev/node_modules"); + // let path_matcher = + // PathMatcher::new(&["**/node_modules/**".to_owned()], PathStyle::Posix).unwrap(); + // assert!( + // path_matcher.is_match(path), + // "Path matcher should match {path:?}" + // ); + // } #[perf] #[cfg(target_os = "windows")] fn test_sanitized_path() { diff --git a/crates/util/src/rel_path.rs b/crates/util/src/rel_path.rs index 60a0e2ef9ef51ee579a30fac57486b0040c42227..8b6ae6e49a67ae1ef2d21b3b1e0c4708f407a5e3 100644 --- a/crates/util/src/rel_path.rs +++ b/crates/util/src/rel_path.rs @@ -27,7 +27,7 @@ pub struct RelPath(str); /// relative and normalized. /// /// This type is to [`RelPath`] as [`std::path::PathBuf`] is to [`std::path::Path`] -#[derive(Clone, Serialize, Deserialize)] +#[derive(PartialEq, Eq, Clone, Serialize, Deserialize)] pub struct RelPathBuf(String); impl RelPath { @@ -228,7 +228,8 @@ impl RelPath { pub fn display(&self, style: PathStyle) -> Cow<'_, str> { match style { PathStyle::Posix => Cow::Borrowed(&self.0), - PathStyle::Windows => Cow::Owned(self.0.replace('/', "\\")), + PathStyle::Windows if self.0.contains('/') => Cow::Owned(self.0.replace('/', "\\")), + PathStyle::Windows => Cow::Borrowed(&self.0), } } @@ -341,6 +342,12 @@ impl AsRef for RelPathBuf { } } +impl AsRef for RelPath { + fn as_ref(&self) -> &RelPath { + self + } +} + impl Deref for RelPathBuf { type Target = RelPath; diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index a62f1b3cd1305a4e396a9fb0dd6b2f3212a321b6..152277af2e3f626aa0da608af275505b04d0af32 100644 --- a/crates/worktree/src/worktree.rs +++ b/crates/worktree/src/worktree.rs @@ -5509,7 +5509,7 @@ impl TryFrom<(&CharBag, &PathMatcher, proto::Entry)> for Entry { let path = RelPath::from_proto(&entry.path).context("invalid relative path in proto message")?; let char_bag = char_bag_for_path(*root_char_bag, &path); - let is_always_included = always_included.is_match(path.as_std_path()); + let is_always_included = always_included.is_match(&path); Ok(Entry { id: ProjectEntryId::from_proto(entry.id), kind, diff --git a/crates/worktree/src/worktree_settings.rs b/crates/worktree/src/worktree_settings.rs index 97723829dd78a3dab517634971f8d0753500aa4b..a86720184ebf6d33755decf415ad97bdcfd7fd8c 100644 --- a/crates/worktree/src/worktree_settings.rs +++ b/crates/worktree/src/worktree_settings.rs @@ -25,25 +25,25 @@ pub struct WorktreeSettings { impl WorktreeSettings { pub fn is_path_private(&self, path: &RelPath) -> bool { path.ancestors() - .any(|ancestor| self.private_files.is_match(ancestor.as_std_path())) + .any(|ancestor| self.private_files.is_match(ancestor)) } pub fn is_path_excluded(&self, path: &RelPath) -> bool { path.ancestors() - .any(|ancestor| self.file_scan_exclusions.is_match(ancestor.as_std_path())) + .any(|ancestor| self.file_scan_exclusions.is_match(ancestor)) } pub fn is_path_always_included(&self, path: &RelPath, is_dir: bool) -> bool { if is_dir { - self.parent_dir_scan_inclusions.is_match(path.as_std_path()) + self.parent_dir_scan_inclusions.is_match(path) } else { - self.file_scan_inclusions.is_match(path.as_std_path()) + self.file_scan_inclusions.is_match(path) } } pub fn is_path_hidden(&self, path: &RelPath) -> bool { path.ancestors() - .any(|ancestor| self.hidden_files.is_match(ancestor.as_std_path())) + .any(|ancestor| self.hidden_files.is_match(ancestor)) } } diff --git a/crates/zeta/src/retrieval_search.rs b/crates/zeta/src/retrieval_search.rs index fd7364cf23ac66fe9baf2f911868ef251d2d25cf..bcc0233ff7e872a151ecddf2cf55a3cb434f02b3 100644 --- a/crates/zeta/src/retrieval_search.rs +++ b/crates/zeta/src/retrieval_search.rs @@ -87,8 +87,7 @@ pub async fn run_retrieval_searches( let exclude_patterns = global_settings .file_scan_exclusions .sources() - .iter() - .chain(global_settings.private_files.sources().iter()); + .chain(global_settings.private_files.sources()); let path_style = project.path_style(cx); anyhow::Ok((PathMatcher::new(exclude_patterns, path_style)?, path_style)) })??;