Skip to content
This repository has been archived by the owner on Aug 16, 2021. It is now read-only.

Adding a new OptionExt trait? #151

Closed
dgrnbrg opened this issue May 25, 2017 · 8 comments
Closed

Adding a new OptionExt trait? #151

dgrnbrg opened this issue May 25, 2017 · 8 comments

Comments

@dgrnbrg
Copy link
Contributor

dgrnbrg commented May 25, 2017

Hello, I'm interested in contributing a new Ext trait for Option, so that I can have chain_err on options when they're None. Often in my code I am working with a vector or iterator that should have a certain number of elements, and I want to bail when that's not the case. Currently, I have tons of option.ok_or(Error::from(format!("problem because of {:?}", context)))? throughout my code, and I think it'd be cool to be able to instead do: option.chain_err(|| format!("problem beause of {:?}", context)). That way, I wouldn't need to compute the error in the success case, and I'd delegate the error casting to type inference. Would you accept this as a PR?

@Yamakaky
Copy link
Contributor

Yes! Though, could you first try to impl ResultExt for Option? I think it should be good.

@dgrnbrg
Copy link
Contributor Author

dgrnbrg commented May 30, 2017

Here's my first draft at the feature--I still need to write tests, and try integrating it throughout my code. Does that look good to you WRT to how I used State?

@Yamakaky
Copy link
Contributor

You can use from_kind which creates a default state for you.

@dgrnbrg
Copy link
Contributor Author

dgrnbrg commented May 31, 2017

So, you're saying to use $crate::Error::from_kind(callback().into()), rather than making a new ChainedError with default State?

@Yamakaky
Copy link
Contributor

Yamakaky commented May 31, 2017

Yes, that's it. It does the same thing: https://brson.github.io/error-chain/src/error_chain/error_chain.rs.html#121-126

@dgrnbrg
Copy link
Contributor Author

dgrnbrg commented Jun 2, 2017

Here's my updated changes: dgrnbrg@d02131f

I'm using from_kind like you suggested, and I've also had to remove the E type from ResultExt, since it turns out ResultExt doesn't ever actually need to be parameterized on E, and it's impossible to determine when chaining with an Option<T>.

I've also included a new method ChainedError::with_boxed_chain, to create a way to start with a boxed Error trait object and get the crate's concrete type of Error with the boxed error as context.

Finally, you can see the commented out code I wrote which would allow for chain_err to be invoked on a boxed error, as I wrote about in #152, if you have any thoughts on how to get the compiler to accept it.

@Yamakaky
Copy link
Contributor

Yamakaky commented Jun 3, 2017

Sorry for the delay.

Seems nice. Could you open a PR?

@dgrnbrg
Copy link
Contributor Author

dgrnbrg commented Jun 5, 2017

I'm closing this in favor of the corresponding PR #156.

@dgrnbrg dgrnbrg closed this as completed Jun 5, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants