Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 20 additions & 9 deletions src/freedesktop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,13 @@ impl PlatformTrashContext {
}
}
impl TrashContext {
pub(crate) fn delete_all_canonicalized(&self, full_paths: Vec<PathBuf>) -> Result<(), Error> {
pub(crate) fn delete_all_canonicalized(&self, full_paths: Vec<PathBuf>) -> Result<Option<Vec<TrashItem>>, 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);
Expand All @@ -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))
}
}

Expand Down Expand Up @@ -450,7 +452,7 @@ fn move_to_trash(
src: impl AsRef<Path>,
trash_folder: impl AsRef<Path>,
_topdir: impl AsRef<Path>,
) -> Result<(), FsError> {
) -> Result<TrashItem, FsError> {
let src = src.as_ref();
let trash_folder = trash_folder.as_ref();
let files_folder = trash_folder.join("files");
Expand Down Expand Up @@ -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 {
Expand All @@ -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;
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment is redundant because time_deleted is already initialized to -1 on line 496. This line can be removed to improve code clarity.

Suggested change
time_deleted = -1;

Copilot uses AI. Check for mistakes.
Ok(())
}
})
Expand All @@ -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(),
Comment on lines +545 to +551
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The expect call assumes that the absolute path has a parent, which should be true for canonicalized paths. However, this could panic if somehow a root path is passed. Consider whether this is an acceptable invariant or if more graceful error handling is needed. If this invariant is guaranteed by earlier canonicalization, this is acceptable, but it would be helpful to add a comment explaining why this is safe.

Suggested change
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(),
// Normally, an absolute path to a trashed item should always have a parent.
// However, in case a root path is ever passed here, handle it gracefully
// instead of panicking, and return an explicit error.
let original_parent = match src.parent() {
Some(parent) => parent.to_path_buf(),
None => {
return Err((
src.to_path_buf(),
std::io::Error::new(
std::io::ErrorKind::Other,
"Trashed item path has no parent directory",
),
));
}
};
return Ok(TrashItem {
id: info_file_path.into(),
name: filename.into(),
original_parent,

Copilot uses AI. Check for mistakes.
time_deleted,
});
}
}
}

Ok(())
}

