refactor(derive): Simplify 'error' handling #2888
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Our goal is to not panic inside of the macro (e.g. clap-rs#2255). Currently, we `.unwrap()` everywhere except when turning an `ArgMatches` into an `enum`. To handle `flatten`, we walk through each `flatten`ed enum to see if it can be instantiated. We don't want to mix this up with any of the other eror cases (including further nested versions of this). If we went straight to `Result<T>`, we'd have to differentiate this case through the `ErrorKind` and `map_err` it in all other cases to prevent it from bubbling up and confusing us. Alternatively, we could do a more complicated type `Result<Option<T>>` where the `Option` exists purely for this case and we can use type checking to make sure we properly turn the `None`s into errors when needed. Or we can bypass all of this and just ask the `flatten`ed` subcommand if it supports the current command.
|
Build succeeded: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Our goal is to not panic inside of the macro (e.g. #2255). Currently,
we
.unwrap()everywhere except when turning anArgMatchesinto anenum. To handleflatten, we walk through eachflattened enum tosee if it can be instantiated. We don't want to mix this up with any of
the other eror cases (including further nested versions of this).
If we went straight to
Result<T>, we'd have to differentiate this casethrough the
ErrorKindandmap_errit in all other cases to preventit from bubbling up and confusing us.
Alternatively, we could do a more complicated type
Result<Option<T>>where the
Optionexists purely for this case and we can use typechecking to make sure we properly turn the
Nones into errors whenneeded.
Or we can bypass all of this and just ask the
flattened` subcommand ifit supports the current command.