catch os.Interrupt (ctrl-C) so that the tty state can be restored

[raff]

catch os.Interrupt (ctrl-C) so that the tty state can be restored before terminating.

On both OSX and Linux a Ctrl-C kills the application and leaves the tty/terminal window with no echo. This patch catches os.Interrupt so that the application closes cleanly.

Tested on OSX (Yosemite) and Linux (Centos 5.11)

[peterh]

Thanks for the patch.

I've been thinking about it for the past day, and I can't decide if I like the idea or not. I change my mind every hour or so.

I really like the idea that liner should always leave things in a safe state, but I don't like the way this changes the behaviour of the application liner is linked with (ie. Ctrl-C stops working between calls to Prompt). I also don't like it when the API's behaviour changes depending on which platform you're running it on.

For a while, I thought that unhooking SIGINT when Prompt returns would be a good compromise, since then the user would still be able to interrupt a long-running process. But in that case, the terminal wouldn't be restored.

Plan	Surprise(s)
No Change	Terminal state is not restored upon SIGINT
Proposed Change	Prompt returns error("Interrupted"), but only on VT100. Apps that don't have their own SIGINT handler can't be interrupted any more (on VT100).
Proposed Change, plus Win32	Prompt returns error("Interrupted") and apps can't be interrupted, except when $TERM=dumb
Proposed Change, plus all platforms	Prompt returns error("Interrupted"), except SIGINT is just ignored when $TERM=dumb. Apps can't be interrupted unless they install their own Interrupt signal handler.
Unhook SIGINT when Prompt returns	Terminal state is only restored upon SIGINT if it arrives during during Prompt. error("Interrupted") may or may not be returned depending on timing and platform.

We should strive to follow the principle of least surprise, but I'm not sure which of the above is least surprising.

Given that the enclosing application has to have its own Interrupt signal handler anyway to do the right thing in all of the above cases, I'm leaning towards the status quo. But I could be convinced otherwise.

(In terms of code review, the attached patch can silently drop SIGINTs when SIGWHINCHes are happening (and vice-versa). Interrupt needs its own channel if you want both signals to be delivered reliably).

[peterh]

A sixth alternative is to always register Interrupt, but never read from the channel. This has the effect of disabling SIGINT unless the enclosing application registers its own handler. I like this alternative best (for now; my opinion is subject to change after I think about it some more).

[michaelmacinnis]

I hope you don't mind me offering one user's perspective. I would prefer if liner did not catch os.Interrupt. I need to catch this signal so it would complicate things for me if liner was also interested in it. But, unless I'm missing something, your library doesn't need a patch to handle this situation. Liner clients only need to call TerminalMode, saving the returned ModeApplier, when they start and make sure they call ApplyMode before they terminate. It would be the client's responsibility to catch Ctrl-C and call ApplyMode before exiting.

[peterh]

Thanks for your comments, @michaelmacinnis.

If I read the source code for signal.Notify correctly, any number of channels can have a given signal delivered each time the signal arrives. I would definitely test this before accepting any proposed patch, to make certain it wouldn't interfere with my applications.

[And clients just need to call Close() on the liner. Unless I've made a mistake somewhere, there should be no need to use TerminalMode for most applications]

[michaelmacinnis]

@peterh you are correct, each channel should receive a copy of the incoming signal. What I mean is that I wouldn't want liner to take an action like restoring the terminal mode when receiving os.Interrupt as my application doesn't always terminate when receiving os.Interrupt.

I completely forgot about Close(). Even easier!

[peterh]

@michaelmacinnis @raff I've pretty much decided that the Right Thing is to handle Ctrl-C as an input character; it creates a new empty prompt (same as bash). What do you think of 28191460c94164a5f9e1fad9280e0ea9075060b2? Instead of handling SIGINT, it asks the terminal to not convert Ctrl-C into SIGINT for the duration of Prompt(). Advantage: SIGINT sent other ways will not be interpreted as a Ctrl-C. SIGINT sent other ways will still arrive in the application's SIGINT handler. Disadvantage: Ctrl-C during a prompt will not arrive in the parent application as a SIGINT. The user will have to use the application's exit command.

I originally started writing it as a thing to turn SIGINT back into Ctrl-C, but a) that gets awkward on Windows, and b) I don't think equating SIGINT with Ctrl-C is the right behaviour.

[raff]

That should work too (I will try it out, but I believe it's exactly what libreadline,that I was using before, was doing)

Thanks for doing this.

[raff]

Just tried on OSX and it works fine for my use case (with no changes in my code). Later I will try also Windows and Linux. Thanks!!!!

[michaelmacinnis]

@peterh would it be possible to have the current call to Prompt return an exit status to indicate that Ctrl-C was pressed. I may be in the middle of parsing a multi-line command and need to know to abort it. I would then call Prompt again to display the new empty prompt. I will have more time to look at this this weekend.