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

[feat] Map Windows errors #96

Open
bicarlsen opened this issue Jan 10, 2024 · 6 comments
Open

[feat] Map Windows errors #96

bicarlsen opened this issue Jan 10, 2024 · 6 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@bicarlsen
Copy link
Contributor

It appears that all windows::core::Error's are mapped to trash::Error::Unknown (code here). Is it possible to use the error code (ref) to map at least some of the more common errors to the corresponding trash::Error variant?

e.g. codes 2 and 3 could map to trash::Error::FileSystem with an error kind of std::io::ErrorKind::NotFound.

Another option could allow for the Unknown variant to also accept an optional error code, but that doesn't feel like the correct way to go about it.

Happy to give implementation a shot if it seems like a good idea.

@Byron Byron added enhancement New feature or request help wanted Extra attention is needed labels Jan 10, 2024
@Byron
Copy link
Owner

Byron commented Jan 10, 2024

Thanks for the suggestion, I am definitely open to making the error type more useful on Windows as well.

@bicarlsen
Copy link
Contributor Author

bicarlsen commented Jan 13, 2024

I'll try to do similarly for the finder method in macos, too (ref).

@bicarlsen
Copy link
Contributor Author

bicarlsen commented Jan 13, 2024

I'm thinking it would be useful to have an ability to pass on error codes for otherwise unhandled errors. I see two ways to do this:

  1. Add a code: Option<i32> field to Error::Unknown.
  2. Create a new Error variant like Error::OS { description: String, code: i32 }.

Thoughts on which is preferred?

@Byron
Copy link
Owner

Byron commented Jan 14, 2024

Since any change to Error is breaking, the question seems to be if Error::Unknown now always has a code or not. If yes, I'd add the code field to it, otherwise, it's probably best to add Error::OS.

I'd also recommend to go with your intuition if anything changes during implementation, the only tries to optimize for being 'least breaking' or 'easily fixable' if it breaks.

@bicarlsen
Copy link
Contributor Author

For macOS the code returned by Command.output may not match the code of interest.
e.g. If a file is not found the code of interest is -10010 but the command exits with code 1, and the code of interest is only available through the message of stderr.

We could use a regex to parse the error, but this seems like a task better left for the end user.
Perhaps best to just document the behaviour.

@Byron
Copy link
Owner

Byron commented Feb 12, 2024

Thanks for sharing!

We could use a regex to parse the error, but this seems like a task better left for the end user.
Perhaps best to just document the behaviour.

I agree, a viable step would be to improve the documentation here. Adding a regex dependency seems out of the question except for when it's behind a feature gate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants