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

Errors with additional info #4

Open
EPashkin opened this issue Nov 14, 2015 · 4 comments
Open

Errors with additional info #4

EPashkin opened this issue Nov 14, 2015 · 4 comments

Comments

@EPashkin
Copy link

In https://github.com/gkoz/gir/blob/master/src/config.rs we use this enum for error of configuration processing:

#[derive(Debug)]
pub enum Error {
    CommandLine(DocoptError),
    Io(IoError, PathBuf),
    Toml(String, PathBuf),
    Options(String, PathBuf),
}

Last 3 errors here also contains file name.

Currently for error conversion we use tuple

try!(File::open(&filename)
     .and_then(|mut f| f.read_to_string(&mut input))
     .map_err(|e| (e, &filename)));

with From definition

impl<P: AsRef<OsStr>> From<(IoError, P)> for Error {
    fn from(e: (IoError, P)) -> Error {
        Error::Io(e.0, PathBuf::from(&e.1))
    }
}

This is the normal way to handle errors?
Currently seems no way to use this method in quick-error.
Can this functionality be added in quick-error?

@tailhook
Copy link
Owner

Hm, it's interesting. I usually use something like this:

try!(File::open(&filename)
     .and_then(|mut f| f.read_to_string(&mut input))
     .map_err(|e| Error::Io(e, filename.to_path_buf())));

It's a little more verbose. But is it bad enough to require another syntax?

@tailhook
Copy link
Owner

What I don't like, is writing .to_path_buf(). But instead of magically converting from tuple I'd rather try to implement a conversion function:

impl Error {
  fn io<P:AsRef<Path>>(e: io::Error, path: P) -> Error {
    Error::Io(e, path.as_ref().to_path_buf())
  } 
}
...
try!(File::open(&filename)
     .and_then(|mut f| f.read_to_string(&mut input))
     .map_err(|e| Error::io(e, &filename)));

I'm not sure if we need any shortcut for this.

What do you think?

@EPashkin
Copy link
Author

Defining function maybe really better as it clearer declares error type, but extra 7+ characters.
Just can't decide 😢

@tailhook
Copy link
Owner

I still feel this is an interesting issue, even if we don't know how to approach the issue yet.

@tailhook tailhook reopened this Feb 12, 2016
@tailhook tailhook mentioned this issue May 17, 2016
6 tasks
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

No branches or pull requests

2 participants