/// An error may mean that a collision was found.
Expand Down
8 changes: 4 additions & 4 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ impl TrashContext {
/// trash::delete("delete_me").unwrap();
/// assert!(File::open("delete_me").is_err());
/// ```
pub fn delete<T: AsRef<Path>>(&self, path: T) -> Result<(), Error> {
pub fn delete<T: AsRef<Path>>(&self, path: T) -> Result<Option<Vec<TrashItem>>, Error> {
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking API change that affects the return type of the delete function. All existing users of this library will need to update their code to handle the new Option<Vec<TrashItem>> return value instead of (). Consider using a major version bump when releasing this change, or alternatively, providing a new method name (e.g., delete_with_info) alongside the existing one to maintain backward compatibility.

Suggested change
pub fn delete<T: AsRef<Path>>(&self, path: T) -> Result<Option<Vec<TrashItem>>, Error> {
pub fn delete<T: AsRef<Path>>(&self, path: T) -> Result<(), Error> {
self.delete_all(&[path])?;
Ok(())
}
/// Like [`delete`], but returns additional information about the trashed items.
pub fn delete_with_info<T: AsRef<Path>>(
&self,
path: T,
) -> Result<Option<Vec<TrashItem>>, Error> {

Copilot uses AI. Check for mistakes.
self.delete_all(&[path])
}

Expand All @@ -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<I, T>(&self, paths: I) -> Result<(), Error>
pub fn delete_all<I, T>(&self, paths: I) -> Result<Option<Vec<TrashItem>>, Error>
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking API change that affects the return type of the delete_all function. All existing users of this library will need to update their code to handle the new Option<Vec<TrashItem>> return value instead of (). Consider using a major version bump when releasing this change, or alternatively, providing a new method name (e.g., delete_all_with_info) alongside the existing one to maintain backward compatibility.

Copilot uses AI. Check for mistakes.
where
I: IntoIterator<Item = T>,
T: AsRef<Path>,
Expand All @@ -116,14 +116,14 @@ impl TrashContext {
/// Convenience method for `DEFAULT_TRASH_CTX.delete()`.
///
/// See: [`TrashContext::delete`](TrashContext::delete)
pub fn delete<T: AsRef<Path>>(path: T) -> Result<(), Error> {
pub fn delete<T: AsRef<Path>>(path: T) -> Result<Option<Vec<TrashItem>>, 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<I, T>(paths: I) -> Result<(), Error>
pub fn delete_all<I, T>(paths: I) -> Result<Option<Vec<TrashItem>>, Error>
where
I: IntoIterator<Item = T>,
T: AsRef<Path>,
Expand Down
9 changes: 5 additions & 4 deletions src/macos/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -74,11 +74,12 @@ impl TrashContextExtMacos for TrashContext {
}
}
impl TrashContext {
pub(crate) fn delete_all_canonicalized(&self, full_paths: Vec<PathBuf>) -> Result<(), Error> {
pub(crate) fn delete_all_canonicalized(&self, full_paths: Vec<PathBuf>) -> Result<Option<Vec<TrashItem>>, 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)
}
}

Expand Down
32 changes: 32 additions & 0 deletions src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,4 +297,36 @@ mod os_limited {
let is_empty = trash::os_limited::is_empty().unwrap();
assert_eq!(is_empty, is_empty_list, "is_empty() should match empty status from list()");
}

#[cfg(all(unix, not(target_os = "macos"), not(target_os = "ios"), not(target_os = "android")))]
#[test]
#[serial]
fn delete_items_info() {
init_logging();
let names = {
let prefix = get_unique_name();
let mut names = Vec::with_capacity(5);
for n in 0..4 {
let name = format!("{prefix}#{n}").into_bytes();
names.push(OsString::from_vec(name));
}

// Throw in an invalid UTF-8 OsString for good measure
let mut name = OsStr::new(&format!("{prefix}#")).to_os_string().into_encoded_bytes();
name.push(168);
names.push(OsString::from_vec(name));

names
};

for name in &names {
File::create_new(name).unwrap();
}

let deleted_names = trash::delete_all(&names).unwrap().expect("Should get a list of deleted items");
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The panic message "Should get a list of deleted items" is not very informative for debugging. Consider using a more descriptive message like "Expected Some(items) on freedesktop platform, but got None" to help identify the issue if this test fails.

Suggested change
let deleted_names = trash::delete_all(&names).unwrap().expect("Should get a list of deleted items");
let deleted_names = trash::delete_all(&names)
.unwrap()
.expect("Expected Some(deleted_items) from trash::delete_all(&names) on freedesktop/unix platform, but got None");

Copilot uses AI. Check for mistakes.
assert_eq!(deleted_names.len(), names.len());
for name in names {
assert!(deleted_names.iter().any(|item| item.name == name));
Comment on lines +326 to +329
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable is named deleted_names but it contains TrashItem objects, not just names. Consider renaming to deleted_items or items to better reflect its contents.

Suggested change
let deleted_names = trash::delete_all(&names).unwrap().expect("Should get a list of deleted items");
assert_eq!(deleted_names.len(), names.len());
for name in names {
assert!(deleted_names.iter().any(|item| item.name == name));
let deleted_items = trash::delete_all(&names).unwrap().expect("Should get a list of deleted items");
assert_eq!(deleted_items.len(), names.len());
for name in names {
assert!(deleted_items.iter().any(|item| item.name == name));

Copilot uses AI. Check for mistakes.
}
Comment on lines +328 to +330
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test should clean up the trashed items after verifying them, similar to how other tests in this file use trash::os_limited::purge_all() to remove test artifacts. Consider adding cleanup code at the end of the test to prevent accumulation of test files in the trash.

Suggested change
for name in names {
assert!(deleted_names.iter().any(|item| item.name == name));
}
for name in &names {
assert!(deleted_names.iter().any(|item| item.name == *name));
}
// Clean up trashed items created by this test to avoid accumulating artifacts.
let targets: Vec<_> = trash::os_limited::list()
.unwrap()
.into_iter()
.filter(|x| names.iter().any(|n| x.name == *n))
.collect();
trash::os_limited::purge_all(targets).unwrap();

Copilot uses AI. Check for mistakes.
}
}
12 changes: 7 additions & 5 deletions src/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<PathBuf>) -> Result<(), Error> {
pub(crate) fn delete_specified_canonicalized(
&self,
full_paths: Vec<PathBuf>,
) -> Result<Option<Vec<TrashItem>>, Error> {
ensure_com_initialized();
unsafe {
let pfo: IFileOperation = CoCreateInstance(&FileOperation as *const _, None, CLSCTX_ALL).unwrap();
Expand Down Expand Up @@ -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<PathBuf>) -> Result<(), Error> {
self.delete_specified_canonicalized(full_paths)?;
Ok(())
pub(crate) fn delete_all_canonicalized(&self, full_paths: Vec<PathBuf>) -> Result<Option<Vec<TrashItem>>, Error> {
self.delete_specified_canonicalized(full_paths)
}
}

Expand Down
Loading