Hey,
When looking to the VTE code to fix focusing upon middle click, I also dig into the long-running problems we have on dealing with ^C and ^D.
First, we try to kill the child (the shell we launched) using SIGINT, but neither BASH nor DASH seem to honor them this way, so we actually only reset the view, thus leading to report like:
* https://sourceforge.net/tracker/index.php?func=detail&aid=2623225&gr... (I think improperly marked as fixed) * https://sourceforge.net/tracker/index.php?func=detail&aid=3518151&gr...
You can check it if you want, either by sending SIGINT to a manually-launched BASH or by checking whether the pid actually quit. Anyway, you'll see nothing gets killed.
So, I propose to replace this by a SIGHUP. I attached a patch that does it, and it works; but I'm not 100% sure if it's OK to do so, although I don't see much problem with it.
Patch is 0001-VTE-Fix-killing-the-child.patch.
The other problem is that we reset the terminal upon ^C -- and worse, upon ^D. OK, ^C is used to send SIGINT to the running child, which generally result in it exiting. But first, one generally expect it to only kill the running child [1], and not the terminal itself; and this is important e.g. if the user still want to read the output (e.g. if a program went wild, it may still have output useful errors). And then, legitimate programs handle SIGINT somewhat gracefully -- the excessive example being Nano which uses ^C as a shortcut.
So, I don't think ^C should reset the terminal/kill the shell, but rather be handled by the shell.
Now ^D. An user expects ^D to send EOF to the stdin of the running child, whatever this might mean to that child; not to behave like ^C and certainly not to reset the terminal/kill the shell.
So, to summarize, I think it's important that both ^C and ^D end up being handled by the shell we run in the VTE, and not by us vainly trying to do something that looks like almost not so bad.
I search a bit how I could forward ^C and ^D to the VTE child, and didn't find much in the VTE docs; but Wikipedia told me that ASCII ETX and EOT were commonly respectively used to mean ^C and ^D for UNIX terminals [2] [3]. And indeed, sending those using `vte_terminal_feed_child()` seems to work just fine. I however don't know how portable/reliable this is, so I ask for your knowledge and opinion here.
Attached patch is 0002-VTE-Really-send-C-and-D-to-the-terminal-rather-than-.patch.
Ah, and note that one can still reset the terminal like before through the context menu.
So, have you opinions, ideas, remarks, something to say? Looking forward to read you on the subject :)
Regards, Colomban
[1] and here we speak of the shell's child, not the VTE child which is the shell. [2] https://en.wikipedia.org/wiki/End-of-text_character [3] https://en.wikipedia.org/wiki/End-of-transmission_character
On 5 December 2012 09:18, Colomban Wendling lists.ban@herbesfolles.org wrote:
Hey,
When looking to the VTE code to fix focusing upon middle click, I also dig into the long-running problems we have on dealing with ^C and ^D.
First, we try to kill the child (the shell we launched) using SIGINT, but neither BASH nor DASH seem to honor them this way, so we actually only reset the view, thus leading to report like:
https://sourceforge.net/tracker/index.php?func=detail&aid=2623225&gr... (I think improperly marked as fixed)
https://sourceforge.net/tracker/index.php?func=detail&aid=3518151&gr...
You can check it if you want, either by sending SIGINT to a manually-launched BASH or by checking whether the pid actually quit. Anyway, you'll see nothing gets killed.
So, I propose to replace this by a SIGHUP. I attached a patch that does it, and it works; but I'm not 100% sure if it's OK to do so, although I don't see much problem with it.
Doesn't this screw up child processes that do something on sigint, since they won't get it now?
Patch is 0001-VTE-Fix-killing-the-child.patch.
The other problem is that we reset the terminal upon ^C -- and worse, upon ^D. OK, ^C is used to send SIGINT to the running child, which generally result in it exiting. But first, one generally expect it to only kill the running child [1], and not the terminal itself; and this is important e.g. if the user still want to read the output (e.g. if a program went wild, it may still have output useful errors). And then, legitimate programs handle SIGINT somewhat gracefully -- the excessive example being Nano which uses ^C as a shortcut.
So, I don't think ^C should reset the terminal/kill the shell, but rather be handled by the shell.
agreed, what happens if we actually send it to the shell?
Now ^D. An user expects ^D to send EOF to the stdin of the running child, whatever this might mean to that child; not to behave like ^C and certainly not to reset the terminal/kill the shell.
Agreed as well.
So, to summarize, I think it's important that both ^C and ^D end up being handled by the shell we run in the VTE, and not by us vainly trying to do something that looks like almost not so bad.
I search a bit how I could forward ^C and ^D to the VTE child, and didn't find much in the VTE docs; but Wikipedia told me that ASCII ETX and EOT were commonly respectively used to mean ^C and ^D for UNIX terminals [2] [3]. And indeed, sending those using `vte_terminal_feed_child()` seems to work just fine. I however don't know how portable/reliable this is, so I ask for your knowledge and opinion here.
This vaguely triggers a memory of a previous discussion on this topic, but I don't have time just now to search for it.
Attached patch is 0002-VTE-Really-send-C-and-D-to-the-terminal-rather-than-.patch.
Ah, and note that one can still reset the terminal like before through the context menu.
So, have you opinions, ideas, remarks, something to say? Looking forward to read you on the subject :)
Can you get any ideas from what the various terminal emulators that use VTE do with these keycodes etc?
Cheers Lex
Regards, Colomban
[1] and here we speak of the shell's child, not the VTE child which is the shell. [2] https://en.wikipedia.org/wiki/End-of-text_character [3] https://en.wikipedia.org/wiki/End-of-transmission_character
Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Le 04/12/2012 23:36, Lex Trotman a écrit :
On 5 December 2012 09:18, Colomban Wendling lists.ban@herbesfolles.org wrote:
Hey,
When looking to the VTE code to fix focusing upon middle click, I also dig into the long-running problems we have on dealing with ^C and ^D.
First, we try to kill the child (the shell we launched) using SIGINT, but neither BASH nor DASH seem to honor them this way, so we actually only reset the view, thus leading to report like:
https://sourceforge.net/tracker/index.php?func=detail&aid=2623225&gr... (I think improperly marked as fixed)
https://sourceforge.net/tracker/index.php?func=detail&aid=3518151&gr...
You can check it if you want, either by sending SIGINT to a manually-launched BASH or by checking whether the pid actually quit. Anyway, you'll see nothing gets killed.
So, I propose to replace this by a SIGHUP. I attached a patch that does it, and it works; but I'm not 100% sure if it's OK to do so, although I don't see much problem with it.
Doesn't this screw up child processes that do something on sigint, since they won't get it now?
Well, we send this signal to the shell we launched and it only, so I expect the shell do what's needed to kill its children. I guess shells are supposed to handle this since it's the old "tty disconnect" signal; and at least Bash saves its history and quits with this one (whereas it doesn't quit with SIGINT and doesn't (obviously) save its history with SIGKILL). If you know a better signal, I'm all ears :)
The other problem is that we reset the terminal upon ^C -- and worse, upon ^D. OK, ^C is used to send SIGINT to the running child, which generally result in it exiting. But first, one generally expect it to only kill the running child [1], and not the terminal itself; and this is important e.g. if the user still want to read the output (e.g. if a program went wild, it may still have output useful errors). And then, legitimate programs handle SIGINT somewhat gracefully -- the excessive example being Nano which uses ^C as a shortcut.
So, I don't think ^C should reset the terminal/kill the shell, but rather be handled by the shell.
agreed, what happens if we actually send it to the shell?
The "problem" is that Geany has a global key handling function that will eat the event before they reach the VTE handler (which will simply send them to the child, as they should).
Though, your remark made me go in that function an realize it'd be quite easy to force it to let ^C and ^D pass through when on the VTE -- there is already quite some code for the VTE here.
So I now have a better patch that simply makes ^C and ^D pass through in any case, no matter "override Geany keybindings" is enabled or not -- since when it is ^C and ^D goes through properly. This approach looks indeed ways better, doesn't involves weird ASCII control characters, and removes quite some code -- and the code is slightly cleaner than before, although the check for ^C and ^D are spread across 2 files (instead of being spread across 2 functions in the same file).
Here's the revised patch (with a revised name): 0001-VTE-Always-let-the-terminal-handle-C-and-D-itself.patch
I think this patch is good (for now, hehe :D), but maybe we want let the other common binding pass through too, like ^Z, maybe others?
[...]
Can you get any ideas from what the various terminal emulators that use VTE do with these keycodes etc?
Nothing, they just let the keybinding go through I guess.
Regards, Colomban
[...]
Well, we send this signal to the shell we launched and it only, so I expect the shell do what's needed to kill its children.
Oops, I misread it and thought we sent it to the child.
I guess shells
are supposed to handle this since it's the old "tty disconnect" signal; and at least Bash saves its history and quits with this one (whereas it doesn't quit with SIGINT and doesn't (obviously) save its history with SIGKILL). If you know a better signal, I'm all ears :)
[...]
agreed, what happens if we actually send it to the shell?
The "problem" is that Geany has a global key handling function that will eat the event before they reach the VTE handler (which will simply send them to the child, as they should).
Though, your remark made me go in that function an realize it'd be quite easy to force it to let ^C and ^D pass through when on the VTE -- there is already quite some code for the VTE here.
So I now have a better patch that simply makes ^C and ^D pass through in any case, no matter "override Geany keybindings" is enabled or not -- since when it is ^C and ^D goes through properly. This approach looks indeed ways better, doesn't involves weird ASCII control characters, and removes quite some code -- and the code is slightly cleaner than before, although the check for ^C and ^D are spread across 2 files (instead of being spread across 2 functions in the same file).
Here's the revised patch (with a revised name): 0001-VTE-Always-let-the-terminal-handle-C-and-D-itself.patch
I think this patch is good (for now, hehe :D), but maybe we want let the other common binding pass through too, like ^Z, maybe others?
Why not just send all by default and only a few very specific keybindings (like switch focus) work in the VTE. The problem with "common" ones is that someone will always find some program that uses some other keys. Of course you could have a second set of keybindings for the VTE window, not :)
[...]
Can you get any ideas from what the various terminal emulators that use VTE do with these keycodes etc?
Nothing, they just let the keybinding go through I guess.
Good idea lets do that :)
Seriously, trying to be smart and decide that some keybindings will/won't work in the VTE window is always going to never be completely right and always risks clashes with bindings in Geany itself.
Cheers Lex
Regards, Colomban
Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Le 05/12/2012 01:42, Lex Trotman a écrit :
[...]
Well, we send this signal to the shell we launched and it only, so I expect the shell do what's needed to kill its children.
Oops, I misread it and thought we sent it to the child.
Actually we don't know about what's running on the shell, and I doubt we can, so yeah, it was the shell itself :)
[...]
I think this patch is good (for now, hehe :D), but maybe we want let the other common binding pass through too, like ^Z, maybe others?
Why not just send all by default and only a few very specific keybindings (like switch focus) work in the VTE. The problem with "common" ones is that someone will always find some program that uses some other keys. Of course you could have a second set of keybindings for the VTE window, not :)
Read ahead.
[...]
Can you get any ideas from what the various terminal emulators that use VTE do with these keycodes etc?
Nothing, they just let the keybinding go through I guess.
Good idea lets do that :)
Seriously, trying to be smart and decide that some keybindings will/won't work in the VTE window is always going to never be completely right and always risks clashes with bindings in Geany itself.
We already have an "Override Geany keybindings" in the terminal prefs, that actually overrides everything but focus keybindings. But for some reason, this is a setting and it is not enabled by default. And if it remains a setting, we somewhat *have to* let required stuff like ^C go through in any cases.
So ok, one might argue that the setting don't make much sense, but it's there and it allows for (most) Geany keybinding behaving like they do everywhere else in the VTE too. Though, at least with the way it is done yet, if you enable "Override Geany keybindings", *all* (but focus) keybindings will be eaten by the terminal no matter whether it or a running process in it will actually handle the stroke; so I can understand one wanting this not to happen.
Regards, Colomban
[...]
We already have an "Override Geany keybindings" in the terminal prefs, that actually overrides everything but focus keybindings. But for some reason, this is a setting and it is not enabled by default. And if it remains a setting, we somewhat *have to* let required stuff like ^C go through in any cases.
Ok, that makes sense, in the overridden case all keys except focus switch ones go to VTE and in the non-overriden case only a specified list go to the VTE (^c, ^d, ^z, ^x, ^o (for nano for git commits) can't think of any others). What about if the vte is not focussed?
These two lists should be in a table in the manual in keybindings section, and the vte setting section referring to that table.
Cheers Lex
So ok, one might argue that the setting don't make much sense, but it's there and it allows for (most) Geany keybinding behaving like they do everywhere else in the VTE too. Though, at least with the way it is done yet, if you enable "Override Geany keybindings", *all* (but focus) keybindings will be eaten by the terminal no matter whether it or a running process in it will actually handle the stroke; so I can understand one wanting this not to happen.
Regards, Colomban _______________________________________________ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Le 06/12/2012 22:53, Lex Trotman a écrit :
[...]
We already have an "Override Geany keybindings" in the terminal prefs, that actually overrides everything but focus keybindings. But for some reason, this is a setting and it is not enabled by default. And if it remains a setting, we somewhat *have to* let required stuff like ^C go through in any cases.
Ok, that makes sense, in the overridden case all keys except focus switch ones go to VTE and in the non-overriden case only a specified list go to the VTE (^c, ^d, ^z, ^x, ^o (for nano for git commits) can't think of any others).
If you want to go down the "my favorite text-based editor for Git commit should work" I think the keys must then me more or less configurable, because not everybody uses Nano, and if they don't use Nano ^X and ^O aren't required (and ^O looks like a keybinding one could want to open the open dialog even in the VTE).
So a really naive "configurability" like the attached patch might be needed then (or even a more real binding description, but that's somewhat a detail).
What about if the vte is not focussed?
Don't worry, the key eating only happens if the VTE is focused :)
Regards, Colomban
These two lists should be in a table in the manual in keybindings section, and the vte setting section referring to that table.
Cheers Lex
On 8 December 2012 01:40, Colomban Wendling lists.ban@herbesfolles.org wrote:
Le 06/12/2012 22:53, Lex Trotman a écrit :
[...]
We already have an "Override Geany keybindings" in the terminal prefs, that actually overrides everything but focus keybindings. But for some reason, this is a setting and it is not enabled by default. And if it remains a setting, we somewhat *have to* let required stuff like ^C go through in any cases.
Ok, that makes sense, in the overridden case all keys except focus switch ones go to VTE and in the non-overriden case only a specified list go to the VTE (^c, ^d, ^z, ^x, ^o (for nano for git commits) can't think of any others).
If you want to go down the "my favorite text-based editor for Git commit should work" I think the keys must then me more or less configurable, because not everybody uses Nano, and if they don't use Nano ^X and ^O aren't required (and ^O looks like a keybinding one could want to open the open dialog even in the VTE).
Well, I would much prefer a git plugin which used Geany itself for the commit editor (and not a separate instance), do you think there will ever be such a thing? ;)
So a really naive "configurability" like the attached patch might be needed then (or even a more real binding description, but that's somewhat a detail).
What about if the vte is not focussed?
Don't worry, the key eating only happens if the VTE is focused :)
Ok, fine.
Cheers Lex
Regards, Colomban
These two lists should be in a table in the manual in keybindings section, and the vte setting section referring to that table.
Cheers Lex
Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
On 05/12/2012 14:36, Colomban Wendling wrote:
We already have an "Override Geany keybindings" in the terminal prefs, that actually overrides everything but focus keybindings. But for some reason, this is a setting and it is not enabled by default. And if it remains a setting, we somewhat*have to* let required stuff like ^C go through in any cases.
Probably we should change it to be on by default.
Letting Ctrl-C/D through when the setting is not enabled is OK, but I would stop there and recommend using the setting for anything else.