Le 04/12/2012 23:36, Lex Trotman a écrit :
> On 5 December 2012 09:18, Colomban Wendling <lists....@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&group_id=153444&atid=787791
>> (I think improperly marked as fixed)
>>  *
>> https://sourceforge.net/tracker/index.php?func=detail&aid=3518151&group_id=153444&atid=787791
>>
>> 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
>From da32dcaf00afbc155dfb4a3c2ef6501fec496a0a Mon Sep 17 00:00:00 2001
From: Colomban Wendling <b...@herbesfolles.org>
Date: Tue, 4 Dec 2012 22:16:57 +0100
Subject: [PATCH] VTE: Always let the terminal handle ^C and ^D itself

This makes the terminal always handle ^C and ^D itself rather than
Geany failing at faking them, so they behave like they are supposed to
in a terminal.  Most importantly, ^D will no longer try to kill the
shell;  but also programs that catches ^C as a shortcut (like Nano)
won't get killed.
---
 src/keybindings.c |    4 +++-
 src/vte.c         |   28 +++-------------------------
 2 files changed, 6 insertions(+), 26 deletions(-)

diff --git a/src/keybindings.c b/src/keybindings.c
index b36c56b..19f102d 100644
--- a/src/keybindings.c
+++ b/src/keybindings.c
@@ -1087,7 +1087,9 @@ static gboolean check_vte(GdkModifierType state, guint keyval)
 	/* let VTE copy/paste override any user keybinding */
 	if (state == (GDK_CONTROL_MASK | GDK_SHIFT_MASK) && (keyval == GDK_c || keyval == GDK_v))
 		return TRUE;
-	if (! vc->enable_bash_keys)
+	if (! vc->enable_bash_keys &&
+		/* force handling of ^C and ^D by the VTE */
+		! (state == GDK_CONTROL_MASK && (keyval == GDK_c || keyval == GDK_d)))
 		return FALSE;
 	/* prevent menubar flickering: */
 	if (state == GDK_SHIFT_MASK && (keyval >= GDK_a && keyval <= GDK_z))
diff --git a/src/vte.c b/src/vte.c
index e5325be..490c53b 100644
--- a/src/vte.c
+++ b/src/vte.c
@@ -120,7 +120,6 @@ static void vte_start(GtkWidget *widget);
 static void vte_restart(GtkWidget *widget);
 static gboolean vte_button_pressed(GtkWidget *widget, GdkEventButton *event, gpointer user_data);
 static gboolean vte_keyrelease_cb(GtkWidget *widget, GdkEventKey *event, gpointer data);
-static gboolean vte_keypress_cb(GtkWidget *widget, GdkEventKey *event, gpointer data);
 static gboolean vte_register_symbols(GModule *module);
 static void vte_popup_menu_clicked(GtkMenuItem *menuitem, gpointer user_data);
 static GtkWidget *vte_create_popup_menu(void);
@@ -278,7 +277,6 @@ static void create_vte(void)
 
 	g_signal_connect(vte, "child-exited", G_CALLBACK(vte_start), NULL);
 	g_signal_connect(vte, "button-press-event", G_CALLBACK(vte_button_pressed), NULL);
-	g_signal_connect(vte, "event", G_CALLBACK(vte_keypress_cb), NULL);
 	g_signal_connect(vte, "key-release-event", G_CALLBACK(vte_keyrelease_cb), NULL);
 	g_signal_connect(vte, "commit", G_CALLBACK(vte_commit_cb), NULL);
 	g_signal_connect(vte, "motion-notify-event", G_CALLBACK(on_motion_event), NULL);
@@ -319,7 +317,9 @@ void vte_close(void)
 static gboolean vte_keyrelease_cb(GtkWidget *widget, GdkEventKey *event, gpointer data)
 {
 	if (ui_is_keyval_enter_or_return(event->keyval) ||
-		((event->keyval == GDK_c) && (event->state & GDK_CONTROL_MASK)))
+		(((event->state & gtk_accelerator_get_default_mod_mask()) == GDK_CONTROL_MASK) &&
+		 (event->keyval == GDK_C || event->keyval == GDK_c ||
+		  event->keyval == GDK_D || event->keyval == GDK_d)))
 	{
 		/* assume any text on the prompt has been executed when pressing Enter/Return */
 		clean = TRUE;
@@ -328,28 +328,6 @@ static gboolean vte_keyrelease_cb(GtkWidget *widget, GdkEventKey *event, gpointe
 }
 
 
-static gboolean vte_keypress_cb(GtkWidget *widget, GdkEventKey *event, gpointer data)
-{
-	if (vc->enable_bash_keys)
-		return FALSE;	/* Ctrl-[CD] will be handled by the VTE itself */
-
-	if (event->type != GDK_KEY_RELEASE)
-		return FALSE;
-
-	if ((event->keyval == GDK_c ||
-		event->keyval == GDK_d ||
-		event->keyval == GDK_C ||
-		event->keyval == GDK_D) &&
-		event->state & GDK_CONTROL_MASK &&
-		! (event->state & GDK_SHIFT_MASK) && ! (event->state & GDK_MOD1_MASK))
-	{
-		vte_restart(widget);
-		return TRUE;
-	}
-	return FALSE;
-}
-
-
 static void vte_commit_cb(VteTerminal *vte, gchar *arg1, guint arg2, gpointer user_data)
 {
 	clean = FALSE;
-- 
1.7.10.4

_______________________________________________
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel

Reply via email to