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

fix: allow cli prompt to be canceled #143

Merged
merged 5 commits into from
Jun 27, 2023
Merged

fix: allow cli prompt to be canceled #143

merged 5 commits into from
Jun 27, 2023

Conversation

verbanicm
Copy link
Contributor

When adding prompt feature to https://github.com/abcxyz/abc, I noticed that prompt cannot be canceled with SIGTERM or SIGINT (even it parent context was created that way). This adds that capability.

Chose to add context to prompt func to allow calling function to determine how the context should be canceled in addition to adding signal.NotifyContext to guarantee Prompt is cancelable.

Copy link
Contributor

@sethvargo sethvargo left a comment

Choose a reason for hiding this comment

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

I'm not sure this is needed? Does it actually fail to cancel on ctrl+c?

cli/command.go Outdated Show resolved Hide resolved
cli/command.go Outdated Show resolved Hide resolved
cli/command.go Outdated Show resolved Hide resolved
cli/command.go Outdated Show resolved Hide resolved
cli/command.go Outdated Show resolved Hide resolved
cli/command.go Outdated Show resolved Hide resolved
cli/command.go Outdated Show resolved Hide resolved
cli/command.go Outdated Show resolved Hide resolved
cli/command.go Outdated Show resolved Hide resolved
cli/command.go Outdated Show resolved Hide resolved
cli/command.go Outdated Show resolved Hide resolved
@verbanicm verbanicm force-pushed the verbanicm/prompt branch 4 times, most recently from 2c1de3b to 1dad0ef Compare June 26, 2023 15:16
@verbanicm verbanicm enabled auto-merge (squash) June 26, 2023 15:26
@verbanicm verbanicm requested a review from sethvargo June 26, 2023 15:26
cli/command.go Show resolved Hide resolved
cli/command.go Outdated Show resolved Hide resolved
@verbanicm verbanicm changed the title fix: allow cli prompt to be canceled and add default value fix: allow cli prompt to be canceled Jun 26, 2023
@verbanicm verbanicm requested a review from drevell June 26, 2023 19:55
cli/command_test.go Outdated Show resolved Hide resolved
cli/command_test.go Outdated Show resolved Hide resolved
cli/command.go Outdated Show resolved Hide resolved
cli/command.go Outdated Show resolved Hide resolved
cli/command.go Outdated Show resolved Hide resolved
@verbanicm verbanicm requested review from sethvargo and drevell June 27, 2023 02:13
@verbanicm verbanicm disabled auto-merge June 27, 2023 02:14
@verbanicm verbanicm dismissed drevell’s stale review June 27, 2023 02:37

merging during off hours to keep moving

@verbanicm verbanicm merged commit acb464d into main Jun 27, 2023
@verbanicm verbanicm deleted the verbanicm/prompt branch June 27, 2023 02:37
@verbanicm verbanicm restored the verbanicm/prompt branch June 27, 2023 02:37
@verbanicm verbanicm deleted the verbanicm/prompt branch June 27, 2023 02:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants