GNOME Bugzilla – Bug 706118
ctrl-c make terminal unusable
Last modified: 2014-06-24 10:08:00 UTC
After I use ctrl-c to exit "nmcli con edit", I do not see anything which is typed into my terminal. Other than that, all works fine :)
Also it would be nice if nmcli exits on ctrl-d
Fix pushed to master: 555e5b4 cli: reset terminal using libreadline when quitting on signal (bgo #706118) 663014e cli: handle POSIX signals in a dedicated thread (As as workaround in the old version, type stty sane<Enter> to fix the terminal.) (In reply to comment #1) > Also it would be nice if nmcli exits on ctrl-d We will evaluate Ctrl-d later.
(About the workaround for the old version, typing "reset" works as well.) In edit mode, I would expect the following behaviour: Ctrl-d: exit nmcli immediately (without asking whether you want to save the connection). Ctrl-c: if nmcli is waiting for user input, abort the currently typed input and show a new line with a prompt. This should also be the case, when nmcli is asking for further parameters (for example "goto" without setting argument). I think that behaviour would be consistent with e.g. bash or interactive python. Currently, there is only the save command, that does something towards the server. Later, there might be an activate command and it would be interesting what should happen when the user types Ctrl-c while waiting for a connection to be activated. Maybe nmcli should show a fresh prompt and just do not wait any more (without stopping the activation itself).
(In reply to comment #3) > (About the workaround for the old version, typing "reset" works as well.) > > > In edit mode, I would expect the following behaviour: > > Ctrl-d: exit nmcli immediately (without asking whether you want to save the > connection). Ctrl+D is just end of input. I would expect it to finish giving input to some component. If you're in edit mode, I would only expect it to quit the edit mode. What you're talking about might be ctrl-\, which is a key combination for quitting the software immediately. > Ctrl-c: if nmcli is waiting for user input, abort the currently typed input and > show a new line with a prompt. +1 This is the usual way shell-like programs behave. > This should also be the case, when nmcli is > asking for further parameters (for example "goto" without setting argument). > > I think that behaviour would be consistent with e.g. bash or interactive > python. +1 > Currently, there is only the save command, that does something towards the > server. Later, there might be an activate command and it would be interesting > what should happen when the user types Ctrl-c while waiting for a connection to > be activated. Simply stop waiting and present the prompt. > Maybe nmcli should show a fresh prompt and just do not wait any > more (without stopping the activation itself). +1
I reopen the bug for implementation of Ctrl-C and Ctrl-D in readline. Ctrl-C in readline (interactive prompt) won't quit nmcli, but cancel the input and show new prompt. Ctrl-D in readline will quit nmcli when line is empty, else it behaves like Delete key. This behaviour is similar as in other interactive programs like bash, python, irb. Though some other may quit on Ctrl-C, e.g. lua. See branch jk/cli-ctrl-c-d
>> cli: don't quit nmcli on Ctrl-C while in readline() + /* In case of Ctrl-C we call nmc_readline() again recursively */ + if (nmc_seen_sigint ()) { + nmc_clear_sigint (); + g_free (str); + str = nmc_readline ("%s", prompt); + } instead of calling recursively, how about "goto label_before_nmc_set_in_readline;" or even better: a while loop. Otherwise, it needs to reconstruct the prompt over and over again and it might stack-overflow if you keep CTRL+C pressed... (maybe not and the stack is large enough... but still). >> cli: mitigate slowness of readline() for some broken versions referencing the bug like this: [1] https://lists.gnu.org/archive/html/bug-readline/2012-06/msg00005.html [2] https://bugzilla.redhat.com/show_bug.cgi?id=1109946 will cause the bzutil.py script to detect this as "fixed bugs"... Could you modify the commit message, so that bug 1109946 is detected as "related"? All in all, I really like this new functionality!! (still testing)
some testing: ./cli/src/nmcli --ask connection add Valid types: [generic, 802-3-...] Connection type: <CTRL-D> sometimes this results in: Connection type: Error: 'type' argument is required. Error: nmcli terminated by signal Quit (3) sometimes in: Connection type: Error: nmcli terminated by signal Quit (3) Also, maybe we should suppress this error message (but I guess we cannot distinguish between an externally sent SIGQUIT and raised internally/during readline. Also, playing around with CTRL-D in edit-mode, I managed to break my terminal, that typed characters were no longer visible. (could be fixed with 'reset').
(In reply to comment #6) > >> cli: don't quit nmcli on Ctrl-C while in readline() > > + /* In case of Ctrl-C we call nmc_readline() again recursively */ > + if (nmc_seen_sigint ()) { > + nmc_clear_sigint (); > + g_free (str); > + str = nmc_readline ("%s", prompt); > + } > > instead of calling recursively, how about "goto > label_before_nmc_set_in_readline;" or even better: a while loop. > > Otherwise, it needs to reconstruct the prompt over and over again and it might > stack-overflow if you keep CTRL+C pressed... (maybe not and the stack is large > enough... but still). > I guess it would not be a real probem, but changed to use goto. > > >> cli: mitigate slowness of readline() for some broken versions > > referencing the bug like this: > [1] https://lists.gnu.org/archive/html/bug-readline/2012-06/msg00005.html > [2] https://bugzilla.redhat.com/show_bug.cgi?id=1109946 > will cause the bzutil.py script to detect this as "fixed bugs"... > > Could you modify the commit message, so that bug 1109946 is detected as > "related"? > What's the keyword for that?
(In reply to comment #7) > some testing: > > ./cli/src/nmcli --ask connection add > Valid types: [generic, 802-3-...] > Connection type: <CTRL-D> > > sometimes this results in: > Connection type: Error: 'type' argument is required. > > Error: nmcli terminated by signal Quit (3) > > sometimes in: > Connection type: > Error: nmcli terminated by signal Quit (3) > > > Also, maybe we should suppress this error message (but I guess we cannot > distinguish between an externally sent SIGQUIT and raised internally/during > readline. > Fixed. > Also, playing around with CTRL-D in edit-mode, I managed to break my terminal, > that typed characters were no longer visible. (could be fixed with 'reset'). I don't see that. It should be already fixed. If you see it again, please try to remember what you did.
Code looks fine to me. I like the behavior change too, it's certianly more consistent with other tools.
Pushed two minor fixups. Now, when you close input with CTRL+D, it prints no newline (I think it should): $ ./cli/src/nmcli --ask connection add Valid types: [generic, 802-3-...] Connection type: $ also, I think in questionary mode (not edit mode), CTRL+C should work differently. If the line is not empty, it should clear the line and show a new prompt (as it does not). If the line is already empty, it should terminate the application. Otherwise you can only abort the questionary mode with CTRL+D or CTRL+\, which is rather obscure and not well known. I cannot reproduce the problem I had before. Maybe it's fixed.
(In reply to comment #11) > Pushed two minor fixups. > Thanks. Squashed the commits and rebased to master. > > Now, when you close input with CTRL+D, it prints no newline (I think it > should): > > $ ./cli/src/nmcli --ask connection add > Valid types: [generic, 802-3-...] > Connection type: $ > > also, I think in questionary mode (not edit mode), CTRL+C should work > differently. If the line is not empty, it should clear the line and show a new > prompt (as it does not). If the line is already empty, it should terminate the > application. Otherwise you can only abort the questionary mode with CTRL+D or > CTRL+\, which is rather obscure and not well known. > I think it is reasonable. I have added a new commit to allow quitting on Ctrl-C when not in editor and line is empty.
(In reply to comment #12) > I think it is reasonable. I have added a new commit to allow quitting on Ctrl-C > when not in editor and line is empty. +/* Global variable defined in nmcli.c */ +extern NmCli nm_cli; nm_cli is now defined externally in 4 C-files. Why not define it in nmcli.h (which is included already)? Rest looks good to me.
(In reply to comment #8) > (In reply to comment #6) > > >> cli: mitigate slowness of readline() for some broken versions > > > > referencing the bug like this: > > [1] https://lists.gnu.org/archive/html/bug-readline/2012-06/msg00005.html > > [2] https://bugzilla.redhat.com/show_bug.cgi?id=1109946 > > will cause the bzutil.py script to detect this as "fixed bugs"... > > > > Could you modify the commit message, so that bug 1109946 is detected as > > "related"? > > > What's the keyword for that? If you prefix the bug by: /([rR]elated(:[ \t]*|[ \t]+|( |\n)to( |\n)(bug( |\n))?))?/ it is marked as related. If you reference the same BZ more then once, you only have to mark it once (related "wins"). Note, you can sneak it in a normal sentence, by saying that this is related to bug bgo#706118. ^^^^^^^^^^^^^^^ Test it with $ contrib/rh-utils/bzutil.py parse --color --show-empty-refs --ref origin/master..origin/jk/cli-ctrl-c-d --no-related
(In reply to comment #13) > +/* Global variable defined in nmcli.c */ > +extern NmCli nm_cli; > > nm_cli is now defined externally in 4 C-files. Why not define it in nmcli.h > (which is included already)? > I left this as it is for now. There's a plan to improve this when NmCli object usage refactoring is done. (In reply to comment #14) > (In reply to comment #8) > > (In reply to comment #6) > > What's the keyword for that? > > > If you prefix the bug by: > > /([rR]elated(:[ \t]*|[ \t]+|( |\n)to( |\n)(bug( |\n))?))?/ > > it is marked as related. If you reference the same BZ more then once, you only > have to mark it once (related "wins"). > > > Note, you can sneak it in a normal sentence, by saying that this is > related to bug bgo#706118. > ^^^^^^^^^^^^^^^ > > > Test it with > $ contrib/rh-utils/bzutil.py parse --color --show-empty-refs --ref > origin/master..origin/jk/cli-ctrl-c-d --no-related Thanks.
Commits in master: c8902e8 Merge changes for Ctrl-C and Ctrl-D handling in nmcli/readline (bgo #706118) 12f284d cli: make Ctrl-C exit nmcli when line is empty *and* we are not in the editor 7e75cce cli: don't print "Error: nmcli terminated by signal Quit (3)" on Ctrl-D c0822f6 cli: append NL to error messages and also print the signal name dbf2d08 cli: allow quitting on Ctrl-D while in readline() 433c067 cli: mitigate slowness of readline() for some broken versions 512fe08 cli: don't quit nmcli on Ctrl-C while in readline()