From d5bb12631a4798f35c2a1a1dc721d2c9337acd68 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sun, 16 Mar 2025 06:58:25 -0400 Subject: [PATCH] Delete tool uses paths instead of globs (#26715) Also made `run` avoid doing work on the main thread. Release Notes: - N/A --- .../assistant_tools/src/delete_path_tool.rs | 136 +++--------------- .../src/delete_path_tool/description.md | 2 +- crates/project/src/project.rs | 3 +- 3 files changed, 24 insertions(+), 117 deletions(-) diff --git a/crates/assistant_tools/src/delete_path_tool.rs b/crates/assistant_tools/src/delete_path_tool.rs index d613cbed125833526648834e6096cda8205ca5a3..4381c26b080eb32cb50117e6f967229df19da9d0 100644 --- a/crates/assistant_tools/src/delete_path_tool.rs +++ b/crates/assistant_tools/src/delete_path_tool.rs @@ -1,16 +1,15 @@ use anyhow::{anyhow, Result}; use assistant_tool::{ActionLog, Tool}; -use gpui::{App, Entity, Task}; +use gpui::{App, AppContext, Entity, Task}; use language_model::LanguageModelRequestMessage; use project::Project; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use std::{fs, path::PathBuf, sync::Arc}; -use util::paths::PathMatcher; +use std::sync::Arc; #[derive(Debug, Serialize, Deserialize, JsonSchema)] pub struct DeletePathToolInput { - /// The glob to match files in the project to delete. + /// The path of the file or directory to delete. /// /// /// If the project has the following files: @@ -19,9 +18,9 @@ pub struct DeletePathToolInput { /// - directory2/a/things.txt /// - directory3/a/other.txt /// - /// You can delete the first two files by providing a glob of "*thing*.txt" + /// You can delete the first file by providing a path of "directory1/a/something.txt" /// - pub glob: String, + pub path: String, } pub struct DeletePathTool; @@ -48,119 +47,26 @@ impl Tool for DeletePathTool { _action_log: Entity, cx: &mut App, ) -> Task> { - let glob = match serde_json::from_value::(input) { - Ok(input) => input.glob, + let path_str = match serde_json::from_value::(input) { + Ok(input) => input.path, Err(err) => return Task::ready(Err(anyhow!(err))), }; - let path_matcher = match PathMatcher::new(&[glob.clone()]) { - Ok(matcher) => matcher, - Err(err) => return Task::ready(Err(anyhow!("Invalid glob: {}", err))), - }; - - struct Match { - display_path: String, - path: PathBuf, - } - - let mut matches = Vec::new(); - let mut deleted_paths = Vec::new(); - let mut errors = Vec::new(); - - for worktree_handle in project.read(cx).worktrees(cx) { - let worktree = worktree_handle.read(cx); - let worktree_root = worktree.abs_path().to_path_buf(); - - // Don't consider ignored entries. - for entry in worktree.entries(false, 0) { - if path_matcher.is_match(&entry.path) { - matches.push(Match { - path: worktree_root.join(&entry.path), - display_path: entry.path.display().to_string(), - }); - } - } - } - - if matches.is_empty() { - return Task::ready(Ok(format!("No paths in the project matched {glob:?}"))); - } - let paths_matched = matches.len(); - - // Delete the files - for Match { path, display_path } in matches { - match fs::remove_file(&path) { - Ok(()) => { - deleted_paths.push(display_path); - } - Err(file_err) => { - // Try to remove directory if it's not a file. Retrying as a directory - // on error saves a syscall compared to checking whether it's - // a directory up front for every single file. - if let Err(dir_err) = fs::remove_dir_all(&path) { - let error = if path.is_dir() { - format!("Failed to delete directory {}: {dir_err}", display_path) - } else { - format!("Failed to delete file {}: {file_err}", display_path) - }; - - errors.push(error); - } else { - deleted_paths.push(display_path); - } - } - } - } - - if errors.is_empty() { - // 0 deleted paths should never happen if there were no errors; - // we already returned if matches was empty. - let answer = if deleted_paths.len() == 1 { - format!( - "Deleted {}", - deleted_paths.first().unwrap_or(&String::new()) - ) - } else { - // Sort to group entries in the same directory together - deleted_paths.sort(); - - let mut buf = format!("Deleted these {} paths:\n", deleted_paths.len()); - - for path in deleted_paths.iter() { - buf.push('\n'); - buf.push_str(path); + match project + .read(cx) + .find_project_path(&path_str, cx) + .and_then(|path| project.update(cx, |project, cx| project.delete_file(path, false, cx))) + { + Some(deletion_task) => cx.background_spawn(async move { + match deletion_task.await { + Ok(()) => Ok(format!("Deleted {}", &path_str)), + Err(err) => Err(anyhow!("Failed to delete {}: {}", &path_str, err)), } - - buf - }; - - Task::ready(Ok(answer)) - } else { - if deleted_paths.is_empty() { - Task::ready(Err(anyhow!( - "{glob:?} matched {} deleted because of {}:\n{}", - if paths_matched == 1 { - "1 path, but it was not".to_string() - } else { - format!("{} paths, but none were", paths_matched) - }, - if errors.len() == 1 { - "this error".to_string() - } else { - format!("{} errors", errors.len()) - }, - errors.join("\n") - ))) - } else { - // Sort to group entries in the same directory together - deleted_paths.sort(); - Task::ready(Ok(format!( - "Deleted {} paths matching glob {glob:?}:\n{}\n\nErrors:\n{}", - deleted_paths.len(), - deleted_paths.join("\n"), - errors.join("\n") - ))) - } + }), + None => Task::ready(Err(anyhow!( + "Couldn't delete {} because that path isn't in this project.", + path_str + ))), } } } diff --git a/crates/assistant_tools/src/delete_path_tool/description.md b/crates/assistant_tools/src/delete_path_tool/description.md index 52b4444a2988546279838d9cd6f1268f18996c6f..dfd4388bf04cf32038d04cacf169e9ea4bf05c56 100644 --- a/crates/assistant_tools/src/delete_path_tool/description.md +++ b/crates/assistant_tools/src/delete_path_tool/description.md @@ -1 +1 @@ -Deletes all files and directories in the project which match the given glob, and returns a list of the paths that were deleted. \ No newline at end of file +Deletes the file or directory (and the directory's contents, recursively) at the specified path in the project, and returns confirmation of the deletion. diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index d29518b2817a0e42ed3e065ffc70b4f4117c80a8..0479657e55df3f7b40b8aaecd373e9e0f30a70d7 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -3842,7 +3842,8 @@ impl Project { /// # Returns /// /// Returns `Some(ProjectPath)` if a matching worktree is found, otherwise `None`. - pub fn find_project_path(&self, path: &Path, cx: &App) -> Option { + pub fn find_project_path(&self, path: impl AsRef, cx: &App) -> Option { + let path = path.as_ref(); let worktree_store = self.worktree_store.read(cx); for worktree in worktree_store.visible_worktrees(cx) {