From b01d07f4902c8875225a23b82ed953ee73b69b77 Mon Sep 17 00:00:00 2001 From: dino Date: Wed, 11 Feb 2026 00:44:33 +0000 Subject: [PATCH 01/10] feat(macos): return trash item info from file manager deletion * Introduce `trash::delete_with_info` and `trash::delete_all_with_info` * Introduce `trash::TrashContext.delete_with_info` and `trash::TrashContext.delete_all_with_info` * Update `trash::macos::delete_using_file_mgr` in order for it to build `TrashItem`s for the deleted paths, ensuring that we return information regarding where the file was moved to in the system's trash At the moment I believe the refactoring can't be introduced to `trash::macos::delete_using_finder` because it leverages AppleScript and the command's output does not returned the trashed file location. We'll still have to do the same refactoring for both Freedesktop and Windows in order to have access to the trashed files on both platforms. --- Cargo.toml | 1 + src/freedesktop.rs | 2 +- src/lib.rs | 46 ++++++++++++++++++++++++++++++++++++++- src/macos/mod.rs | 54 ++++++++++++++++++++++++++++++++++++++-------- src/windows.rs | 2 +- 5 files changed, 93 insertions(+), 12 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d2f7186b..34f809e2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,6 +36,7 @@ objc2-foundation = { version = "0.3.2", default-features = false, features = [ "std", "NSError", "NSFileManager", + "NSPathUtilities", "NSString", "NSURL", ] } diff --git a/src/freedesktop.rs b/src/freedesktop.rs index 99065d65..b671c7a6 100644 --- a/src/freedesktop.rs +++ b/src/freedesktop.rs @@ -33,7 +33,7 @@ impl PlatformTrashContext { } } impl TrashContext { - pub(crate) fn delete_all_canonicalized(&self, full_paths: Vec) -> Result<(), Error> { + pub(crate) fn delete_all_canonicalized(&self, full_paths: Vec, _with_info: bool) -> Result<(), Error> { let home_trash = home_trash()?; let sorted_mount_points = get_sorted_mount_points()?; let home_topdir = home_topdir(&sorted_mount_points)?; diff --git a/src/lib.rs b/src/lib.rs index adccaea6..adfefd7b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -109,7 +109,33 @@ impl TrashContext { trace!("Starting canonicalize_paths"); let full_paths = canonicalize_paths(paths)?; trace!("Finished canonicalize_paths"); - self.delete_all_canonicalized(full_paths) + + match self.delete_all_canonicalized(full_paths, false) { + Ok(_) => Ok(()), + Err(err) => Err(err), + } + } + + /// Same as `delete`, but returns a `TrashItem` describing where the file + /// ended up in trash. + /// Returns `None` on platforms that don't support returning trash info. + pub fn delete_with_info>(&self, path: T) -> Result, Error> { + match self.delete_all_with_info(&[path])? { + Some(mut items) => Ok(items.pop()), + None => Ok(None), + } + } + + /// Same as `delete_all` but returns `TrashItem`s describing where files + /// ended up in the trash. + /// Returns `None` on platforms that don't support returning trash info. + pub fn delete_all_with_info(&self, paths: I) -> Result>, Error> + where + I: IntoIterator, + T: AsRef, + { + let full_paths = canonicalize_paths(paths)?; + self.delete_all_canonicalized(full_paths, true) } } @@ -120,6 +146,13 @@ pub fn delete>(path: T) -> Result<(), Error> { DEFAULT_TRASH_CTX.delete(path) } +/// Convenience method for `DEFAULT_TRASH_CTX.delete_with_info()`. +/// +/// See: [`TrashContext::delete_with_info`](TrashContext::delete_with_info) +pub fn delete_with_info>(path: T) -> Result, Error> { + DEFAULT_TRASH_CTX.delete_with_info(path) +} + /// Convenience method for `DEFAULT_TRASH_CTX.delete_all()`. /// /// See: [`TrashContext::delete_all`](TrashContext::delete_all) @@ -131,6 +164,17 @@ where DEFAULT_TRASH_CTX.delete_all(paths) } +/// Convenience method for `DEFAULT_TRASH_CTX.delete_all_with_info()`. +/// +/// See: [`TrashContext::delete_all_with_info`](TrashContext::delete_all_with_info) +pub fn delete_all_with_info(paths: I) -> Result>, Error> +where + I: IntoIterator, + T: AsRef, +{ + DEFAULT_TRASH_CTX.delete_all_with_info(paths) +} + /// Provides information about an error. #[derive(Debug)] pub enum Error { diff --git a/src/macos/mod.rs b/src/macos/mod.rs index 824371d4..932a228f 100644 --- a/src/macos/mod.rs +++ b/src/macos/mod.rs @@ -2,12 +2,13 @@ use std::{ ffi::OsString, path::{Path, PathBuf}, process::Command, + time::SystemTime, }; use log::trace; use objc2_foundation::{NSFileManager, NSString, NSURL}; -use crate::{into_unknown, Error, TrashContext}; +use crate::{into_unknown, Error, TrashContext, TrashItem}; #[derive(Copy, Clone, Debug)] /// There are 2 ways to trash files: via the ā‰¯Finder app or via the OS NsFileManager call @@ -74,17 +75,23 @@ impl TrashContextExtMacos for TrashContext { } } impl TrashContext { - pub(crate) fn delete_all_canonicalized(&self, full_paths: Vec) -> Result<(), Error> { + pub(crate) fn delete_all_canonicalized( + &self, + full_paths: Vec, + with_info: bool, + ) -> Result>, Error> { match self.platform_specific.delete_method { - DeleteMethod::Finder => delete_using_finder(&full_paths), - DeleteMethod::NsFileManager => delete_using_file_mgr(&full_paths), + DeleteMethod::Finder => delete_using_finder(&full_paths, with_info), + DeleteMethod::NsFileManager => delete_using_file_mgr(&full_paths, with_info), } } } -fn delete_using_file_mgr>(full_paths: &[P]) -> Result<(), Error> { +fn delete_using_file_mgr>(full_paths: &[P], with_info: bool) -> Result>, Error> { trace!("Starting delete_using_file_mgr"); let file_mgr = NSFileManager::defaultManager(); + let mut trash_items = Vec::::new(); + for path in full_paths { let path = path.as_ref().as_os_str().as_encoded_bytes(); let path = match std::str::from_utf8(path) { @@ -96,8 +103,10 @@ fn delete_using_file_mgr>(full_paths: &[P]) -> Result<(), Error> let url = NSURL::fileURLWithPath(&path); trace!("Finished fileURLWithPath"); + let mut trash_url = Some(NSURL::new()); + trace!("Calling trashItemAtURL"); - let res = file_mgr.trashItemAtURL_resultingItemURL_error(&url, None); + let res = file_mgr.trashItemAtURL_resultingItemURL_error(&url, Some(&mut trash_url)); trace!("Finished trashItemAtURL"); if let Err(err) = res { @@ -105,11 +114,35 @@ fn delete_using_file_mgr>(full_paths: &[P]) -> Result<(), Error> description: format!("While deleting '{:?}', `trashItemAtURL` failed: {err}", &path), }); } + + if with_info { + trash_items.push(TrashItem { + name: OsString::from(path.lastPathComponent().to_string()), + original_parent: path.stringByDeletingLastPathComponent().to_string().into(), + id: trash_url + .and_then(|url| url.path()) + .map(|path| OsString::from(path.to_string())) + .unwrap_or_default(), + time_deleted: SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .map(|duration| duration.as_secs() as i64) + .unwrap_or(-1), + }); + } + } + + if with_info { + Ok(Some(trash_items)) + } else { + Ok(None) } - Ok(()) } -fn delete_using_finder>(full_paths: &[P]) -> Result<(), Error> { +/// This method currently has a limitation where `with_info` doesn't change the +/// result of this method, seeing as The AppleScript `delete` command returns +/// Finder object references rather than POSIX paths, so we don't attempt to +/// parse them into TrashItems. +fn delete_using_finder>(full_paths: &[P], _with_info: bool) -> Result>, Error> { // AppleScript command to move files (or directories) to Trash looks like // osascript -e 'tell application "Finder" to delete { POSIX file "file1", POSIX "file2" }' // The `-e` flag is used to execute only one line of AppleScript. @@ -149,7 +182,10 @@ fn delete_using_finder>(full_paths: &[P]) -> Result<(), Error> { } }; } - Ok(()) + + // The AppleScript `delete` command returns Finder object references rather + // than POSIX paths, so we don't attempt to parse them into TrashItems. + Ok(None) } /// std's from_utf8_lossy, but non-utf8 byte sequences are %-encoded instead of being replaced by a special symbol. diff --git a/src/windows.rs b/src/windows.rs index 87277067..536b3f9b 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -71,7 +71,7 @@ impl TrashContext { } /// Removes all files and folder paths recursively. - pub(crate) fn delete_all_canonicalized(&self, full_paths: Vec) -> Result<(), Error> { + pub(crate) fn delete_all_canonicalized(&self, full_paths: Vec, _with_info: bool) -> Result<(), Error> { self.delete_specified_canonicalized(full_paths)?; Ok(()) } From 341fbd203142a43133a0b8b4a16fcec01a77a0c8 Mon Sep 17 00:00:00 2001 From: dino Date: Wed, 11 Feb 2026 13:10:07 +0000 Subject: [PATCH 02/10] test: add test for delete_with_info with ns file manager --- tests/trash.rs | 39 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/tests/trash.rs b/tests/trash.rs index 7ea478fb..2e3ce2b3 100644 --- a/tests/trash.rs +++ b/tests/trash.rs @@ -1,10 +1,11 @@ -use std::fs::{create_dir, File}; +use std::env; +use std::fs::{create_dir, remove_file, File}; use std::path::{Path, PathBuf}; use log::trace; use serial_test::serial; -use trash::{delete, delete_all}; +use trash::{delete, delete_all, TrashContext}; mod util { use std::sync::atomic::{AtomicI32, Ordering}; @@ -177,3 +178,37 @@ fn recursive_file_with_content_deletion() { trash::delete(parent_dir).unwrap(); assert!(!parent_dir.exists()); } + +#[test] +#[serial] +#[cfg(target_os = "macos")] +fn test_delete_with_info_ns_file_manager() { + use trash::macos::{DeleteMethod, TrashContextExtMacos}; + + // Create the test file to be deleted, ensuring that we include the current + // directory so we can later assert that the `original_parent` is preserved. + let path = env::current_dir().expect("Should be able to get current directory").join(get_unique_name()); + File::create_new(&path).unwrap(); + + // Create new test context with `NSFileManager`, as that is currently the + // only implementation that actually supports returning the trashed item + // information. + let mut trash = TrashContext::new(); + trash.set_delete_method(DeleteMethod::NsFileManager); + + match trash.delete_with_info(&path) { + Ok(Some(trash_item)) => { + // Before asserting any of the fields, we'll go ahead and remove the + // trashed file from the trash, otherwise it'll be kept around after + // the test is finished, as the test literally moves the file to + // macOS' trash. + // The returned `Result` is ignored, as we don't want the test to + // fail in case we're not able to remove the file from trash. + let _ = remove_file(PathBuf::from(trash_item.id)); + + assert_eq!(trash_item.name, path.components().last().expect("Should have last component").as_os_str()); + assert_eq!(trash_item.original_parent, path.parent().expect("Should have parent").as_os_str()); + } + _ => panic!("Calling delete_with_info failed to return TrashItem."), + } +} From 71a40ef7d54368be6a70bf8f98f67eff94afabc0 Mon Sep 17 00:00:00 2001 From: dino Date: Thu, 12 Feb 2026 17:04:04 +0000 Subject: [PATCH 03/10] feat(freedesktop): return trash item info from deletion Populate and return `TrashItem` from `move_to_trash` on Freedesktop, mirroring the existing macOS implementation. Uses `SystemTime` for `time_deleted` to stay consistent across platforms. --- src/freedesktop.rs | 47 +++++++++++++++++++++++++++++++++++++--------- tests/trash.rs | 44 ++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 79 insertions(+), 12 deletions(-) diff --git a/src/freedesktop.rs b/src/freedesktop.rs index b671c7a6..29200059 100644 --- a/src/freedesktop.rs +++ b/src/freedesktop.rs @@ -17,6 +17,7 @@ use std::{ fs::PermissionsExt, }, path::{Component, Path, PathBuf}, + time::SystemTime, }; use log::{debug, warn}; @@ -33,10 +34,16 @@ impl PlatformTrashContext { } } impl TrashContext { - pub(crate) fn delete_all_canonicalized(&self, full_paths: Vec, _with_info: bool) -> Result<(), Error> { + pub(crate) fn delete_all_canonicalized( + &self, + full_paths: Vec, + with_info: bool, + ) -> Result>, Error> { let home_trash = home_trash()?; let sorted_mount_points = get_sorted_mount_points()?; let home_topdir = home_topdir(&sorted_mount_points)?; + let mut items: Vec = Vec::new(); + debug!("The home topdir is {:?}", home_topdir); let uid = unsafe { libc::getuid() }; for path in full_paths { @@ -47,18 +54,37 @@ impl TrashContext { debug!("The topdir was identical to the home topdir, so moving to the home trash."); // Note that the following function creates the trash folder // and its required subfolders in case they don't exist. - move_to_trash(path, &home_trash, topdir).map_err(|(p, e)| fs_error(p, e))?; + let item = move_to_trash(path, &home_trash, topdir).map_err(|(p, e)| fs_error(p, e))?; + if with_info { + items.push(item); + } } else if topdir.to_str() == Some("/var/home") && home_topdir.to_str() == Some("/") { debug!("The topdir is '/var/home' but the home_topdir is '/', moving to the home trash anyway."); - move_to_trash(path, &home_trash, topdir).map_err(|(p, e)| fs_error(p, e))?; + let item = move_to_trash(path, &home_trash, topdir).map_err(|(p, e)| fs_error(p, e))?; + if with_info { + items.push(item); + } } else { + let mut item = None; execute_on_mounted_trash_folders(uid, topdir, true, true, |trash_path| { - move_to_trash(&path, trash_path, topdir) + let trash_item = move_to_trash(&path, trash_path, topdir)?; + item = Some(trash_item); + Ok(()) }) .map_err(|(p, e)| fs_error(p, e))?; + if with_info { + if let Some(trash_item) = item { + items.push(trash_item); + } + } } } - Ok(()) + + if with_info { + Ok(Some(items)) + } else { + Ok(None) + } } } @@ -450,7 +476,7 @@ fn move_to_trash( src: impl AsRef, trash_folder: impl AsRef, _topdir: impl AsRef, -) -> Result<(), FsError> { +) -> Result { let src = src.as_ref(); let trash_folder = trash_folder.as_ref(); let files_folder = trash_folder.join("files"); @@ -537,12 +563,15 @@ fn move_to_trash( } Ok(_) => { // We did it! - break; + let original_parent = src.parent().map(Path::to_owned).unwrap_or_default(); + let name = src.file_name().map(OsStr::to_owned).unwrap_or_default(); + let time_deleted = + SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).map(|d| d.as_secs() as i64).unwrap_or(-1); + + return Ok(TrashItem { id: info_file_path.into_os_string(), name, original_parent, time_deleted }); } } } - - Ok(()) } /// An error may mean that a collision was found. diff --git a/tests/trash.rs b/tests/trash.rs index 2e3ce2b3..8758e762 100644 --- a/tests/trash.rs +++ b/tests/trash.rs @@ -1,11 +1,15 @@ -use std::env; -use std::fs::{create_dir, remove_file, File}; +use std::fs::{create_dir, File}; use std::path::{Path, PathBuf}; use log::trace; use serial_test::serial; -use trash::{delete, delete_all, TrashContext}; +use trash::{delete, delete_all}; + +#[cfg(not(target_os = "windows"))] +use std::env; +use std::fs::remove_file; +use trash::TrashContext; mod util { use std::sync::atomic::{AtomicI32, Ordering}; @@ -212,3 +216,37 @@ fn test_delete_with_info_ns_file_manager() { _ => panic!("Calling delete_with_info failed to return TrashItem."), } } + +#[test] +#[serial] +#[cfg(target_os = "linux")] +fn test_delete_with_info() { + // Create the test file to be deleted, ensuring that we include the current + // directory so we can later assert that the `original_parent` is preserved. + let path = env::current_dir().expect("Should be able to get current directory").join(get_unique_name()); + File::create_new(&path).unwrap(); + + // Create a new trash context for deleting the file with info. + let trash = TrashContext::new(); + + match trash.delete_with_info(&path) { + Ok(Some(trash_item)) => { + // Before asserting any of the fields, we'll go ahead and remove the + // trashed file from the Freedesktop trash, otherwise it'll be kept + // around after the test is finished. + // On Freedesktop, `id` points to the `.trashinfo` file. The actual + // trashed file lives in the sibling `files/` directory, so we need + // to clean up both. + let info_path = PathBuf::from(&trash_item.id); + let _ = remove_file(&info_path); + if let Some(info_dir) = info_path.parent().and_then(|p| p.parent()) { + let file_in_trash = info_dir.join("files").join(info_path.file_stem().unwrap_or_default()); + let _ = std::fs::remove_file(&file_in_trash).or_else(|_| std::fs::remove_dir_all(&file_in_trash)); + } + + assert_eq!(trash_item.name, path.components().last().expect("Should have last component").as_os_str()); + assert_eq!(trash_item.original_parent, path.parent().expect("Should have parent").as_os_str()); + } + _ => panic!("Calling delete_with_info failed to return TrashItem."), + } +} From 417430be2d439d5444bf35a0b83f8b280e43e38c Mon Sep 17 00:00:00 2001 From: dino Date: Fri, 13 Feb 2026 16:38:02 +0000 Subject: [PATCH 04/10] feat(windows): implement delete_with_info to return TrashItem metadata Implement IFileOperationProgressSink to capture item IDs during delete operations on Windows, enabling delete_with_info and delete_all_with_info to return TrashItem structs with id, name, original_parent, and time_deleted. The implementation: - Adds TrashProgressSink COM object implementing IFileOperationProgressSink - Collects recycle bin item IDs in PostDeleteItem callback - Looks up full metadata after operations complete (as metadata isn't immediately available during the callback) - Adds windows-core dependency and "implement" feature for COM support Also adds a Windows-specific test for delete_with_info that verifies the returned TrashItem fields and cleans up via purge_all. --- Cargo.toml | 2 + src/windows.rs | 230 ++++++++++++++++++++++++++++++++++++++++++++++--- tests/trash.rs | 34 +++++++- 3 files changed, 253 insertions(+), 13 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 34f809e2..34c124cc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -56,12 +56,14 @@ once_cell = "1.7.2" [target.'cfg(windows)'.dependencies] windows = { version = "0.56.0", features = [ + "implement", "Win32_Foundation", "Win32_Storage_EnhancedStorage", "Win32_System_Com_StructuredStorage", "Win32_System_SystemServices", "Win32_UI_Shell_PropertiesSystem", ] } +windows-core = "0.56.0" scopeguard = "1.2.0" # workaround for https://github.com/cross-rs/cross/issues/1345 diff --git a/src/windows.rs b/src/windows.rs index 536b3f9b..2e510afc 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -4,13 +4,14 @@ use std::{ ffi::{c_void, OsStr, OsString}, os::windows::{ffi::OsStrExt, prelude::*}, path::PathBuf, + sync::{Arc, Mutex}, }; use windows::Win32::{ Foundation::*, Storage::EnhancedStorage::*, System::Com::*, System::SystemServices::*, UI::Shell::PropertiesSystem::*, UI::Shell::*, }; use windows::{ - core::{Interface, PCWSTR, PWSTR}, + core::{implement, Interface, PCWSTR, PWSTR}, Win32::System::Com::StructuredStorage::PropVariantToBSTR, }; @@ -35,14 +36,31 @@ impl PlatformTrashContext { } } impl TrashContext { + /// Removes all files and folder paths recursively. /// See https://docs.microsoft.com/en-us/windows/win32/api/shellapi/ns-shellapi-_shfileopstructa - pub(crate) fn delete_specified_canonicalized(&self, full_paths: Vec) -> Result<(), Error> { + pub(crate) fn delete_all_canonicalized( + &self, + full_paths: Vec, + with_info: bool, + ) -> Result>, Error> { ensure_com_initialized(); unsafe { - let pfo: IFileOperation = CoCreateInstance(&FileOperation as *const _, None, CLSCTX_ALL).unwrap(); + let pfo: IFileOperation = CoCreateInstance(&FileOperation as *const _, None, CLSCTX_ALL)?; pfo.SetOperationFlags(FOF_NO_UI | FOF_ALLOWUNDO | FOF_WANTNUKEWARNING)?; + // Set up progress sink to collect trash item IDs if requested + // We need to keep sink_interface alive until after PerformOperations completes + let (ids_arc, _sink_interface) = if with_info { + let sink = TrashProgressSink::new(); + let ids_arc = sink.ids.clone(); + let sink_interface: IFileOperationProgressSink = sink.into(); + pfo.Advise(&sink_interface)?; + (Some(ids_arc), Some(sink_interface)) + } else { + (None, None) + }; + for full_path in full_paths.iter() { let path_prefix = ['\\' as u16, '\\' as u16, '?' as u16, '\\' as u16]; let wide_path_container = to_wide_path(full_path); @@ -66,14 +84,24 @@ impl TrashContext { // the list of HRESULT codes is not documented. return Err(Error::Unknown { description: "Some operations were aborted".into() }); } - Ok(()) - } - } - /// Removes all files and folder paths recursively. - pub(crate) fn delete_all_canonicalized(&self, full_paths: Vec, _with_info: bool) -> Result<(), Error> { - self.delete_specified_canonicalized(full_paths)?; - Ok(()) + // Look up full TrashItem metadata for collected IDs + if let Some(ids_arc) = ids_arc { + let ids = ids_arc + .lock() + .map_err(|e| Error::Unknown { description: format!("Failed to lock trash item ids: {}", e) })?; + + let mut items = Vec::with_capacity(ids.len()); + for id in ids.iter() { + if let Ok(item) = get_trash_item_by_id(id) { + items.push(item); + } + } + Ok(Some(items)) + } else { + Ok(None) + } + } } } @@ -269,6 +297,188 @@ unsafe fn get_date_deleted_unix(item: &IShellItem2) -> Result { Ok(seconds_since_unix_epoch as i64) } +/// Look up a TrashItem by its ID (parsing name) from the recycle bin +unsafe fn get_trash_item_by_id(id: &OsStr) -> Result { + let id_as_wide = to_wide_path(id); + let parsing_name = PCWSTR(id_as_wide.as_ptr()); + let item: IShellItem = SHCreateItemFromParsingName(parsing_name, None)?; + let item2: IShellItem2 = item.cast()?; + + let name = get_display_name(&item, SIGDN_PARENTRELATIVE)?; + let original_location_variant = item2.GetProperty(&SCID_ORIGINAL_LOCATION)?; + let original_location_bstr = PropVariantToBSTR(&original_location_variant)?; + let original_location = OsString::from_wide(original_location_bstr.as_wide()); + let time_deleted = get_date_deleted_unix(&item2)?; + + Ok(TrashItem { + id: id.to_os_string(), + name: name.into_string().map_err(|original| Error::ConvertOsString { original })?.into(), + original_parent: PathBuf::from(original_location), + time_deleted, + }) +} + +/// A COM object implementing IFileOperationProgressSink to collect trash item IDs +/// during delete operations. The PostDeleteItem callback receives the newly created +/// item in the Recycle Bin, and we collect its ID (parsing name) for later lookup. +/// +/// We only collect IDs here because the full metadata (original location, delete time) +/// may not be available immediately in the callback. After operations complete, we +/// look up the full metadata using these IDs. +#[implement(IFileOperationProgressSink)] +pub(crate) struct TrashProgressSink { + /// Collected IDs (parsing names) of items moved to the recycle bin + pub ids: Arc>>, +} + +impl TrashProgressSink { + pub fn new() -> Self { + Self { ids: Arc::new(Mutex::new(Vec::new())) } + } +} + +impl IFileOperationProgressSink_Impl for TrashProgressSink { + fn StartOperations(&self) -> windows::core::Result<()> { + Ok(()) + } + + fn FinishOperations(&self, _hrresult: windows::core::HRESULT) -> windows::core::Result<()> { + Ok(()) + } + + fn PreRenameItem( + &self, + _dwflags: u32, + _psiitem: Option<&IShellItem>, + _psznewname: &PCWSTR, + ) -> windows::core::Result<()> { + Ok(()) + } + + fn PostRenameItem( + &self, + _dwflags: u32, + _psiitem: Option<&IShellItem>, + _psznewname: &PCWSTR, + _hrrename: windows::core::HRESULT, + _psinewlycreated: Option<&IShellItem>, + ) -> windows::core::Result<()> { + Ok(()) + } + + fn PreMoveItem( + &self, + _dwflags: u32, + _psiitem: Option<&IShellItem>, + _psidestinationfolder: Option<&IShellItem>, + _psznewname: &PCWSTR, + ) -> windows::core::Result<()> { + Ok(()) + } + + fn PostMoveItem( + &self, + _dwflags: u32, + _psiitem: Option<&IShellItem>, + _psidestinationfolder: Option<&IShellItem>, + _psznewname: &PCWSTR, + _hrmove: windows::core::HRESULT, + _psinewlycreated: Option<&IShellItem>, + ) -> windows::core::Result<()> { + Ok(()) + } + + fn PreCopyItem( + &self, + _dwflags: u32, + _psiitem: Option<&IShellItem>, + _psidestinationfolder: Option<&IShellItem>, + _psznewname: &PCWSTR, + ) -> windows::core::Result<()> { + Ok(()) + } + + fn PostCopyItem( + &self, + _dwflags: u32, + _psiitem: Option<&IShellItem>, + _psidestinationfolder: Option<&IShellItem>, + _psznewname: &PCWSTR, + _hrcopy: windows::core::HRESULT, + _psinewlycreated: Option<&IShellItem>, + ) -> windows::core::Result<()> { + Ok(()) + } + + fn PreDeleteItem(&self, _dwflags: u32, _psiitem: Option<&IShellItem>) -> windows::core::Result<()> { + Ok(()) + } + + fn PostDeleteItem( + &self, + _dwflags: u32, + _psiitem: Option<&IShellItem>, + hrdelete: windows::core::HRESULT, + psinewlycreated: Option<&IShellItem>, + ) -> windows::core::Result<()> { + // Only process if the delete succeeded and we have a new item in the trash + // HRESULT success codes have the high bit clear (SUCCEEDED macro check) + // hrdelete.is_ok() only checks for S_OK (0), but other success codes exist + let succeeded = hrdelete.0 >= 0; + if succeeded { + if let Some(trash_item) = psinewlycreated { + // Just collect the ID (parsing name) - we'll look up full metadata later + unsafe { + if let Ok(id) = get_display_name(trash_item, SIGDN_DESKTOPABSOLUTEPARSING) { + if let Ok(mut ids) = self.ids.lock() { + ids.push(id); + } + } + } + } + } + Ok(()) + } + + fn PreNewItem( + &self, + _dwflags: u32, + _psidestinationfolder: Option<&IShellItem>, + _psznewname: &PCWSTR, + ) -> windows::core::Result<()> { + Ok(()) + } + + fn PostNewItem( + &self, + _dwflags: u32, + _psidestinationfolder: Option<&IShellItem>, + _psznewname: &PCWSTR, + _psztemplatename: &PCWSTR, + _dwfileattributes: u32, + _hrnew: windows::core::HRESULT, + _psinewitem: Option<&IShellItem>, + ) -> windows::core::Result<()> { + Ok(()) + } + + fn UpdateProgress(&self, _iworktotal: u32, _iworksofar: u32) -> windows::core::Result<()> { + Ok(()) + } + + fn ResetTimer(&self) -> windows::core::Result<()> { + Ok(()) + } + + fn PauseTimer(&self) -> windows::core::Result<()> { + Ok(()) + } + + fn ResumeTimer(&self) -> windows::core::Result<()> { + Ok(()) + } +} + struct CoInitializer {} impl CoInitializer { fn new() -> CoInitializer { diff --git a/tests/trash.rs b/tests/trash.rs index 8758e762..9f913b46 100644 --- a/tests/trash.rs +++ b/tests/trash.rs @@ -6,9 +6,7 @@ use log::trace; use serial_test::serial; use trash::{delete, delete_all}; -#[cfg(not(target_os = "windows"))] use std::env; -use std::fs::remove_file; use trash::TrashContext; mod util { @@ -238,7 +236,7 @@ fn test_delete_with_info() { // trashed file lives in the sibling `files/` directory, so we need // to clean up both. let info_path = PathBuf::from(&trash_item.id); - let _ = remove_file(&info_path); + let _ = std::fs::remove_file(&info_path); if let Some(info_dir) = info_path.parent().and_then(|p| p.parent()) { let file_in_trash = info_dir.join("files").join(info_path.file_stem().unwrap_or_default()); let _ = std::fs::remove_file(&file_in_trash).or_else(|_| std::fs::remove_dir_all(&file_in_trash)); @@ -250,3 +248,33 @@ fn test_delete_with_info() { _ => panic!("Calling delete_with_info failed to return TrashItem."), } } + +#[test] +#[serial] +#[cfg(target_os = "windows")] +fn test_delete_with_info() { + use trash::os_limited::purge_all; + + // Create the test file to be deleted, ensuring that we include the current + // directory so we can later assert that the `original_parent` is preserved. + let path = env::current_dir().expect("Should be able to get current directory").join(get_unique_name()); + File::create_new(&path).unwrap(); + + // Create a new trash context for deleting the file with info. + let trash = TrashContext::new(); + + match trash.delete_with_info(&path) { + Ok(Some(trash_item)) => { + // Before asserting any of the fields, we'll go ahead and remove the + // trashed file from the Recycle Bin, otherwise it'll be kept around + // after the test is finished. + // The returned `Result` is ignored, as we don't want the test to + // fail in case we're not able to remove the file from trash. + let _ = purge_all([&trash_item]); + + assert_eq!(trash_item.name, path.components().last().expect("Should have last component").as_os_str()); + assert_eq!(trash_item.original_parent, path.parent().expect("Should have parent")); + } + _ => panic!("Calling delete_with_info failed to return TrashItem."), + } +} From 4b69fd227fa18614f546abec964e8192c7f10b5e Mon Sep 17 00:00:00 2001 From: dino Date: Mon, 23 Feb 2026 12:10:20 +0000 Subject: [PATCH 05/10] refactor: smaller improvements and fixes * macOS: Ensure that `trash_url` is initialized as `None` instead of `NSURL::new()` to avoid allocation in the case of failure * macOS: Fix tests due to a missing `std::fs::remove_file` import * misc: Update documentation to further specify when are `TrashItem` impossible to construct, even when `with_info` is set to `true` * misc: Prefer use of `map` instead of `match` in `delete_all` --- src/lib.rs | 11 +++++------ src/macos/mod.rs | 2 +- src/windows.rs | 6 ++++-- tests/trash.rs | 2 +- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index adfefd7b..3db94209 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -110,15 +110,13 @@ impl TrashContext { let full_paths = canonicalize_paths(paths)?; trace!("Finished canonicalize_paths"); - match self.delete_all_canonicalized(full_paths, false) { - Ok(_) => Ok(()), - Err(err) => Err(err), - } + self.delete_all_canonicalized(full_paths, false).map(|_| ()) } /// Same as `delete`, but returns a `TrashItem` describing where the file /// ended up in trash. - /// Returns `None` on platforms that don't support returning trash info. + /// Returns `None` when trash item info is unavailable (e.g. when using the + /// Finder delete method on macOS). pub fn delete_with_info>(&self, path: T) -> Result, Error> { match self.delete_all_with_info(&[path])? { Some(mut items) => Ok(items.pop()), @@ -128,7 +126,8 @@ impl TrashContext { /// Same as `delete_all` but returns `TrashItem`s describing where files /// ended up in the trash. - /// Returns `None` on platforms that don't support returning trash info. + /// Returns `None` when trash item info is unavailable (e.g. when using the + /// Finder delete method on macOS). pub fn delete_all_with_info(&self, paths: I) -> Result>, Error> where I: IntoIterator, diff --git a/src/macos/mod.rs b/src/macos/mod.rs index 932a228f..328ca411 100644 --- a/src/macos/mod.rs +++ b/src/macos/mod.rs @@ -103,7 +103,7 @@ fn delete_using_file_mgr>(full_paths: &[P], with_info: bool) -> R let url = NSURL::fileURLWithPath(&path); trace!("Finished fileURLWithPath"); - let mut trash_url = Some(NSURL::new()); + let mut trash_url = None; trace!("Calling trashItemAtURL"); let res = file_mgr.trashItemAtURL_resultingItemURL_error(&url, Some(&mut trash_url)); diff --git a/src/windows.rs b/src/windows.rs index 2e510afc..c6dbee7b 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -1,4 +1,5 @@ use crate::{Error, TrashContext, TrashItem, TrashItemMetadata, TrashItemSize}; +use log::warn; use std::{ borrow::Borrow, ffi::{c_void, OsStr, OsString}, @@ -93,8 +94,9 @@ impl TrashContext { let mut items = Vec::with_capacity(ids.len()); for id in ids.iter() { - if let Ok(item) = get_trash_item_by_id(id) { - items.push(item); + match get_trash_item_by_id(id) { + Ok(item) => items.push(item), + Err(err) => warn!("Failed to look up trash item metadata for {:?}: {}", id, err), } } Ok(Some(items)) diff --git a/tests/trash.rs b/tests/trash.rs index 9f913b46..4c852dcc 100644 --- a/tests/trash.rs +++ b/tests/trash.rs @@ -206,7 +206,7 @@ fn test_delete_with_info_ns_file_manager() { // macOS' trash. // The returned `Result` is ignored, as we don't want the test to // fail in case we're not able to remove the file from trash. - let _ = remove_file(PathBuf::from(trash_item.id)); + let _ = std::fs::remove_file(PathBuf::from(trash_item.id)); assert_eq!(trash_item.name, path.components().last().expect("Should have last component").as_os_str()); assert_eq!(trash_item.original_parent, path.parent().expect("Should have parent").as_os_str()); From ab7d0dab03278eaabd2595726180143b84f61d31 Mon Sep 17 00:00:00 2001 From: dino Date: Mon, 23 Feb 2026 12:58:12 +0000 Subject: [PATCH 06/10] refactor(windows): avoid unnecessary type conversion Since `name` is already an OsString, we don't need to convert it from `OsString` to `String` to then convert back to `OsString`. This changes comes out of an exploration to try and get rid of the way that, on Windows, in order to return trashed items, we first need to collect the IDs of the items in `PostDeleteItem` to then fetch the item information one by one, in the Recycle Bin, using the item IDs. Unfortunately this doesn't seem possible. When trying to update the implementation so as to build the `TrashItem` in `PostDeleteItem`, it was noted that, for the newly created `IShellItem`, its extended properties, like the deletion date, are not yet populated at callback time, so the `ERR_NOT_FOUND (0x80070490)` error was being returned when trying to fetch those properties. As such, seems like we need to continue collecting only the IDs during the callback and look up full metadata via `SHCreateItemFromParsingName` after `PerformOperations` completes, so the properties are available. --- src/windows.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/windows.rs b/src/windows.rs index c6dbee7b..be524a47 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -312,12 +312,7 @@ unsafe fn get_trash_item_by_id(id: &OsStr) -> Result { let original_location = OsString::from_wide(original_location_bstr.as_wide()); let time_deleted = get_date_deleted_unix(&item2)?; - Ok(TrashItem { - id: id.to_os_string(), - name: name.into_string().map_err(|original| Error::ConvertOsString { original })?.into(), - original_parent: PathBuf::from(original_location), - time_deleted, - }) + Ok(TrashItem { id: id.to_os_string(), name, original_parent: PathBuf::from(original_location), time_deleted }) } /// A COM object implementing IFileOperationProgressSink to collect trash item IDs From 63d71bac79c32930c5cd4d75c2bdeaa1beefdfde Mon Sep 17 00:00:00 2001 From: dino Date: Mon, 23 Feb 2026 14:29:02 +0000 Subject: [PATCH 07/10] feat(macos): simplify public api by returning item on finder deletion Implement trash item info retrieval for the Finder delete method by converting AppleScript Finder object references to POSIX paths via `as alias`. Results are newline-delimited to avoid ambiguity with commas in filenames. This allows the public API to return `Result` and `Result>` instead of `Result>` and `Result>>` since all platforms and configurations now support returning trash item info. --- src/lib.rs | 26 ++++++++-------- src/macos/mod.rs | 78 ++++++++++++++++++++++++++++++++++++++---------- tests/trash.rs | 39 ++++++++++++++++++++++-- 3 files changed, 112 insertions(+), 31 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 3db94209..9f950c17 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -115,26 +115,26 @@ impl TrashContext { /// Same as `delete`, but returns a `TrashItem` describing where the file /// ended up in trash. - /// Returns `None` when trash item info is unavailable (e.g. when using the - /// Finder delete method on macOS). - pub fn delete_with_info>(&self, path: T) -> Result, Error> { - match self.delete_all_with_info(&[path])? { - Some(mut items) => Ok(items.pop()), - None => Ok(None), - } + pub fn delete_with_info>(&self, path: T) -> Result { + self.delete_all_with_info(&[path])? + .pop() + .ok_or(Error::Unknown { description: "delete_with_info did not return trash item information".into() }) } /// Same as `delete_all` but returns `TrashItem`s describing where files /// ended up in the trash. - /// Returns `None` when trash item info is unavailable (e.g. when using the - /// Finder delete method on macOS). - pub fn delete_all_with_info(&self, paths: I) -> Result>, Error> + pub fn delete_all_with_info(&self, paths: I) -> Result, Error> where I: IntoIterator, T: AsRef, { let full_paths = canonicalize_paths(paths)?; - self.delete_all_canonicalized(full_paths, true) + + self.delete_all_canonicalized(full_paths, true).and_then(|items| { + items.ok_or(Error::Unknown { + description: "delete_all_with_info did not return trash item information".into(), + }) + }) } } @@ -148,7 +148,7 @@ pub fn delete>(path: T) -> Result<(), Error> { /// Convenience method for `DEFAULT_TRASH_CTX.delete_with_info()`. /// /// See: [`TrashContext::delete_with_info`](TrashContext::delete_with_info) -pub fn delete_with_info>(path: T) -> Result, Error> { +pub fn delete_with_info>(path: T) -> Result { DEFAULT_TRASH_CTX.delete_with_info(path) } @@ -166,7 +166,7 @@ where /// Convenience method for `DEFAULT_TRASH_CTX.delete_all_with_info()`. /// /// See: [`TrashContext::delete_all_with_info`](TrashContext::delete_all_with_info) -pub fn delete_all_with_info(paths: I) -> Result>, Error> +pub fn delete_all_with_info(paths: I) -> Result, Error> where I: IntoIterator, T: AsRef, diff --git a/src/macos/mod.rs b/src/macos/mod.rs index 328ca411..9baed8a4 100644 --- a/src/macos/mod.rs +++ b/src/macos/mod.rs @@ -138,27 +138,48 @@ fn delete_using_file_mgr>(full_paths: &[P], with_info: bool) -> R } } -/// This method currently has a limitation where `with_info` doesn't change the -/// result of this method, seeing as The AppleScript `delete` command returns -/// Finder object references rather than POSIX paths, so we don't attempt to -/// parse them into TrashItems. -fn delete_using_finder>(full_paths: &[P], _with_info: bool) -> Result>, Error> { +fn delete_using_finder>(full_paths: &[P], with_info: bool) -> Result>, Error> { // AppleScript command to move files (or directories) to Trash looks like - // osascript -e 'tell application "Finder" to delete { POSIX file "file1", POSIX "file2" }' - // The `-e` flag is used to execute only one line of AppleScript. + // the snippet below, with `-e` being used to execute only one line of + // AppleScript. + // + // ``` + // osascript -e 'tell application "Finder" to delete { POSIX file "file1", POSIX "file2" }' + // ``` let mut command = Command::new("osascript"); let posix_files = full_paths .iter() - .map(|p| { - let path_b = p.as_ref().as_os_str().as_encoded_bytes(); - match std::str::from_utf8(path_b) { + .map(|path| { + let path_bytes = path.as_ref().as_os_str().as_encoded_bytes(); + + match std::str::from_utf8(path_bytes) { Ok(path_utf8) => format!(r#"POSIX file "{}""#, esc_quote(path_utf8)), // utf-8 path, escape \" - Err(_) => format!(r#"POSIX file "{}""#, esc_quote(&percent_encode(path_b))), // binary path, %-encode it and escape \" + Err(_) => format!(r#"POSIX file "{}""#, esc_quote(&percent_encode(path_bytes))), // binary path, %-encode it and escape \" } }) .collect::>() .join(", "); - let script = format!("tell application \"Finder\" to delete {{ {posix_files} }}"); + + // When `with_info` is requested, we convert the Finder object references + // returned by `delete` into POSIX paths using `as alias`. The results are + // newline-delimited so we can split them reliably (commas would be + // ambiguous for filenames containing commas). + let script = if with_info { + format!( + r#"tell application "Finder" + set trashedItems to delete {{ {posix_files} }} + if class of trashedItems is not list then set trashedItems to {{trashedItems}} + set posixPaths to "" + repeat with t in trashedItems + if posixPaths is not "" then set posixPaths to posixPaths & linefeed + set posixPaths to posixPaths & POSIX path of (t as alias) + end repeat + return posixPaths +end tell"# + ) + } else { + format!("tell application \"Finder\" to delete {{ {posix_files} }}") + }; let argv: Vec = vec!["-e".into(), script.into()]; command.args(argv); @@ -183,9 +204,36 @@ fn delete_using_finder>(full_paths: &[P], _with_info: bool) -> Re }; } - // The AppleScript `delete` command returns Finder object references rather - // than POSIX paths, so we don't attempt to parse them into TrashItems. - Ok(None) + if with_info { + let stdout = String::from_utf8_lossy(&result.stdout); + let time_deleted = SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .map(|duration| duration.as_secs() as i64) + .unwrap_or(-1); + + // In practice, the Finder `delete` command returns results in the same + // order as the input. We rely on this to pair trash paths with their + // original paths via `.zip()`. + let trash_items: Vec = stdout + .lines() + .zip(full_paths.iter()) + .map(|(trash_path, original_path)| { + let trash_path = Path::new(trash_path); + let original = original_path.as_ref(); + + TrashItem { + id: trash_path.as_os_str().to_os_string(), + name: original.file_name().map(|name| name.to_os_string()).unwrap_or_default(), + original_parent: original.parent().map(Path::to_owned).unwrap_or_default(), + time_deleted, + } + }) + .collect(); + + Ok(Some(trash_items)) + } else { + Ok(None) + } } /// std's from_utf8_lossy, but non-utf8 byte sequences are %-encoded instead of being replaced by a special symbol. diff --git a/tests/trash.rs b/tests/trash.rs index 4c852dcc..f132fd2e 100644 --- a/tests/trash.rs +++ b/tests/trash.rs @@ -199,7 +199,7 @@ fn test_delete_with_info_ns_file_manager() { trash.set_delete_method(DeleteMethod::NsFileManager); match trash.delete_with_info(&path) { - Ok(Some(trash_item)) => { + Ok(trash_item) => { // Before asserting any of the fields, we'll go ahead and remove the // trashed file from the trash, otherwise it'll be kept around after // the test is finished, as the test literally moves the file to @@ -215,6 +215,39 @@ fn test_delete_with_info_ns_file_manager() { } } +#[test] +#[serial] +#[cfg(target_os = "macos")] +fn test_delete_with_info_finder() { + use trash::macos::{DeleteMethod, TrashContextExtMacos}; + + // Create the test file to be deleted, ensuring that we include the current + // directory so we can later assert that the `original_parent` is preserved. + let path = env::current_dir().expect("Should be able to get current directory").join(get_unique_name()); + File::create_new(&path).unwrap(); + + let mut trash = TrashContext::new(); + trash.set_delete_method(DeleteMethod::Finder); + + match trash.delete_with_info(&path) { + Ok(trash_item) => { + // Clean up the trashed file so it doesn't linger in macOS' trash. + let _ = std::fs::remove_file(PathBuf::from(&trash_item.id)); + + assert_eq!(trash_item.name, path.components().last().expect("Should have last component").as_os_str()); + assert_eq!(trash_item.original_parent, path.parent().expect("Should have parent").as_os_str()); + // Verify that the id points to a path inside the .Trash directory + let id_path = PathBuf::from(&trash_item.id); + assert!( + id_path.to_string_lossy().contains(".Trash"), + "Expected trash item id to contain '.Trash', got: {:?}", + id_path + ); + } + _ => panic!("Calling delete_with_info with Finder method failed to return TrashItem."), + } +} + #[test] #[serial] #[cfg(target_os = "linux")] @@ -228,7 +261,7 @@ fn test_delete_with_info() { let trash = TrashContext::new(); match trash.delete_with_info(&path) { - Ok(Some(trash_item)) => { + Ok(trash_item) => { // Before asserting any of the fields, we'll go ahead and remove the // trashed file from the Freedesktop trash, otherwise it'll be kept // around after the test is finished. @@ -264,7 +297,7 @@ fn test_delete_with_info() { let trash = TrashContext::new(); match trash.delete_with_info(&path) { - Ok(Some(trash_item)) => { + Ok(trash_item) => { // Before asserting any of the fields, we'll go ahead and remove the // trashed file from the Recycle Bin, otherwise it'll be kept around // after the test is finished. From a69bfdf0f82541abed897e8b26f097975f467184 Mon Sep 17 00:00:00 2001 From: dino Date: Wed, 25 Feb 2026 22:49:56 +0000 Subject: [PATCH 08/10] refactor: move macos tests and add path clean up helper * Move newly introduced macOS tests from `tests/trash.rs` to `src/macos/tests.rs`, as I did not notice we already had macOS-specific tests in this file * Introduce `CleanupPaths` to help ensure that files are removed after tests are run, regardless of whether the tests succeeds or fails --- src/macos/tests.rs | 72 ++++++++++++++++++++++++++++++++++++++++++++++ tests/trash.rs | 67 ------------------------------------------ 2 files changed, 72 insertions(+), 67 deletions(-) diff --git a/src/macos/tests.rs b/src/macos/tests.rs index 7d4fcf05..21ec52ca 100644 --- a/src/macos/tests.rs +++ b/src/macos/tests.rs @@ -10,6 +10,30 @@ use std::os::unix::ffi::OsStrExt; use std::path::PathBuf; use std::process::Command; +/// Holds a list of paths to files to clean up after a test. +/// +/// Simply push the paths for whatever file was created during tests and the +/// `Drop` implementation will clean up the files after the test. +struct CleanupPaths(Vec); + +impl CleanupPaths { + pub fn new() -> Self { + Self(Vec::new()) + } + + pub fn push(&mut self, path: PathBuf) { + self.0.push(path); + } +} + +impl Drop for CleanupPaths { + fn drop(&mut self) { + for path in &self.0 { + let _ = std::fs::remove_file(path); + } + } +} + #[test] #[serial] fn test_delete_with_finder_quoted_paths() { @@ -102,3 +126,51 @@ fn create_hfs_volume() -> std::io::Result<(impl Drop, tempfile::TempDir)> { }; Ok((cleanup, tmp)) } + +#[test] +#[serial] +fn test_delete_with_info_ns_file_manager() { + let mut cleanup_paths = CleanupPaths::new(); + let path = std::env::current_dir().expect("Should be able to get current directory").join(get_unique_name()); + cleanup_paths.push(path.clone()); + File::create_new(&path).unwrap(); + + let mut trash = TrashContext::new(); + trash.set_delete_method(DeleteMethod::NsFileManager); + + match trash.delete_with_info(&path) { + Ok(trash_item) => { + let id_path = PathBuf::from(&trash_item.id); + cleanup_paths.push(id_path.clone()); + + assert_eq!(trash_item.name, path.components().last().expect("Should have last component").as_os_str()); + assert_eq!(trash_item.original_parent, path.parent().expect("Should have parent").as_os_str()); + assert!(id_path.to_string_lossy().contains(".Trash")) + } + _ => panic!("Calling delete_with_info failed to return TrashItem."), + } +} + +#[test] +#[serial] +fn test_delete_with_info_finder() { + let mut cleanup_paths = CleanupPaths::new(); + let path = std::env::current_dir().expect("Should be able to get current directory").join(get_unique_name()); + cleanup_paths.push(path.clone()); + File::create_new(&path).unwrap(); + + let mut trash = TrashContext::new(); + trash.set_delete_method(DeleteMethod::Finder); + + match trash.delete_with_info(&path) { + Ok(trash_item) => { + let id_path = PathBuf::from(&trash_item.id); + cleanup_paths.push(id_path.clone()); + + assert_eq!(trash_item.name, path.components().last().expect("Should have last component").as_os_str()); + assert_eq!(trash_item.original_parent, path.parent().expect("Should have parent").as_os_str()); + assert!(id_path.to_string_lossy().contains(".Trash")) + } + _ => panic!("Calling delete_with_info with Finder method failed to return TrashItem."), + } +} diff --git a/tests/trash.rs b/tests/trash.rs index f132fd2e..378d218f 100644 --- a/tests/trash.rs +++ b/tests/trash.rs @@ -181,73 +181,6 @@ fn recursive_file_with_content_deletion() { assert!(!parent_dir.exists()); } -#[test] -#[serial] -#[cfg(target_os = "macos")] -fn test_delete_with_info_ns_file_manager() { - use trash::macos::{DeleteMethod, TrashContextExtMacos}; - - // Create the test file to be deleted, ensuring that we include the current - // directory so we can later assert that the `original_parent` is preserved. - let path = env::current_dir().expect("Should be able to get current directory").join(get_unique_name()); - File::create_new(&path).unwrap(); - - // Create new test context with `NSFileManager`, as that is currently the - // only implementation that actually supports returning the trashed item - // information. - let mut trash = TrashContext::new(); - trash.set_delete_method(DeleteMethod::NsFileManager); - - match trash.delete_with_info(&path) { - Ok(trash_item) => { - // Before asserting any of the fields, we'll go ahead and remove the - // trashed file from the trash, otherwise it'll be kept around after - // the test is finished, as the test literally moves the file to - // macOS' trash. - // The returned `Result` is ignored, as we don't want the test to - // fail in case we're not able to remove the file from trash. - let _ = std::fs::remove_file(PathBuf::from(trash_item.id)); - - assert_eq!(trash_item.name, path.components().last().expect("Should have last component").as_os_str()); - assert_eq!(trash_item.original_parent, path.parent().expect("Should have parent").as_os_str()); - } - _ => panic!("Calling delete_with_info failed to return TrashItem."), - } -} - -#[test] -#[serial] -#[cfg(target_os = "macos")] -fn test_delete_with_info_finder() { - use trash::macos::{DeleteMethod, TrashContextExtMacos}; - - // Create the test file to be deleted, ensuring that we include the current - // directory so we can later assert that the `original_parent` is preserved. - let path = env::current_dir().expect("Should be able to get current directory").join(get_unique_name()); - File::create_new(&path).unwrap(); - - let mut trash = TrashContext::new(); - trash.set_delete_method(DeleteMethod::Finder); - - match trash.delete_with_info(&path) { - Ok(trash_item) => { - // Clean up the trashed file so it doesn't linger in macOS' trash. - let _ = std::fs::remove_file(PathBuf::from(&trash_item.id)); - - assert_eq!(trash_item.name, path.components().last().expect("Should have last component").as_os_str()); - assert_eq!(trash_item.original_parent, path.parent().expect("Should have parent").as_os_str()); - // Verify that the id points to a path inside the .Trash directory - let id_path = PathBuf::from(&trash_item.id); - assert!( - id_path.to_string_lossy().contains(".Trash"), - "Expected trash item id to contain '.Trash', got: {:?}", - id_path - ); - } - _ => panic!("Calling delete_with_info with Finder method failed to return TrashItem."), - } -} - #[test] #[serial] #[cfg(target_os = "linux")] From 68aa191d7b551008d4f600250b8ebbbbc3d28e6a Mon Sep 17 00:00:00 2001 From: dino Date: Mon, 2 Mar 2026 21:23:11 +0000 Subject: [PATCH 09/10] feat(macos)!: change default delete method to file manager Switch the default delete method on macOS from `Finder` to `NsFileManager`. The NSFileManager approach is faster (no process spawning or IPC), requires no extra permissions, and avoids playing the Finder trash sound. --- src/macos/mod.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/macos/mod.rs b/src/macos/mod.rs index 9baed8a4..7167d9aa 100644 --- a/src/macos/mod.rs +++ b/src/macos/mod.rs @@ -27,7 +27,6 @@ pub enum DeleteMethod { /// - Produces the sound that Finder usually makes when deleting a file /// - Shows the "Put Back" option in the context menu, when using the Finder application /// - /// This is the default. Finder, /// Use `trashItemAtURL` from the `NSFileManager` object to delete the files. @@ -40,12 +39,14 @@ pub enum DeleteMethod { /// at: /// - /// - + /// + /// This is the default. NsFileManager, } impl DeleteMethod { - /// Returns `DeleteMethod::Finder` + /// Returns `DeleteMethod::NsFileManager` pub const fn new() -> Self { - DeleteMethod::Finder + DeleteMethod::NsFileManager } } impl Default for DeleteMethod { From badc176c16225a5bf0dc7bfdda0151b382ad9224 Mon Sep 17 00:00:00 2001 From: dino Date: Wed, 25 Mar 2026 19:14:01 +0000 Subject: [PATCH 10/10] docs: better explanation regarding window's trash progress sink --- src/windows.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/windows.rs b/src/windows.rs index be524a47..1db4e32d 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -50,8 +50,11 @@ impl TrashContext { pfo.SetOperationFlags(FOF_NO_UI | FOF_ALLOWUNDO | FOF_WANTNUKEWARNING)?; - // Set up progress sink to collect trash item IDs if requested - // We need to keep sink_interface alive until after PerformOperations completes + // The `PostDeleteItem` callback provides the item's ID immediately, + // but the full shell metadata (original location, delete time) may + // not yet be written to the Recycle Bin at that point. + // We collect IDs during the operation and do a full metadata lookup + // only after `PerformOperations` completes. let (ids_arc, _sink_interface) = if with_info { let sink = TrashProgressSink::new(); let ids_arc = sink.ids.clone();