Skip to content

Issue/pr create: exit with nonzero status code when "Cancel" was chosen#3023

Merged
mislav merged 6 commits intotrunkfrom
cancel-error-status
Mar 4, 2021
Merged

Issue/pr create: exit with nonzero status code when "Cancel" was chosen#3023
mislav merged 6 commits intotrunkfrom
cancel-error-status

Conversation

@mislav
Copy link
Contributor

@mislav mislav commented Feb 23, 2021

This is to indicate that the command had not finished successfully.

Ref. #3002

This is to indicate that the command had not finished successfully.
@mislav mislav requested a review from a team February 23, 2021 16:08
…mpts

Either InterruptErr or SilentErr will be present when the user has
chosen "Cancel" or pressed Ctrl-C in prompts. We don't want the recovery
mechanism to kick in these cases because the cancellation was likely
willingly initiated by the user.
@mislav mislav requested a review from vilmibm February 23, 2021 19:11
@mislav
Copy link
Contributor Author

mislav commented Feb 23, 2021

@vilmibm I've pushed some tweaks regarding avoiding the PreserveInput mechanism in these cases. Please take a look!

Copy link
Contributor

@vilmibm vilmibm left a comment

Choose a reason for hiding this comment

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

ah, good catch.

I'm a little nervous about SilentError meaning user cancellation implicitly but am fine with it for now 👍

@mislav
Copy link
Contributor Author

mislav commented Mar 2, 2021

I'm a little nervous about SilentError meaning user cancellation implicitly but am fine with it for now 👍

That's a great point. I've now introduced a new concept of "CancelError" which, together with "interrupt" error when Ctrl-C is pressed during Survey, always means explicit user cancellation. For scripts to be able to detect the operation being explicitly cancelled by the user, the gh process now exits with status code "2" in those cases instead of status code "1".

@mislav mislav requested a review from vilmibm March 2, 2021 12:55
Copy link
Contributor

@vilmibm vilmibm left a comment

Choose a reason for hiding this comment

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

oh awesome, thank you! I feel much better about this.

Here are the statuses:
- 0: success
- 1: misc. error
- 2: user interrupt/cancellation
- 4: authentication needed

These old exit codes are now changed to "1":
- we used to return "2" for config file errors;
- we used to return "2" for alias expansion errors;
- we used to return "3" for alias runtime errors.

I do not believe that there is a need to distinguish these specific
cases via exit status, and converting them to "1" frees codes "2" and
"3" for more practical use.
@mislav mislav merged commit e96d974 into trunk Mar 4, 2021
@mislav mislav deleted the cancel-error-status branch March 4, 2021 12:45
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.

3 participants