Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhanced error reporting #98

Merged
merged 3 commits into from
Feb 5, 2024
Merged

Enhanced error reporting #98

merged 3 commits into from
Feb 5, 2024

Conversation

bicarlsen
Copy link
Contributor

Addresses #96.

Adds an Error::Os variant so clients can more easily handle errors.
On Windows, maps errors immediately if a file is not found or not accessible.

@bicarlsen
Copy link
Contributor Author

bicarlsen commented Feb 4, 2024

Here's a first go at an update.

I opted for add a new Error variant because it appears that some of the freedesktop errors don't have a code (e.g.) and on macOS the error from Command is not guaranteed (e.g.).

Copy link
Owner

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your help!

Just one small issue to address and the PR can be merged.

src/windows.rs Outdated
@@ -79,7 +79,9 @@ impl TrashContext {
/// Removes all files and folder paths recursively.
pub(crate) fn delete_all_canonicalized(&self, full_paths: Vec<PathBuf>) -> Result<(), Error> {
let mut collection = Vec::new();
log::trace!("Starting traverse_paths_recursively");
Copy link
Owner

Choose a reason for hiding this comment

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

Could you remove all added logging? I think it's out of scope for this PR.

@Byron Byron merged commit 452be83 into Byron:master Feb 5, 2024
4 checks passed
@Byron
Copy link
Owner

Byron commented Feb 5, 2024

Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants