From 69e74a5f7240140d06a9d1d65bc8f9b7af6d462d Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Wed, 4 Dec 2024 18:13:51 +0700 Subject: [PATCH 01/41] Return trash items that are created by delete (old FR) --- src/freedesktop.rs | 29 ++++++++++++++++++++--------- src/lib.rs | 8 ++++---- src/macos/mod.rs | 9 +++++---- src/windows.rs | 12 +++++++----- 4 files changed, 36 insertions(+), 22 deletions(-) diff --git a/src/freedesktop.rs b/src/freedesktop.rs index a8bca87..82471c2 100644 --- a/src/freedesktop.rs +++ b/src/freedesktop.rs @@ -33,12 +33,13 @@ 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) -> Result>, Error> { let home_trash = home_trash()?; let sorted_mount_points = get_sorted_mount_points()?; let home_topdir = home_topdir(&sorted_mount_points)?; debug!("The home topdir is {:?}", home_topdir); let uid = unsafe { libc::getuid() }; + let mut items = Vec::with_capacity(full_paths.len()); for path in full_paths { debug!("Deleting {:?}", path); let topdir = get_first_topdir_containing_path(&path, &sorted_mount_points); @@ -47,18 +48,19 @@ 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))?; + items.push(move_to_trash(path, &home_trash, topdir).map_err(|(p, e)| fs_error(p, e))?); } 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))?; + items.push(move_to_trash(path, &home_trash, topdir).map_err(|(p, e)| fs_error(p, e))?); } else { execute_on_mounted_trash_folders(uid, topdir, true, true, |trash_path| { - move_to_trash(&path, trash_path, topdir) + items.push(move_to_trash(&path, trash_path, topdir)?); + Ok(()) }) .map_err(|(p, e)| fs_error(p, e))?; } } - Ok(()) + Ok(Some(items)) } } @@ -450,7 +452,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"); @@ -491,6 +493,7 @@ fn move_to_trash( info_name.push(".trashinfo"); let info_file_path = info_folder.join(&info_name); let info_result = OpenOptions::new().create_new(true).write(true).open(&info_file_path); + let mut time_deleted = -1; match info_result { Err(error) => { if error.kind() == std::io::ErrorKind::AlreadyExists { @@ -510,10 +513,12 @@ fn move_to_trash( #[cfg(feature = "chrono")] { let now = chrono::Local::now(); + time_deleted = now.timestamp(); writeln!(file, "DeletionDate={}", now.format("%Y-%m-%dT%H:%M:%S")) } #[cfg(not(feature = "chrono"))] { + time_deleted = -1; Ok(()) } }) @@ -537,12 +542,18 @@ fn move_to_trash( } Ok(_) => { // We did it! - break; + return Ok(TrashItem { + id: info_file_path.into(), + name: filename.into(), + original_parent: src + .parent() + .expect("Absolute path to trashed item should have a parent") + .to_path_buf(), + time_deleted, + }); } } } - - Ok(()) } /// An error may mean that a collision was found. diff --git a/src/lib.rs b/src/lib.rs index adccaea..42cf31d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -81,7 +81,7 @@ impl TrashContext { /// trash::delete("delete_me").unwrap(); /// assert!(File::open("delete_me").is_err()); /// ``` - pub fn delete>(&self, path: T) -> Result<(), Error> { + pub fn delete>(&self, path: T) -> Result>, Error> { self.delete_all(&[path]) } @@ -101,7 +101,7 @@ impl TrashContext { /// assert!(File::open("delete_me_1").is_err()); /// assert!(File::open("delete_me_2").is_err()); /// ``` - pub fn delete_all(&self, paths: I) -> Result<(), Error> + pub fn delete_all(&self, paths: I) -> Result>, Error> where I: IntoIterator, T: AsRef, @@ -116,14 +116,14 @@ impl TrashContext { /// Convenience method for `DEFAULT_TRASH_CTX.delete()`. /// /// See: [`TrashContext::delete`](TrashContext::delete) -pub fn delete>(path: T) -> Result<(), Error> { +pub fn delete>(path: T) -> Result>, Error> { DEFAULT_TRASH_CTX.delete(path) } /// Convenience method for `DEFAULT_TRASH_CTX.delete_all()`. /// /// See: [`TrashContext::delete_all`](TrashContext::delete_all) -pub fn delete_all(paths: I) -> Result<(), Error> +pub fn delete_all(paths: I) -> Result>, Error> where I: IntoIterator, T: AsRef, diff --git a/src/macos/mod.rs b/src/macos/mod.rs index 62027bd..dcaf3e4 100644 --- a/src/macos/mod.rs +++ b/src/macos/mod.rs @@ -7,7 +7,7 @@ use std::{ 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,11 +74,12 @@ 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) -> 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)?, + DeleteMethod::NsFileManager => delete_using_file_mgr(&full_paths)?, } + Ok(None) } } diff --git a/src/windows.rs b/src/windows.rs index 8727706..7b97bef 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -36,7 +36,10 @@ impl PlatformTrashContext { } impl TrashContext { /// 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_specified_canonicalized( + &self, + full_paths: Vec, + ) -> Result>, Error> { ensure_com_initialized(); unsafe { let pfo: IFileOperation = CoCreateInstance(&FileOperation as *const _, None, CLSCTX_ALL).unwrap(); @@ -66,14 +69,13 @@ impl TrashContext { // the list of HRESULT codes is not documented. return Err(Error::Unknown { description: "Some operations were aborted".into() }); } - Ok(()) + Ok(None) } } /// Removes all files and folder paths recursively. - pub(crate) fn delete_all_canonicalized(&self, full_paths: Vec) -> Result<(), Error> { - self.delete_specified_canonicalized(full_paths)?; - Ok(()) + pub(crate) fn delete_all_canonicalized(&self, full_paths: Vec) -> Result>, Error> { + self.delete_specified_canonicalized(full_paths) } } From 8e5cb9d6ca795125c7b954efed8489af82b09794 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Wed, 4 Dec 2024 18:18:27 +0700 Subject: [PATCH 02/41] fix mac return signatures --- src/macos/mod.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/macos/mod.rs b/src/macos/mod.rs index dcaf3e4..2851b13 100644 --- a/src/macos/mod.rs +++ b/src/macos/mod.rs @@ -76,14 +76,13 @@ impl TrashContextExtMacos for TrashContext { impl TrashContext { pub(crate) fn delete_all_canonicalized(&self, full_paths: Vec) -> 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), + DeleteMethod::NsFileManager => delete_using_file_mgr(&full_paths), } - Ok(None) } } -fn delete_using_file_mgr>(full_paths: &[P]) -> Result<(), Error> { +fn delete_using_file_mgr>(full_paths: &[P]) -> Result>, Error> { trace!("Starting delete_using_file_mgr"); let file_mgr = unsafe { NSFileManager::defaultManager() }; for path in full_paths { @@ -107,10 +106,10 @@ fn delete_using_file_mgr>(full_paths: &[P]) -> Result<(), Error> }); } } - Ok(()) + Ok(None) } -fn delete_using_finder>(full_paths: &[P]) -> Result<(), Error> { +fn delete_using_finder>(full_paths: &[P]) -> 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. @@ -150,7 +149,7 @@ fn delete_using_finder>(full_paths: &[P]) -> Result<(), Error> { } }; } - Ok(()) + Ok(None) } /// std's from_utf8_lossy, but non-utf8 byte sequences are %-encoded instead of being replaced by a special symbol. From 029e5e9307d8d5967e661bc1b62b320108aa0327 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Wed, 4 Dec 2024 19:31:45 +0700 Subject: [PATCH 03/41] update doc --- src/lib.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 42cf31d..8eb191d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -270,6 +270,9 @@ pub struct TrashItem { /// /// On Linux it is an absolute path to the `.trashinfo` file associated with /// the item. + /// + /// On macOS it is the string returned by the `.path()` method on the `NSURL` item + /// returned by the `trashItemAtURL_resultingItemURL_error` call. pub id: OsString, /// The name of the item. For example if the folder '/home/user/New Folder' @@ -286,7 +289,11 @@ pub struct TrashItem { /// The number of non-leap seconds elapsed between the UNIX Epoch and the /// moment the file was deleted. - /// Without the "chrono" feature, this will be a negative number on linux only. + /// Without the "chrono" feature, this will be a negative number on linux/macOS only. + /// macOS has the number, but there is no information on how to get it, + /// the usual 'kMDItemDateAdded' attribute doesn't exist for files @ trash + /// apple.stackexchange.com/questions/437475/how-can-i-find-out-when-a-file-had-been-moved-to-trash + /// stackoverflow.com/questions/53341670/access-the-file-date-added-in-terminal pub time_deleted: i64, } From 3cd198e1833037d15f884392e6122f6adf62e742 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Wed, 4 Dec 2024 23:43:25 +0700 Subject: [PATCH 04/41] dep: add chrono to macOS --- Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.toml b/Cargo.toml index 93dc38b..7a95e07 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,6 +39,7 @@ objc2-foundation = { version = "0.2.0", features = [ "NSURL", ] } percent-encoding = "2.3.1" +chrono = { version = "0.4.31", optional = true, default-features = false, features = ["clock",] } [target.'cfg(all(unix, not(target_os = "macos"), not(target_os = "ios"), not(target_os = "android")))'.dependencies] chrono = { version = "0.4.31", optional = true, default-features = false, features = [ From 6cc803b04ab5305746ab91af627692f29cf547a3 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Wed, 4 Dec 2024 18:35:57 +0700 Subject: [PATCH 05/41] add support for getting trashed items' data actual deleted time as recorded in file metadata (`kMDItemDateAdded`) seems inaccessible to us when a file is in the trash, so this follows the Linux approach of getting system time instead --- src/macos/mod.rs | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/src/macos/mod.rs b/src/macos/mod.rs index 2851b13..68c5555 100644 --- a/src/macos/mod.rs +++ b/src/macos/mod.rs @@ -4,8 +4,9 @@ use std::{ process::Command, }; -use log::trace; +use log::{trace,warn}; use objc2_foundation::{NSFileManager, NSString, NSURL}; +use objc2::rc::Retained; use crate::{into_unknown, Error, TrashContext, TrashItem}; @@ -85,8 +86,10 @@ impl TrashContext { fn delete_using_file_mgr>(full_paths: &[P]) -> Result>, Error> { trace!("Starting delete_using_file_mgr"); let file_mgr = unsafe { NSFileManager::defaultManager() }; + let mut items = Vec::with_capacity(full_paths.len()); for path in full_paths { - let path = path.as_ref().as_os_str().as_encoded_bytes(); + let path_r = path.as_ref(); + let path = path_r.as_os_str().as_encoded_bytes(); let path = match std::str::from_utf8(path) { Ok(path_utf8) => NSString::from_str(path_utf8), // utf-8 path, use as is Err(_) => NSString::from_str(&percent_encode(path)), // binary path, %-encode it @@ -97,16 +100,31 @@ fn delete_using_file_mgr>(full_paths: &[P]) -> Result> = None; + let res = unsafe { file_mgr.trashItemAtURL_resultingItemURL_error(&url, Some(&mut out_res_nsurl)) }; trace!("Finished trashItemAtURL"); if let Err(err) = res { return Err(Error::Unknown { - description: format!("While deleting '{:?}', `trashItemAtURL` failed: {err}", path.as_ref()), + description: format!("While deleting '{:?}', `trashItemAtURL` failed: {err}", path), }); + } else { + if let Some(out_nsurl) = out_res_nsurl { + let mut time_deleted = -1; + #[cfg( feature = "chrono") ] {let now = chrono::Local::now(); time_deleted = now.timestamp();} + #[cfg(not(feature = "chrono"))] { time_deleted = -1;} + if let Some(nspath) = unsafe {out_nsurl.path()} { // Option> + items.push(TrashItem { + id : nspath.to_string().into(), + name : path_r.file_name().expect("Item to be trashed should have a name" ).into(), + original_parent: path_r.parent ().expect("Item to be trashed should have a parent").to_path_buf(), + time_deleted, + }); + } else {warn!("OS did not return path string from the URL of the trashed item '{:?}', originally located at: '{:?}'", out_nsurl, path);} + } else {warn!("OS did not return a path to the trashed file, originally located at: '{:?}'" , path);} } } - Ok(None) + Ok(Some(items)) } fn delete_using_finder>(full_paths: &[P]) -> Result>, Error> { From ffe57a417f0b441d78927015eb3173004d9d415c Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Thu, 5 Dec 2024 00:25:13 +0700 Subject: [PATCH 06/41] return a single TrashItem for a single trashed item instead of currently returning a vec just because 1 delete still goes through delete_all --- src/lib.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 8eb191d..5651c14 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -81,8 +81,13 @@ impl TrashContext { /// trash::delete("delete_me").unwrap(); /// assert!(File::open("delete_me").is_err()); /// ``` - pub fn delete>(&self, path: T) -> Result>, Error> { - self.delete_all(&[path]) + pub fn delete>(&self, path: T) -> Result, Error> { + match self.delete_all(&[path]) { // Result>> + Ok(maybe_items) => match maybe_items { + Some(mut items) => Ok(items.pop()), // no need to check that vec.len=2? + None => Ok(None), }, + Err(e) => Err(e), + } } /// Removes all files/directories specified by the collection of paths provided as an argument. @@ -116,7 +121,7 @@ impl TrashContext { /// Convenience method for `DEFAULT_TRASH_CTX.delete()`. /// /// See: [`TrashContext::delete`](TrashContext::delete) -pub fn delete>(path: T) -> Result>, Error> { +pub fn delete>(path: T) -> Result, Error> { DEFAULT_TRASH_CTX.delete(path) } From 9a56e8b54e5799bcd9f08399a5981b12061ba3a5 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Thu, 5 Dec 2024 00:52:01 +0700 Subject: [PATCH 07/41] update docs --- src/macos/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/macos/mod.rs b/src/macos/mod.rs index 68c5555..352c0fd 100644 --- a/src/macos/mod.rs +++ b/src/macos/mod.rs @@ -16,6 +16,7 @@ use crate::{into_unknown, Error, TrashContext, TrashItem}; /// |
Feature |≝
Finder |
NsFileManager | /// |:-----------------------|:--------------:|:----------------:| /// |Undo via "Put back" | ✓ | ✗ | +/// |Get trashed paths | ✗ | ✓ | /// |Speed | ✗
Slower | ✓
Faster | /// |No sound | ✗ | ✓ | /// |No extra permissions | ✗ | ✓ | @@ -40,6 +41,7 @@ pub enum DeleteMethod { /// at: /// - /// - + /// - Allows getting the paths to the trashed items NsFileManager, } impl DeleteMethod { From db0868bf8119856daaa5a92731984741c17405db Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Thu, 5 Dec 2024 00:25:37 +0700 Subject: [PATCH 08/41] __TODO why is this needed? --- src/macos/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/macos/mod.rs b/src/macos/mod.rs index 352c0fd..d60b2c1 100644 --- a/src/macos/mod.rs +++ b/src/macos/mod.rs @@ -112,6 +112,7 @@ fn delete_using_file_mgr>(full_paths: &[P]) -> Result Date: Thu, 5 Dec 2024 01:03:32 +0700 Subject: [PATCH 09/41] mac: split delete functions into 2 categories: those that return trashed items and those that done so that you can avoid the overhead of storing all the info you might not need --- src/lib.rs | 48 ++++++++++++++++++++++++++++++++++++++++++------ src/macos/mod.rs | 22 ++++++++++++---------- 2 files changed, 54 insertions(+), 16 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 5651c14..cca84da 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -70,7 +70,7 @@ impl TrashContext { /// Removes a single file or directory. /// /// When a symbolic link is provided to this function, the symbolic link will be removed and the link - /// target will be kept intact. + /// target will be kept intact. Successful results will have always have None trash items. /// /// # Example /// @@ -81,8 +81,13 @@ impl TrashContext { /// trash::delete("delete_me").unwrap(); /// assert!(File::open("delete_me").is_err()); /// ``` - pub fn delete>(&self, path: T) -> Result, Error> { - match self.delete_all(&[path]) { // Result>> + pub fn delete>(&self, path: T) -> Result>, Error> { + self.delete_all(&[path]) + } + + /// Same as `delete`, but returns `TrashItem` if available. + pub fn delete_with_info>(&self, path: T) -> Result, Error> { + match self.delete_all_with_info(&[path]) { // Result>> Ok(maybe_items) => match maybe_items { Some(mut items) => Ok(items.pop()), // no need to check that vec.len=2? None => Ok(None), }, @@ -93,7 +98,7 @@ impl TrashContext { /// Removes all files/directories specified by the collection of paths provided as an argument. /// /// When a symbolic link is provided to this function, the symbolic link will be removed and the link - /// target will be kept intact. + /// target will be kept intact. Successful results will have always have None trash items. /// /// # Example /// @@ -114,17 +119,37 @@ impl TrashContext { trace!("Starting canonicalize_paths"); let full_paths = canonicalize_paths(paths)?; trace!("Finished canonicalize_paths"); - self.delete_all_canonicalized(full_paths) + self.delete_all_canonicalized(full_paths, false) + } + + /// Same as `delete_all, but returns `TrashItem`s if available. + pub fn delete_all_with_info(&self, paths: I) -> Result>, Error> + where + I: IntoIterator, + T: AsRef, + { + trace!("Starting canonicalize_paths"); + let full_paths = canonicalize_paths(paths)?; + trace!("Finished canonicalize_paths"); + self.delete_all_canonicalized(full_paths, true) } + } /// Convenience method for `DEFAULT_TRASH_CTX.delete()`. /// /// See: [`TrashContext::delete`](TrashContext::delete) -pub fn delete>(path: T) -> Result, Error> { +pub fn delete>(path: T) -> Result>, Error> { DEFAULT_TRASH_CTX.delete(path) } +/// Convenience method for `DEFAULT_TRASH_CTX.delete_with_info()`. +/// +/// See: [`TrashContext::delete`](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) @@ -136,6 +161,17 @@ where DEFAULT_TRASH_CTX.delete_all(paths) } +/// Convenience method for `DEFAULT_TRASH_CTX.delete_all_with_info()`. +/// +/// See: [`TrashContext::delete_all`](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 d60b2c1..48dfd40 100644 --- a/src/macos/mod.rs +++ b/src/macos/mod.rs @@ -77,18 +77,19 @@ 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), //Finder doesn't return trashed paths + 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 = unsafe { NSFileManager::defaultManager() }; - let mut items = Vec::with_capacity(full_paths.len()); + let mut items = if with_info { Vec::with_capacity(full_paths.len()) + } else { vec![] }; for path in full_paths { let path_r = path.as_ref(); let path = path_r.as_os_str().as_encoded_bytes(); @@ -103,14 +104,15 @@ fn delete_using_file_mgr>(full_paths: &[P]) -> Result> = None; - let res = unsafe { file_mgr.trashItemAtURL_resultingItemURL_error(&url, Some(&mut out_res_nsurl)) }; + let res = if with_info {unsafe { file_mgr.trashItemAtURL_resultingItemURL_error(&url, None ) } + } else {unsafe { file_mgr.trashItemAtURL_resultingItemURL_error(&url, Some(&mut out_res_nsurl)) }}; trace!("Finished trashItemAtURL"); if let Err(err) = res { return Err(Error::Unknown { description: format!("While deleting '{:?}', `trashItemAtURL` failed: {err}", path), }); - } else { + } else if with_info { if let Some(out_nsurl) = out_res_nsurl { #[allow(unused_assignments)] let mut time_deleted = -1; @@ -124,10 +126,10 @@ fn delete_using_file_mgr>(full_paths: &[P]) -> Result>(full_paths: &[P]) -> Result>, Error> { From d20a453c3765d7017400ba1fa28b4bedc6d3fd58 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Thu, 5 Dec 2024 16:19:27 +0700 Subject: [PATCH 10/41] fmt --- src/lib.rs | 13 +++++----- src/macos/mod.rs | 63 +++++++++++++++++++++++++++++++++--------------- 2 files changed, 51 insertions(+), 25 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index cca84da..d57bbbd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -81,17 +81,19 @@ impl TrashContext { /// trash::delete("delete_me").unwrap(); /// assert!(File::open("delete_me").is_err()); /// ``` - pub fn delete>(&self, path: T) -> Result>, Error> { + pub fn delete>(&self, path: T) -> Result>, Error> { self.delete_all(&[path]) } /// Same as `delete`, but returns `TrashItem` if available. pub fn delete_with_info>(&self, path: T) -> Result, Error> { - match self.delete_all_with_info(&[path]) { // Result>> - Ok(maybe_items) => match maybe_items { + match self.delete_all_with_info(&[path]) { + // Result>> + Ok(maybe_items) => match maybe_items { Some(mut items) => Ok(items.pop()), // no need to check that vec.len=2? - None => Ok(None), }, - Err(e) => Err(e), + None => Ok(None), + }, + Err(e) => Err(e), } } @@ -133,7 +135,6 @@ impl TrashContext { trace!("Finished canonicalize_paths"); self.delete_all_canonicalized(full_paths, true) } - } /// Convenience method for `DEFAULT_TRASH_CTX.delete()`. diff --git a/src/macos/mod.rs b/src/macos/mod.rs index 48dfd40..192345a 100644 --- a/src/macos/mod.rs +++ b/src/macos/mod.rs @@ -4,9 +4,9 @@ use std::{ process::Command, }; -use log::{trace,warn}; -use objc2_foundation::{NSFileManager, NSString, NSURL}; +use log::{trace, warn}; use objc2::rc::Retained; +use objc2_foundation::{NSFileManager, NSString, NSURL}; use crate::{into_unknown, Error, TrashContext, TrashItem}; @@ -77,10 +77,14 @@ impl TrashContextExtMacos for TrashContext { } } 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> { match self.platform_specific.delete_method { - DeleteMethod::Finder => delete_using_finder(&full_paths), //Finder doesn't return trashed paths - DeleteMethod::NsFileManager => delete_using_file_mgr(&full_paths, with_info), + DeleteMethod::Finder => delete_using_finder(&full_paths), //Finder doesn't return trashed paths + DeleteMethod::NsFileManager => delete_using_file_mgr(&full_paths, with_info), } } } @@ -88,8 +92,7 @@ impl TrashContext { fn delete_using_file_mgr>(full_paths: &[P], with_info: bool) -> Result>, Error> { trace!("Starting delete_using_file_mgr"); let file_mgr = unsafe { NSFileManager::defaultManager() }; - let mut items = if with_info { Vec::with_capacity(full_paths.len()) - } else { vec![] }; + let mut items = if with_info { Vec::with_capacity(full_paths.len()) } else { vec![] }; for path in full_paths { let path_r = path.as_ref(); let path = path_r.as_os_str().as_encoded_bytes(); @@ -104,8 +107,11 @@ fn delete_using_file_mgr>(full_paths: &[P], with_info: bool) -> R trace!("Calling trashItemAtURL"); let mut out_res_nsurl: Option> = None; - let res = if with_info {unsafe { file_mgr.trashItemAtURL_resultingItemURL_error(&url, None ) } - } else {unsafe { file_mgr.trashItemAtURL_resultingItemURL_error(&url, Some(&mut out_res_nsurl)) }}; + let res = if with_info { + unsafe { file_mgr.trashItemAtURL_resultingItemURL_error(&url, None) } + } else { + unsafe { file_mgr.trashItemAtURL_resultingItemURL_error(&url, Some(&mut out_res_nsurl)) } + }; trace!("Finished trashItemAtURL"); if let Err(err) = res { @@ -116,20 +122,39 @@ fn delete_using_file_mgr>(full_paths: &[P], with_info: bool) -> R if let Some(out_nsurl) = out_res_nsurl { #[allow(unused_assignments)] let mut time_deleted = -1; - #[cfg( feature = "chrono") ] {let now = chrono::Local::now(); time_deleted = now.timestamp();} - #[cfg(not(feature = "chrono"))] { time_deleted = -1;} - if let Some(nspath) = unsafe {out_nsurl.path()} { // Option> + #[cfg(feature = "chrono")] + { + let now = chrono::Local::now(); + time_deleted = now.timestamp(); + } + #[cfg(not(feature = "chrono"))] + { + time_deleted = -1; + } + if let Some(nspath) = unsafe { out_nsurl.path() } { + // Option> items.push(TrashItem { - id : nspath.to_string().into(), - name : path_r.file_name().expect("Item to be trashed should have a name" ).into(), - original_parent: path_r.parent ().expect("Item to be trashed should have a parent").to_path_buf(), + id: nspath.to_string().into(), + name: path_r.file_name().expect("Item to be trashed should have a name").into(), + original_parent: path_r + .parent() + .expect("Item to be trashed should have a parent") + .to_path_buf(), time_deleted, }); - } else {warn!("OS did not return path string from the URL of the trashed item '{:?}', originally located at: '{:?}'", out_nsurl, path);} - } else {warn!("OS did not return a path to the trashed file, originally located at: '{:?}'" , path);}} + } else { + warn!("OS did not return path string from the URL of the trashed item '{:?}', originally located at: '{:?}'", out_nsurl, path); + } + } else { + warn!("OS did not return a path to the trashed file, originally located at: '{:?}'", path); + } + } + } + if with_info { + Ok(Some(items)) + } else { + Ok(None) } - if with_info { Ok(Some(items)) - } else { Ok(None) } } fn delete_using_finder>(full_paths: &[P]) -> Result>, Error> { From a2db98072ca3cbf6ce03ed79e18cc333982f7018 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Thu, 5 Dec 2024 17:05:25 +0700 Subject: [PATCH 11/41] fix wrong delete arg order --- src/macos/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/macos/mod.rs b/src/macos/mod.rs index 192345a..4cb9829 100644 --- a/src/macos/mod.rs +++ b/src/macos/mod.rs @@ -108,9 +108,9 @@ fn delete_using_file_mgr>(full_paths: &[P], with_info: bool) -> R trace!("Calling trashItemAtURL"); let mut out_res_nsurl: Option> = None; let res = if with_info { - unsafe { file_mgr.trashItemAtURL_resultingItemURL_error(&url, None) } - } else { unsafe { file_mgr.trashItemAtURL_resultingItemURL_error(&url, Some(&mut out_res_nsurl)) } + } else { + unsafe { file_mgr.trashItemAtURL_resultingItemURL_error(&url, None) } }; trace!("Finished trashItemAtURL"); From bcb755e91a45806d4856ed2ddb5bea76da989469 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Thu, 5 Dec 2024 16:22:03 +0700 Subject: [PATCH 12/41] add stub arguments for Windows/Linux --- src/freedesktop.rs | 2 +- src/windows.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/freedesktop.rs b/src/freedesktop.rs index 82471c2..86bdb5c 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/windows.rs b/src/windows.rs index 7b97bef..f0b24d7 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -74,7 +74,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) } } From 34fe204eaad0a461dd03bc49f017426b2c87cee8 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Thu, 5 Dec 2024 16:57:48 +0700 Subject: [PATCH 13/41] fmt --- src/freedesktop.rs | 6 +++++- src/windows.rs | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/freedesktop.rs b/src/freedesktop.rs index 86bdb5c..0bc70c8 100644 --- a/src/freedesktop.rs +++ b/src/freedesktop.rs @@ -33,7 +33,11 @@ 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)?; diff --git a/src/windows.rs b/src/windows.rs index f0b24d7..e8434e0 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -74,7 +74,11 @@ impl TrashContext { } /// Removes all files and folder paths recursively. - 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> { self.delete_specified_canonicalized(full_paths) } } From 3c393ded197cb27277911094576fa616acc4fac2 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Thu, 5 Dec 2024 17:47:46 +0700 Subject: [PATCH 14/41] upd comment --- src/macos/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/macos/mod.rs b/src/macos/mod.rs index 4cb9829..206bb2c 100644 --- a/src/macos/mod.rs +++ b/src/macos/mod.rs @@ -83,7 +83,7 @@ impl TrashContext { with_info: bool, ) -> Result>, Error> { match self.platform_specific.delete_method { - DeleteMethod::Finder => delete_using_finder(&full_paths), //Finder doesn't return trashed paths + DeleteMethod::Finder => delete_using_finder(&full_paths), // TODO: parse Finder's output to get paths? DeleteMethod::NsFileManager => delete_using_file_mgr(&full_paths, with_info), } } From 17f212ba76c3658c37b11e52a77a010f6f4503d1 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Fri, 6 Dec 2024 20:38:45 +0700 Subject: [PATCH 15/41] test: add test_delete_binary_path_with_ns_file_manager_with_info --- src/macos/tests.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/macos/tests.rs b/src/macos/tests.rs index 7d4fcf0..0160af0 100644 --- a/src/macos/tests.rs +++ b/src/macos/tests.rs @@ -1,4 +1,5 @@ use crate::{ + canonicalize_paths, macos::{percent_encode, DeleteMethod, TrashContextExtMacos}, tests::{get_unique_name, init_logging}, TrashContext, @@ -41,6 +42,29 @@ fn test_delete_with_ns_file_manager() { assert!(File::open(&path).is_err()); } +#[test] +#[serial] +fn test_delete_binary_path_with_ns_file_manager_with_info() { + init_logging(); + let mut trash_ctx = TrashContext::default(); + trash_ctx.set_delete_method(DeleteMethod::NsFileManager); + + let mut path = PathBuf::from(get_unique_name()); + let paths = canonicalize_paths(&[path]).unwrap(); // need full path to get parent + assert!(!paths.is_empty()); + path = paths[0].clone(); + let name = path.file_name().unwrap(); + let original_parent = path.parent().unwrap(); + + File::create_new(&path).unwrap(); + let trash_item = trash_ctx.delete_with_info(&path).unwrap().unwrap(); + assert!(File::open(&path).is_err()); + assert_eq!(name, trash_item.name); + assert_eq!(original_parent, trash_item.original_parent); + // TrashItem's date deleted not tested since we can't guarantee the date we calculate here will match the date calculated @ delete + // TrashItem's path@trash not tested since we can't guarantee the ours will be identical to the one FileManager decides to use (names change if they're dupe, also trash path is a bit tricky to get right as it changes depending on the user/admin) +} + #[test] #[serial] fn test_delete_binary_path_with_ns_file_manager() { From e9717d47ce7a40415876df93fc5067904902f44b Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Fri, 6 Dec 2024 20:57:44 +0700 Subject: [PATCH 16/41] dep: add osakit to parse AppleScript's output --- Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.toml b/Cargo.toml index 7a95e07..a0e385e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,6 +40,7 @@ objc2-foundation = { version = "0.2.0", features = [ ] } percent-encoding = "2.3.1" chrono = { version = "0.4.31", optional = true, default-features = false, features = ["clock",] } +osakit = "0.2.4" [target.'cfg(all(unix, not(target_os = "macos"), not(target_os = "ios"), not(target_os = "android")))'.dependencies] chrono = { version = "0.4.31", optional = true, default-features = false, features = [ From 1609ec174258aef5718beedbcf40e183844945f2 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Fri, 6 Dec 2024 22:30:10 +0700 Subject: [PATCH 17/41] update description --- src/lib.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index d57bbbd..b8ac6b4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -319,6 +319,9 @@ pub struct TrashItem { /// The name of the item. For example if the folder '/home/user/New Folder' /// was deleted, its `name` is 'New Folder' + /// macOS: when trashing with DeleteMethod::Finder, files are passed to Finder in a single batch, + /// so if the size of the returned list of trashed paths is different from the list of items we sent + /// to trash, we can't match input to output, so will leave this field "" blank pub name: OsString, /// The path to the parent folder of this item before it was put inside the @@ -327,6 +330,9 @@ pub struct TrashItem { /// /// To get the full path to the file in its original location use the /// `original_path` function. + /// macOS: when trashing with DeleteMethod::Finder, files are passed to Finder in a single batch, + /// so if the size of the returned list of trashed paths is different from the list of items we sent + /// to trash, we can't match input to output, so will leave this field "" blank pub original_parent: PathBuf, /// The number of non-leap seconds elapsed between the UNIX Epoch and the @@ -334,8 +340,10 @@ pub struct TrashItem { /// Without the "chrono" feature, this will be a negative number on linux/macOS only. /// macOS has the number, but there is no information on how to get it, /// the usual 'kMDItemDateAdded' attribute doesn't exist for files @ trash - /// apple.stackexchange.com/questions/437475/how-can-i-find-out-when-a-file-had-been-moved-to-trash - /// stackoverflow.com/questions/53341670/access-the-file-date-added-in-terminal + /// apple.stackexchange.com/questions/437475/how-can-i-find-out-when-a-file-had-been-moved-to-trash + /// stackoverflow.com/questions/53341670/access-the-file-date-added-in-terminal + /// macOS: when trashing with DeleteMethod::Finder, files are passed to Finder in a single batch, so the timing will be + /// set to the time before a batch is trashed and is the same for all files in a batch pub time_deleted: i64, } From 5576cf7bf44770b1c4e8fbbe05658fbffcd93312 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Fri, 6 Dec 2024 20:46:20 +0700 Subject: [PATCH 18/41] mac: add support for Finder returned url to trashed items replace cmd osascript with Objc bindings with osakit to allow proper results parsing --- src/macos/mod.rs | 94 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 63 insertions(+), 31 deletions(-) diff --git a/src/macos/mod.rs b/src/macos/mod.rs index 206bb2c..8ffe78a 100644 --- a/src/macos/mod.rs +++ b/src/macos/mod.rs @@ -1,14 +1,12 @@ use std::{ - ffi::OsString, path::{Path, PathBuf}, - process::Command, }; use log::{trace, warn}; use objc2::rc::Retained; use objc2_foundation::{NSFileManager, NSString, NSURL}; -use crate::{into_unknown, Error, TrashContext, TrashItem}; +use crate::{Error, TrashContext, TrashItem}; #[derive(Copy, Clone, Debug)] /// There are 2 ways to trash files: via the ≝Finder app or via the OS NsFileManager call @@ -83,7 +81,7 @@ impl TrashContext { with_info: bool, ) -> Result>, Error> { match self.platform_specific.delete_method { - DeleteMethod::Finder => delete_using_finder(&full_paths), // TODO: parse Finder's output to get paths? + DeleteMethod::Finder => delete_using_finder(&full_paths, with_info), DeleteMethod::NsFileManager => delete_using_file_mgr(&full_paths, with_info), } } @@ -157,11 +155,12 @@ fn delete_using_file_mgr>(full_paths: &[P], with_info: bool) -> R } } -fn delete_using_finder>(full_paths: &[P]) -> 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. - let mut command = Command::new("osascript"); +fn delete_using_finder + std::fmt::Debug>(full_paths: &[P], with_info: bool) -> Result>, Error> { + // TODO: should we convert to trashing item by item instead of in batches to have a guaranteed match of input to output? + // which method is faster? + // what about with a lot of items? will a huge script combining all paths still work? + use osakit::{Language, Script}; + let mut items: Vec = if with_info { Vec::with_capacity(full_paths.len()) } else { vec![] }; let posix_files = full_paths .iter() .map(|p| { @@ -173,29 +172,62 @@ fn delete_using_finder>(full_paths: &[P]) -> Result>() .join(", "); - let script = format!("tell application \"Finder\" to delete {{ {posix_files} }}"); + let script_text = if with_info { format!(r#" + tell application "Finder" + set Trash_items to delete {{ {posix_files} }} + end tell + repeat with aFile in Trash_items -- Finder reference + set contents of aFile to (POSIX path of (aFile as alias)) -- can't get paths of Finder reference, coersion to alias needed + end repeat + return Trash_items + "#) + } else {format!(r#"tell application "Finder" to delete {{ {posix_files} }}"#)}; + let mut script = Script::new_from_source(Language::AppleScript, &script_text); - let argv: Vec = vec!["-e".into(), script.into()]; - command.args(argv); - - // Execute command - let result = command.output().map_err(into_unknown)?; - if !result.status.success() { - let stderr = String::from_utf8_lossy(&result.stderr); - match result.status.code() { - None => { - return Err(Error::Unknown { - description: format!("The AppleScript exited with error. stderr: {}", stderr), - }) - } - - Some(code) => { - return Err(Error::Os { - code, - description: format!("The AppleScript exited with error. stderr: {}", stderr), - }) - } - }; + // Compile and Execute script + match script.compile() { + Ok(_) => match script.execute() { + Ok(res) => { if with_info { + #[allow(unused_assignments)] + let mut time_deleted = -1; + #[cfg(feature = "chrono")] + { + let now = chrono::Local::now(); + time_deleted = now.timestamp(); + } + #[cfg(not(feature = "chrono"))] + { + time_deleted = -1; + } + // todo: check if single file still produces an array + if let Some(file_list) = res.as_array() { + let len_match = full_paths.len()==file_list.len(); + if ! len_match {warn!("AppleScript returned a list of trashed paths len {} ≠ {} expected items sent to be trashed, to trashed items will have empty names/original parents as we cant be certain which trash path matches which trashed item",full_paths.len(),file_list.len());} + for (i,posix_path) in file_list.iter().enumerate() { + if let Some(file_path) = posix_path.as_str() { // Finder's return paths should be utf8 (%-encoded?)? + //let p=PathBuf::from(file_path); + //println!("✓converted posix_path:{} + // \nexists {} {:?}", posix_path, p.exists(),p); + let path_r = if len_match {full_paths[i].as_ref()} else {Path::new("")}; + items.push(TrashItem { + id: file_path.into(), + name: if len_match {path_r.file_name().expect("Item to be trashed should have a name").into() + } else {"".into()}, + original_parent: if len_match {path_r.parent().expect("Item to be trashed should have a parent").to_path_buf() + } else {"".into()}, + time_deleted, + }); + } else {warn!("Failed to parse AppleScript's returned path to the trashed file: {:?}",&posix_path);} + } + return Ok(Some(items)) + } else { + let ss = if full_paths.len() > 1 {"s"} else {""}; + warn!("AppleScript did not return a list of path{} to the trashed file{}, originally located at: {:?}",&ss,&ss,&full_paths); + } + }}, + Err(e) => return Err(Error::Unknown {description: format!("The AppleScript failed with error: {}", e),}), + }, + Err(e) => return Err(Error::Unknown {description: format!("The AppleScript failed to compile with error: {}", e),}), } Ok(None) } From 5b1c2c61e3426d9eec5b9b00fdad6b3f1a1747ae Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Fri, 6 Dec 2024 20:47:33 +0700 Subject: [PATCH 19/41] test: add test_delete_with_finder_with_info --- src/macos/tests.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/macos/tests.rs b/src/macos/tests.rs index 0160af0..6d080e9 100644 --- a/src/macos/tests.rs +++ b/src/macos/tests.rs @@ -11,6 +11,29 @@ use std::os::unix::ffi::OsStrExt; use std::path::PathBuf; use std::process::Command; +#[test] +#[serial] +fn test_delete_with_finder_with_info() { + init_logging(); + let mut trash_ctx = TrashContext::default(); + trash_ctx.set_delete_method(DeleteMethod::Finder); + + let mut path1 = PathBuf::from(get_unique_name()); + let mut path2 = PathBuf::from(get_unique_name()); + path1.set_extension(r#"a"b,"#); + path2.set_extension(r#"x80=%80 slash=\ pc=% quote=" comma=,"#); + File::create_new(&path1).unwrap(); + File::create_new(&path2).unwrap(); + let trashed_items = trash_ctx.delete_all_with_info(&[path1.clone(),path2.clone()]).unwrap().unwrap(); //Ok + Some trashed paths + assert!(File::open(&path1).is_err()); // original files deleted + assert!(File::open(&path2).is_err()); + for item in trashed_items { + let trashed_path = item.id; + assert!(!File::open(&trashed_path).is_err()); // returned trash items exist + std::fs::remove_file(&trashed_path).unwrap(); // clean up + assert!( File::open(&trashed_path).is_err()); // cleaned up trash items + } +} #[test] #[serial] fn test_delete_with_finder_quoted_paths() { From d11b8e0f500228615ca638569f3bf556196ae1f5 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Fri, 6 Dec 2024 22:38:54 +0700 Subject: [PATCH 20/41] test: add test_delete_with_finder_with_info_invalid --- src/macos/tests.rs | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/macos/tests.rs b/src/macos/tests.rs index 6d080e9..5a7c616 100644 --- a/src/macos/tests.rs +++ b/src/macos/tests.rs @@ -34,6 +34,33 @@ fn test_delete_with_finder_with_info() { assert!( File::open(&trashed_path).is_err()); // cleaned up trash items } } + + +// #[test] +// #[serial] +// fn test_delete_with_finder_with_info_invalid() { +// // add invalid byt +// init_logging(); +// let mut trash_ctx = TrashContext::default(); +// trash_ctx.set_delete_method(DeleteMethod::Finder); + +// let p = PathBuf::from("/Volumes/Untitled"); + +// let invalid_utf8 = b"\x80"; // lone continuation byte (128) (invalid utf8) + +// let mut path1 = p.clone(); +// let mut path2 = p.clone(); +// path1.push(get_unique_name()); +// path2.push(get_unique_name()); +// path1.set_extension(OsStr::from_bytes(invalid_utf8)); +// path2.set_extension(OsStr::from_bytes(invalid_utf8)); + +// File::create_new(&path1).unwrap(); +// File::create_new(&path2).unwrap(); +// trash_ctx.delete_all_with_info(&[path1,path2]).unwrap(); +// // assert!(File::open(&path1).is_err()); +// // assert!(File::open(&path2).is_err()); +// } #[test] #[serial] fn test_delete_with_finder_quoted_paths() { From 568041777d50f4403e714b394f684a49bdadb24e Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Fri, 6 Dec 2024 22:43:54 +0700 Subject: [PATCH 21/41] test: add test_delete_binary_with_finder_with_info --- src/macos/tests.rs | 53 ++++++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/src/macos/tests.rs b/src/macos/tests.rs index 5a7c616..108bf6d 100644 --- a/src/macos/tests.rs +++ b/src/macos/tests.rs @@ -36,31 +36,34 @@ fn test_delete_with_finder_with_info() { } -// #[test] -// #[serial] -// fn test_delete_with_finder_with_info_invalid() { -// // add invalid byt -// init_logging(); -// let mut trash_ctx = TrashContext::default(); -// trash_ctx.set_delete_method(DeleteMethod::Finder); - -// let p = PathBuf::from("/Volumes/Untitled"); - -// let invalid_utf8 = b"\x80"; // lone continuation byte (128) (invalid utf8) - -// let mut path1 = p.clone(); -// let mut path2 = p.clone(); -// path1.push(get_unique_name()); -// path2.push(get_unique_name()); -// path1.set_extension(OsStr::from_bytes(invalid_utf8)); -// path2.set_extension(OsStr::from_bytes(invalid_utf8)); - -// File::create_new(&path1).unwrap(); -// File::create_new(&path2).unwrap(); -// trash_ctx.delete_all_with_info(&[path1,path2]).unwrap(); -// // assert!(File::open(&path1).is_err()); -// // assert!(File::open(&path2).is_err()); -// } +#[test] +#[serial] +fn test_delete_binary_with_finder_with_info() { + init_logging(); + let (_cleanup, tmp) = create_hfs_volume().unwrap(); + let parent_fs_supports_binary = tmp.path(); + let mut trash_ctx = TrashContext::default(); + trash_ctx.set_delete_method(DeleteMethod::Finder); + + let mut path1 = parent_fs_supports_binary.join(get_unique_name()); + let mut path2 = parent_fs_supports_binary.join(get_unique_name()); + path1.set_extension(OsStr::from_bytes(b"\x80a\"b")); // \x80 = lone continuation byte (128) (invalid utf8) + path2.set_extension(OsStr::from_bytes(b"\x80=%80 slash=\\ pc=% quote=\" comma=,")); + File::create_new(&path1).unwrap(); + File::create_new(&path2).unwrap(); + assert!(&path1.exists()); + assert!(&path2.exists()); + let trashed_items = trash_ctx.delete_all_with_info(&[path1.clone(),path2.clone()]).unwrap().unwrap(); //Ok + Some trashed paths + assert!(File::open(&path1).is_err()); // original files deleted + assert!(File::open(&path2).is_err()); + for item in trashed_items { + let trashed_path = item.id; + assert!(!File::open(&trashed_path).is_err()); // returned trash items exist + std::fs::remove_file(&trashed_path).unwrap(); // clean up + assert!( File::open(&trashed_path).is_err()); // cleaned up trash items + } +} + #[test] #[serial] fn test_delete_with_finder_quoted_paths() { From 99ae32352372794edfb6e8fbf2cfde7ad0a7a8ed Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Fri, 6 Dec 2024 23:15:27 +0700 Subject: [PATCH 22/41] fix 1 item not being returned from AppleScript was treated as a list but it was a single item --- src/macos/mod.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/macos/mod.rs b/src/macos/mod.rs index 8ffe78a..fb073d6 100644 --- a/src/macos/mod.rs +++ b/src/macos/mod.rs @@ -176,6 +176,9 @@ fn delete_using_finder + std::fmt::Debug>(full_paths: &[P], with_ tell application "Finder" set Trash_items to delete {{ {posix_files} }} end tell + if Trash_items is not list then -- if only 1 file is deleted, returns a file, not a list + return (POSIX path of (Trash_items as alias)) + end if repeat with aFile in Trash_items -- Finder reference set contents of aFile to (POSIX path of (aFile as alias)) -- can't get paths of Finder reference, coersion to alias needed end repeat @@ -199,8 +202,10 @@ fn delete_using_finder + std::fmt::Debug>(full_paths: &[P], with_ { time_deleted = -1; } - // todo: check if single file still produces an array - if let Some(file_list) = res.as_array() { + let res_arr = if let Some(file_path) = res.as_str() { // convert a single value into an array for ease of handling later + vec![file_path].into() + } else {res}; + if let Some(file_list) = res_arr.as_array() { let len_match = full_paths.len()==file_list.len(); if ! len_match {warn!("AppleScript returned a list of trashed paths len {} ≠ {} expected items sent to be trashed, to trashed items will have empty names/original parents as we cant be certain which trash path matches which trashed item",full_paths.len(),file_list.len());} for (i,posix_path) in file_list.iter().enumerate() { From 9021350101f72963092c0a7e545da90b836f18cb Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Fri, 6 Dec 2024 23:15:35 +0700 Subject: [PATCH 23/41] test: add 1-file delete test to test_delete_with_finder_with_info --- src/macos/tests.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/macos/tests.rs b/src/macos/tests.rs index 108bf6d..55902b4 100644 --- a/src/macos/tests.rs +++ b/src/macos/tests.rs @@ -33,6 +33,17 @@ fn test_delete_with_finder_with_info() { std::fs::remove_file(&trashed_path).unwrap(); // clean up assert!( File::open(&trashed_path).is_err()); // cleaned up trash items } + + // test a single file (in case returned paths aren't an array anymore) + let mut path3 = PathBuf::from(get_unique_name()); + path3.set_extension(r#"a"b,"#); + File::create_new(&path3).unwrap(); + let item = trash_ctx.delete_with_info(&path3).unwrap().unwrap(); //Ok + Some trashed paths + assert!(File::open(&path3).is_err()); // original files deleted + let trashed_path = item.id; + assert!(!File::open(&trashed_path).is_err()); // returned trash items exist + std::fs::remove_file(&trashed_path).unwrap(); // clean up + assert!( File::open(&trashed_path).is_err()); // cleaned up trash items } From cb7550a6bb7c6e6d6df1868b99627768a2e6b48b Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Fri, 6 Dec 2024 23:18:27 +0700 Subject: [PATCH 24/41] fmt --- src/macos/mod.rs | 125 +++++++++++++++++++++++++++------------------ src/macos/tests.rs | 11 ++-- 2 files changed, 81 insertions(+), 55 deletions(-) diff --git a/src/macos/mod.rs b/src/macos/mod.rs index fb073d6..8f7680a 100644 --- a/src/macos/mod.rs +++ b/src/macos/mod.rs @@ -1,6 +1,4 @@ -use std::{ - path::{Path, PathBuf}, -}; +use std::path::{Path, PathBuf}; use log::{trace, warn}; use objc2::rc::Retained; @@ -155,10 +153,13 @@ fn delete_using_file_mgr>(full_paths: &[P], with_info: bool) -> R } } -fn delete_using_finder + std::fmt::Debug>(full_paths: &[P], with_info: bool) -> Result>, Error> { +fn delete_using_finder + std::fmt::Debug>( + full_paths: &[P], + with_info: bool, +) -> Result>, Error> { // TODO: should we convert to trashing item by item instead of in batches to have a guaranteed match of input to output? - // which method is faster? - // what about with a lot of items? will a huge script combining all paths still work? + // which method is faster? + // what about with a lot of items? will a huge script combining all paths still work? use osakit::{Language, Script}; let mut items: Vec = if with_info { Vec::with_capacity(full_paths.len()) } else { vec![] }; let posix_files = full_paths @@ -172,7 +173,9 @@ fn delete_using_finder + std::fmt::Debug>(full_paths: &[P], with_ }) .collect::>() .join(", "); - let script_text = if with_info { format!(r#" + let script_text = if with_info { + format!( + r#" tell application "Finder" set Trash_items to delete {{ {posix_files} }} end tell @@ -183,56 +186,80 @@ fn delete_using_finder + std::fmt::Debug>(full_paths: &[P], with_ set contents of aFile to (POSIX path of (aFile as alias)) -- can't get paths of Finder reference, coersion to alias needed end repeat return Trash_items - "#) - } else {format!(r#"tell application "Finder" to delete {{ {posix_files} }}"#)}; + "# + ) + } else { + format!(r#"tell application "Finder" to delete {{ {posix_files} }}"#) + }; let mut script = Script::new_from_source(Language::AppleScript, &script_text); // Compile and Execute script match script.compile() { Ok(_) => match script.execute() { - Ok(res) => { if with_info { - #[allow(unused_assignments)] - let mut time_deleted = -1; - #[cfg(feature = "chrono")] - { - let now = chrono::Local::now(); - time_deleted = now.timestamp(); - } - #[cfg(not(feature = "chrono"))] - { - time_deleted = -1; - } - let res_arr = if let Some(file_path) = res.as_str() { // convert a single value into an array for ease of handling later - vec![file_path].into() - } else {res}; - if let Some(file_list) = res_arr.as_array() { - let len_match = full_paths.len()==file_list.len(); - if ! len_match {warn!("AppleScript returned a list of trashed paths len {} ≠ {} expected items sent to be trashed, to trashed items will have empty names/original parents as we cant be certain which trash path matches which trashed item",full_paths.len(),file_list.len());} - for (i,posix_path) in file_list.iter().enumerate() { - if let Some(file_path) = posix_path.as_str() { // Finder's return paths should be utf8 (%-encoded?)? - //let p=PathBuf::from(file_path); - //println!("✓converted posix_path:{} - // \nexists {} {:?}", posix_path, p.exists(),p); - let path_r = if len_match {full_paths[i].as_ref()} else {Path::new("")}; - items.push(TrashItem { - id: file_path.into(), - name: if len_match {path_r.file_name().expect("Item to be trashed should have a name").into() - } else {"".into()}, - original_parent: if len_match {path_r.parent().expect("Item to be trashed should have a parent").to_path_buf() - } else {"".into()}, - time_deleted, - }); - } else {warn!("Failed to parse AppleScript's returned path to the trashed file: {:?}",&posix_path);} + Ok(res) => { + if with_info { + #[allow(unused_assignments)] + let mut time_deleted = -1; + #[cfg(feature = "chrono")] + { + let now = chrono::Local::now(); + time_deleted = now.timestamp(); + } + #[cfg(not(feature = "chrono"))] + { + time_deleted = -1; + } + let res_arr = if let Some(file_path) = res.as_str() { + // convert a single value into an array for ease of handling later + vec![file_path].into() + } else { + res + }; + if let Some(file_list) = res_arr.as_array() { + let len_match = full_paths.len() == file_list.len(); + if !len_match { + warn!("AppleScript returned a list of trashed paths len {} ≠ {} expected items sent to be trashed, to trashed items will have empty names/original parents as we cant be certain which trash path matches which trashed item",full_paths.len(),file_list.len()); + } + for (i, posix_path) in file_list.iter().enumerate() { + if let Some(file_path) = posix_path.as_str() { + // Finder's return paths should be utf8 (%-encoded?)? + //let p=PathBuf::from(file_path); + //println!("✓converted posix_path:{} + // \nexists {} {:?}", posix_path, p.exists(),p); + let path_r = if len_match { full_paths[i].as_ref() } else { Path::new("") }; + items.push(TrashItem { + id: file_path.into(), + name: if len_match { + path_r.file_name().expect("Item to be trashed should have a name").into() + } else { + "".into() + }, + original_parent: if len_match { + path_r.parent().expect("Item to be trashed should have a parent").to_path_buf() + } else { + "".into() + }, + time_deleted, + }); + } else { + warn!( + "Failed to parse AppleScript's returned path to the trashed file: {:?}", + &posix_path + ); + } + } + return Ok(Some(items)); + } else { + let ss = if full_paths.len() > 1 { "s" } else { "" }; + warn!("AppleScript did not return a list of path{} to the trashed file{}, originally located at: {:?}",&ss,&ss,&full_paths); } - return Ok(Some(items)) - } else { - let ss = if full_paths.len() > 1 {"s"} else {""}; - warn!("AppleScript did not return a list of path{} to the trashed file{}, originally located at: {:?}",&ss,&ss,&full_paths); } - }}, - Err(e) => return Err(Error::Unknown {description: format!("The AppleScript failed with error: {}", e),}), + } + Err(e) => return Err(Error::Unknown { description: format!("The AppleScript failed with error: {}", e) }), }, - Err(e) => return Err(Error::Unknown {description: format!("The AppleScript failed to compile with error: {}", e),}), + Err(e) => { + return Err(Error::Unknown { description: format!("The AppleScript failed to compile with error: {}", e) }) + } } Ok(None) } diff --git a/src/macos/tests.rs b/src/macos/tests.rs index 55902b4..509e58a 100644 --- a/src/macos/tests.rs +++ b/src/macos/tests.rs @@ -24,14 +24,14 @@ fn test_delete_with_finder_with_info() { path2.set_extension(r#"x80=%80 slash=\ pc=% quote=" comma=,"#); File::create_new(&path1).unwrap(); File::create_new(&path2).unwrap(); - let trashed_items = trash_ctx.delete_all_with_info(&[path1.clone(),path2.clone()]).unwrap().unwrap(); //Ok + Some trashed paths + let trashed_items = trash_ctx.delete_all_with_info(&[path1.clone(), path2.clone()]).unwrap().unwrap(); //Ok + Some trashed paths assert!(File::open(&path1).is_err()); // original files deleted assert!(File::open(&path2).is_err()); for item in trashed_items { let trashed_path = item.id; assert!(!File::open(&trashed_path).is_err()); // returned trash items exist std::fs::remove_file(&trashed_path).unwrap(); // clean up - assert!( File::open(&trashed_path).is_err()); // cleaned up trash items + assert!(File::open(&trashed_path).is_err()); // cleaned up trash items } // test a single file (in case returned paths aren't an array anymore) @@ -43,10 +43,9 @@ fn test_delete_with_finder_with_info() { let trashed_path = item.id; assert!(!File::open(&trashed_path).is_err()); // returned trash items exist std::fs::remove_file(&trashed_path).unwrap(); // clean up - assert!( File::open(&trashed_path).is_err()); // cleaned up trash items + assert!(File::open(&trashed_path).is_err()); // cleaned up trash items } - #[test] #[serial] fn test_delete_binary_with_finder_with_info() { @@ -64,14 +63,14 @@ fn test_delete_binary_with_finder_with_info() { File::create_new(&path2).unwrap(); assert!(&path1.exists()); assert!(&path2.exists()); - let trashed_items = trash_ctx.delete_all_with_info(&[path1.clone(),path2.clone()]).unwrap().unwrap(); //Ok + Some trashed paths + let trashed_items = trash_ctx.delete_all_with_info(&[path1.clone(), path2.clone()]).unwrap().unwrap(); //Ok + Some trashed paths assert!(File::open(&path1).is_err()); // original files deleted assert!(File::open(&path2).is_err()); for item in trashed_items { let trashed_path = item.id; assert!(!File::open(&trashed_path).is_err()); // returned trash items exist std::fs::remove_file(&trashed_path).unwrap(); // clean up - assert!( File::open(&trashed_path).is_err()); // cleaned up trash items + assert!(File::open(&trashed_path).is_err()); // cleaned up trash items } } From 7637858aaed5000a9bbfc813ee7fb6c9662e8315 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Sat, 7 Dec 2024 01:00:31 +0700 Subject: [PATCH 25/41] fix variable type detection in AppleScript --- src/macos/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/macos/mod.rs b/src/macos/mod.rs index 8f7680a..d9f3d75 100644 --- a/src/macos/mod.rs +++ b/src/macos/mod.rs @@ -179,7 +179,7 @@ fn delete_using_finder + std::fmt::Debug>( tell application "Finder" set Trash_items to delete {{ {posix_files} }} end tell - if Trash_items is not list then -- if only 1 file is deleted, returns a file, not a list + if (class of Trash_items) is not list then -- if only 1 file is deleted, returns a file, not a list return (POSIX path of (Trash_items as alias)) end if repeat with aFile in Trash_items -- Finder reference From 91b0bc67a21983073371da407b032e7e41792158 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Sat, 7 Dec 2024 01:14:04 +0700 Subject: [PATCH 26/41] set an empty return to AS with no info so it can be parsed by osakit --- src/macos/mod.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/macos/mod.rs b/src/macos/mod.rs index d9f3d75..b2b290b 100644 --- a/src/macos/mod.rs +++ b/src/macos/mod.rs @@ -189,7 +189,10 @@ fn delete_using_finder + std::fmt::Debug>( "# ) } else { - format!(r#"tell application "Finder" to delete {{ {posix_files} }}"#) + format!( + r#"tell application "Finder" to delete {{ {posix_files} }} + return "" "# + ) }; let mut script = Script::new_from_source(Language::AppleScript, &script_text); From bc43ab2fcdaf19817faf52510ef84a45a901a144 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Sat, 7 Dec 2024 16:44:20 +0700 Subject: [PATCH 27/41] test: add test_delete_with_finder --- src/macos/tests.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/macos/tests.rs b/src/macos/tests.rs index 509e58a..ffc9a81 100644 --- a/src/macos/tests.rs +++ b/src/macos/tests.rs @@ -105,6 +105,20 @@ fn test_delete_with_ns_file_manager() { assert!(File::open(&path).is_err()); } +#[test] +#[serial] +fn test_delete_with_finder() { + init_logging(); + let mut trash_ctx = TrashContext::default(); + trash_ctx.set_delete_method(DeleteMethod::Finder); + + let path = PathBuf::from(get_unique_name()); + File::create_new(&path).unwrap(); + assert!(path.exists()); + trash_ctx.delete(&path).unwrap(); + assert!(!path.exists()); +} + #[test] #[serial] fn test_delete_binary_path_with_ns_file_manager_with_info() { From d2eaf199de2b42143b2961c302be4281a7f80415 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Sun, 8 Dec 2024 00:18:57 +0700 Subject: [PATCH 28/41] add ScriptMethod --- src/macos/mod.rs | 40 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/src/macos/mod.rs b/src/macos/mod.rs index b2b290b..3ba852d 100644 --- a/src/macos/mod.rs +++ b/src/macos/mod.rs @@ -51,18 +51,50 @@ impl Default for DeleteMethod { Self::new() } } + + +#[derive(Copy, Clone, Debug)] +/// There are 2 ways to ask Finder to trash files: by calling the `osascript` binary or directly into the `OSAKit` Framework. +/// The `OSAKit` method should be faster, but it can unexpectedly bug by stalling until the default 2 min timeout expires, +/// so the default is `osascript`. +/// +pub enum ScriptMethod { + /// Spawn a process calling the standalone `osascript` binary to run AppleScript. Slower, but more reliable. + /// + /// This is the default. + Cli, + + /// Call into `OSAKit` directly via ObjC-bindings. Faster, but can sometimes to fail to trigger Finder action, stalling for 2 min. + /// + Osakit, +} +impl ScriptMethod { + /// Returns `ScriptMethod::Cli` + pub const fn new() -> Self { + ScriptMethod::Cli + } +} +impl Default for ScriptMethod { + fn default() -> Self { + Self::new() + } +} + #[derive(Clone, Default, Debug)] pub struct PlatformTrashContext { delete_method: DeleteMethod, + script_method: ScriptMethod, } impl PlatformTrashContext { pub const fn new() -> Self { - Self { delete_method: DeleteMethod::new() } + Self { delete_method: DeleteMethod::new(), script_method: ScriptMethod::new() } } } pub trait TrashContextExtMacos { fn set_delete_method(&mut self, method: DeleteMethod); fn delete_method(&self) -> DeleteMethod; + fn set_script_method(&mut self, method: ScriptMethod); + fn script_method(&self) -> ScriptMethod; } impl TrashContextExtMacos for TrashContext { fn set_delete_method(&mut self, method: DeleteMethod) { @@ -71,6 +103,12 @@ impl TrashContextExtMacos for TrashContext { fn delete_method(&self) -> DeleteMethod { self.platform_specific.delete_method } + fn set_script_method(&mut self, method: ScriptMethod) { + self.platform_specific.script_method = method; + } + fn script_method(&self) -> ScriptMethod { + self.platform_specific.script_method + } } impl TrashContext { pub(crate) fn delete_all_canonicalized( From 401d5e352104dd80f5dc85ef634f0784146c7a0e Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Sun, 8 Dec 2024 00:56:16 +0700 Subject: [PATCH 29/41] add back cli applesript runs since osakit can bug, but retain it as an option for those who might not encounter such bugs --- src/macos/mod.rs | 259 +++++++++++++++++++++++++++++++++-------------- 1 file changed, 183 insertions(+), 76 deletions(-) diff --git a/src/macos/mod.rs b/src/macos/mod.rs index 3ba852d..8a1c891 100644 --- a/src/macos/mod.rs +++ b/src/macos/mod.rs @@ -1,4 +1,7 @@ +use crate::into_unknown; +use std::ffi::OsString; use std::path::{Path, PathBuf}; +use std::process::Command; use log::{trace, warn}; use objc2::rc::Retained; @@ -52,7 +55,6 @@ impl Default for DeleteMethod { } } - #[derive(Copy, Clone, Debug)] /// There are 2 ways to ask Finder to trash files: by calling the `osascript` binary or directly into the `OSAKit` Framework. /// The `OSAKit` method should be faster, but it can unexpectedly bug by stalling until the default 2 min timeout expires, @@ -117,7 +119,10 @@ impl TrashContext { with_info: bool, ) -> Result>, Error> { match self.platform_specific.delete_method { - DeleteMethod::Finder => delete_using_finder(&full_paths, with_info), + DeleteMethod::Finder => match self.platform_specific.script_method { + ScriptMethod::Cli => delete_using_finder(&full_paths, with_info, true), + ScriptMethod::Osakit => delete_using_finder(&full_paths, with_info, false), + }, DeleteMethod::NsFileManager => delete_using_file_mgr(&full_paths, with_info), } } @@ -194,11 +199,11 @@ fn delete_using_file_mgr>(full_paths: &[P], with_info: bool) -> R fn delete_using_finder + std::fmt::Debug>( full_paths: &[P], with_info: bool, + as_cli: bool, ) -> Result>, Error> { // TODO: should we convert to trashing item by item instead of in batches to have a guaranteed match of input to output? // which method is faster? // what about with a lot of items? will a huge script combining all paths still work? - use osakit::{Language, Script}; let mut items: Vec = if with_info { Vec::with_capacity(full_paths.len()) } else { vec![] }; let posix_files = full_paths .iter() @@ -211,95 +216,197 @@ fn delete_using_finder + std::fmt::Debug>( }) .collect::>() .join(", "); + + const LIST_SEP: &str = " /// "; let script_text = if with_info { - format!( - r#" - tell application "Finder" - set Trash_items to delete {{ {posix_files} }} - end tell - if (class of Trash_items) is not list then -- if only 1 file is deleted, returns a file, not a list - return (POSIX path of (Trash_items as alias)) - end if - repeat with aFile in Trash_items -- Finder reference - set contents of aFile to (POSIX path of (aFile as alias)) -- can't get paths of Finder reference, coersion to alias needed - end repeat - return Trash_items - "# - ) + if as_cli { + // since paths can have any char, use " /// " triple path separator for parsing ouput of list paths + format!( + r#" + tell application "Finder" + set Trash_items to delete {{ {posix_files} }} + end tell + if (class of Trash_items) is not list then -- if only 1 file is deleted, returns a file, not a list + return (POSIX path of (Trash_items as alias)) + end if + repeat with aFile in Trash_items -- Finder reference + set contents of aFile to (POSIX path of (aFile as alias)) -- can't get paths of Finder reference, coersion to alias needed + end repeat + set text item delimiters to "{LIST_SEP}" -- hopefully no legal path can have this + return Trash_items as text -- coersion to text forces item delimiters for lists + "# + ) + } else { + format!( + r#" + tell application "Finder" + set Trash_items to delete {{ {posix_files} }} + end tell + if (class of Trash_items) is not list then -- if only 1 file is deleted, returns a file, not a list + return (POSIX path of (Trash_items as alias)) + end if + repeat with aFile in Trash_items -- Finder reference + set contents of aFile to (POSIX path of (aFile as alias)) -- can't get paths of Finder reference, coersion to alias needed + end repeat + return Trash_items + "# + ) + } } else { + // no ouput parsing required, so script is the same for Cli and Osakit format!( r#"tell application "Finder" to delete {{ {posix_files} }} return "" "# ) }; - let mut script = Script::new_from_source(Language::AppleScript, &script_text); + use osakit::{Language, Script}; + if as_cli { + let mut command = Command::new("osascript"); + let argv: Vec = vec!["-e".into(), script_text.into()]; + command.args(argv); + + // Execute command + let result = command.output().map_err(into_unknown)?; + if result.status.success() { + if with_info { + // parse stdout into a list of paths and convert to TrashItems + #[allow(unused_assignments)] + let mut time_deleted = -1; + #[cfg(feature = "chrono")] + { + let now = chrono::Local::now(); + time_deleted = now.timestamp(); + } + #[cfg(not(feature = "chrono"))] + { + time_deleted = -1; + } + let stdout = String::from_utf8_lossy(&result.stdout); // Finder's return paths should be utf8 (%-encoded?)? + let file_list = stdout.strip_suffix("\n").unwrap_or(&stdout).split(LIST_SEP).collect::>(); - // Compile and Execute script - match script.compile() { - Ok(_) => match script.execute() { - Ok(res) => { - if with_info { - #[allow(unused_assignments)] - let mut time_deleted = -1; - #[cfg(feature = "chrono")] - { - let now = chrono::Local::now(); - time_deleted = now.timestamp(); + if !file_list.is_empty() { + let len_match = full_paths.len() == file_list.len(); + if !len_match { + warn!("AppleScript returned a list of trashed paths len {} ≠ {} expected items sent to be trashed, so trashed items will have empty names/original parents as we can't be certain which trash path matches which trashed item",full_paths.len(),file_list.len()); } - #[cfg(not(feature = "chrono"))] - { - time_deleted = -1; + for (i, file_path) in file_list.iter().enumerate() { + let path_r = if len_match { full_paths[i].as_ref() } else { Path::new("") }; + items.push(TrashItem { + id: file_path.into(), + name: if len_match { + path_r.file_name().expect("Item to be trashed should have a name").into() + } else { + "".into() + }, + original_parent: if len_match { + path_r.parent().expect("Item to be trashed should have a parent").to_path_buf() + } else { + "".into() + }, + time_deleted, + }); } - let res_arr = if let Some(file_path) = res.as_str() { - // convert a single value into an array for ease of handling later - vec![file_path].into() - } else { - res - }; - if let Some(file_list) = res_arr.as_array() { - let len_match = full_paths.len() == file_list.len(); - if !len_match { - warn!("AppleScript returned a list of trashed paths len {} ≠ {} expected items sent to be trashed, to trashed items will have empty names/original parents as we cant be certain which trash path matches which trashed item",full_paths.len(),file_list.len()); + return Ok(Some(items)); + } else { + let ss = if full_paths.len() > 1 { "s" } else { "" }; + warn!("AppleScript did not return a list of path{} to the trashed file{}, originally located at: {:?}",&ss,&ss,&full_paths); + } + } + } else { + let stderr = String::from_utf8_lossy(&result.stderr); + match result.status.code() { + None => { + return Err(Error::Unknown { + description: format!("The AppleScript exited with error. stderr: {}", stderr), + }) + } + + Some(code) => { + return Err(Error::Os { + code, + description: format!("The AppleScript exited with error. stderr: {}", stderr), + }) + } + }; + } + } else { + // use Osakit + let mut script = Script::new_from_source(Language::AppleScript, &script_text); + + // Compile and Execute script + match script.compile() { + Ok(_) => match script.execute() { + Ok(res) => { + if with_info { + #[allow(unused_assignments)] + let mut time_deleted = -1; + #[cfg(feature = "chrono")] + { + let now = chrono::Local::now(); + time_deleted = now.timestamp(); } - for (i, posix_path) in file_list.iter().enumerate() { - if let Some(file_path) = posix_path.as_str() { - // Finder's return paths should be utf8 (%-encoded?)? - //let p=PathBuf::from(file_path); - //println!("✓converted posix_path:{} - // \nexists {} {:?}", posix_path, p.exists(),p); - let path_r = if len_match { full_paths[i].as_ref() } else { Path::new("") }; - items.push(TrashItem { - id: file_path.into(), - name: if len_match { - path_r.file_name().expect("Item to be trashed should have a name").into() - } else { - "".into() - }, - original_parent: if len_match { - path_r.parent().expect("Item to be trashed should have a parent").to_path_buf() - } else { - "".into() - }, - time_deleted, - }); - } else { - warn!( - "Failed to parse AppleScript's returned path to the trashed file: {:?}", - &posix_path - ); + #[cfg(not(feature = "chrono"))] + { + time_deleted = -1; + } + let res_arr = if let Some(file_path) = res.as_str() { + // convert a single value into an array for ease of handling later + vec![file_path].into() + } else { + res + }; + if let Some(file_list) = res_arr.as_array() { + let len_match = full_paths.len() == file_list.len(); + if !len_match { + warn!("AppleScript returned a list of trashed paths len {} ≠ {} expected items sent to be trashed, so trashed items will have empty names/original parents as we can't be certain which trash path matches which trashed item",full_paths.len(),file_list.len()); } + for (i, posix_path) in file_list.iter().enumerate() { + if let Some(file_path) = posix_path.as_str() { + // Finder's return paths should be utf8 (%-encoded?)? + //let p=PathBuf::from(file_path); + //println!("✓converted posix_path:{} + // \nexists {} {:?}", posix_path, p.exists(),p); + let path_r = if len_match { full_paths[i].as_ref() } else { Path::new("") }; + items.push(TrashItem { + id: file_path.into(), + name: if len_match { + path_r.file_name().expect("Item to be trashed should have a name").into() + } else { + "".into() + }, + original_parent: if len_match { + path_r + .parent() + .expect("Item to be trashed should have a parent") + .to_path_buf() + } else { + "".into() + }, + time_deleted, + }); + } else { + warn!( + "Failed to parse AppleScript's returned path to the trashed file: {:?}", + &posix_path + ); + } + } + return Ok(Some(items)); + } else { + let ss = if full_paths.len() > 1 { "s" } else { "" }; + warn!("AppleScript did not return a list of path{} to the trashed file{}, originally located at: {:?}",&ss,&ss,&full_paths); } - return Ok(Some(items)); - } else { - let ss = if full_paths.len() > 1 { "s" } else { "" }; - warn!("AppleScript did not return a list of path{} to the trashed file{}, originally located at: {:?}",&ss,&ss,&full_paths); } } + Err(e) => { + return Err(Error::Unknown { description: format!("The AppleScript failed with error: {}", e) }) + } + }, + Err(e) => { + return Err(Error::Unknown { + description: format!("The AppleScript failed to compile with error: {}", e), + }) } - Err(e) => return Err(Error::Unknown { description: format!("The AppleScript failed with error: {}", e) }), - }, - Err(e) => { - return Err(Error::Unknown { description: format!("The AppleScript failed to compile with error: {}", e) }) } } Ok(None) From b55ddca8c3d3a00ab1c7f52028ea06fb9715ed42 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Sun, 8 Dec 2024 01:12:15 +0700 Subject: [PATCH 30/41] test: update --- src/macos/tests.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/macos/tests.rs b/src/macos/tests.rs index ffc9a81..5cc0a14 100644 --- a/src/macos/tests.rs +++ b/src/macos/tests.rs @@ -1,6 +1,6 @@ use crate::{ canonicalize_paths, - macos::{percent_encode, DeleteMethod, TrashContextExtMacos}, + macos::{percent_encode, DeleteMethod, ScriptMethod, TrashContextExtMacos}, tests::{get_unique_name, init_logging}, TrashContext, }; @@ -17,6 +17,7 @@ fn test_delete_with_finder_with_info() { init_logging(); let mut trash_ctx = TrashContext::default(); trash_ctx.set_delete_method(DeleteMethod::Finder); + trash_ctx.set_script_method(ScriptMethod::Cli); let mut path1 = PathBuf::from(get_unique_name()); let mut path2 = PathBuf::from(get_unique_name()); From 4e5c6e6d6eb8db6f3acb291a01d112fe8f827de4 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Sun, 8 Dec 2024 01:12:12 +0700 Subject: [PATCH 31/41] test: add test_delete_with_finder_osakit_with_info disabled as it can timeout --- src/macos/tests.rs | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/macos/tests.rs b/src/macos/tests.rs index 5cc0a14..64f3a61 100644 --- a/src/macos/tests.rs +++ b/src/macos/tests.rs @@ -47,6 +47,43 @@ fn test_delete_with_finder_with_info() { assert!(File::open(&trashed_path).is_err()); // cleaned up trash items } +/* +#[test] +#[serial] +fn test_delete_with_finder_osakit_with_info() { // tested to work, but not always: can randomly timeout, so disabled by default + init_logging(); + let mut trash_ctx = TrashContext::default(); + trash_ctx.set_delete_method(DeleteMethod::Finder); + trash_ctx.set_script_method(ScriptMethod::Osakit); + + let mut path1 = PathBuf::from(get_unique_name()); + let mut path2 = PathBuf::from(get_unique_name()); + path1.set_extension(r#"a"b,"#); + path2.set_extension(r#"x80=%80 slash=\ pc=% quote=" comma=,"#); + File::create_new(&path1).unwrap(); + File::create_new(&path2).unwrap(); + let trashed_items = trash_ctx.delete_all_with_info(&[path1.clone(), path2.clone()]).unwrap().unwrap(); //Ok + Some trashed paths + assert!(File::open(&path1).is_err()); // original files deleted + assert!(File::open(&path2).is_err()); + for item in trashed_items { + let trashed_path = item.id; + assert!(!File::open(&trashed_path).is_err()); // returned trash items exist + std::fs::remove_file(&trashed_path).unwrap(); // clean up + assert!(File::open(&trashed_path).is_err()); // cleaned up trash items + } + + // test a single file (in case returned paths aren't an array anymore) + let mut path3 = PathBuf::from(get_unique_name()); + path3.set_extension(r#"a"b,"#); + File::create_new(&path3).unwrap(); + let item = trash_ctx.delete_with_info(&path3).unwrap().unwrap(); //Ok + Some trashed paths + assert!(File::open(&path3).is_err()); // original files deleted + let trashed_path = item.id; + assert!(!File::open(&trashed_path).is_err()); // returned trash items exist + std::fs::remove_file(&trashed_path).unwrap(); // clean up + assert!(File::open(&trashed_path).is_err()); // cleaned up trash items +}*/ + #[test] #[serial] fn test_delete_binary_with_finder_with_info() { From 29c7a62ed057b4cccbda2ee3588a804077ae3a3d Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Sun, 8 Dec 2024 18:33:54 +0700 Subject: [PATCH 32/41] update doc --- src/macos/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/macos/mod.rs b/src/macos/mod.rs index 8a1c891..3b0e363 100644 --- a/src/macos/mod.rs +++ b/src/macos/mod.rs @@ -66,7 +66,8 @@ pub enum ScriptMethod { /// This is the default. Cli, - /// Call into `OSAKit` directly via ObjC-bindings. Faster, but can sometimes to fail to trigger Finder action, stalling for 2 min. + /// Call into `OSAKit` directly via ObjC-bindings. Faster, but can sometimes fail to trigger any Finder action, + /// stalling for 2 min instead. /// Osakit, } From 8c69c26bead064be157d5cfc0f4c57fef0afa82d Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Mon, 9 Dec 2024 01:05:21 +0700 Subject: [PATCH 33/41] update doc to reflect the need to run Finder scripts on the main thread --- src/macos/mod.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/macos/mod.rs b/src/macos/mod.rs index 3b0e363..f0c601e 100644 --- a/src/macos/mod.rs +++ b/src/macos/mod.rs @@ -56,9 +56,9 @@ impl Default for DeleteMethod { } #[derive(Copy, Clone, Debug)] -/// There are 2 ways to ask Finder to trash files: by calling the `osascript` binary or directly into the `OSAKit` Framework. -/// The `OSAKit` method should be faster, but it can unexpectedly bug by stalling until the default 2 min timeout expires, -/// so the default is `osascript`. +/// There are 2 ways to ask Finder to trash files: ≝1. by calling the `osascript` binary or 2. calling directly into the `OSAKit` Framework. +/// The `OSAKit` method should be faster, but it MUST be run on the main thread, otherwise it can fail, stalling until the default 2 min +/// timeout expires. /// pub enum ScriptMethod { /// Spawn a process calling the standalone `osascript` binary to run AppleScript. Slower, but more reliable. @@ -66,9 +66,7 @@ pub enum ScriptMethod { /// This is the default. Cli, - /// Call into `OSAKit` directly via ObjC-bindings. Faster, but can sometimes fail to trigger any Finder action, - /// stalling for 2 min instead. - /// + /// Call into `OSAKit` directly via ObjC-bindings. Faster, but MUST be run on the main thread, or it can fail, stalling for 2 min. Osakit, } impl ScriptMethod { From 1ec79042714dedafdbec8875df2816da8711662d Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Mon, 9 Dec 2024 14:47:22 +0700 Subject: [PATCH 34/41] test: move osakit to a separate test with no harness to force it run on the main thread --- Cargo.toml | 5 +++++ src/macos/tests.rs | 37 ---------------------------------- tests/osakit.rs | 49 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 37 deletions(-) create mode 100644 tests/osakit.rs diff --git a/Cargo.toml b/Cargo.toml index a0e385e..9fbc944 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,6 +29,11 @@ env_logger = "0.10.0" tempfile = "3.8.0" defer = "0.2.1" +[[test]] +name = "osakit" +path = "tests/osakit.rs" +harness = false + [target.'cfg(target_os = "macos")'.dependencies] objc2 = "0.5.1" diff --git a/src/macos/tests.rs b/src/macos/tests.rs index 64f3a61..5cc0a14 100644 --- a/src/macos/tests.rs +++ b/src/macos/tests.rs @@ -47,43 +47,6 @@ fn test_delete_with_finder_with_info() { assert!(File::open(&trashed_path).is_err()); // cleaned up trash items } -/* -#[test] -#[serial] -fn test_delete_with_finder_osakit_with_info() { // tested to work, but not always: can randomly timeout, so disabled by default - init_logging(); - let mut trash_ctx = TrashContext::default(); - trash_ctx.set_delete_method(DeleteMethod::Finder); - trash_ctx.set_script_method(ScriptMethod::Osakit); - - let mut path1 = PathBuf::from(get_unique_name()); - let mut path2 = PathBuf::from(get_unique_name()); - path1.set_extension(r#"a"b,"#); - path2.set_extension(r#"x80=%80 slash=\ pc=% quote=" comma=,"#); - File::create_new(&path1).unwrap(); - File::create_new(&path2).unwrap(); - let trashed_items = trash_ctx.delete_all_with_info(&[path1.clone(), path2.clone()]).unwrap().unwrap(); //Ok + Some trashed paths - assert!(File::open(&path1).is_err()); // original files deleted - assert!(File::open(&path2).is_err()); - for item in trashed_items { - let trashed_path = item.id; - assert!(!File::open(&trashed_path).is_err()); // returned trash items exist - std::fs::remove_file(&trashed_path).unwrap(); // clean up - assert!(File::open(&trashed_path).is_err()); // cleaned up trash items - } - - // test a single file (in case returned paths aren't an array anymore) - let mut path3 = PathBuf::from(get_unique_name()); - path3.set_extension(r#"a"b,"#); - File::create_new(&path3).unwrap(); - let item = trash_ctx.delete_with_info(&path3).unwrap().unwrap(); //Ok + Some trashed paths - assert!(File::open(&path3).is_err()); // original files deleted - let trashed_path = item.id; - assert!(!File::open(&trashed_path).is_err()); // returned trash items exist - std::fs::remove_file(&trashed_path).unwrap(); // clean up - assert!(File::open(&trashed_path).is_err()); // cleaned up trash items -}*/ - #[test] #[serial] fn test_delete_binary_with_finder_with_info() { diff --git a/tests/osakit.rs b/tests/osakit.rs new file mode 100644 index 0000000..6084ece --- /dev/null +++ b/tests/osakit.rs @@ -0,0 +1,49 @@ +// Separate file to force running the tests on the main thread, which is required for any macOS OSAkit APIs, which otherwise fail after a 2-min stall +use trash::{ + canonicalize_paths, + macos::{percent_encode, DeleteMethod, ScriptMethod, TrashContextExtMacos}, + tests::{get_unique_name, init_logging}, + TrashContext, +}; +use serial_test::serial; +use std::ffi::OsStr; +use std::fs::File; +use std::os::unix::ffi::OsStrExt; +use std::path::PathBuf; +use std::process::Command; + +#[test] +#[serial] +fn test_delete_with_finder_osakit_with_info() { // tested to work, but not always: can randomly timeout, so disabled by default + init_logging(); + let mut trash_ctx = TrashContext::default(); + trash_ctx.set_delete_method(DeleteMethod::Finder); + trash_ctx.set_script_method(ScriptMethod::Osakit); + + let mut path1 = PathBuf::from(get_unique_name()); + let mut path2 = PathBuf::from(get_unique_name()); + path1.set_extension(r#"a"b,"#); + path2.set_extension(r#"x80=%80 slash=\ pc=% quote=" comma=,"#); + File::create_new(&path1).unwrap(); + File::create_new(&path2).unwrap(); + let trashed_items = trash_ctx.delete_all_with_info(&[path1.clone(), path2.clone()]).unwrap().unwrap(); //Ok + Some trashed paths + assert!(File::open(&path1).is_err()); // original files deleted + assert!(File::open(&path2).is_err()); + for item in trashed_items { + let trashed_path = item.id; + assert!(!File::open(&trashed_path).is_err()); // returned trash items exist + std::fs::remove_file(&trashed_path).unwrap(); // clean up + assert!(File::open(&trashed_path).is_err()); // cleaned up trash items + } + + // test a single file (in case returned paths aren't an array anymore) + let mut path3 = PathBuf::from(get_unique_name()); + path3.set_extension(r#"a"b,"#); + File::create_new(&path3).unwrap(); + let item = trash_ctx.delete_with_info(&path3).unwrap().unwrap(); //Ok + Some trashed paths + assert!(File::open(&path3).is_err()); // original files deleted + let trashed_path = item.id; + assert!(!File::open(&trashed_path).is_err()); // returned trash items exist + std::fs::remove_file(&trashed_path).unwrap(); // clean up + assert!(File::open(&trashed_path).is_err()); // cleaned up trash items +} From 674d7be00d00302c9be738ee4e9c70dccb8d4ef0 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Mon, 9 Dec 2024 17:34:43 +0700 Subject: [PATCH 35/41] test: update osakit --- tests/osakit.rs | 42 ++++++++++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/tests/osakit.rs b/tests/osakit.rs index 6084ece..895d6f0 100644 --- a/tests/osakit.rs +++ b/tests/osakit.rs @@ -1,20 +1,42 @@ -// Separate file to force running the tests on the main thread, which is required for any macOS OSAkit APIs, which otherwise fail after a 2-min stall +// Separate file to force running the tests on the main thread, required for any macOS OSAkit APIs, which otherwise fail after a 2-min stall +// Uses cargo-nextest and a custom libtest_mimic test harness for that since the default Cargo test doesn't. +// ADD "test_main_thread_" prefix to test names so that the main cargo test run filter them out with `--skip "test_main_thread_"` +#[path = "../src/tests.rs"] +mod trash_tests; +use serial_test::serial; use trash::{ - canonicalize_paths, - macos::{percent_encode, DeleteMethod, ScriptMethod, TrashContextExtMacos}, - tests::{get_unique_name, init_logging}, + macos::{DeleteMethod, ScriptMethod, TrashContextExtMacos}, TrashContext, }; -use serial_test::serial; -use std::ffi::OsStr; +use trash_tests::{get_unique_name, init_logging}; // not pub, so import directly +// use std::ffi::OsStr; +// use std::os::unix::ffi::OsStrExt; use std::fs::File; -use std::os::unix::ffi::OsStrExt; use std::path::PathBuf; -use std::process::Command; +// use std::process::Command; -#[test] +use libtest_mimic::{Arguments, Trial}; +#[ignore] +fn main() { + let args = Arguments::from_args(); // Parse command line arguments + let tests = vec![ + // Create a list of tests and/or benchmarks + Trial::test("test_main_thread_delete_with_finder_osakit_with_info", || { + Ok(test_main_thread_delete_with_finder_osakit_with_info()) + }), + ]; + libtest_mimic::run(&args, tests).exit(); // Run all tests and exit the application appropriatly +} + +use std::thread; #[serial] -fn test_delete_with_finder_osakit_with_info() { // tested to work, but not always: can randomly timeout, so disabled by default +pub fn test_main_thread_delete_with_finder_osakit_with_info() { + // OSAkit must be run on the main thread + if let Some("main") = thread::current().name() { + } else { + eprintln!("This test is NOT thread-safe, so must be run on the main thread, and is not, thus can fail…"); + }; + init_logging(); let mut trash_ctx = TrashContext::default(); trash_ctx.set_delete_method(DeleteMethod::Finder); From bbb12894f5037c675eea5ab9b423aba7d57aa0fd Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Mon, 9 Dec 2024 17:51:23 +0700 Subject: [PATCH 36/41] test: run threading tests in a short loop to make sure there are no threading issues --- tests/osakit.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/osakit.rs b/tests/osakit.rs index 895d6f0..680d8bf 100644 --- a/tests/osakit.rs +++ b/tests/osakit.rs @@ -9,11 +9,8 @@ use trash::{ TrashContext, }; use trash_tests::{get_unique_name, init_logging}; // not pub, so import directly -// use std::ffi::OsStr; -// use std::os::unix::ffi::OsStrExt; use std::fs::File; use std::path::PathBuf; -// use std::process::Command; use libtest_mimic::{Arguments, Trial}; #[ignore] @@ -46,16 +43,19 @@ pub fn test_main_thread_delete_with_finder_osakit_with_info() { let mut path2 = PathBuf::from(get_unique_name()); path1.set_extension(r#"a"b,"#); path2.set_extension(r#"x80=%80 slash=\ pc=% quote=" comma=,"#); - File::create_new(&path1).unwrap(); - File::create_new(&path2).unwrap(); - let trashed_items = trash_ctx.delete_all_with_info(&[path1.clone(), path2.clone()]).unwrap().unwrap(); //Ok + Some trashed paths - assert!(File::open(&path1).is_err()); // original files deleted - assert!(File::open(&path2).is_err()); - for item in trashed_items { - let trashed_path = item.id; - assert!(!File::open(&trashed_path).is_err()); // returned trash items exist - std::fs::remove_file(&trashed_path).unwrap(); // clean up - assert!(File::open(&trashed_path).is_err()); // cleaned up trash items + for _i in 1..10 { + // run a few times since previously threading issues didn't always appear on 1st run + File::create_new(&path1).unwrap(); + File::create_new(&path2).unwrap(); + let trashed_items = trash_ctx.delete_all_with_info(&[path1.clone(), path2.clone()]).unwrap().unwrap(); //Ok + Some trashed paths + assert!(File::open(&path1).is_err()); // original files deleted + assert!(File::open(&path2).is_err()); + for item in trashed_items { + let trashed_path = item.id; + assert!(!File::open(&trashed_path).is_err()); // returned trash items exist + std::fs::remove_file(&trashed_path).unwrap(); // clean up + assert!(File::open(&trashed_path).is_err()); // cleaned up trash items + } } // test a single file (in case returned paths aren't an array anymore) From 2ee8045c580647bde06b0e1433376d9913384e76 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Mon, 9 Dec 2024 17:35:25 +0700 Subject: [PATCH 37/41] dep: add custom test harness supporting forcing main thread --- Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.toml b/Cargo.toml index 9fbc944..6237963 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,6 +22,7 @@ log = "0.4" [dev-dependencies] serial_test = { version = "2.0.0", default-features = false } +libtest-mimic = "0.8.1" chrono = { version = "0.4.31", default-features = false, features = ["clock"] } rand = "0.8.5" once_cell = "1.18.0" From 369c4c80b5d0d074fc6cb9a81b0b48527b584973 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Mon, 9 Dec 2024 18:04:24 +0700 Subject: [PATCH 38/41] move tests behind macos cfg compilation --- tests/osakit.rs | 126 +++++++++++++++++++++++++----------------------- 1 file changed, 67 insertions(+), 59 deletions(-) diff --git a/tests/osakit.rs b/tests/osakit.rs index 680d8bf..e22343e 100644 --- a/tests/osakit.rs +++ b/tests/osakit.rs @@ -1,71 +1,79 @@ // Separate file to force running the tests on the main thread, required for any macOS OSAkit APIs, which otherwise fail after a 2-min stall // Uses cargo-nextest and a custom libtest_mimic test harness for that since the default Cargo test doesn't. // ADD "test_main_thread_" prefix to test names so that the main cargo test run filter them out with `--skip "test_main_thread_"` +fn main() { + #[cfg(target_os = "macos")] + test_main_thread_mac::main_mac() +} + +#[cfg(target_os = "macos")] #[path = "../src/tests.rs"] mod trash_tests; -use serial_test::serial; -use trash::{ - macos::{DeleteMethod, ScriptMethod, TrashContextExtMacos}, - TrashContext, -}; -use trash_tests::{get_unique_name, init_logging}; // not pub, so import directly -use std::fs::File; -use std::path::PathBuf; -use libtest_mimic::{Arguments, Trial}; -#[ignore] -fn main() { - let args = Arguments::from_args(); // Parse command line arguments - let tests = vec![ - // Create a list of tests and/or benchmarks - Trial::test("test_main_thread_delete_with_finder_osakit_with_info", || { - Ok(test_main_thread_delete_with_finder_osakit_with_info()) - }), - ]; - libtest_mimic::run(&args, tests).exit(); // Run all tests and exit the application appropriatly -} +#[cfg(target_os = "macos")] +mod test_main_thread_mac { + use crate::trash_tests::{get_unique_name, init_logging}; + use serial_test::serial; + use std::{fs::File, path::PathBuf}; + use trash::{ + macos::{DeleteMethod, ScriptMethod, TrashContextExtMacos}, + TrashContext, + }; // not pub, so import directly -use std::thread; -#[serial] -pub fn test_main_thread_delete_with_finder_osakit_with_info() { - // OSAkit must be run on the main thread - if let Some("main") = thread::current().name() { - } else { - eprintln!("This test is NOT thread-safe, so must be run on the main thread, and is not, thus can fail…"); - }; + pub(crate) fn main_mac() { + use libtest_mimic::{Arguments, Trial}; + let args = Arguments::from_args(); // Parse command line arguments + let tests = vec![ + // Create a list of tests and/or benchmarks + Trial::test("test_main_thread_delete_with_finder_osakit_with_info", || { + Ok(test_main_thread_delete_with_finder_osakit_with_info()) + }), + ]; + libtest_mimic::run(&args, tests).exit(); // Run all tests and exit the application appropriatly + } + + use std::thread; + #[serial] + pub fn test_main_thread_delete_with_finder_osakit_with_info() { + // OSAkit must be run on the main thread + if let Some("main") = thread::current().name() { + } else { + eprintln!("This test is NOT thread-safe, so must be run on the main thread, and is not, thus can fail…"); + }; - init_logging(); - let mut trash_ctx = TrashContext::default(); - trash_ctx.set_delete_method(DeleteMethod::Finder); - trash_ctx.set_script_method(ScriptMethod::Osakit); + init_logging(); + let mut trash_ctx = TrashContext::default(); + trash_ctx.set_delete_method(DeleteMethod::Finder); + trash_ctx.set_script_method(ScriptMethod::Osakit); - let mut path1 = PathBuf::from(get_unique_name()); - let mut path2 = PathBuf::from(get_unique_name()); - path1.set_extension(r#"a"b,"#); - path2.set_extension(r#"x80=%80 slash=\ pc=% quote=" comma=,"#); - for _i in 1..10 { - // run a few times since previously threading issues didn't always appear on 1st run - File::create_new(&path1).unwrap(); - File::create_new(&path2).unwrap(); - let trashed_items = trash_ctx.delete_all_with_info(&[path1.clone(), path2.clone()]).unwrap().unwrap(); //Ok + Some trashed paths - assert!(File::open(&path1).is_err()); // original files deleted - assert!(File::open(&path2).is_err()); - for item in trashed_items { - let trashed_path = item.id; - assert!(!File::open(&trashed_path).is_err()); // returned trash items exist - std::fs::remove_file(&trashed_path).unwrap(); // clean up - assert!(File::open(&trashed_path).is_err()); // cleaned up trash items + let mut path1 = PathBuf::from(get_unique_name()); + let mut path2 = PathBuf::from(get_unique_name()); + path1.set_extension(r#"a"b,"#); + path2.set_extension(r#"x80=%80 slash=\ pc=% quote=" comma=,"#); + for _i in 1..10 { + // run a few times since previously threading issues didn't always appear on 1st run + File::create_new(&path1).unwrap(); + File::create_new(&path2).unwrap(); + let trashed_items = trash_ctx.delete_all_with_info(&[path1.clone(), path2.clone()]).unwrap().unwrap(); //Ok + Some trashed paths + assert!(File::open(&path1).is_err()); // original files deleted + assert!(File::open(&path2).is_err()); + for item in trashed_items { + let trashed_path = item.id; + assert!(!File::open(&trashed_path).is_err()); // returned trash items exist + std::fs::remove_file(&trashed_path).unwrap(); // clean up + assert!(File::open(&trashed_path).is_err()); // cleaned up trash items + } } - } - // test a single file (in case returned paths aren't an array anymore) - let mut path3 = PathBuf::from(get_unique_name()); - path3.set_extension(r#"a"b,"#); - File::create_new(&path3).unwrap(); - let item = trash_ctx.delete_with_info(&path3).unwrap().unwrap(); //Ok + Some trashed paths - assert!(File::open(&path3).is_err()); // original files deleted - let trashed_path = item.id; - assert!(!File::open(&trashed_path).is_err()); // returned trash items exist - std::fs::remove_file(&trashed_path).unwrap(); // clean up - assert!(File::open(&trashed_path).is_err()); // cleaned up trash items + // test a single file (in case returned paths aren't an array anymore) + let mut path3 = PathBuf::from(get_unique_name()); + path3.set_extension(r#"a"b,"#); + File::create_new(&path3).unwrap(); + let item = trash_ctx.delete_with_info(&path3).unwrap().unwrap(); //Ok + Some trashed paths + assert!(File::open(&path3).is_err()); // original files deleted + let trashed_path = item.id; + assert!(!File::open(&trashed_path).is_err()); // returned trash items exist + std::fs::remove_file(&trashed_path).unwrap(); // clean up + assert!(File::open(&trashed_path).is_err()); // cleaned up trash items + } } From 41acbf16b81e56b5501ec023d6cc6bfe7de49c7a Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Mon, 9 Dec 2024 18:18:05 +0700 Subject: [PATCH 39/41] update comment --- tests/osakit.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/osakit.rs b/tests/osakit.rs index e22343e..4b1755f 100644 --- a/tests/osakit.rs +++ b/tests/osakit.rs @@ -1,5 +1,5 @@ -// Separate file to force running the tests on the main thread, required for any macOS OSAkit APIs, which otherwise fail after a 2-min stall -// Uses cargo-nextest and a custom libtest_mimic test harness for that since the default Cargo test doesn't. +// Separate file to force running tests on the main thread, a must for macOS OSAkit APIs, which can otherwise fail after a 2-min stall +// Uses a custom libtest_mimic test harness for that since the default Cargo test doesn't support it. // ADD "test_main_thread_" prefix to test names so that the main cargo test run filter them out with `--skip "test_main_thread_"` fn main() { #[cfg(target_os = "macos")] From a166605ac29003b1e2d4598b6e67a08ab3d3d06e Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Mon, 9 Dec 2024 22:13:41 +0700 Subject: [PATCH 40/41] test: fail unless on main thread --- tests/osakit.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/osakit.rs b/tests/osakit.rs index 4b1755f..d9667c0 100644 --- a/tests/osakit.rs +++ b/tests/osakit.rs @@ -38,7 +38,7 @@ mod test_main_thread_mac { // OSAkit must be run on the main thread if let Some("main") = thread::current().name() { } else { - eprintln!("This test is NOT thread-safe, so must be run on the main thread, and is not, thus can fail…"); + panic!("OSAkit is NOT thread-safe, so this test must run on the main thread, and it's not"); }; init_logging(); From 75a23d2426794f60a427866d3015c6733d2457a2 Mon Sep 17 00:00:00 2001 From: eugenesvk Date: Mon, 9 Dec 2024 17:46:08 +0700 Subject: [PATCH 41/41] workflow: split running tests required to be on the main thread into a separate workflow item with a custom argument (also requires a custom harness supporting main threads) --- .github/workflows/rust.yml | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 011fba6..535a71f 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -53,13 +53,21 @@ jobs: echo "target flag is: ${{ env.TARGET_FLAGS }}" echo "target dir is: ${{ env.TARGET_DIR }}" - - name: cargo test + - name: cargo test (excluding main_thread) if: ${{ !startsWith(matrix.build, 'netbsd') }} - run: ${{ env.CARGO }} test --verbose ${{ env.TARGET_FLAGS }} + run: ${{ env.CARGO }} test --verbose ${{ env.TARGET_FLAGS }} -- --skip test_main_thread - - name: cargo test (without chrono) + - name: cargo test (only main_thread) if: ${{ !startsWith(matrix.build, 'netbsd') }} - run: ${{ env.CARGO }} test --verbose --no-default-features --features coinit_apartmentthreaded ${{ env.TARGET_FLAGS }} + run: ${{ env.CARGO }} test test_main_thread --verbose ${{ env.TARGET_FLAGS }} -- --test-threads=1 + + - name: cargo test (without chrono, excluding main_thread) + if: ${{ !startsWith(matrix.build, 'netbsd') }} + run: ${{ env.CARGO }} test --verbose --no-default-features --features coinit_apartmentthreaded ${{ env.TARGET_FLAGS }} -- --skip test_main_thread + + - name: cargo test (without chrono, only main_thread) + if: ${{ !startsWith(matrix.build, 'netbsd') }} + run: ${{ env.CARGO }} test test_main_thread --verbose --no-default-features --features coinit_apartmentthreaded ${{ env.TARGET_FLAGS }} -- --test-threads=1 - name: cargo build if: ${{ startsWith(matrix.build, 'netbsd') }}