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

Ctrl-c-ing out of checkbox prompt leaves terminal in broken state #1286

Closed
maschwenk opened this issue Aug 23, 2023 · 7 comments
Closed

Ctrl-c-ing out of checkbox prompt leaves terminal in broken state #1286

maschwenk opened this issue Aug 23, 2023 · 7 comments

Comments

@maschwenk
Copy link

maschwenk commented Aug 23, 2023

On@inquirer/checkbox@1.3.8

Screenshot 2023-08-23 at 11 24 32 AM

When ctrl-cing out of a prompt, the terminal cursor is left in broken state.

Cannot reproduce this with old inquirer package. Have to open a new terminal to fix.

@SBoudrias
Copy link
Owner

Hi @maschwenk, can you explain what you mean by "broken state"? Broken how? What is the expected state?

@tommy-mitchell
Copy link

Think this has to do with c51e33d downgrading cli-cursor, which causes restore-cursor to fail and not restore the cursor to the terminal prompt: sindresorhus/np#689 (comment)

@SBoudrias
Copy link
Owner

Hi @tommy-mitchell, I doubt this is related. This package is only used on the legacy version; and this one doesn't have the issue of the cursor disappearing on ctrl+C.

It appears to only happen with the @inquirer/prompts version. I'm not exactly sure where the regression came from; that was reported and fixed "recently" (but since it's crtl+c/sigint, I wasn't able to add a regression test that wouldn't kill the test runner 😫)

The code restoring the cursor as a back up is here: https://github.com/SBoudrias/Inquirer.js/blob/master/packages/core/src/lib/screen-manager.mts#L128 - I wasn't able to figure it out yet.

@tommy-mitchell
Copy link

tommy-mitchell commented Sep 4, 2023

Seems it's only the select/list prompt in my case, as it was happening with the old inquirer package as well. sigint during other prompts doesn't have the same issue.

@SBoudrias
Copy link
Owner

SBoudrias commented Sep 4, 2023

@tommy-mitchell I'm not sure I'm fully following. Which version of the old Inquirer are you running into this issue with? I wasn't able to reproduce this myself.

I can only reproduce with those 2 prompts on @inquirer/prompts.

@tommy-mitchell
Copy link

Hm, now I'm unable to reproduce on the old Inquirer either. It was occurring on this prompt in np: https://github.com/sindresorhus/np/blob/main/source/ui.js#L222, but now it's working correctly locally.

@SBoudrias
Copy link
Owner

This should now work properly.

Not sure how it happens, but signals became 0 without names (just null.) And so the force exit handler wasn't running anymore. It could be a side-effect of the Node async hooks; but really weird. Anyway, now detection should be more reliable for the future.

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

No branches or pull requests

3 participants