From 107d8ca483276263362c23482612d1cadea81c71 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sat, 19 Apr 2025 18:53:30 -0600 Subject: [PATCH] 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 --- assets/prompts/assistant_system_prompt.hbs | 32 +- assets/settings/default.json | 155 ++----- crates/assistant_tools/src/assistant_tools.rs | 6 +- crates/assistant_tools/src/batch_tool.rs | 4 +- crates/assistant_tools/src/grep_tool.rs | 424 ++++++++++++++++++ .../src/grep_tool/description.md | 8 + .../src/list_directory_tool/description.md | 2 +- .../src/path_search_tool/description.md | 2 +- .../assistant_tools/src/regex_search_tool.rs | 206 --------- .../src/regex_search_tool/description.md | 6 - crates/assistant_tools/src/replace.rs | 1 + crates/collab/src/tests/integration_tests.rs | 1 + .../random_project_collaboration_tests.rs | 1 + .../src/chat_panel/message_editor.rs | 1 + crates/editor/src/editor.rs | 1 + .../thread_criteria.md | 2 +- crates/eval/src/example.rs | 2 +- crates/gpui/src/app/test_context.rs | 9 +- crates/gpui/src/text_system/line_wrapper.rs | 2 +- crates/gpui_macros/src/test.rs | 6 +- crates/project/src/project.rs | 2 +- crates/project/src/project_tests.rs | 23 + crates/project/src/search.rs | 26 +- crates/project/src/worktree_store.rs | 27 +- crates/proto/proto/buffer.proto | 1 + .../remote_server/src/remote_editing_tests.rs | 1 + crates/search/src/buffer_search.rs | 2 + crates/search/src/project_search.rs | 15 + crates/terminal_view/src/terminal_view.rs | 1 + 29 files changed, 579 insertions(+), 390 deletions(-) create mode 100644 crates/assistant_tools/src/grep_tool.rs create mode 100644 crates/assistant_tools/src/grep_tool/description.md delete mode 100644 crates/assistant_tools/src/regex_search_tool.rs delete mode 100644 crates/assistant_tools/src/regex_search_tool/description.md diff --git a/assets/prompts/assistant_system_prompt.hbs b/assets/prompts/assistant_system_prompt.hbs index be186f1738c7882dd9a3c50c7e0a6eecd53d1cb2..eb91c4f4a8066cb044692653fe88a6675acc16fe 100644 --- a/assets/prompts/assistant_system_prompt.hbs +++ b/assets/prompts/assistant_system_prompt.hbs @@ -8,16 +8,6 @@ You are a highly skilled software engineer with extensive knowledge in many prog 4. NEVER lie or make things up. 5. Refrain from apologizing all the time when results are unexpected. Instead, just try your best to proceed or explain the circumstances to the user without apologizing. -## Searching and Reading - -If you are unsure about the answer to the user's request or how to satiate their request, you should gather more information. -This can be done with additional tool calls, asking clarifying questions, etc. - -For example, if you've performed a semantic search, and the results may not fully answer the user's request, or merit gathering more information, feel free to call more tools. Similarly, if you've performed an edit that may partially -satiate the user's query, but you're not confident, gather more information or use more tools before ending your turn. - -Bias towards not asking the user for help if you can find the answer yourself. - ## Tool Use 1. Make sure to adhere to the tools schema. @@ -26,6 +16,22 @@ Bias towards not asking the user for help if you can find the answer yourself. 4. Use only the tools that are currently available. 5. DO NOT use a tool that is not available just because it appears in the conversation. This means the user turned it off. +## Searching and Reading + +If you are unsure how to fulfill the user's request, gather more information with tool calls and/or clarifying questions. + +{{! TODO: If there are files, we should mention it but otherwise omit that fact }} +If appropriate, use tool calls to explore the current project, which contains the following root directories: + +{{#each worktrees}} +- `{{root_name}}` +{{/each}} + +- When providing paths to tools, the path should always begin with a path that starts with a project root directory listed above. +- When looking for symbols in the project, prefer the `grep` tool. +- As you learn about the structure of the project, use that information to scope `grep` searches to targeted subtrees of the project. +- Bias towards not asking the user for help if you can find the answer yourself. + ## Fixing Diagnostics 1. Make 1-2 attempts at fixing diagnostics, then defer to the user. @@ -50,12 +56,6 @@ Otherwise, follow debugging best practices: Operating System: {{os}} Default Shell: {{shell}} -The user has opened a project that contains the following root directories/files. Whenever you specify a path in the project, it must be a relative path which begins with one of these root directories/files: - -{{#each worktrees}} -- `{{root_name}}` -{{/each}} - {{#if (or has_rules has_default_user_rules)}} ## User's Custom Instructions diff --git a/assets/settings/default.json b/assets/settings/default.json index 069a9b910fd6c9aa671630e524d9d059b21412e6..d93a6d39fc3553cb15725ae5d5ec67bf14cdc412 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -212,14 +212,7 @@ // The default number of lines to expand excerpts in the multibuffer by. "expand_excerpt_lines": 3, // Globs to match against file paths to determine if a file is private. - "private_files": [ - "**/.env*", - "**/*.pem", - "**/*.key", - "**/*.cert", - "**/*.crt", - "**/secrets.yml" - ], + "private_files": ["**/.env*", "**/*.pem", "**/*.key", "**/*.cert", "**/*.crt", "**/secrets.yml"], // Whether to use additional LSP queries to format (and amend) the code after // every "trigger" symbol input, defined by LSP server capabilities. "use_on_type_format": true, @@ -655,7 +648,7 @@ "now": true, "path_search": true, "read_file": true, - "regex_search": true, + "grep": true, "thinking": true, "web_search": true } @@ -679,7 +672,7 @@ "now": false, "path_search": true, "read_file": true, - "regex_search": true, + "grep": true, "rename": false, "symbol_info": false, "terminal": true, @@ -719,9 +712,7 @@ // The list of language servers to use (or disable) for all languages. // // This is typically customized on a per-language basis. - "language_servers": [ - "..." - ], + "language_servers": ["..."], // When to automatically save edited buffers. This setting can // take four values. // @@ -917,9 +908,7 @@ // for files that are not tracked by git, but are still important to your project. Note that globs // that are overly broad can slow down Zed's file scanning. `file_scan_exclusions` takes // precedence over these inclusions. - "file_scan_inclusions": [ - ".env*" - ], + "file_scan_inclusions": [".env*"], // Git gutter behavior configuration. "git": { // Control whether the git gutter is shown. May take 2 values: @@ -971,15 +960,7 @@ // Any addition to this list will be merged with the default list. // Globs are matched relative to the worktree root, // except when starting with a slash (/) or equivalent in Windows. - "disabled_globs": [ - "**/.env*", - "**/*.pem", - "**/*.key", - "**/*.cert", - "**/*.crt", - "**/.dev.vars", - "**/secrets.yml" - ], + "disabled_globs": ["**/.env*", "**/*.pem", "**/*.key", "**/*.cert", "**/*.crt", "**/.dev.vars", "**/secrets.yml"], // When to show edit predictions previews in buffer. // This setting takes two possible values: // 1. Display predictions inline when there are no language server completions available. @@ -1112,12 +1093,7 @@ // Default directories to search for virtual environments, relative // to the current working directory. We recommend overriding this // in your project's settings, rather than globally. - "directories": [ - ".env", - "env", - ".venv", - "venv" - ], + "directories": [".env", "env", ".venv", "venv"], // Can also be `csh`, `fish`, `nushell` and `power_shell` "activate_script": "default" } @@ -1181,15 +1157,8 @@ // } // "file_types": { - "JSONC": [ - "**/.zed/**/*.json", - "**/zed/**/*.json", - "**/Zed/**/*.json", - "**/.vscode/**/*.json" - ], - "Shell Script": [ - ".env.*" - ] + "JSONC": ["**/.zed/**/*.json", "**/zed/**/*.json", "**/Zed/**/*.json", "**/.vscode/**/*.json"], + "Shell Script": [".env.*"] }, // By default use a recent system version of node, or install our own. // You can override this to use a version of node that is not in $PATH with: @@ -1262,15 +1231,10 @@ // Different settings for specific languages. "languages": { "Astro": { - "language_servers": [ - "astro-language-server", - "..." - ], + "language_servers": ["astro-language-server", "..."], "prettier": { "allowed": true, - "plugins": [ - "prettier-plugin-astro" - ] + "plugins": ["prettier-plugin-astro"] } }, "Blade": { @@ -1306,19 +1270,10 @@ "ensure_final_newline_on_save": false }, "Elixir": { - "language_servers": [ - "elixir-ls", - "!next-ls", - "!lexical", - "..." - ] + "language_servers": ["elixir-ls", "!next-ls", "!lexical", "..."] }, "Erlang": { - "language_servers": [ - "erlang-ls", - "!elp", - "..." - ] + "language_servers": ["erlang-ls", "!elp", "..."] }, "Git Commit": { "allow_rewrap": "anywhere" @@ -1334,12 +1289,7 @@ } }, "HEEX": { - "language_servers": [ - "elixir-ls", - "!next-ls", - "!lexical", - "..." - ] + "language_servers": ["elixir-ls", "!next-ls", "!lexical", "..."] }, "HTML": { "prettier": { @@ -1349,17 +1299,11 @@ "Java": { "prettier": { "allowed": true, - "plugins": [ - "prettier-plugin-java" - ] + "plugins": ["prettier-plugin-java"] } }, "JavaScript": { - "language_servers": [ - "!typescript-language-server", - "vtsls", - "..." - ], + "language_servers": ["!typescript-language-server", "vtsls", "..."], "prettier": { "allowed": true } @@ -1377,10 +1321,7 @@ "LaTeX": { "format_on_save": "on", "formatter": "language_server", - "language_servers": [ - "texlab", - "..." - ], + "language_servers": ["texlab", "..."], "prettier": { "allowed": false } @@ -1395,16 +1336,10 @@ } }, "PHP": { - "language_servers": [ - "phpactor", - "!intelephense", - "..." - ], + "language_servers": ["phpactor", "!intelephense", "..."], "prettier": { "allowed": true, - "plugins": [ - "@prettier/plugin-php" - ], + "plugins": ["@prettier/plugin-php"], "parser": "php" } }, @@ -1412,12 +1347,7 @@ "allow_rewrap": "anywhere" }, "Ruby": { - "language_servers": [ - "solargraph", - "!ruby-lsp", - "!rubocop", - "..." - ] + "language_servers": ["solargraph", "!ruby-lsp", "!rubocop", "..."] }, "SCSS": { "prettier": { @@ -1427,36 +1357,21 @@ "SQL": { "prettier": { "allowed": true, - "plugins": [ - "prettier-plugin-sql" - ] + "plugins": ["prettier-plugin-sql"] } }, "Starlark": { - "language_servers": [ - "starpls", - "!buck2-lsp", - "..." - ] + "language_servers": ["starpls", "!buck2-lsp", "..."] }, "Svelte": { - "language_servers": [ - "svelte-language-server", - "..." - ], + "language_servers": ["svelte-language-server", "..."], "prettier": { "allowed": true, - "plugins": [ - "prettier-plugin-svelte" - ] + "plugins": ["prettier-plugin-svelte"] } }, "TSX": { - "language_servers": [ - "!typescript-language-server", - "vtsls", - "..." - ], + "language_servers": ["!typescript-language-server", "vtsls", "..."], "prettier": { "allowed": true } @@ -1467,20 +1382,13 @@ } }, "TypeScript": { - "language_servers": [ - "!typescript-language-server", - "vtsls", - "..." - ], + "language_servers": ["!typescript-language-server", "vtsls", "..."], "prettier": { "allowed": true } }, "Vue.js": { - "language_servers": [ - "vue-language-server", - "..." - ], + "language_servers": ["vue-language-server", "..."], "prettier": { "allowed": true } @@ -1488,9 +1396,7 @@ "XML": { "prettier": { "allowed": true, - "plugins": [ - "@prettier/plugin-xml" - ] + "plugins": ["@prettier/plugin-xml"] } }, "YAML": { @@ -1499,10 +1405,7 @@ } }, "Zig": { - "language_servers": [ - "zls", - "..." - ] + "language_servers": ["zls", "..."] } }, // Different settings for specific language models. diff --git a/crates/assistant_tools/src/assistant_tools.rs b/crates/assistant_tools/src/assistant_tools.rs index b68a273b4e4090aec0dfa53abcc6d61bf3a8027c..92e388407c6893344a036a9ab83e0ce2e80a6334 100644 --- a/crates/assistant_tools/src/assistant_tools.rs +++ b/crates/assistant_tools/src/assistant_tools.rs @@ -9,13 +9,13 @@ mod delete_path_tool; mod diagnostics_tool; mod edit_file_tool; mod fetch_tool; +mod grep_tool; mod list_directory_tool; mod move_path_tool; mod now_tool; mod open_tool; mod path_search_tool; mod read_file_tool; -mod regex_search_tool; mod rename_tool; mod replace; mod schema; @@ -44,12 +44,12 @@ use crate::delete_path_tool::DeletePathTool; use crate::diagnostics_tool::DiagnosticsTool; use crate::edit_file_tool::EditFileTool; use crate::fetch_tool::FetchTool; +use crate::grep_tool::GrepTool; use crate::list_directory_tool::ListDirectoryTool; use crate::now_tool::NowTool; use crate::open_tool::OpenTool; use crate::path_search_tool::PathSearchTool; use crate::read_file_tool::ReadFileTool; -use crate::regex_search_tool::RegexSearchTool; use crate::rename_tool::RenameTool; use crate::symbol_info_tool::SymbolInfoTool; use crate::terminal_tool::TerminalTool; @@ -77,7 +77,7 @@ pub fn init(http_client: Arc, cx: &mut App) { registry.register_tool(ContentsTool); registry.register_tool(PathSearchTool); registry.register_tool(ReadFileTool); - registry.register_tool(RegexSearchTool); + registry.register_tool(GrepTool); registry.register_tool(RenameTool); registry.register_tool(ThinkingTool); registry.register_tool(FetchTool::new(http_client)); diff --git a/crates/assistant_tools/src/batch_tool.rs b/crates/assistant_tools/src/batch_tool.rs index bfd27845e406ef5fdcc46d16ff59514a55908068..6e4caf4a103e65de121a525e135584a39474dba8 100644 --- a/crates/assistant_tools/src/batch_tool.rs +++ b/crates/assistant_tools/src/batch_tool.rs @@ -43,7 +43,7 @@ pub struct BatchToolInput { /// } /// }, /// { - /// "name": "regex_search", + /// "name": "grep", /// "input": { /// "regex": "fn run\\(" /// } @@ -91,7 +91,7 @@ pub struct BatchToolInput { /// { /// "invocations": [ /// { - /// "name": "regex_search", + /// "name": "grep", /// "input": { /// "regex": "impl Database" /// } diff --git a/crates/assistant_tools/src/grep_tool.rs b/crates/assistant_tools/src/grep_tool.rs new file mode 100644 index 0000000000000000000000000000000000000000..45f3cacf7bbb91f57824b9c71602f38cd1be02ad --- /dev/null +++ b/crates/assistant_tools/src/grep_tool.rs @@ -0,0 +1,424 @@ +use crate::schema::json_schema_for; +use anyhow::{Result, anyhow}; +use assistant_tool::{ActionLog, Tool, ToolResult}; +use futures::StreamExt; +use gpui::{App, Entity, Task}; +use language::OffsetRangeExt; +use language_model::{LanguageModelRequestMessage, LanguageModelToolSchemaFormat}; +use project::{ + Project, + search::{SearchQuery, SearchResult}, +}; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; +use std::{cmp, fmt::Write, sync::Arc}; +use ui::IconName; +use util::markdown::MarkdownString; +use util::paths::PathMatcher; + +#[derive(Debug, Serialize, Deserialize, JsonSchema)] +pub struct GrepToolInput { + /// A regex pattern to search for in the entire project. Note that the regex + /// will be parsed by the Rust `regex` crate. + pub regex: String, + + /// A glob pattern for the paths of files to include in the search. + /// Supports standard glob patterns like "**/*.rs" or "src/**/*.ts". + /// If omitted, all files in the project will be searched. + pub include_pattern: Option, + + /// Optional starting position for paginated results (0-based). + /// When not provided, starts from the beginning. + #[serde(default)] + pub offset: u32, + + /// Whether the regex is case-sensitive. Defaults to false (case-insensitive). + #[serde(default)] + pub case_sensitive: bool, +} + +impl GrepToolInput { + /// Which page of search results this is. + pub fn page(&self) -> u32 { + 1 + (self.offset / RESULTS_PER_PAGE) + } +} + +const RESULTS_PER_PAGE: u32 = 20; + +pub struct GrepTool; + +impl Tool for GrepTool { + fn name(&self) -> String { + "grep".into() + } + + fn needs_confirmation(&self, _: &serde_json::Value, _: &App) -> bool { + false + } + + fn description(&self) -> String { + include_str!("./grep_tool/description.md").into() + } + + fn icon(&self) -> IconName { + IconName::Regex + } + + fn input_schema(&self, format: LanguageModelToolSchemaFormat) -> Result { + json_schema_for::(format) + } + + fn ui_text(&self, input: &serde_json::Value) -> String { + match serde_json::from_value::(input.clone()) { + Ok(input) => { + let page = input.page(); + let regex_str = MarkdownString::inline_code(&input.regex); + let case_info = if input.case_sensitive { + " (case-sensitive)" + } else { + "" + }; + + if page > 1 { + format!("Get page {page} of search results for regex {regex_str}{case_info}") + } else { + format!("Search files for regex {regex_str}{case_info}") + } + } + Err(_) => "Search with regex".to_string(), + } + } + + fn run( + self: Arc, + input: serde_json::Value, + _messages: &[LanguageModelRequestMessage], + project: Entity, + _action_log: Entity, + cx: &mut App, + ) -> ToolResult { + const CONTEXT_LINES: u32 = 2; + + let input = match serde_json::from_value::(input) { + Ok(input) => input, + Err(error) => { + return Task::ready(Err(anyhow!("Failed to parse input: {}", error))).into(); + } + }; + + let include_matcher = match PathMatcher::new( + input + .include_pattern + .as_ref() + .into_iter() + .collect::>(), + ) { + Ok(matcher) => matcher, + Err(error) => { + return Task::ready(Err(anyhow!("invalid include glob pattern: {}", error))).into(); + } + }; + + let query = match SearchQuery::regex( + &input.regex, + false, + input.case_sensitive, + false, + false, + include_matcher, + PathMatcher::default(), // For now, keep it simple and don't enable an exclude pattern. + true, // Always match file include pattern against *full project paths* that start with a project root. + None, + ) { + Ok(query) => query, + Err(error) => return Task::ready(Err(error)).into(), + }; + + let results = project.update(cx, |project, cx| project.search(query, cx)); + + cx.spawn(async move|cx| { + futures::pin_mut!(results); + + let mut output = String::new(); + let mut skips_remaining = input.offset; + let mut matches_found = 0; + let mut has_more_matches = false; + + while let Some(SearchResult::Buffer { buffer, ranges }) = results.next().await { + if ranges.is_empty() { + continue; + } + + buffer.read_with(cx, |buffer, cx| -> Result<(), anyhow::Error> { + if let Some(path) = buffer.file().map(|file| file.full_path(cx)) { + let mut file_header_written = false; + let mut ranges = ranges + .into_iter() + .map(|range| { + let mut point_range = range.to_point(buffer); + point_range.start.row = + point_range.start.row.saturating_sub(CONTEXT_LINES); + point_range.start.column = 0; + point_range.end.row = cmp::min( + buffer.max_point().row, + point_range.end.row + CONTEXT_LINES, + ); + point_range.end.column = buffer.line_len(point_range.end.row); + point_range + }) + .peekable(); + + while let Some(mut range) = ranges.next() { + if skips_remaining > 0 { + skips_remaining -= 1; + continue; + } + + // We'd already found a full page of matches, and we just found one more. + if matches_found >= RESULTS_PER_PAGE { + has_more_matches = true; + return Ok(()); + } + + while let Some(next_range) = ranges.peek() { + if range.end.row >= next_range.start.row { + range.end = next_range.end; + ranges.next(); + } else { + break; + } + } + + if !file_header_written { + writeln!(output, "\n## Matches in {}", path.display())?; + file_header_written = true; + } + + let start_line = range.start.row + 1; + let end_line = range.end.row + 1; + writeln!(output, "\n### Lines {start_line}-{end_line}\n```")?; + output.extend(buffer.text_for_range(range)); + output.push_str("\n```\n"); + + matches_found += 1; + } + } + + Ok(()) + })??; + } + + if matches_found == 0 { + Ok("No matches found".to_string()) + } else if has_more_matches { + Ok(format!( + "Showing matches {}-{} (there were more matches found; use offset: {} to see next page):\n{output}", + input.offset + 1, + input.offset + matches_found, + input.offset + RESULTS_PER_PAGE, + )) + } else { + Ok(format!("Found {matches_found} matches:\n{output}")) + } + }).into() + } +} + +#[cfg(test)] +mod tests { + use super::*; + use assistant_tool::Tool; + use gpui::{AppContext, TestAppContext}; + use project::{FakeFs, Project}; + use settings::SettingsStore; + use util::path; + + #[gpui::test] + async fn test_grep_tool_with_include_pattern(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor().clone()); + fs.insert_tree( + "/root", + serde_json::json!({ + "src": { + "main.rs": "fn main() {\n println!(\"Hello, world!\");\n}", + "utils": { + "helper.rs": "fn helper() {\n println!(\"I'm a helper!\");\n}", + }, + }, + "tests": { + "test_main.rs": "fn test_main() {\n assert!(true);\n}", + } + }), + ) + .await; + + let project = Project::test(fs.clone(), [path!("/root").as_ref()], cx).await; + + // Test with include pattern for Rust files inside the root of the project + let input = serde_json::to_value(GrepToolInput { + regex: "println".to_string(), + include_pattern: Some("root/**/*.rs".to_string()), + offset: 0, + case_sensitive: false, + }) + .unwrap(); + + let result = run_grep_tool(input, project.clone(), cx).await; + assert!(result.contains("main.rs"), "Should find matches in main.rs"); + assert!( + result.contains("helper.rs"), + "Should find matches in helper.rs" + ); + assert!( + !result.contains("test_main.rs"), + "Should not include test_main.rs even though it's a .rs file (because it doesn't have the pattern)" + ); + + // Test with include pattern for src directory only + let input = serde_json::to_value(GrepToolInput { + regex: "fn".to_string(), + include_pattern: Some("root/**/src/**".to_string()), + offset: 0, + case_sensitive: false, + }) + .unwrap(); + + let result = run_grep_tool(input, project.clone(), cx).await; + assert!( + result.contains("main.rs"), + "Should find matches in src/main.rs" + ); + assert!( + result.contains("helper.rs"), + "Should find matches in src/utils/helper.rs" + ); + assert!( + !result.contains("test_main.rs"), + "Should not include test_main.rs as it's not in src directory" + ); + + // Test with empty include pattern (should default to all files) + let input = serde_json::to_value(GrepToolInput { + regex: "fn".to_string(), + include_pattern: None, + offset: 0, + case_sensitive: false, + }) + .unwrap(); + + let result = run_grep_tool(input, project.clone(), cx).await; + assert!(result.contains("main.rs"), "Should find matches in main.rs"); + assert!( + result.contains("helper.rs"), + "Should find matches in helper.rs" + ); + assert!( + result.contains("test_main.rs"), + "Should include test_main.rs" + ); + } + + #[gpui::test] + async fn test_grep_tool_with_case_sensitivity(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor().clone()); + fs.insert_tree( + "/root", + serde_json::json!({ + "case_test.txt": "This file has UPPERCASE and lowercase text.\nUPPERCASE patterns should match only with case_sensitive: true", + }), + ) + .await; + + let project = Project::test(fs.clone(), [path!("/root").as_ref()], cx).await; + + // Test case-insensitive search (default) + let input = serde_json::to_value(GrepToolInput { + regex: "uppercase".to_string(), + include_pattern: Some("**/*.txt".to_string()), + offset: 0, + case_sensitive: false, + }) + .unwrap(); + + let result = run_grep_tool(input, project.clone(), cx).await; + assert!( + result.contains("UPPERCASE"), + "Case-insensitive search should match uppercase" + ); + + // Test case-sensitive search + let input = serde_json::to_value(GrepToolInput { + regex: "uppercase".to_string(), + include_pattern: Some("**/*.txt".to_string()), + offset: 0, + case_sensitive: true, + }) + .unwrap(); + + let result = run_grep_tool(input, project.clone(), cx).await; + assert!( + !result.contains("UPPERCASE"), + "Case-sensitive search should not match uppercase" + ); + + // Test case-sensitive search + let input = serde_json::to_value(GrepToolInput { + regex: "LOWERCASE".to_string(), + include_pattern: Some("**/*.txt".to_string()), + offset: 0, + case_sensitive: true, + }) + .unwrap(); + + let result = run_grep_tool(input, project.clone(), cx).await; + + assert!( + !result.contains("lowercase"), + "Case-sensitive search should match lowercase" + ); + + // Test case-sensitive search for lowercase pattern + let input = serde_json::to_value(GrepToolInput { + regex: "lowercase".to_string(), + include_pattern: Some("**/*.txt".to_string()), + offset: 0, + case_sensitive: true, + }) + .unwrap(); + + let result = run_grep_tool(input, project.clone(), cx).await; + assert!( + result.contains("lowercase"), + "Case-sensitive search should match lowercase text" + ); + } + + async fn run_grep_tool( + input: serde_json::Value, + project: Entity, + cx: &mut TestAppContext, + ) -> String { + let tool = Arc::new(GrepTool); + let action_log = cx.new(|_cx| ActionLog::new(project.clone())); + let task = cx.update(|cx| tool.run(input, &[], project, action_log, cx)); + + match task.output.await { + Ok(result) => result, + Err(e) => panic!("Failed to run grep tool: {}", e), + } + } + + fn init_test(cx: &mut TestAppContext) { + cx.update(|cx| { + let settings_store = SettingsStore::test(cx); + cx.set_global(settings_store); + language::init(cx); + Project::init_settings(cx); + }); + } +} diff --git a/crates/assistant_tools/src/grep_tool/description.md b/crates/assistant_tools/src/grep_tool/description.md new file mode 100644 index 0000000000000000000000000000000000000000..33983e66ddc1d128a43513ab29efc40e9942c4ae --- /dev/null +++ b/crates/assistant_tools/src/grep_tool/description.md @@ -0,0 +1,8 @@ +Searches the contents of files in the project with a regular expression + +- Prefer this tool to path search when searching for symbols in the project, because you won't need to guess what path it's in. +- Supports full regex syntax (eg. "log.*Error", "function\\s+\\w+", etc.) +- Pass an `include_pattern` if you know how to narrow your search on the files system +- Never use this tool to search for paths. Only search file contents with this tool. +- Use this tool when you need to find files containing specific patterns +- Results are paginated with 20 matches per page. Use the optional 'offset' parameter to request subsequent pages. diff --git a/crates/assistant_tools/src/list_directory_tool/description.md b/crates/assistant_tools/src/list_directory_tool/description.md index 1daf3e3a9f29a4885f2954235b790a803df7eacc..7fe3a6d441c2504a80b8fc83bc3042b1fce3c2bc 100644 --- a/crates/assistant_tools/src/list_directory_tool/description.md +++ b/crates/assistant_tools/src/list_directory_tool/description.md @@ -1 +1 @@ -Lists files and directories in a given path. Prefer the `regex_search` or `path_search` tools when searching the codebase. +Lists files and directories in a given path. Prefer the `grep` or `path_search` tools when searching the codebase. diff --git a/crates/assistant_tools/src/path_search_tool/description.md b/crates/assistant_tools/src/path_search_tool/description.md index 73345bff8e807124a6b9fce48accc600235995c8..8939aec0a973fee255d75148989cf7a257c2af81 100644 --- a/crates/assistant_tools/src/path_search_tool/description.md +++ b/crates/assistant_tools/src/path_search_tool/description.md @@ -2,6 +2,6 @@ Fast file pattern matching tool that works with any codebase size - Supports glob patterns like "**/*.js" or "src/**/*.ts" - Returns matching file paths sorted alphabetically -- Prefer the `regex_search` tool to this tool when searching for symbols unless you have specific information about paths. +- Prefer the `grep` tool to this tool when searching for symbols unless you have specific information about paths. - Use this tool when you need to find files by name patterns - Results are paginated with 50 matches per page. Use the optional 'offset' parameter to request subsequent pages. diff --git a/crates/assistant_tools/src/regex_search_tool.rs b/crates/assistant_tools/src/regex_search_tool.rs deleted file mode 100644 index acd408112f7979024ba91ae067bf0775372a88a4..0000000000000000000000000000000000000000 --- a/crates/assistant_tools/src/regex_search_tool.rs +++ /dev/null @@ -1,206 +0,0 @@ -use crate::schema::json_schema_for; -use anyhow::{Result, anyhow}; -use assistant_tool::{ActionLog, Tool, ToolResult}; -use futures::StreamExt; -use gpui::{App, Entity, Task}; -use language::OffsetRangeExt; -use language_model::{LanguageModelRequestMessage, LanguageModelToolSchemaFormat}; -use project::{ - Project, - search::{SearchQuery, SearchResult}, -}; -use schemars::JsonSchema; -use serde::{Deserialize, Serialize}; -use std::{cmp, fmt::Write, sync::Arc}; -use ui::IconName; -use util::markdown::MarkdownString; -use util::paths::PathMatcher; - -#[derive(Debug, Serialize, Deserialize, JsonSchema)] -pub struct RegexSearchToolInput { - /// A regex pattern to search for in the entire project. Note that the regex - /// will be parsed by the Rust `regex` crate. - pub regex: String, - - /// Optional starting position for paginated results (0-based). - /// When not provided, starts from the beginning. - #[serde(default)] - pub offset: u32, - - /// Whether the regex is case-sensitive. Defaults to false (case-insensitive). - #[serde(default)] - pub case_sensitive: bool, -} - -impl RegexSearchToolInput { - /// Which page of search results this is. - pub fn page(&self) -> u32 { - 1 + (self.offset / RESULTS_PER_PAGE) - } -} - -const RESULTS_PER_PAGE: u32 = 20; - -pub struct RegexSearchTool; - -impl Tool for RegexSearchTool { - fn name(&self) -> String { - "regex_search".into() - } - - fn needs_confirmation(&self, _: &serde_json::Value, _: &App) -> bool { - false - } - - fn description(&self) -> String { - include_str!("./regex_search_tool/description.md").into() - } - - fn icon(&self) -> IconName { - IconName::Regex - } - - fn input_schema(&self, format: LanguageModelToolSchemaFormat) -> Result { - json_schema_for::(format) - } - - fn ui_text(&self, input: &serde_json::Value) -> String { - match serde_json::from_value::(input.clone()) { - Ok(input) => { - let page = input.page(); - let regex_str = MarkdownString::inline_code(&input.regex); - let case_info = if input.case_sensitive { - " (case-sensitive)" - } else { - "" - }; - - if page > 1 { - format!("Get page {page} of search results for regex {regex_str}{case_info}") - } else { - format!("Search files for regex {regex_str}{case_info}") - } - } - Err(_) => "Search with regex".to_string(), - } - } - - fn run( - self: Arc, - input: serde_json::Value, - _messages: &[LanguageModelRequestMessage], - project: Entity, - _action_log: Entity, - cx: &mut App, - ) -> ToolResult { - const CONTEXT_LINES: u32 = 2; - - let (offset, regex, case_sensitive) = - match serde_json::from_value::(input) { - Ok(input) => (input.offset, input.regex, input.case_sensitive), - Err(err) => return Task::ready(Err(anyhow!(err))).into(), - }; - - let query = match SearchQuery::regex( - ®ex, - false, - case_sensitive, - false, - false, - PathMatcher::default(), - PathMatcher::default(), - None, - ) { - Ok(query) => query, - Err(error) => return Task::ready(Err(error)).into(), - }; - - let results = project.update(cx, |project, cx| project.search(query, cx)); - - cx.spawn(async move|cx| { - futures::pin_mut!(results); - - let mut output = String::new(); - let mut skips_remaining = offset; - let mut matches_found = 0; - let mut has_more_matches = false; - - while let Some(SearchResult::Buffer { buffer, ranges }) = results.next().await { - if ranges.is_empty() { - continue; - } - - buffer.read_with(cx, |buffer, cx| -> Result<(), anyhow::Error> { - if let Some(path) = buffer.file().map(|file| file.full_path(cx)) { - let mut file_header_written = false; - let mut ranges = ranges - .into_iter() - .map(|range| { - let mut point_range = range.to_point(buffer); - point_range.start.row = - point_range.start.row.saturating_sub(CONTEXT_LINES); - point_range.start.column = 0; - point_range.end.row = cmp::min( - buffer.max_point().row, - point_range.end.row + CONTEXT_LINES, - ); - point_range.end.column = buffer.line_len(point_range.end.row); - point_range - }) - .peekable(); - - while let Some(mut range) = ranges.next() { - if skips_remaining > 0 { - skips_remaining -= 1; - continue; - } - - // We'd already found a full page of matches, and we just found one more. - if matches_found >= RESULTS_PER_PAGE { - has_more_matches = true; - return Ok(()); - } - - while let Some(next_range) = ranges.peek() { - if range.end.row >= next_range.start.row { - range.end = next_range.end; - ranges.next(); - } else { - break; - } - } - - if !file_header_written { - writeln!(output, "\n## Matches in {}", path.display())?; - file_header_written = true; - } - - let start_line = range.start.row + 1; - let end_line = range.end.row + 1; - writeln!(output, "\n### Lines {start_line}-{end_line}\n```")?; - output.extend(buffer.text_for_range(range)); - output.push_str("\n```\n"); - - matches_found += 1; - } - } - - Ok(()) - })??; - } - - if matches_found == 0 { - Ok("No matches found".to_string()) - } else if has_more_matches { - Ok(format!( - "Showing matches {}-{} (there were more matches found; use offset: {} to see next page):\n{output}", - offset + 1, - offset + matches_found, - offset + RESULTS_PER_PAGE, - )) - } else { - Ok(format!("Found {matches_found} matches:\n{output}")) - } - }).into() - } -} diff --git a/crates/assistant_tools/src/regex_search_tool/description.md b/crates/assistant_tools/src/regex_search_tool/description.md deleted file mode 100644 index 674dd6043b6efc4ae41dcf3cac1fc4a8b6c00e73..0000000000000000000000000000000000000000 --- a/crates/assistant_tools/src/regex_search_tool/description.md +++ /dev/null @@ -1,6 +0,0 @@ -Searches the entire project for the given regular expression. - -- Prefer this tool when searching for files containing symbols in the project. -- Supports full regex syntax (eg. "log.*Error", "function\\s+\\w+", etc.) -- Use this tool when you need to find files containing specific patterns -- Results are paginated with 20 matches per page. Use the optional 'offset' parameter to request subsequent pages. diff --git a/crates/assistant_tools/src/replace.rs b/crates/assistant_tools/src/replace.rs index 17243b4a12f4ba5c575ccae877f6fdac2104be00..83019a47cd7193e5c5ffbadad21669e2aecddb96 100644 --- a/crates/assistant_tools/src/replace.rs +++ b/crates/assistant_tools/src/replace.rs @@ -14,6 +14,7 @@ pub async fn replace_exact(old: &str, new: &str, snapshot: &BufferSnapshot) -> O true, PathMatcher::new(iter::empty::<&str>()).ok()?, PathMatcher::new(iter::empty::<&str>()).ok()?, + false, None, ) .log_err()?; diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index a965f2395a34ad3f8b054a3d1896148953945b20..941bd84ff90b99c77a1bcd31495e72ef60efb876 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -5091,6 +5091,7 @@ async fn test_project_search( false, Default::default(), Default::default(), + false, None, ) .unwrap(), diff --git a/crates/collab/src/tests/random_project_collaboration_tests.rs b/crates/collab/src/tests/random_project_collaboration_tests.rs index 474a0fc2e346fd74e8ee63566e8bf895dfbea659..69f0fca438da7021d94f289e89066708a5e94027 100644 --- a/crates/collab/src/tests/random_project_collaboration_tests.rs +++ b/crates/collab/src/tests/random_project_collaboration_tests.rs @@ -882,6 +882,7 @@ impl RandomizedTest for ProjectCollaborationTest { false, Default::default(), Default::default(), + false, None, ) .unwrap(), diff --git a/crates/collab_ui/src/chat_panel/message_editor.rs b/crates/collab_ui/src/chat_panel/message_editor.rs index bc1baa7ec4d332ca6c8c8357faf07923c6ecec2f..25e0684c6bf3b5c04defa3f79be1f9080bb880bf 100644 --- a/crates/collab_ui/src/chat_panel/message_editor.rs +++ b/crates/collab_ui/src/chat_panel/message_editor.rs @@ -37,6 +37,7 @@ static MENTIONS_SEARCH: LazyLock = LazyLock::new(|| { false, Default::default(), Default::default(), + false, None, ) .unwrap() diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 6210b032228c1ae67781161965c0e7768b448751..6bda454847e4de6f1da76956eb89230475525581 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -5537,6 +5537,7 @@ impl Editor { false, Default::default(), Default::default(), + false, None, ) .unwrap() diff --git a/crates/eval/examples/find_and_replace_diff_card/thread_criteria.md b/crates/eval/examples/find_and_replace_diff_card/thread_criteria.md index ee5c640a44154ade73ee69edf15c8e9216f870fc..f5e1ef0401b88667674937d0e4664e012a5c4e54 100644 --- a/crates/eval/examples/find_and_replace_diff_card/thread_criteria.md +++ b/crates/eval/examples/find_and_replace_diff_card/thread_criteria.md @@ -1,3 +1,3 @@ -1. The first tool call should be to path search including "find_replace_file_tool.rs" in the string. (*Not* regex_search, for example, or reading the file based on a guess at the path.) This is because we gave the model a filename and it needs to turn that into a real path. +1. The first tool call should be to path search including "find_replace_file_tool.rs" in the string. (*Not* grep, for example, or reading the file based on a guess at the path.) This is because we gave the model a filename and it needs to turn that into a real path. 2. After obtaining the correct path of "zed/crates/assistant_tools/src/find_replace_file_tool.rs", it should read the contents of that path. 3. When trying to find information about the Render trait, it should *not* begin with a path search, because it doesn't yet have any information on what path the Render trait might be in. diff --git a/crates/eval/src/example.rs b/crates/eval/src/example.rs index 78b7eb7af91e8812ff3cf86f78adb9c6c29fd6bf..b61a169d33fa302bcb76cdcf8b40126078932411 100644 --- a/crates/eval/src/example.rs +++ b/crates/eval/src/example.rs @@ -948,7 +948,7 @@ impl RequestMarkdown { if tool_result.is_error { messages.push_str("**ERROR:**\n"); } - messages.push_str(&format!("{}\n", tool_result.content)); + messages.push_str(&format!("{}\n\n", tool_result.content)); } } } diff --git a/crates/gpui/src/app/test_context.rs b/crates/gpui/src/app/test_context.rs index 2de8a9dbbe012d84be6787fd0e51a80f81ef4472..a861b82ff6c6d7c52f64993648832149acb8aa8e 100644 --- a/crates/gpui/src/app/test_context.rs +++ b/crates/gpui/src/app/test_context.rs @@ -113,7 +113,7 @@ impl AppContext for TestAppContext { impl TestAppContext { /// Creates a new `TestAppContext`. Usually you can rely on `#[gpui::test]` to do this for you. - pub fn new(dispatcher: TestDispatcher, fn_name: Option<&'static str>) -> Self { + pub fn build(dispatcher: TestDispatcher, fn_name: Option<&'static str>) -> Self { let arc_dispatcher = Arc::new(dispatcher.clone()); let background_executor = BackgroundExecutor::new(arc_dispatcher.clone()); let foreground_executor = ForegroundExecutor::new(arc_dispatcher); @@ -146,7 +146,7 @@ impl TestAppContext { /// returns a new `TestAppContext` re-using the same executors to interleave tasks. pub fn new_app(&self) -> TestAppContext { - Self::new(self.dispatcher.clone(), self.fn_name) + Self::build(self.dispatcher.clone(), self.fn_name) } /// Called by the test helper to end the test. @@ -178,6 +178,11 @@ impl TestAppContext { &self.foreground_executor } + fn new(&mut self, build_entity: impl FnOnce(&mut Context) -> T) -> Entity { + let mut cx = self.app.borrow_mut(); + cx.new(build_entity) + } + /// Gives you an `&mut App` for the duration of the closure pub fn update(&self, f: impl FnOnce(&mut App) -> R) -> R { let mut cx = self.app.borrow_mut(); diff --git a/crates/gpui/src/text_system/line_wrapper.rs b/crates/gpui/src/text_system/line_wrapper.rs index d7bc4c1f242ac935a181982a38dca3e31eadc29e..29fc95c7afae90a83ad983b458e9854cc49baae3 100644 --- a/crates/gpui/src/text_system/line_wrapper.rs +++ b/crates/gpui/src/text_system/line_wrapper.rs @@ -329,7 +329,7 @@ mod tests { fn build_wrapper() -> LineWrapper { let dispatcher = TestDispatcher::new(StdRng::seed_from_u64(0)); - let cx = TestAppContext::new(dispatcher, None); + let cx = TestAppContext::build(dispatcher, None); let id = cx.text_system().font_id(&font("Zed Plex Mono")).unwrap(); LineWrapper::new(id, px(16.), cx.text_system().platform_text_system.clone()) } diff --git a/crates/gpui_macros/src/test.rs b/crates/gpui_macros/src/test.rs index 62edca2489133f93f1b9c3f13c9827581af7baae..aeec19a3a383bbda483942a625b2ab3398b060dd 100644 --- a/crates/gpui_macros/src/test.rs +++ b/crates/gpui_macros/src/test.rs @@ -104,7 +104,7 @@ fn try_test(args: Vec, function: TokenStream) -> Result, function: TokenStream) -> Result, function: TokenStream) -> Result { let cx_varname = format_ident!("cx_{}", ix); cx_vars.extend(quote!( - let mut #cx_varname = gpui::TestAppContext::new( + let mut #cx_varname = gpui::TestAppContext::build( dispatcher.clone(), Some(stringify!(#outer_fn_name)) ); diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index f339da9c29ff0bfd9039e3ac7d2627830109f8c7..9e99a093e3873c01dff96f0b96153dc20ed61959 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -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 diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index d54eaa6e556c0e571aa5bfd1f93ba8bf1969c034..63168283a7a964a640418d81c4d03e40e48a4e8a 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -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(), diff --git a/crates/project/src/search.rs b/crates/project/src/search.rs index d23bb9a9b8afe757c435b4860714c9d1038f632b..fe7d83b6fb0c82b58688faa304573bbd8d8e3c56 100644 --- a/crates/project/src/search.rs +++ b/crates/project/src/search.rs @@ -36,6 +36,7 @@ pub struct SearchInputs { query: Arc, files_to_include: PathMatcher, files_to_exclude: PathMatcher, + match_full_paths: bool, buffers: Option>>, } @@ -83,6 +84,10 @@ static WORD_MATCH_TEST: LazyLock = 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>>, ) -> Result { 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>>, ) -> Result { 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) { diff --git a/crates/project/src/worktree_store.rs b/crates/project/src/worktree_store.rs index 26974992b267da1e9bbca8a166700d801119d2c1..ea66cd84cc5c839dcb4185e3c5bb066f2eadf4ff 100644 --- a/crates/project/src/worktree_store.rs +++ b/crates/project/src/worktree_store.rs @@ -734,7 +734,6 @@ impl WorktreeStore { snapshot: &'a worktree::Snapshot, path: &'a Path, query: &'a SearchQuery, - include_root: bool, filter_tx: &'a Sender, output_tx: &'a Sender>, ) -> 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, output_tx: Sender>, ) -> 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; diff --git a/crates/proto/proto/buffer.proto b/crates/proto/proto/buffer.proto index 0495355e50739ed2d4323ae714e290cc7dffe2fc..e5b610ee55e03bd99dcc414c258740868dcd65a2 100644 --- a/crates/proto/proto/buffer.proto +++ b/crates/proto/proto/buffer.proto @@ -279,6 +279,7 @@ message SearchQuery { bool case_sensitive = 5; string files_to_include = 6; string files_to_exclude = 7; + bool match_full_paths = 9; bool include_ignored = 8; } diff --git a/crates/remote_server/src/remote_editing_tests.rs b/crates/remote_server/src/remote_editing_tests.rs index b5c825e7e1411ff2b81b0f133a9db8ae949e84d5..f81af17be9fdcf085998ddd9a75ff62cdd47786a 100644 --- a/crates/remote_server/src/remote_editing_tests.rs +++ b/crates/remote_server/src/remote_editing_tests.rs @@ -200,6 +200,7 @@ async fn test_remote_project_search(cx: &mut TestAppContext, server_cx: &mut Tes false, Default::default(), Default::default(), + false, None, ) .unwrap(), diff --git a/crates/search/src/buffer_search.rs b/crates/search/src/buffer_search.rs index 12b9ed53497bea6bfd0ebf6aef6f1493fcb7042b..456943d476bdf7f6a474067513171e26a21e2c0f 100644 --- a/crates/search/src/buffer_search.rs +++ b/crates/search/src/buffer_search.rs @@ -1252,6 +1252,7 @@ impl BufferSearchBar { .contains(SearchOptions::ONE_MATCH_PER_LINE), Default::default(), Default::default(), + false, None, ) { Ok(query) => query.with_replacement(self.replacement(cx)), @@ -1270,6 +1271,7 @@ impl BufferSearchBar { false, Default::default(), Default::default(), + false, None, ) { Ok(query) => query.with_replacement(self.replacement(cx)), diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index 23e51054d60fa16c0f4d0c8d4f9f9af3efa8ecb0..6223e32190fd782456cdf88371b32acbafd27949 100644 --- a/crates/search/src/project_search.rs +++ b/crates/search/src/project_search.rs @@ -1047,6 +1047,19 @@ impl ProjectSearchView { } }; + // If the project contains multiple visible worktrees, we match the + // include/exclude patterns against full paths to allow them to be + // disambiguated. For single worktree projects we use worktree relative + // paths for convenience. + let match_full_paths = self + .entity + .read(cx) + .project + .read(cx) + .visible_worktrees(cx) + .count() + > 1; + let query = if self.search_options.contains(SearchOptions::REGEX) { match SearchQuery::regex( text, @@ -1057,6 +1070,7 @@ impl ProjectSearchView { .contains(SearchOptions::ONE_MATCH_PER_LINE), included_files, excluded_files, + match_full_paths, open_buffers, ) { Ok(query) => { @@ -1084,6 +1098,7 @@ impl ProjectSearchView { self.search_options.contains(SearchOptions::INCLUDE_IGNORED), included_files, excluded_files, + match_full_paths, open_buffers, ) { Ok(query) => { diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index 76d5a26f33401b564d5e7b1e01921a59ab70f612..ee25792f35e7606c5f95d0181d1aebbe14e66fbf 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/crates/terminal_view/src/terminal_view.rs @@ -1669,6 +1669,7 @@ impl SearchableItem for TerminalView { query.include_ignored(), query.files_to_include().clone(), query.files_to_exclude().clone(), + false, None, ) .unwrap()),