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

Feature:(issue_1090): Add unwrap for ExitCoder #1545

Merged
merged 2 commits into from Oct 28, 2022

Conversation

dearchap
Copy link
Contributor

What type of PR is this?

(REQUIRED)

  • feature

What this PR does / why we need it:

(REQUIRED)

Which issue(s) this PR fixes:

(REQUIRED)

Fixes #1090

Special notes for your reviewer:

(fill-in or delete this section)

Testing

make test

(fill-in or delete this section)

Release Notes

(REQUIRED)


@dearchap dearchap requested a review from a team as a code owner October 23, 2022 22:55
var err error

switch e := message.(type) {
case ErrorFormatter:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any difference between this and the default handling?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well ErrorFormatter implies that the error message can be formatted dfferently than a "regular" error.

Comment on lines +106 to +113
switch e := message.(type) {
case ErrorFormatter:
err = fmt.Errorf("%+v", message)
case error:
err = e
default:
err = fmt.Errorf("%+v", message)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well ErrorFormatter implies that the error message can be formatted dfferently than a "regular" error.

Buf the handling of ErrorFormatter same as default

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. So what do you suggest be done ? I can collapse the ErrorFormatter into default.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually collapsing ErrorFormatter into default doesnt seem to work. I'm going to leave it as it is.

@dearchap dearchap mentioned this pull request Oct 27, 2022
3 tasks
@dearchap dearchap merged commit c344b46 into urfave:main Oct 28, 2022
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

Successfully merging this pull request may close these issues.

ExitCoder should implement Unwrap
2 participants