Skip to content

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

Merged
dearchap merged 2 commits intourfave:mainfrom
dearchap:issue_1090
Oct 28, 2022
Merged

Feature:(issue_1090): Add unwrap for ExitCoder#1545
dearchap merged 2 commits intourfave:mainfrom
dearchap:issue_1090

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