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(cli/repl): keyboard interrupt should continue #7960

Merged

Conversation

caspervonb
Copy link
Contributor

This changes the behavior of keyboard interrupts (ctrl+c) to continue, clearing the current line instead of exiting.

Exit can still be done with ctrl+d or by calling close().

This changes the behavior of keyboard interrupts (ctrl+c) to continue,
clearing the current line instead of exiting.

Exit can still be done with ctrl+d or by calling close().
@littledivy
Copy link
Member

This seems to be the default behavior with Python, although Node does ask for confirmation.
image

@caspervonb
Copy link
Contributor Author

caspervonb commented Oct 13, 2020

This sounds really bad, ctrl+c means kill the process. It has always meant that. What is the point of this bizarre perversion of Unix?!

It means interrupt, SIGINT after all.

Anyway; based on prior art:

Bash clears and continues.
Python clears and continues.
Ruby clears and continues.
Node clears and continues (but does allow you to hit it again immediately to exit).

@ry
Copy link
Member

ry commented Oct 13, 2020

Maybe print "exit using ctrl+d or close()" again on ^c ?

@nayeemrmn
Copy link
Collaborator

nayeemrmn commented Oct 13, 2020

Note that the existing SIGINT behaviour still remains when an actual execution is interrupted:
image

@caspervonb
Copy link
Contributor Author

caspervonb commented Oct 14, 2020

Maybe print "exit using ctrl+d or close()" again on ^c ?

Since you phrased that as maybe, I think its a bit noisy, never liked Node doing it.
Prefer mimicking bash/zsh as a good examples of shells but I'll yield if anyone feels strongly about it.

Note that the existing SIGINT behaviour still remains when an actual execution is interrupted:

Been playing around with the new Runtime.terminateExecution method but that ends up killing the whole process which is somewhat unexpected.

@bartlomieju
Copy link
Member

@nayeemrmn @caspervonb this is a long standing issue - #1725 - we need some kind of signal watchdog to implement that (I know tokio has APIs for that), and most likely tinker with deno_core to get it working.

@caspervonb
Copy link
Contributor Author

@nayeemrmn @caspervonb this is a long standing issue - #1725 - we need some kind of signal watchdog to implement that (I know tokio has APIs for that), and most likely tinker with deno_core to get it working.

In this case we are catching the signal, it's just a matter of what do we do with it but research required so out of scope for this PR.

@bartlomieju
Copy link
Member

Maybe print "exit using ctrl+d or close()" again on ^c ?

That sounds good to me, but I agree with @caspervonb that we should clear the input line on ^c.

Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

LGTM

@bartlomieju bartlomieju merged commit 8eb4453 into denoland:master Oct 19, 2020
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.

5 participants