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: improve logbox interrupt handling #133

Merged
merged 2 commits into from
Jul 24, 2024
Merged

fix: improve logbox interrupt handling #133

merged 2 commits into from
Jul 24, 2024

Conversation

ctrox
Copy link
Contributor

@ctrox ctrox commented Jul 24, 2024

This fixes a number of issues with the cancel/error handling while the
logbox (tea program) is running. Previously input was sent to the tea
program, which meant we had to handle ctrl+c/ctrl+d explicitly. Now the
tea program is stopped using the context and also in afterWait since
it's important to call p.Wait() so that the console is restored
properly before nctl exits.

create/application.go Outdated Show resolved Hide resolved
@ctrox ctrox force-pushed the logbox-interrupt branch from cc911b4 to f9b777a Compare July 24, 2024 12:41
ctrox added 2 commits July 24, 2024 15:08
This fixes a number of issues with the cancel/error handling while the
logbox (tea program) is running. Previously input was sent to the tea
program, which meant we had to handle ctrl+c/ctrl+d explicitly. Now the
tea program is stopped using the context and also in afterWait since
it's important to call `p.Wait()` so that the console is restored
properly before nctl exits.
When a user quits nctl while it is waiting for create/delete, we should
only show "timeout waiting for ..." if the context deadline is reached.
If the contact was simply canceled we should just exit without a special
message.
@ctrox ctrox force-pushed the logbox-interrupt branch from f9b777a to 823062b Compare July 24, 2024 13:08
@ctrox ctrox merged commit f7895fe into main Jul 24, 2024
2 checks passed
@ctrox ctrox deleted the logbox-interrupt branch July 24, 2024 13:24
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.

2 participants