-
Notifications
You must be signed in to change notification settings - Fork 80
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
add GlobError::into_error so that errors can be easily converted #66
Conversation
@@ -61,6 +61,7 @@ | |||
#![deny(missing_docs)] | |||
#![cfg_attr(all(test, windows), feature(std_misc))] | |||
|
|||
#[allow(unused_imports)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought I would throw this in as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What imports cause these warning?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it only affects the next statement, so use std::ascii::AsciiExt
. This was introduced in recent rust versions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yeah that makes sense then. :)
This is related to, but does not solve #64 (that is a separate issue related to using the library directly, this is for libraries that depend on this to be able to convert the types). |
@@ -279,12 +280,18 @@ impl GlobError { | |||
pub fn error(&self) -> &io::Error { | |||
&self.error | |||
} | |||
|
|||
/// Consumes self, returning the _raw_ underlying `io::Error` | |||
pub fn into_error(self) -> io::Error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems a bit weird to have a into_error()
method on a type that already is a Error - maybe something like into_io_error()
to be more explicit?
Alternativly, if its clear in the API that this just wraps an io::Error
, into_inner()
might also be appropiate to follow the std conventions about wrapper types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went with into_error()
because the method directly above it (which returns io::Error
) is called just error()
. I do agree that I like into_io_error
better, but should I also create an io_error()
method and deprecate error()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed that there is precedent with the error()
method. I guess then the name into_error()
is the consistent choice.
For the
ergo_fs
crate I would like to convertGlobError
intoPathError
. However I cannot without some performance penalty, since there is no way to get the originalio::Error
object.