Rename regex search tool to grep and accept an include glob pattern (#29100)

This PR renames the `regex_search` tool to `grep` because I think it
conveys more meaning to the model, the idea of searching the filesystem
with a regular expression. It's also one word and the model seems to be
using it effectively after some additional prompt tuning.

It also takes an include pattern to filter on the specific files we try
to search. I'd like to encourage the model to scope its searches more
aggressively, as in my testing, I'm only seeing it filter on file
extension.

Release Notes:

- N/A
This commit is contained in:
Nathan Sobo 2025-04-19 18:53:30 -06:00 committed by GitHub
parent 4278d894d2
commit 107d8ca483
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
29 changed files with 579 additions and 390 deletions

View file

@ -3665,7 +3665,7 @@ impl Project {
.filter(|buffer| {
let b = buffer.read(cx);
if let Some(file) = b.file() {
if !search_query.file_matches(file.path()) {
if !search_query.match_path(file.path()) {
return false;
}
if let Some(entry) = b

View file

@ -4822,6 +4822,7 @@ async fn test_search(cx: &mut gpui::TestAppContext) {
false,
Default::default(),
Default::default(),
false,
None
)
.unwrap(),
@ -4856,6 +4857,7 @@ async fn test_search(cx: &mut gpui::TestAppContext) {
false,
Default::default(),
Default::default(),
false,
None,
)
.unwrap(),
@ -4900,6 +4902,7 @@ async fn test_search_with_inclusions(cx: &mut gpui::TestAppContext) {
false,
PathMatcher::new(&["*.odd".to_owned()]).unwrap(),
Default::default(),
false,
None
)
.unwrap(),
@ -4921,6 +4924,7 @@ async fn test_search_with_inclusions(cx: &mut gpui::TestAppContext) {
false,
PathMatcher::new(&["*.rs".to_owned()]).unwrap(),
Default::default(),
false,
None
)
.unwrap(),
@ -4945,6 +4949,7 @@ async fn test_search_with_inclusions(cx: &mut gpui::TestAppContext) {
false,
PathMatcher::new(&["*.ts".to_owned(), "*.odd".to_owned()]).unwrap(),
Default::default(),
false,
None,
)
.unwrap(),
@ -4970,6 +4975,7 @@ async fn test_search_with_inclusions(cx: &mut gpui::TestAppContext) {
PathMatcher::new(&["*.rs".to_owned(), "*.ts".to_owned(), "*.odd".to_owned()])
.unwrap(),
Default::default(),
false,
None,
)
.unwrap(),
@ -5016,6 +5022,7 @@ async fn test_search_with_exclusions(cx: &mut gpui::TestAppContext) {
false,
Default::default(),
PathMatcher::new(&["*.odd".to_owned()]).unwrap(),
false,
None,
)
.unwrap(),
@ -5042,6 +5049,7 @@ async fn test_search_with_exclusions(cx: &mut gpui::TestAppContext) {
false,
Default::default(),
PathMatcher::new(&["*.rs".to_owned()]).unwrap(),
false,
None,
)
.unwrap(),
@ -5066,6 +5074,7 @@ async fn test_search_with_exclusions(cx: &mut gpui::TestAppContext) {
false,
Default::default(),
PathMatcher::new(&["*.ts".to_owned(), "*.odd".to_owned()]).unwrap(),
false,
None,
)
.unwrap(),
@ -5091,6 +5100,7 @@ async fn test_search_with_exclusions(cx: &mut gpui::TestAppContext) {
Default::default(),
PathMatcher::new(&["*.rs".to_owned(), "*.ts".to_owned(), "*.odd".to_owned()])
.unwrap(),
false,
None,
)
.unwrap(),
@ -5132,6 +5142,7 @@ async fn test_search_with_exclusions_and_inclusions(cx: &mut gpui::TestAppContex
false,
PathMatcher::new(&["*.odd".to_owned()]).unwrap(),
PathMatcher::new(&["*.odd".to_owned()]).unwrap(),
false,
None,
)
.unwrap(),
@ -5153,6 +5164,7 @@ async fn test_search_with_exclusions_and_inclusions(cx: &mut gpui::TestAppContex
false,
PathMatcher::new(&["*.ts".to_owned()]).unwrap(),
PathMatcher::new(&["*.ts".to_owned()]).unwrap(),
false,
None,
)
.unwrap(),
@ -5174,6 +5186,7 @@ async fn test_search_with_exclusions_and_inclusions(cx: &mut gpui::TestAppContex
false,
PathMatcher::new(&["*.ts".to_owned(), "*.odd".to_owned()]).unwrap(),
PathMatcher::new(&["*.ts".to_owned(), "*.odd".to_owned()]).unwrap(),
false,
None,
)
.unwrap(),
@ -5195,6 +5208,7 @@ async fn test_search_with_exclusions_and_inclusions(cx: &mut gpui::TestAppContex
false,
PathMatcher::new(&["*.ts".to_owned(), "*.odd".to_owned()]).unwrap(),
PathMatcher::new(&["*.rs".to_owned(), "*.odd".to_owned()]).unwrap(),
false,
None,
)
.unwrap(),
@ -5249,6 +5263,7 @@ async fn test_search_multiple_worktrees_with_inclusions(cx: &mut gpui::TestAppCo
false,
PathMatcher::new(&["worktree-a/*.rs".to_owned()]).unwrap(),
Default::default(),
true,
None,
)
.unwrap(),
@ -5269,6 +5284,7 @@ async fn test_search_multiple_worktrees_with_inclusions(cx: &mut gpui::TestAppCo
false,
PathMatcher::new(&["worktree-b/*.rs".to_owned()]).unwrap(),
Default::default(),
true,
None,
)
.unwrap(),
@ -5290,6 +5306,7 @@ async fn test_search_multiple_worktrees_with_inclusions(cx: &mut gpui::TestAppCo
false,
PathMatcher::new(&["*.ts".to_owned()]).unwrap(),
Default::default(),
false,
None,
)
.unwrap(),
@ -5345,6 +5362,7 @@ async fn test_search_in_gitignored_dirs(cx: &mut gpui::TestAppContext) {
false,
Default::default(),
Default::default(),
false,
None,
)
.unwrap(),
@ -5367,6 +5385,7 @@ async fn test_search_in_gitignored_dirs(cx: &mut gpui::TestAppContext) {
true,
Default::default(),
Default::default(),
false,
None,
)
.unwrap(),
@ -5410,6 +5429,7 @@ async fn test_search_in_gitignored_dirs(cx: &mut gpui::TestAppContext) {
true,
files_to_include,
files_to_exclude,
false,
None,
)
.unwrap(),
@ -5448,6 +5468,7 @@ async fn test_search_with_unicode(cx: &mut gpui::TestAppContext) {
false,
Default::default(),
Default::default(),
false,
None,
);
assert_matches!(unicode_case_sensitive_query, Ok(SearchQuery::Text { .. }));
@ -5468,6 +5489,7 @@ async fn test_search_with_unicode(cx: &mut gpui::TestAppContext) {
false,
Default::default(),
Default::default(),
false,
None,
);
assert_matches!(
@ -5495,6 +5517,7 @@ async fn test_search_with_unicode(cx: &mut gpui::TestAppContext) {
false,
Default::default(),
Default::default(),
false,
None,
)
.unwrap(),

View file

@ -36,6 +36,7 @@ pub struct SearchInputs {
query: Arc<str>,
files_to_include: PathMatcher,
files_to_exclude: PathMatcher,
match_full_paths: bool,
buffers: Option<Vec<Entity<Buffer>>>,
}
@ -83,6 +84,10 @@ static WORD_MATCH_TEST: LazyLock<Regex> = LazyLock::new(|| {
});
impl SearchQuery {
/// Create a text query
///
/// If `match_full_paths` is true, include/exclude patterns will always be matched against fully qualified project paths beginning with a project root.
/// If `match_full_paths` is false, patterns will be matched against full paths only when the project has multiple roots.
pub fn text(
query: impl ToString,
whole_word: bool,
@ -90,6 +95,7 @@ impl SearchQuery {
include_ignored: bool,
files_to_include: PathMatcher,
files_to_exclude: PathMatcher,
match_full_paths: bool,
buffers: Option<Vec<Entity<Buffer>>>,
) -> Result<Self> {
let query = query.to_string();
@ -105,6 +111,7 @@ impl SearchQuery {
false,
files_to_include,
files_to_exclude,
false,
buffers,
);
}
@ -115,6 +122,7 @@ impl SearchQuery {
query: query.into(),
files_to_exclude,
files_to_include,
match_full_paths,
buffers,
};
Ok(Self::Text {
@ -127,6 +135,11 @@ impl SearchQuery {
})
}
/// Create a regex query
///
/// If `match_full_paths` is true, include/exclude patterns will be matched against fully qualified project paths
/// beginning with a project root name. If false, they will be matched against project-relative paths (which don't start
/// with their respective project root).
pub fn regex(
query: impl ToString,
whole_word: bool,
@ -135,6 +148,7 @@ impl SearchQuery {
one_match_per_line: bool,
files_to_include: PathMatcher,
files_to_exclude: PathMatcher,
match_full_paths: bool,
buffers: Option<Vec<Entity<Buffer>>>,
) -> Result<Self> {
let mut query = query.to_string();
@ -163,6 +177,7 @@ impl SearchQuery {
query: initial_query,
files_to_exclude,
files_to_include,
match_full_paths,
buffers,
};
Ok(Self::Regex {
@ -187,6 +202,7 @@ impl SearchQuery {
false,
deserialize_path_matches(&message.files_to_include)?,
deserialize_path_matches(&message.files_to_exclude)?,
message.match_full_paths,
None, // search opened only don't need search remote
)
} else {
@ -197,6 +213,7 @@ impl SearchQuery {
message.include_ignored,
deserialize_path_matches(&message.files_to_include)?,
deserialize_path_matches(&message.files_to_exclude)?,
false,
None, // search opened only don't need search remote
)
}
@ -227,6 +244,7 @@ impl SearchQuery {
include_ignored: self.include_ignored(),
files_to_include: self.files_to_include().sources().join(","),
files_to_exclude: self.files_to_exclude().sources().join(","),
match_full_paths: self.match_full_paths(),
}
}
@ -459,7 +477,13 @@ impl SearchQuery {
&& self.files_to_include().sources().is_empty())
}
pub fn file_matches(&self, file_path: &Path) -> bool {
pub fn match_full_paths(&self) -> bool {
self.as_inner().match_full_paths
}
/// 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();
loop {
if self.files_to_exclude().is_match(&path) {

View file

@ -734,7 +734,6 @@ impl WorktreeStore {
snapshot: &'a worktree::Snapshot,
path: &'a Path,
query: &'a SearchQuery,
include_root: bool,
filter_tx: &'a Sender<MatchingEntry>,
output_tx: &'a Sender<oneshot::Receiver<ProjectPath>>,
) -> BoxFuture<'a, Result<()>> {
@ -773,12 +772,12 @@ impl WorktreeStore {
for (path, is_file) in results {
if is_file {
if query.filters_path() {
let matched_path = if include_root {
let matched_path = if query.match_full_paths() {
let mut full_path = PathBuf::from(snapshot.root_name());
full_path.push(&path);
query.file_matches(&full_path)
query.match_path(&full_path)
} else {
query.file_matches(&path)
query.match_path(&path)
};
if !matched_path {
continue;
@ -797,16 +796,8 @@ impl WorktreeStore {
})
.await?;
} else {
Self::scan_ignored_dir(
fs,
snapshot,
&path,
query,
include_root,
filter_tx,
output_tx,
)
.await?;
Self::scan_ignored_dir(fs, snapshot, &path, query, filter_tx, output_tx)
.await?;
}
}
Ok(())
@ -822,7 +813,6 @@ impl WorktreeStore {
filter_tx: Sender<MatchingEntry>,
output_tx: Sender<oneshot::Receiver<ProjectPath>>,
) -> Result<()> {
let include_root = snapshots.len() > 1;
for (snapshot, settings) in snapshots {
for entry in snapshot.entries(query.include_ignored(), 0) {
if entry.is_dir() && entry.is_ignored {
@ -832,7 +822,6 @@ impl WorktreeStore {
&snapshot,
&entry.path,
&query,
include_root,
&filter_tx,
&output_tx,
)
@ -846,12 +835,12 @@ impl WorktreeStore {
}
if query.filters_path() {
let matched_path = if include_root {
let matched_path = if query.match_full_paths() {
let mut full_path = PathBuf::from(snapshot.root_name());
full_path.push(&entry.path);
query.file_matches(&full_path)
query.match_path(&full_path)
} else {
query.file_matches(&entry.path)
query.match_path(&entry.path)
};
if !matched_path {
continue;