-
Notifications
You must be signed in to change notification settings - Fork 4
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: FromError
unwraps based on an interface so it works with *exec.ExitError
as well
#95
Conversation
Oh that's cool! I actually had no idea that |
if errors.As(err, &e) { | ||
return e.Code | ||
return e.ExitCode() |
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.
Can't tell why is this better? Is it to capture anything that might not be derived from Error
but can implement ExitCode()
?
Also should the variable Code
be not exported out, given you have the function ExitCode
introduced below?
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.
Can't tell why is this better?
This is based on @alecthomas's feedback here — that instead of unwrapping based on type, it'd be better to unwrap based on interface, which totally makes sense.
Also should the variable
Code
be not exported out, given you have the functionExitCode
introduced below?
I don't have a strong opinion on that; but the rest of this change should be nonbreaking for anything that uses square/exit
, while renaming Code
to code
could be breaking.
} else if err == nil { | ||
return OK | ||
} else { | ||
return NotOK | ||
} | ||
} | ||
|
||
func WrapIf(err error, code Code) 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.
Is it used anywhere?
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.
Not in this library!
but I've found myself writing the following a lot of times:
err := someFunctionThatParsesOrValidatesUserInput()
if err != nil {
err = exit.Wrap(err, exit.UsageError)
}
and I'd rather be more declarative and say "errors returned by this function are a certain class of error" — more like this:
err := exit.WrapIf(someFunctionThatParsesOrValidatesUserInput(), exit.UsageError)
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.
Looks good! Few questions.
And can fall through to Kong based on alecthomas/kong#507