From e58f34e761d612a17402e62f69547a6ea9475886 Mon Sep 17 00:00:00 2001 From: dino Date: Sun, 1 Mar 2026 23:02:10 +0000 Subject: [PATCH 01/17] chore(fs): add trash dependency --- Cargo.lock | 85 ++++++++++++++++++++++++++++++++++++++------ Cargo.toml | 2 +- crates/fs/Cargo.toml | 1 + 3 files changed, 76 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 06def4875c2517..39990c989ddab6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4961,7 +4961,7 @@ dependencies = [ "libc", "option-ext", "redox_users 0.5.2", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -5727,7 +5727,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -6506,6 +6506,7 @@ dependencies = [ "tempfile", "text", "time", + "trash", "util", "windows 0.61.3", ] @@ -10485,7 +10486,7 @@ version = "0.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "536bfad37a309d62069485248eeaba1e8d9853aaf951caaeaed0585a95346f08" dependencies = [ - "windows-sys 0.60.2", + "windows-sys 0.61.2", ] [[package]] @@ -10895,7 +10896,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -11193,9 +11194,9 @@ checksum = "ef25abbcd74fb2609453eb695bd2f860d389e457f67dc17cafc8b8cbc89d0c33" [[package]] name = "objc2-foundation" -version = "0.3.1" +version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "900831247d2fe1a09a683278e5384cfb8c80c79fe6b166f9d14bfdde0ea1b03c" +checksum = "e3e0adef53c21f888deb4fa59fc59f7eb17404926ee8a6f59f5df0fd7f9f3272" dependencies = [ "bitflags 2.10.0", "block2", @@ -13172,7 +13173,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "22505a5c94da8e3b7c2996394d1c933236c4d743e81a410bcca4e6989fc066a4" dependencies = [ "bytes 1.11.1", - "heck 0.4.1", + "heck 0.5.0", "itertools 0.12.1", "log", "multimap 0.10.1", @@ -14644,7 +14645,7 @@ dependencies = [ "errno 0.3.14", "libc", "linux-raw-sys 0.11.0", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -17096,7 +17097,7 @@ dependencies = [ "getrandom 0.3.4", "once_cell", "rustix 1.1.2", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -17996,7 +17997,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2fb391ac70462b3097a755618fbf9c8f95ecc1eb379a414f7b46f202ed10db1f" dependencies = [ "cc", - "windows-targets 0.48.5", + "windows-targets 0.52.6", ] [[package]] @@ -18020,6 +18021,24 @@ dependencies = [ "strength_reduce", ] +[[package]] +name = "trash" +version = "5.2.5" +source = "git+https://github.com/dinocosta/trash-rs?branch=5039-macos-restore#4afa31ae6099fc73bcbb800996582998ff1cb418" +dependencies = [ + "chrono", + "libc", + "log", + "objc2", + "objc2-foundation", + "once_cell", + "percent-encoding", + "scopeguard", + "urlencoding", + "windows 0.56.0", + "windows-core 0.56.0", +] + [[package]] name = "tree-sitter" version = "0.26.3" @@ -20070,7 +20089,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.48.0", + "windows-sys 0.61.2", ] [[package]] @@ -20098,6 +20117,16 @@ dependencies = [ "wasmtime-environ", ] +[[package]] +name = "windows" +version = "0.56.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1de69df01bdf1ead2f4ac895dc77c9351aefff65b2f3db429a343f9cbf05e132" +dependencies = [ + "windows-core 0.56.0", + "windows-targets 0.52.6", +] + [[package]] name = "windows" version = "0.57.0" @@ -20186,6 +20215,18 @@ dependencies = [ "windows-core 0.62.2", ] +[[package]] +name = "windows-core" +version = "0.56.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4698e52ed2d08f8658ab0c39512a7c00ee5fe2688c65f8c0a4f06750d729f2a6" +dependencies = [ + "windows-implement 0.56.0", + "windows-interface 0.56.0", + "windows-result 0.1.2", + "windows-targets 0.52.6", +] + [[package]] name = "windows-core" version = "0.57.0" @@ -20259,6 +20300,17 @@ dependencies = [ "windows-threading 0.2.1", ] +[[package]] +name = "windows-implement" +version = "0.56.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f6fc35f58ecd95a9b71c4f2329b911016e6bec66b3f2e6a4aad86bd2e99e2f9b" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.106", +] + [[package]] name = "windows-implement" version = "0.57.0" @@ -20292,6 +20344,17 @@ dependencies = [ "syn 2.0.106", ] +[[package]] +name = "windows-interface" +version = "0.56.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08990546bf4edef8f431fa6326e032865f27138718c587dc21bc0265bbcb57cc" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.106", +] + [[package]] name = "windows-interface" version = "0.57.0" diff --git a/Cargo.toml b/Cargo.toml index cb388e08a5b0c4..0e132f2cb80dab 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -594,7 +594,7 @@ nbformat = "1.1.0" nix = "0.29" num-format = "0.4.4" objc = "0.2" -objc2-foundation = { version = "=0.3.1", default-features = false, features = [ +objc2-foundation = { version = "=0.3.2", default-features = false, features = [ "NSArray", "NSAttributedString", "NSBundle", diff --git a/crates/fs/Cargo.toml b/crates/fs/Cargo.toml index 6355524e4f328d..ad2f56c455883d 100644 --- a/crates/fs/Cargo.toml +++ b/crates/fs/Cargo.toml @@ -41,6 +41,7 @@ time.workspace = true util.workspace = true is_executable = "1.0.5" notify = "8.2.0" +trash = { git = "https://github.com/dinocosta/trash-rs", branch = "5039-macos-restore" } [target.'cfg(target_os = "macos")'.dependencies] objc.workspace = true From fdbd61dbc39f61ab8779a38446a563508d58d6e1 Mon Sep 17 00:00:00 2001 From: dino Date: Sun, 1 Mar 2026 23:19:27 +0000 Subject: [PATCH 02/17] fix: explicit type annotation and unsafe block removal - Remove `unsafe` block in `telemetry.rs` now that the updated `objc2-foundation` no longer requires it for `operatingSystemVersionString()` - Add explicit type annotation on `.collect()` in `path_key.rs` to fix a type inference overflow caused by the `objc2-foundation` update --- crates/client/src/telemetry.rs | 2 +- crates/multi_buffer/src/path_key.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/client/src/telemetry.rs b/crates/client/src/telemetry.rs index 70056f19653b09..58681d2e14bb8b 100644 --- a/crates/client/src/telemetry.rs +++ b/crates/client/src/telemetry.rs @@ -129,7 +129,7 @@ pub fn os_version() -> String { { use objc2_foundation::NSProcessInfo; let process_info = NSProcessInfo::processInfo(); - let version_nsstring = unsafe { process_info.operatingSystemVersionString() }; + let version_nsstring = process_info.operatingSystemVersionString(); // "Version 15.6.1 (Build 24G90)" -> "15.6.1 (Build 24G90)" let version_string = version_nsstring.to_string().replace("Version ", ""); // "15.6.1 (Build 24G90)" -> "15.6.1" diff --git a/crates/multi_buffer/src/path_key.rs b/crates/multi_buffer/src/path_key.rs index 09d17d7b7fe2e9..9ed8a1dfe62ff5 100644 --- a/crates/multi_buffer/src/path_key.rs +++ b/crates/multi_buffer/src/path_key.rs @@ -495,7 +495,7 @@ impl MultiBuffer { .cloned() // todo(lw): There is a logic bug somewhere that causes excerpt_ids to not necessarily be in order by locator .sorted_by_cached_key(|&id| snapshot.excerpt_locator_for_id(id)) - .collect(); + .collect::>(); for &excerpt_id in &excerpt_ids { self.paths_by_excerpt.insert(excerpt_id, path.clone()); } From 9c0411d94f660087c97e285e22b056ff87d244e4 Mon Sep 17 00:00:00 2001 From: dino Date: Sun, 1 Mar 2026 23:23:13 +0000 Subject: [PATCH 03/17] refactor(fs): replace platform-specific trash implementations Replace per-platform `trash_file` implementations (macOS/Linux/Windows) with a single call to `trash::delete`. --- crates/fs/src/fs.rs | 59 +-------------------------------------------- 1 file changed, 1 insertion(+), 58 deletions(-) diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 2db9e48a2e77bd..2c2de8afa71fbd 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -647,65 +647,8 @@ impl Fs for RealFs { } } - #[cfg(target_os = "macos")] async fn trash_file(&self, path: &Path, _options: RemoveOptions) -> Result<()> { - use cocoa::{ - base::{id, nil}, - foundation::{NSAutoreleasePool, NSString}, - }; - use objc::{class, msg_send, sel, sel_impl}; - - unsafe { - /// Allow NSString::alloc use here because it sets autorelease - #[allow(clippy::disallowed_methods)] - unsafe fn ns_string(string: &str) -> id { - unsafe { NSString::alloc(nil).init_str(string).autorelease() } - } - - let url: id = msg_send![class!(NSURL), fileURLWithPath: ns_string(path.to_string_lossy().as_ref())]; - let array: id = msg_send![class!(NSArray), arrayWithObject: url]; - let workspace: id = msg_send![class!(NSWorkspace), sharedWorkspace]; - - let _: id = msg_send![workspace, recycleURLs: array completionHandler: nil]; - } - Ok(()) - } - - #[cfg(any(target_os = "linux", target_os = "freebsd"))] - async fn trash_file(&self, path: &Path, _options: RemoveOptions) -> Result<()> { - if let Ok(Some(metadata)) = self.metadata(path).await - && metadata.is_symlink - { - // TODO: trash_file does not support trashing symlinks yet - https://github.com/bilelmoussaoui/ashpd/issues/255 - return self.remove_file(path, RemoveOptions::default()).await; - } - let file = smol::fs::File::open(path).await?; - match trash::trash_file(&file.as_fd()).await { - Ok(_) => Ok(()), - Err(err) => { - log::error!("Failed to trash file: {}", err); - // Trashing files can fail if you don't have a trashing dbus service configured. - // In that case, delete the file directly instead. - return self.remove_file(path, RemoveOptions::default()).await; - } - } - } - - #[cfg(target_os = "windows")] - async fn trash_file(&self, path: &Path, _options: RemoveOptions) -> Result<()> { - use util::paths::SanitizedPath; - use windows::{ - Storage::{StorageDeleteOption, StorageFile}, - core::HSTRING, - }; - // todo(windows) - // When new version of `windows-rs` release, make this operation `async` - let path = path.canonicalize()?; - let path = SanitizedPath::new(&path); - let path_string = path.to_string(); - let file = StorageFile::GetFileFromPathAsync(&HSTRING::from(path_string))?.get()?; - file.DeleteAsync(StorageDeleteOption::Default)?.get()?; - Ok(()) + Ok(trash::delete(path)?) } #[cfg(target_os = "macos")] From 566158443db84b7c7682b9a1dd58a1937de518d0 Mon Sep 17 00:00:00 2001 From: dino Date: Mon, 2 Mar 2026 12:47:45 +0000 Subject: [PATCH 04/17] refactor(fs): replace platform-specific trash_dir implementations * Replace per-platform `trash_dir` implementations with a single call to `trash_file. * Add `util/test-support` feature to `fs` crate so we can actually run the `fs` test suite using `cargo t -p fs`. --- crates/fs/Cargo.toml | 2 +- crates/fs/src/fs.rs | 24 ------------------------ 2 files changed, 1 insertion(+), 25 deletions(-) diff --git a/crates/fs/Cargo.toml b/crates/fs/Cargo.toml index ad2f56c455883d..87244fe8d84c46 100644 --- a/crates/fs/Cargo.toml +++ b/crates/fs/Cargo.toml @@ -59,4 +59,4 @@ gpui = { workspace = true, features = ["test-support"] } git = { workspace = true, features = ["test-support"] } [features] -test-support = ["gpui/test-support", "git/test-support"] +test-support = ["gpui/test-support", "git/test-support", "util/test-support"] diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 2c2de8afa71fbd..b3941d0beb832b 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -651,34 +651,10 @@ impl Fs for RealFs { Ok(trash::delete(path)?) } - #[cfg(target_os = "macos")] - async fn trash_dir(&self, path: &Path, options: RemoveOptions) -> Result<()> { - self.trash_file(path, options).await - } - - #[cfg(any(target_os = "linux", target_os = "freebsd"))] async fn trash_dir(&self, path: &Path, options: RemoveOptions) -> Result<()> { self.trash_file(path, options).await } - #[cfg(target_os = "windows")] - async fn trash_dir(&self, path: &Path, _options: RemoveOptions) -> Result<()> { - use util::paths::SanitizedPath; - use windows::{ - Storage::{StorageDeleteOption, StorageFolder}, - core::HSTRING, - }; - - // todo(windows) - // When new version of `windows-rs` release, make this operation `async` - let path = path.canonicalize()?; - let path = SanitizedPath::new(&path); - let path_string = path.to_string(); - let folder = StorageFolder::GetFolderFromPathAsync(&HSTRING::from(path_string))?.get()?; - folder.DeleteAsync(StorageDeleteOption::Default)?.get()?; - Ok(()) - } - async fn open_sync(&self, path: &Path) -> Result> { Ok(Box::new(std::fs::File::open(path)?)) } From fac34ca243e6b492b66e93f88cc849da194b59de Mon Sep 17 00:00:00 2001 From: dino Date: Mon, 2 Mar 2026 16:24:20 +0000 Subject: [PATCH 05/17] feat(fs): keep track of trashed files and directories * Introduce a `fs::TrashedEntry` struct to track the location of trashed files and directories in the system's trash. * Update the return type for both `Fs.trash_file` and `Fs.trash_dir` to `Result` instead of `Result<()>`. * Update `RealFs.trash_file` and `RealFs.trash_dir` to leverage the `trash::delete_with_info` function instead of `trash::delete`, so that there's actually a `trash::TrashItem` to work with and that can be converted into a `fs::TrashedEntry`. * Add a `FakeFsState.trash` vector so that we can keep track of the trashed entries. While this might be irrelevant for now, it's meant to be leveraged when adding supporting for restoring trashed entries. * In order to avoid duplication, the `FakeFs.remove_file` and `FakeFs.remove_dir` implementations have been extracted to a `FakeFs.remove_file_inner` and `FakeFs.remove_dir_inner` methods, respectively, so that they can be reused by `FakeFs.trash_file` and `FakeFs.trash_dir`, while also adding the ability to track trashed entries in the `FakeFsState`. * This was done because, before these changes, the default `Fs.trash_file` and `Fs.trash_dir` implementations were just forwarding the call to `Fs.remove_file` and `Fs.remove_dir`, respectively. --- crates/fs/src/fs.rs | 226 +++++++++++++++++++++++------- crates/fs/tests/integration/fs.rs | 67 +++++++++ 2 files changed, 239 insertions(+), 54 deletions(-) diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index b3941d0beb832b..860b27acee8ac4 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -1,6 +1,7 @@ pub mod fs_watcher; use parking_lot::Mutex; +use std::ffi::OsString; use std::sync::atomic::{AtomicU8, AtomicUsize, Ordering}; use std::time::Instant; use util::maybe; @@ -103,14 +104,27 @@ pub trait Fs: Send + Sync { ) -> Result<()>; async fn copy_file(&self, source: &Path, target: &Path, options: CopyOptions) -> Result<()>; async fn rename(&self, source: &Path, target: &Path, options: RenameOptions) -> Result<()>; + + /// Removes a directory from the filesystem. + /// There is no expectation that the directory will be preserved in the + /// system trash. async fn remove_dir(&self, path: &Path, options: RemoveOptions) -> Result<()>; - async fn trash_dir(&self, path: &Path, options: RemoveOptions) -> Result<()> { - self.remove_dir(path, options).await - } + + /// Moves a directory to the system trash. + /// Returns a [`TrashedEntry`] that can be used to keep track of the + /// location of the trashed directory in the system's trash. + async fn trash_dir(&self, path: &Path, options: RemoveOptions) -> Result; + + /// Removes a file from the filesystem. + /// There is no expectation that the file will be preserved in the system + /// trash. async fn remove_file(&self, path: &Path, options: RemoveOptions) -> Result<()>; - async fn trash_file(&self, path: &Path, options: RemoveOptions) -> Result<()> { - self.remove_file(path, options).await - } + + /// Moves a file to the system trash. + /// Returns a [`TrashedEntry`] that can be used to keep track of the + /// location of the trashed file in the system's trash. + async fn trash_file(&self, path: &Path, options: RemoveOptions) -> Result; + async fn open_handle(&self, path: &Path) -> Result>; async fn open_sync(&self, path: &Path) -> Result>; async fn load(&self, path: &Path) -> Result { @@ -157,6 +171,31 @@ pub trait Fs: Send + Sync { } } +/// Represents a file or directory that was moved to the system's trash. +#[derive(Clone)] +pub struct TrashedEntry { + /// Platform-specific identifier for the file/directory in the trash. + /// + /// * Freedesktop – Path to the `.trashinfo` file. + /// * macOS & Windows – Full path to the file/directory in the system's + /// trash. + pub id: OsString, + // Original name of the file/directory before it was moved to the trash. + pub name: OsString, + // Original parent directory. + pub original_parent: PathBuf, +} + +impl From for TrashedEntry { + fn from(item: trash::TrashItem) -> Self { + Self { + id: item.id, + name: item.name, + original_parent: item.original_parent, + } + } +} + struct GlobalFs(Arc); impl Global for GlobalFs {} @@ -647,11 +686,11 @@ impl Fs for RealFs { } } - async fn trash_file(&self, path: &Path, _options: RemoveOptions) -> Result<()> { - Ok(trash::delete(path)?) + async fn trash_file(&self, path: &Path, _options: RemoveOptions) -> Result { + Ok(trash::delete_with_info(path)?.into()) } - async fn trash_dir(&self, path: &Path, options: RemoveOptions) -> Result<()> { + async fn trash_dir(&self, path: &Path, options: RemoveOptions) -> Result { self.trash_file(path, options).await } @@ -1135,6 +1174,7 @@ struct FakeFsState { path_write_counts: std::collections::HashMap, moves: std::collections::HashMap, job_event_subscribers: Arc>>, + trash: Vec<(TrashedEntry, FakeFsEntry)>, } #[cfg(feature = "test-support")] @@ -1420,6 +1460,7 @@ impl FakeFs { path_write_counts: Default::default(), moves: Default::default(), job_event_subscribers: Arc::new(Mutex::new(Vec::new())), + trash: Vec::new(), })), }); @@ -2120,6 +2161,90 @@ impl FakeFs { fn simulate_random_delay(&self) -> impl futures::Future { self.executor.simulate_random_delay() } + + /// Returns list of all tracked trash entries. + pub fn trash_entries(&self) -> Vec { + self.state + .lock() + .trash + .iter() + .map(|(entry, _)| entry.clone()) + .collect() + } + + async fn remove_dir_inner( + &self, + path: &Path, + options: RemoveOptions, + ) -> Result> { + self.simulate_random_delay().await; + + let path = normalize_path(path); + let parent_path = path.parent().context("cannot remove the root")?; + let base_name = path.file_name().context("cannot remove the root")?; + + let mut state = self.state.lock(); + let parent_entry = state.entry(parent_path)?; + let entry = parent_entry + .dir_entries(parent_path)? + .entry(base_name.to_str().unwrap().into()); + + let removed = match entry { + btree_map::Entry::Vacant(_) => { + if !options.ignore_if_not_exists { + anyhow::bail!("{path:?} does not exist"); + } + + None + } + btree_map::Entry::Occupied(mut entry) => { + { + let children = entry.get_mut().dir_entries(&path)?; + if !options.recursive && !children.is_empty() { + anyhow::bail!("{path:?} is not empty"); + } + } + + Some(entry.remove()) + } + }; + + state.emit_event([(path, Some(PathEventKind::Removed))]); + Ok(removed) + } + + async fn remove_file_inner( + &self, + path: &Path, + options: RemoveOptions, + ) -> Result> { + self.simulate_random_delay().await; + + let path = normalize_path(path); + let parent_path = path.parent().context("cannot remove the root")?; + let base_name = path.file_name().unwrap(); + let mut state = self.state.lock(); + let parent_entry = state.entry(parent_path)?; + let entry = parent_entry + .dir_entries(parent_path)? + .entry(base_name.to_str().unwrap().into()); + let removed = match entry { + btree_map::Entry::Vacant(_) => { + if !options.ignore_if_not_exists { + anyhow::bail!("{path:?} does not exist"); + } + + None + } + btree_map::Entry::Occupied(mut entry) => { + entry.get_mut().file_content(&path)?; + Some(entry.remove()) + } + }; + + state.emit_event([(path, Some(PathEventKind::Removed))]); + Ok(removed) + } } #[cfg(feature = "test-support")] @@ -2419,62 +2544,55 @@ impl Fs for FakeFs { } async fn remove_dir(&self, path: &Path, options: RemoveOptions) -> Result<()> { - self.simulate_random_delay().await; + self.remove_dir_inner(path, options).await.map(|_| ()) + } - let path = normalize_path(path); - let parent_path = path.parent().context("cannot remove the root")?; - let base_name = path.file_name().context("cannot remove the root")?; + async fn trash_dir(&self, path: &Path, options: RemoveOptions) -> Result { + let normalized_path = normalize_path(path); + let parent_path = normalized_path.parent().context("cannot remove the root")?; + let base_name = normalized_path.file_name().unwrap(); - let mut state = self.state.lock(); - let parent_entry = state.entry(parent_path)?; - let entry = parent_entry - .dir_entries(parent_path)? - .entry(base_name.to_str().unwrap().into()); + match self.remove_dir_inner(path, options).await? { + Some(fake_entry) => { + let trashed_entry = TrashedEntry { + id: base_name.to_str().unwrap().into(), + name: base_name.to_str().unwrap().into(), + original_parent: parent_path.to_path_buf(), + }; - match entry { - btree_map::Entry::Vacant(_) => { - if !options.ignore_if_not_exists { - anyhow::bail!("{path:?} does not exist"); - } - } - btree_map::Entry::Occupied(mut entry) => { - { - let children = entry.get_mut().dir_entries(&path)?; - if !options.recursive && !children.is_empty() { - anyhow::bail!("{path:?} is not empty"); - } - } - entry.remove(); + let mut state = self.state.lock(); + state.trash.push((trashed_entry.clone(), fake_entry)); + state.emit_event([(path, Some(PathEventKind::Removed))]); + Ok(trashed_entry) } + None => anyhow::bail!("{normalized_path:?} does not exist"), } - state.emit_event([(path, Some(PathEventKind::Removed))]); - Ok(()) } async fn remove_file(&self, path: &Path, options: RemoveOptions) -> Result<()> { - self.simulate_random_delay().await; + self.remove_file_inner(path, options).await.map(|_| ()) + } - let path = normalize_path(path); - let parent_path = path.parent().context("cannot remove the root")?; - let base_name = path.file_name().unwrap(); - let mut state = self.state.lock(); - let parent_entry = state.entry(parent_path)?; - let entry = parent_entry - .dir_entries(parent_path)? - .entry(base_name.to_str().unwrap().into()); - match entry { - btree_map::Entry::Vacant(_) => { - if !options.ignore_if_not_exists { - anyhow::bail!("{path:?} does not exist"); - } - } - btree_map::Entry::Occupied(mut entry) => { - entry.get_mut().file_content(&path)?; - entry.remove(); + async fn trash_file(&self, path: &Path, options: RemoveOptions) -> Result { + let normalized_path = normalize_path(path); + let parent_path = normalized_path.parent().context("cannot remove the root")?; + let base_name = normalized_path.file_name().unwrap(); + + match self.remove_file_inner(path, options).await? { + Some(fake_entry) => { + let trashed_entry = TrashedEntry { + id: base_name.to_str().unwrap().into(), + name: base_name.to_str().unwrap().into(), + original_parent: parent_path.to_path_buf(), + }; + + let mut state = self.state.lock(); + state.trash.push((trashed_entry.clone(), fake_entry)); + state.emit_event([(path, Some(PathEventKind::Removed))]); + Ok(trashed_entry) } + None => anyhow::bail!("{normalized_path:?} does not exist"), } - state.emit_event([(path, Some(PathEventKind::Removed))]); - Ok(()) } async fn open_sync(&self, path: &Path) -> Result> { diff --git a/crates/fs/tests/integration/fs.rs b/crates/fs/tests/integration/fs.rs index dd5e694e23c997..c95df0ec211102 100644 --- a/crates/fs/tests/integration/fs.rs +++ b/crates/fs/tests/integration/fs.rs @@ -562,3 +562,70 @@ async fn test_realfs_symlink_loop_metadata(executor: BackgroundExecutor) { assert!(!metadata.is_executable); // don't care about len or mtime on symlinks? } + +#[gpui::test] +async fn test_fake_fs_trash_file(executor: BackgroundExecutor) { + let fs = FakeFs::new(executor.clone()); + fs.insert_tree( + path!("/root"), + json!({ + "file_a.txt": "File A", + "file_b.txt": "File B", + }), + ) + .await; + + let root_path = PathBuf::from(path!("/root")); + let path = path!("/root/file_a.txt").as_ref(); + let trashed_entry = fs + .trash_file(path, Default::default()) + .await + .expect("should be able to trash {path:?}"); + + assert_eq!(trashed_entry.name, "file_a.txt"); + assert_eq!(trashed_entry.original_parent, root_path); + assert_eq!(fs.files(), vec![PathBuf::from(path!("/root/file_b.txt"))]); + + let trash_entries = fs.trash_entries(); + assert_eq!(trash_entries.len(), 1); + assert_eq!(trash_entries[0].name, "file_a.txt"); + assert_eq!(trash_entries[0].original_parent, root_path); +} + +#[gpui::test] +async fn test_fake_fs_trash_dir(executor: BackgroundExecutor) { + let fs = FakeFs::new(executor.clone()); + fs.insert_tree( + path!("/root"), + json!({ + "src": { + "file_a.txt": "File A", + "file_b.txt": "File B", + }, + "file_c.txt": "File C", + }), + ) + .await; + + let root_path = PathBuf::from(path!("/root")); + let path = path!("/root/src").as_ref(); + let trashed_entry = fs + .trash_dir( + path, + RemoveOptions { + recursive: true, + ..Default::default() + }, + ) + .await + .expect("should be able to trash {path:?}"); + + assert_eq!(trashed_entry.name, "src"); + assert_eq!(trashed_entry.original_parent, root_path); + assert_eq!(fs.files(), vec![PathBuf::from(path!("/root/file_c.txt"))]); + + let trash_entries = fs.trash_entries(); + assert_eq!(trash_entries.len(), 1); + assert_eq!(trash_entries[0].name, "src"); + assert_eq!(trash_entries[0].original_parent, root_path); +} From fc108de49e04f1ec925aa741aea0c20f9f667d8a Mon Sep 17 00:00:00 2001 From: dino Date: Tue, 3 Mar 2026 17:00:23 +0000 Subject: [PATCH 06/17] feat(fs): add restore trait method * Introduce a new `restore` method to the `Fs` trait, meant to be used to restore a trashed entry (directory or file). * Implement `Fs.restore` for `RealFs` by simply leveraging `trash::restore_all` * This also meant introducing `TrashedEntry.into_trash_item` so we can easily convert the `TrashedEntry` to a `trash::TrashItem` * Implement `Fs.restore` for `FakeFs` to actually leverage the existing `FakeFsState`, so we can actually simulate entries being trashed and restored * This will come in handy when adding undo/redo support for the Project Panel, so that we can actually test how the panel behaves when a collision happens, for example --- crates/fs/src/fs.rs | 75 +++++++++++++- crates/fs/tests/integration/fs.rs | 156 ++++++++++++++++++++++++++++++ 2 files changed, 230 insertions(+), 1 deletion(-) diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 860b27acee8ac4..77c095d5f3cd82 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -165,6 +165,12 @@ pub trait Fs: Send + Sync { async fn is_case_sensitive(&self) -> bool; fn subscribe_to_jobs(&self) -> JobEventReceiver; + // TODO!: Probably need to have our own `Error` enum so we don't make + // callers depend on `trash::Error`? + /// Restores a given `TrashedEntry`, moving it from the system's trash back + /// to the original path. + async fn restore(&self, trashed_entry: TrashedEntry) -> std::result::Result<(), trash::Error>; + #[cfg(feature = "test-support")] fn as_fake(&self) -> Arc { panic!("called as_fake on a real fs"); @@ -172,7 +178,7 @@ pub trait Fs: Send + Sync { } /// Represents a file or directory that was moved to the system's trash. -#[derive(Clone)] +#[derive(Clone, Debug, PartialEq)] pub struct TrashedEntry { /// Platform-specific identifier for the file/directory in the trash. /// @@ -196,6 +202,20 @@ impl From for TrashedEntry { } } +impl TrashedEntry { + fn into_trash_item(self) -> trash::TrashItem { + trash::TrashItem { + id: self.id, + name: self.name, + original_parent: self.original_parent, + // TODO!: Figure out if `time_deleted` is relevant for the restore + // functionality, in which case we probably want to also preserve it + // in the `From for TrashedEntry` implementation. + time_deleted: 0, + } + } +} + struct GlobalFs(Arc); impl Global for GlobalFs {} @@ -1139,6 +1159,10 @@ impl Fs for RealFs { ); res } + + async fn restore(&self, trashed_entry: TrashedEntry) -> std::result::Result<(), trash::Error> { + trash::restore_all([trashed_entry.into_trash_item()]) + } } #[cfg(not(any(target_os = "linux", target_os = "freebsd")))] @@ -1174,6 +1198,8 @@ struct FakeFsState { path_write_counts: std::collections::HashMap, moves: std::collections::HashMap, job_event_subscribers: Arc>>, + // TODO!: We might want to convert this to `HashMap` to make it easier to + // find and remove entries, instead of relying on `Vec.find` and `Vec.retain`. trash: Vec<(TrashedEntry, FakeFsEntry)>, } @@ -2845,6 +2871,53 @@ impl Fs for FakeFs { receiver } + async fn restore(&self, trashed_entry: TrashedEntry) -> std::result::Result<(), trash::Error> { + let mut state = self.state.lock(); + + let Some((trashed_entry, fake_entry)) = state + .trash + .iter() + .find(|(entry, _)| *entry == trashed_entry) + .cloned() + else { + // TODO!: Is there a better or more appropriate error we can use + // when the trashed entry does not exist in trash? + return Err(trash::Error::Unknown { + description: format!("Trashed entry not found {:?}", trashed_entry), + }); + }; + + let path = trashed_entry + .original_parent + .join(trashed_entry.name.clone()); + + let result = state.write_path(&path, |entry| match entry { + btree_map::Entry::Vacant(entry) => { + entry.insert(fake_entry); + Ok(()) + } + btree_map::Entry::Occupied(_) => { + anyhow::bail!("Failed to restore {:?}", path); + } + }); + + match result { + Ok(_) => { + state.trash.retain(|(entry, _)| *entry != trashed_entry); + Ok(()) + } + Err(_) => { + // For now we'll just assume that this failed because it was a + // collision error, which I think that, for the time being, is + // the only case where this could fail? + Err(trash::Error::RestoreCollision { + path, + remaining_items: vec![], + }) + } + } + } + #[cfg(feature = "test-support")] fn as_fake(&self) -> Arc { self.this.upgrade().unwrap() diff --git a/crates/fs/tests/integration/fs.rs b/crates/fs/tests/integration/fs.rs index c95df0ec211102..6b02381a87d1e7 100644 --- a/crates/fs/tests/integration/fs.rs +++ b/crates/fs/tests/integration/fs.rs @@ -1,4 +1,5 @@ use std::{ + ffi::OsString, io::Write, path::{Path, PathBuf}, }; @@ -629,3 +630,158 @@ async fn test_fake_fs_trash_dir(executor: BackgroundExecutor) { assert_eq!(trash_entries[0].name, "src"); assert_eq!(trash_entries[0].original_parent, root_path); } + +#[gpui::test] +async fn test_fake_fs_restore(executor: BackgroundExecutor) { + let fs = FakeFs::new(executor.clone()); + fs.insert_tree( + path!("/root"), + json!({ + "src": { + "file_a.txt": "File A", + "file_b.txt": "File B", + }, + "file_c.txt": "File C", + }), + ) + .await; + + // Providing a non-existent `TrashedEntry` should result in an error. + let id: OsString = "/trash/file_c.txt".into(); + let name: OsString = "file_c.txt".into(); + let original_parent = PathBuf::from(path!("/root")); + let trashed_entry = TrashedEntry { + id, + name, + original_parent, + }; + let result = fs.restore(trashed_entry).await; + assert!(matches!(result, Err(trash::Error::Unknown { .. }))); + + // Attempt deleting a file, asserting that the filesystem no longer reports + // it as part of its list of files, restore it and verify that the list of + // files and trash has been updated accordingly. + let path = path!("/root/src/file_a.txt").as_ref(); + let trashed_entry = fs.trash_file(path, Default::default()).await.unwrap(); + + assert_eq!(fs.trash_entries().len(), 1); + assert_eq!( + fs.files(), + vec![ + PathBuf::from(path!("/root/file_c.txt")), + PathBuf::from(path!("/root/src/file_b.txt")) + ] + ); + + fs.restore(trashed_entry).await.unwrap(); + + assert_eq!(fs.trash_entries().len(), 0); + assert_eq!( + fs.files(), + vec![ + PathBuf::from(path!("/root/file_c.txt")), + PathBuf::from(path!("/root/src/file_a.txt")), + PathBuf::from(path!("/root/src/file_b.txt")) + ] + ); + + // Deleting and restoring a directory should also remove all of its files + // but create a single trashed entry, which should be removed after + // restoration. + let options = RemoveOptions { + recursive: true, + ..Default::default() + }; + let path = path!("/root/src/").as_ref(); + let trashed_entry = fs.trash_dir(path, options).await.unwrap(); + + assert_eq!(fs.trash_entries().len(), 1); + assert_eq!(fs.files(), vec![PathBuf::from(path!("/root/file_c.txt"))]); + + fs.restore(trashed_entry).await.unwrap(); + + assert_eq!( + fs.files(), + vec![ + PathBuf::from(path!("/root/file_c.txt")), + PathBuf::from(path!("/root/src/file_a.txt")), + PathBuf::from(path!("/root/src/file_b.txt")) + ] + ); + assert_eq!(fs.trash_entries().len(), 0); + + // A collision error should be returned in case a file is being restored to + // a path where a file already exists. + let path = path!("/root/src/file_a.txt").as_ref(); + let trashed_entry = fs.trash_file(path, Default::default()).await.unwrap(); + + assert_eq!(fs.trash_entries().len(), 1); + assert_eq!( + fs.files(), + vec![ + PathBuf::from(path!("/root/file_c.txt")), + PathBuf::from(path!("/root/src/file_b.txt")) + ] + ); + + fs.write(path, "New File A".as_bytes()).await.unwrap(); + + assert_eq!(fs.trash_entries().len(), 1); + assert_eq!( + fs.files(), + vec![ + PathBuf::from(path!("/root/file_c.txt")), + PathBuf::from(path!("/root/src/file_a.txt")), + PathBuf::from(path!("/root/src/file_b.txt")) + ] + ); + + // TODO!: Assert that the file contents are still "New File A". + assert!(fs.restore(trashed_entry).await.is_err()); + + // A collision error should be returned in case a directory is being + // restored to a path where a directory already exists. + let options = RemoveOptions { + recursive: true, + ..Default::default() + }; + let path = path!("/root/src/").as_ref(); + let trashed_entry = fs.trash_dir(path, options).await.unwrap(); + + assert_eq!(fs.trash_entries().len(), 2); + assert_eq!(fs.files(), vec![PathBuf::from(path!("/root/file_c.txt"))]); + + fs.create_dir(path).await.unwrap(); + + assert_eq!(fs.files(), vec![PathBuf::from(path!("/root/file_c.txt"))]); + assert_eq!(fs.trash_entries().len(), 2); + + let result = fs.restore(trashed_entry).await; + assert!(result.is_err()); + + assert_eq!(fs.files(), vec![PathBuf::from(path!("/root/file_c.txt"))]); + assert_eq!(fs.trash_entries().len(), 2); + + // assert_eq! + // let root_path = PathBuf::from(path!("/root")); + // let path = path!("/root/src").as_ref(); + // let trashed_entry = fs + // .trash_dir( + // path, + // RemoveOptions { + // recursive: true, + // ..Default::default() + // }, + // ) + // .await + // .expect("should be able to trash {path:?}"); + + // assert_eq!(trashed_entry.name, "src"); + // assert_eq!(trashed_entry.original_parent, root_path); + // assert_eq!(fs.files(), vec![PathBuf::from(path!("/root/file_c.txt"))]); + + // let trash_entries = fs.trash_entries(); + // assert_eq!(trash_entries.len(), 1); + // assert_eq!(trash_entries[0].name, "src"); + // assert_eq!(trash_entries[0].original_parent, root_path); +} From 311417ee9f219050a43b69c5d048ed234e280e90 Mon Sep 17 00:00:00 2001 From: dino Date: Tue, 3 Mar 2026 17:04:13 +0000 Subject: [PATCH 07/17] chore: remove duplicate removed event emission Update both `FakeFs.trash_file` and `FakeFs.trash_dir` to avoid emitting the `PathEventKind::Removed` event, as both implementations already leverage the `FakeFs.remove_file_inner` and `FakeFs.remove_dir_inner`, respectively, which already emits this event, so there's no need to emit it again. --- crates/fs/src/fs.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 860b27acee8ac4..dd7accdc06c733 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -2562,7 +2562,6 @@ impl Fs for FakeFs { let mut state = self.state.lock(); state.trash.push((trashed_entry.clone(), fake_entry)); - state.emit_event([(path, Some(PathEventKind::Removed))]); Ok(trashed_entry) } None => anyhow::bail!("{normalized_path:?} does not exist"), @@ -2588,7 +2587,6 @@ impl Fs for FakeFs { let mut state = self.state.lock(); state.trash.push((trashed_entry.clone(), fake_entry)); - state.emit_event([(path, Some(PathEventKind::Removed))]); Ok(trashed_entry) } None => anyhow::bail!("{normalized_path:?} does not exist"), From 1cebdf8b05dc81031b8cf2aa3c49a5eb5d3e6461 Mon Sep 17 00:00:00 2001 From: dino Date: Tue, 3 Mar 2026 17:26:16 +0000 Subject: [PATCH 08/17] feat(fs): introduce trash restoration error * Introduce `fs::TrashRestoreError` so we don't depend on the `trash::Error` enum * Update all `Fs.restore` implementations to leverage this new error enum * Update existing `FakeFs.restore` test to match on `fs::TrashRestoreError` rather than `trash::Error` --- crates/fs/src/fs.rs | 54 ++++++++++++++++++++++--------- crates/fs/tests/integration/fs.rs | 2 +- 2 files changed, 39 insertions(+), 17 deletions(-) diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 77c095d5f3cd82..874207ee7e82a0 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -165,11 +165,12 @@ pub trait Fs: Send + Sync { async fn is_case_sensitive(&self) -> bool; fn subscribe_to_jobs(&self) -> JobEventReceiver; - // TODO!: Probably need to have our own `Error` enum so we don't make - // callers depend on `trash::Error`? /// Restores a given `TrashedEntry`, moving it from the system's trash back /// to the original path. - async fn restore(&self, trashed_entry: TrashedEntry) -> std::result::Result<(), trash::Error>; + async fn restore( + &self, + trashed_entry: TrashedEntry, + ) -> std::result::Result<(), TrashRestoreError>; #[cfg(feature = "test-support")] fn as_fake(&self) -> Arc { @@ -216,6 +217,28 @@ impl TrashedEntry { } } +#[derive(Debug)] +pub enum TrashRestoreError { + /// The specified `path` was not found in the system's trash. + NotFound { path: PathBuf }, + /// A file or directory already exists at the restore destination. + Collision { path: PathBuf }, + /// Any other platform-specific error. + Unknown { description: String }, +} + +impl From for TrashRestoreError { + fn from(err: trash::Error) -> Self { + match err { + trash::Error::RestoreCollision { path, .. } => Self::Collision { path }, + trash::Error::Unknown { description } => Self::Unknown { description }, + other => Self::Unknown { + description: other.to_string(), + }, + } + } +} + struct GlobalFs(Arc); impl Global for GlobalFs {} @@ -1160,8 +1183,11 @@ impl Fs for RealFs { res } - async fn restore(&self, trashed_entry: TrashedEntry) -> std::result::Result<(), trash::Error> { - trash::restore_all([trashed_entry.into_trash_item()]) + async fn restore( + &self, + trashed_entry: TrashedEntry, + ) -> std::result::Result<(), TrashRestoreError> { + trash::restore_all([trashed_entry.into_trash_item()]).map_err(Into::into) } } @@ -1198,8 +1224,6 @@ struct FakeFsState { path_write_counts: std::collections::HashMap, moves: std::collections::HashMap, job_event_subscribers: Arc>>, - // TODO!: We might want to convert this to `HashMap` to make it easier to - // find and remove entries, instead of relying on `Vec.find` and `Vec.retain`. trash: Vec<(TrashedEntry, FakeFsEntry)>, } @@ -2871,7 +2895,10 @@ impl Fs for FakeFs { receiver } - async fn restore(&self, trashed_entry: TrashedEntry) -> std::result::Result<(), trash::Error> { + async fn restore( + &self, + trashed_entry: TrashedEntry, + ) -> std::result::Result<(), TrashRestoreError> { let mut state = self.state.lock(); let Some((trashed_entry, fake_entry)) = state @@ -2880,10 +2907,8 @@ impl Fs for FakeFs { .find(|(entry, _)| *entry == trashed_entry) .cloned() else { - // TODO!: Is there a better or more appropriate error we can use - // when the trashed entry does not exist in trash? - return Err(trash::Error::Unknown { - description: format!("Trashed entry not found {:?}", trashed_entry), + return Err(TrashRestoreError::NotFound { + path: PathBuf::from(trashed_entry.id), }); }; @@ -2910,10 +2935,7 @@ impl Fs for FakeFs { // For now we'll just assume that this failed because it was a // collision error, which I think that, for the time being, is // the only case where this could fail? - Err(trash::Error::RestoreCollision { - path, - remaining_items: vec![], - }) + Err(TrashRestoreError::Collision { path }) } } } diff --git a/crates/fs/tests/integration/fs.rs b/crates/fs/tests/integration/fs.rs index 6b02381a87d1e7..a1efa695b7bc6f 100644 --- a/crates/fs/tests/integration/fs.rs +++ b/crates/fs/tests/integration/fs.rs @@ -656,7 +656,7 @@ async fn test_fake_fs_restore(executor: BackgroundExecutor) { original_parent, }; let result = fs.restore(trashed_entry).await; - assert!(matches!(result, Err(trash::Error::Unknown { .. }))); + assert!(matches!(result, Err(TrashRestoreError::NotFound { .. }))); // Attempt deleting a file, asserting that the filesystem no longer reports // it as part of its list of files, restore it and verify that the list of From b459a08eaef568ee885aae1172d946bbc33858db Mon Sep 17 00:00:00 2001 From: dino Date: Wed, 4 Mar 2026 10:00:32 +0000 Subject: [PATCH 09/17] refactor(fs): remove unused dependencies and improve test * Remove `objc` and `cocoa` dependencies from `fs` crate, as they're no longer required * Update the `test_fake_fs_restore` test in order to assert that the file contents are not changed when there's a restore collision --- Cargo.lock | 2 -- crates/fs/Cargo.toml | 4 ---- crates/fs/tests/integration/fs.rs | 29 +++++------------------------ 3 files changed, 5 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3d29be7c27469b..c3a6cd39be85b1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6515,7 +6515,6 @@ dependencies = [ "ashpd", "async-tar", "async-trait", - "cocoa 0.26.0", "collections", "fs", "futures 0.3.31", @@ -6526,7 +6525,6 @@ dependencies = [ "libc", "log", "notify 8.2.0", - "objc", "parking_lot", "paths", "proto", diff --git a/crates/fs/Cargo.toml b/crates/fs/Cargo.toml index 87244fe8d84c46..7cccf5109790aa 100644 --- a/crates/fs/Cargo.toml +++ b/crates/fs/Cargo.toml @@ -43,10 +43,6 @@ is_executable = "1.0.5" notify = "8.2.0" trash = { git = "https://github.com/dinocosta/trash-rs", branch = "5039-macos-restore" } -[target.'cfg(target_os = "macos")'.dependencies] -objc.workspace = true -cocoa = "0.26" - [target.'cfg(target_os = "windows")'.dependencies] windows.workspace = true diff --git a/crates/fs/tests/integration/fs.rs b/crates/fs/tests/integration/fs.rs index a1efa695b7bc6f..cc6711ad0fc3b9 100644 --- a/crates/fs/tests/integration/fs.rs +++ b/crates/fs/tests/integration/fs.rs @@ -736,8 +736,12 @@ async fn test_fake_fs_restore(executor: BackgroundExecutor) { ] ); - // TODO!: Assert that the file contents are still "New File A". + let file_contents = fs.files_with_contents(path); assert!(fs.restore(trashed_entry).await.is_err()); + assert_eq!( + file_contents, + vec![(PathBuf::from(path), b"New File A".to_vec())] + ); // A collision error should be returned in case a directory is being // restored to a path where a directory already exists. @@ -761,27 +765,4 @@ async fn test_fake_fs_restore(executor: BackgroundExecutor) { assert_eq!(fs.files(), vec![PathBuf::from(path!("/root/file_c.txt"))]); assert_eq!(fs.trash_entries().len(), 2); - - // assert_eq! - // let root_path = PathBuf::from(path!("/root")); - // let path = path!("/root/src").as_ref(); - // let trashed_entry = fs - // .trash_dir( - // path, - // RemoveOptions { - // recursive: true, - // ..Default::default() - // }, - // ) - // .await - // .expect("should be able to trash {path:?}"); - - // assert_eq!(trashed_entry.name, "src"); - // assert_eq!(trashed_entry.original_parent, root_path); - // assert_eq!(fs.files(), vec![PathBuf::from(path!("/root/file_c.txt"))]); - - // let trash_entries = fs.trash_entries(); - // assert_eq!(trash_entries.len(), 1); - // assert_eq!(trash_entries[0].name, "src"); - // assert_eq!(trash_entries[0].original_parent, root_path); } From 2a94100854e62c427cb22b590d32642bd973f924 Mon Sep 17 00:00:00 2001 From: dino Date: Fri, 20 Mar 2026 13:44:55 +0000 Subject: [PATCH 10/17] fix(fs): fix missing test attribute --- crates/fs/tests/integration/fs.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/fs/tests/integration/fs.rs b/crates/fs/tests/integration/fs.rs index a5c37f1f31343a..f265f59935a067 100644 --- a/crates/fs/tests/integration/fs.rs +++ b/crates/fs/tests/integration/fs.rs @@ -693,6 +693,7 @@ async fn test_fake_fs_trash_dir(executor: BackgroundExecutor) { assert_eq!(trash_entries[0].original_parent, root_path); } +#[gpui::test] #[ignore = "stress test; run explicitly when needed"] async fn test_realfs_watch_stress_reports_missed_paths( executor: BackgroundExecutor, From b4148920787646001597802bf7a9bcf2666d3047 Mon Sep 17 00:00:00 2001 From: dino Date: Fri, 20 Mar 2026 14:21:59 +0000 Subject: [PATCH 11/17] fix(fs): fix import name conflict on linux --- crates/fs/src/fs.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 082fe13e829d2d..e51ba7d474ff78 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -7,8 +7,6 @@ use std::time::Instant; use util::maybe; use anyhow::{Context as _, Result, anyhow}; -#[cfg(any(target_os = "linux", target_os = "freebsd"))] -use ashpd::desktop::trash; use futures::stream::iter; use gpui::App; use gpui::BackgroundExecutor; From ebfb3552462c4ce8f95a3e14b8cb69a5f8cbc044 Mon Sep 17 00:00:00 2001 From: dino Date: Wed, 25 Mar 2026 19:28:50 +0000 Subject: [PATCH 12/17] docs: improve trashed entry documentation Explain why we decided to create our own type instead of just rolling with `trash::TrashItem`, as well as ensuring that both `TrashedEntry::name` and `TrashedEntry::original_parent` have a doc comment. --- crates/fs/src/fs.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index e51ba7d474ff78..731945adf3a119 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -176,7 +176,11 @@ pub trait Fs: Send + Sync { } } -/// Represents a file or directory that was moved to the system's trash. +// We use our own type rather than `trash::TrashItem` directly to avoid carrying +// over fields we don't need (e.g. `time_deleted`) and to insulate callers and +// tests from changes to that crate's API surface. +/// Represents a file or directory that has been moved to the system trash, +/// retaining enough information to restore it to its original location. #[derive(Clone)] pub struct TrashedEntry { /// Platform-specific identifier for the file/directory in the trash. @@ -185,9 +189,9 @@ pub struct TrashedEntry { /// * macOS & Windows – Full path to the file/directory in the system's /// trash. pub id: OsString, - // Original name of the file/directory before it was moved to the trash. + /// Original name of the file/directory before it was moved to the trash. pub name: OsString, - // Original parent directory. + /// Original parent directory. pub original_parent: PathBuf, } From bbe30a4719aa3f09b9e0570f5039a78b3f1d5a1f Mon Sep 17 00:00:00 2001 From: dino Date: Wed, 25 Mar 2026 19:38:16 +0000 Subject: [PATCH 13/17] refactor(fs): remove unused dependencies Remove `objc` and `cocoa` dependencies from `fs` crate, as they're no longer required. --- Cargo.lock | 2 -- crates/fs/Cargo.toml | 4 ---- 2 files changed, 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 85147f6f45760a..9452a59a299fdb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6600,7 +6600,6 @@ dependencies = [ "ashpd", "async-tar", "async-trait", - "cocoa 0.26.0", "collections", "dunce", "fs", @@ -6612,7 +6611,6 @@ dependencies = [ "libc", "log", "notify 8.2.0", - "objc", "parking_lot", "paths", "proto", diff --git a/crates/fs/Cargo.toml b/crates/fs/Cargo.toml index c6a6078f10269a..2e2c9f08dbc64e 100644 --- a/crates/fs/Cargo.toml +++ b/crates/fs/Cargo.toml @@ -43,10 +43,6 @@ is_executable = "1.0.5" notify = "8.2.0" trash = { git = "https://github.com/dinocosta/trash-rs", branch = "5039-macos-restore" } -[target.'cfg(target_os = "macos")'.dependencies] -objc.workspace = true -cocoa = "0.26" - [target.'cfg(target_os = "windows")'.dependencies] windows.workspace = true dunce.workspace = true From 88bfddcd8c0a78ed3ae0ac9cf98b74d7d8feba1a Mon Sep 17 00:00:00 2001 From: dino Date: Wed, 25 Mar 2026 19:43:07 +0000 Subject: [PATCH 14/17] chore: clean up todo comment --- crates/fs/src/fs.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 3e3c6a7f6e1bbc..d1412fc5a2dedd 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -218,9 +218,8 @@ impl TrashedEntry { id: self.id, name: self.name, original_parent: self.original_parent, - // TODO!: Figure out if `time_deleted` is relevant for the restore - // functionality, in which case we probably want to also preserve it - // in the `From for TrashedEntry` implementation. + // `TrashedEntry` doesn't preserve `time_deleted` as we don't + // currently need it for restore, so we default it to 0 here. time_deleted: 0, } } From 794b4e7785a747d3495fa09e2c590d48b091f0af Mon Sep 17 00:00:00 2001 From: dino Date: Thu, 26 Mar 2026 15:42:54 +0000 Subject: [PATCH 15/17] refactor(fs): remove unused options argument Remove the `options` argument from both `Fs::trash_dir` and `Fs::trash_file` trait methods. Before the change to the trait method return type, these options were only used by the default implementation, as any other implementation of these methods as always using the default options (`Default::default`) which sets all fields to `false`. Since we no longer have any default implementation, it doesn't make sense to continue accepting the `options` argument, as now it is truly unused. The only place where it made a difference was in the `FakeFs` implementation, where it is passed to `FakeFs::remove_file_inner`, but even then, we can simply have `recursive: true` in the `FakeFs::trash_dir` implementation. --- crates/fs/src/fs.rs | 20 ++++++++++++-------- crates/fs/tests/integration/fs.rs | 10 ++-------- crates/worktree/src/worktree.rs | 11 ++--------- 3 files changed, 16 insertions(+), 25 deletions(-) diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 731945adf3a119..d6ca52a34359c0 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -118,7 +118,7 @@ pub trait Fs: Send + Sync { /// Moves a directory to the system trash. /// Returns a [`TrashedEntry`] that can be used to keep track of the /// location of the trashed directory in the system's trash. - async fn trash_dir(&self, path: &Path, options: RemoveOptions) -> Result; + async fn trash_dir(&self, path: &Path) -> Result; /// Removes a file from the filesystem. /// There is no expectation that the file will be preserved in the system @@ -128,7 +128,7 @@ pub trait Fs: Send + Sync { /// Moves a file to the system trash. /// Returns a [`TrashedEntry`] that can be used to keep track of the /// location of the trashed file in the system's trash. - async fn trash_file(&self, path: &Path, options: RemoveOptions) -> Result; + async fn trash_file(&self, path: &Path) -> Result; async fn open_handle(&self, path: &Path) -> Result>; async fn open_sync(&self, path: &Path) -> Result>; @@ -759,12 +759,12 @@ impl Fs for RealFs { } } - async fn trash_file(&self, path: &Path, _options: RemoveOptions) -> Result { + async fn trash_file(&self, path: &Path) -> Result { Ok(trash::delete_with_info(path)?.into()) } - async fn trash_dir(&self, path: &Path, options: RemoveOptions) -> Result { - self.trash_file(path, options).await + async fn trash_dir(&self, path: &Path) -> Result { + self.trash_file(path).await } async fn open_sync(&self, path: &Path) -> Result> { @@ -2624,10 +2624,14 @@ impl Fs for FakeFs { self.remove_dir_inner(path, options).await.map(|_| ()) } - async fn trash_dir(&self, path: &Path, options: RemoveOptions) -> Result { + async fn trash_dir(&self, path: &Path) -> Result { let normalized_path = normalize_path(path); let parent_path = normalized_path.parent().context("cannot remove the root")?; let base_name = normalized_path.file_name().unwrap(); + let options = RemoveOptions { + recursive: true, + ..Default::default() + }; match self.remove_dir_inner(path, options).await? { Some(fake_entry) => { @@ -2649,12 +2653,12 @@ impl Fs for FakeFs { self.remove_file_inner(path, options).await.map(|_| ()) } - async fn trash_file(&self, path: &Path, options: RemoveOptions) -> Result { + async fn trash_file(&self, path: &Path) -> Result { let normalized_path = normalize_path(path); let parent_path = normalized_path.parent().context("cannot remove the root")?; let base_name = normalized_path.file_name().unwrap(); - match self.remove_file_inner(path, options).await? { + match self.remove_file_inner(path, Default::default()).await? { Some(fake_entry) => { let trashed_entry = TrashedEntry { id: base_name.to_str().unwrap().into(), diff --git a/crates/fs/tests/integration/fs.rs b/crates/fs/tests/integration/fs.rs index f265f59935a067..83956c76c9f1db 100644 --- a/crates/fs/tests/integration/fs.rs +++ b/crates/fs/tests/integration/fs.rs @@ -641,7 +641,7 @@ async fn test_fake_fs_trash_file(executor: BackgroundExecutor) { let root_path = PathBuf::from(path!("/root")); let path = path!("/root/file_a.txt").as_ref(); let trashed_entry = fs - .trash_file(path, Default::default()) + .trash_file(path) .await .expect("should be able to trash {path:?}"); @@ -673,13 +673,7 @@ async fn test_fake_fs_trash_dir(executor: BackgroundExecutor) { let root_path = PathBuf::from(path!("/root")); let path = path!("/root/src").as_ref(); let trashed_entry = fs - .trash_dir( - path, - RemoveOptions { - recursive: true, - ..Default::default() - }, - ) + .trash_dir(path) .await .expect("should be able to trash {path:?}"); diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index 5d726cc9e712e7..e5cfcf0767cb9b 100644 --- a/crates/worktree/src/worktree.rs +++ b/crates/worktree/src/worktree.rs @@ -1606,19 +1606,12 @@ impl LocalWorktree { let delete = cx.background_spawn(async move { if entry.is_file() { if trash { - fs.trash_file(&abs_path, Default::default()).await?; + fs.trash_file(&abs_path).await?; } else { fs.remove_file(&abs_path, Default::default()).await?; } } else if trash { - fs.trash_dir( - &abs_path, - RemoveOptions { - recursive: true, - ignore_if_not_exists: false, - }, - ) - .await?; + fs.trash_dir(&abs_path).await?; } else { fs.remove_dir( &abs_path, From 500e08c2fb6394dcb8ed09da907d92cacd5eaaf3 Mon Sep 17 00:00:00 2001 From: dino Date: Wed, 8 Apr 2026 08:27:26 +0100 Subject: [PATCH 16/17] chore: update trash dependency Update the `trash` dependency repository from my personal fork to Zed's own fork, as that is now updated with the necessary changes to allow both tracking the trashed file's location as well as restore support for macOS. --- Cargo.lock | 6 +++--- crates/fs/Cargo.toml | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9452a59a299fdb..6829ad25696cde 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8506,7 +8506,7 @@ dependencies = [ "js-sys", "log", "wasm-bindgen", - "windows-core 0.62.2", + "windows-core 0.57.0", ] [[package]] @@ -18281,7 +18281,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2fb391ac70462b3097a755618fbf9c8f95ecc1eb379a414f7b46f202ed10db1f" dependencies = [ "cc", - "windows-targets 0.52.6", + "windows-targets 0.48.5", ] [[package]] @@ -18308,7 +18308,7 @@ dependencies = [ [[package]] name = "trash" version = "5.2.5" -source = "git+https://github.com/dinocosta/trash-rs?branch=5039-macos-restore#4afa31ae6099fc73bcbb800996582998ff1cb418" +source = "git+https://github.com/zed-industries/trash-rs?rev=3bf27effd4eb8699f2e484d3326b852fe3e53af7#3bf27effd4eb8699f2e484d3326b852fe3e53af7" dependencies = [ "chrono", "libc", diff --git a/crates/fs/Cargo.toml b/crates/fs/Cargo.toml index 2e2c9f08dbc64e..f8c5ae91699721 100644 --- a/crates/fs/Cargo.toml +++ b/crates/fs/Cargo.toml @@ -41,7 +41,7 @@ time.workspace = true util.workspace = true is_executable = "1.0.5" notify = "8.2.0" -trash = { git = "https://github.com/dinocosta/trash-rs", branch = "5039-macos-restore" } +trash = { git = "https://github.com/zed-industries/trash-rs", rev = "3bf27effd4eb8699f2e484d3326b852fe3e53af7" } [target.'cfg(target_os = "windows")'.dependencies] windows.workspace = true From 46de14fdfc3e0127a16eeffafc0fa367a1c3682e Mon Sep 17 00:00:00 2001 From: dino Date: Wed, 8 Apr 2026 08:55:29 +0100 Subject: [PATCH 17/17] test: fix failing test The `FakeFs::trash_file` and `FakeFs::trash_file` implementations no longer accept options, so the `test_fake_fs_restore` test has been updated accordingly. --- crates/fs/src/fs.rs | 2 +- crates/fs/tests/integration/fs.rs | 16 ++++------------ 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 5674b7099d3e42..bdeb139088bf33 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -188,7 +188,7 @@ pub trait Fs: Send + Sync { // tests from changes to that crate's API surface. /// Represents a file or directory that has been moved to the system trash, /// retaining enough information to restore it to its original location. -#[derive(Clone)] +#[derive(Clone, PartialEq)] pub struct TrashedEntry { /// Platform-specific identifier for the file/directory in the trash. /// diff --git a/crates/fs/tests/integration/fs.rs b/crates/fs/tests/integration/fs.rs index d5ad4b796826e8..fce8a98dea64fb 100644 --- a/crates/fs/tests/integration/fs.rs +++ b/crates/fs/tests/integration/fs.rs @@ -719,7 +719,7 @@ async fn test_fake_fs_restore(executor: BackgroundExecutor) { // it as part of its list of files, restore it and verify that the list of // files and trash has been updated accordingly. let path = path!("/root/src/file_a.txt").as_ref(); - let trashed_entry = fs.trash_file(path, Default::default()).await.unwrap(); + let trashed_entry = fs.trash_file(path).await.unwrap(); assert_eq!(fs.trash_entries().len(), 1); assert_eq!( @@ -745,12 +745,8 @@ async fn test_fake_fs_restore(executor: BackgroundExecutor) { // Deleting and restoring a directory should also remove all of its files // but create a single trashed entry, which should be removed after // restoration. - let options = RemoveOptions { - recursive: true, - ..Default::default() - }; let path = path!("/root/src/").as_ref(); - let trashed_entry = fs.trash_dir(path, options).await.unwrap(); + let trashed_entry = fs.trash_dir(path).await.unwrap(); assert_eq!(fs.trash_entries().len(), 1); assert_eq!(fs.files(), vec![PathBuf::from(path!("/root/file_c.txt"))]); @@ -770,7 +766,7 @@ async fn test_fake_fs_restore(executor: BackgroundExecutor) { // A collision error should be returned in case a file is being restored to // a path where a file already exists. let path = path!("/root/src/file_a.txt").as_ref(); - let trashed_entry = fs.trash_file(path, Default::default()).await.unwrap(); + let trashed_entry = fs.trash_file(path).await.unwrap(); assert_eq!(fs.trash_entries().len(), 1); assert_eq!( @@ -802,12 +798,8 @@ async fn test_fake_fs_restore(executor: BackgroundExecutor) { // A collision error should be returned in case a directory is being // restored to a path where a directory already exists. - let options = RemoveOptions { - recursive: true, - ..Default::default() - }; let path = path!("/root/src/").as_ref(); - let trashed_entry = fs.trash_dir(path, options).await.unwrap(); + let trashed_entry = fs.trash_dir(path).await.unwrap(); assert_eq!(fs.trash_entries().len(), 2); assert_eq!(fs.files(), vec![PathBuf::from(path!("/root/file_c.txt"))]);