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: make sure the interactive printers can cleanup after Ctrl+C #383

Merged
merged 2 commits into from Jul 26, 2022

Conversation

jochil
Copy link
Contributor

@jochil jochil commented Jul 25, 2022

Description

After canceling the interactive printers with Ctrl+C, os.Exit was
directly called in the keyboard listener. Because of this the
defer statements of the surrounding function were not called
(for example cursor.Show) and the keyboard listener
was still running.
This resulted in a broken user "terminal" (no Cursor, no Ctrl+C)

Scope

What is affected by this pull request?

  • Bug Fix
  • New Feature
  • Documentation
  • Other

Related Issue

none

To-Do Checklist

  • I tested my changes
  • I have commented every method that I created/changed
  • I updated the examples to fit with my changes
  • I have added tests for my newly created methods

@github-actions github-actions bot added fix and removed fix labels Jul 25, 2022
@codecov
Copy link

codecov bot commented Jul 25, 2022

Codecov Report

Merging #383 (63b4ba3) into master (814a52d) will decrease coverage by 0.11%.
The diff coverage is 16.66%.

@@            Coverage Diff             @@
##           master     #383      +/-   ##
==========================================
- Coverage   81.90%   81.78%   -0.12%     
==========================================
  Files          28       28              
  Lines        2006     2015       +9     
==========================================
+ Hits         1643     1648       +5     
- Misses        349      355       +6     
+ Partials       14       12       -2     
Impacted Files Coverage Δ
interactive_multiselect_printer.go 0.00% <0.00%> (ø)
interactive_textinput_printer.go 4.76% <0.00%> (-0.12%) ⬇️
interactive_select_printer.go 46.57% <50.00%> (+2.51%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 814a52d...63b4ba3. Read the comment docs.

@jochil jochil force-pushed the jochil/cleanup-after-cancelation branch from 07dbed1 to f359cb3 Compare July 25, 2022 14:27
After canceling the interactive printers with Ctrl+C, `os.Exit` was
directly called in the keyboard listener. Because of this the
`defer` statements of the surrounding function were not called
(for example `cursor.Show`) and the keyboard listener
was still running.
This resulted in a broken user "terminal" (no Cursor, no Ctrl+C)
@jochil jochil force-pushed the jochil/cleanup-after-cancelation branch from f359cb3 to be55629 Compare July 25, 2022 14:36
@jochil jochil marked this pull request as ready for review July 25, 2022 14:37
Copy link
Contributor

@luisdavim luisdavim left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@MarvinJWendt MarvinJWendt left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for contributing 🥳

This PR should be released in the next 12h. If not, feel free to ping me :)

@MarvinJWendt MarvinJWendt merged commit c98a71d into pterm:master Jul 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants