Accelerator-only actions, take two

[Matthias Clasen <maclas gmx de>]

Here is another attempt to provide accelerator-only actions. This patch
also switches to installing accelerators on the actions themself, not on
the proxies. But unlike my first attempt, the lifecycle of accelerators
is exactly as it in the current code: they are installed when a proxy
menuitem is connected and uninstalled when it is disconnected again. In
order to properly handle multiple proxies for the same action, we have
to count how often the accelerator for an action has been installed, and
only uninstall it when the count goes down to zero again.
Accelerator-only actions are supported via 
<accelerator name="foo" action="bar"/> elements below the root <ui>
element in the ui descriptions.

Does this look better ?

Matthias


Index: gtkaction.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkaction.c,v
retrieving revision 1.8
diff -u -p -r1.8 gtkaction.c
--- gtkaction.c	6 Sep 2003 23:52:00 -0000	1.8
+++ gtkaction.c	8 Sep 2003 22:32:31 -0000
@@ -59,7 +59,10 @@ struct _GtkActionPrivate 
   guint is_important    : 1;
 
   /* accelerator */
-  GQuark accel_quark;
+  guint          accel_count;
+  GtkAccelGroup *accel_group;
+  GClosure      *accel_closure;
+  GQuark         accel_quark;
 
   /* list of proxy widgets */
   GSList *proxies;
@@ -130,10 +133,16 @@ static void gtk_action_get_property (GOb
 
 static GtkWidget *create_menu_item    (GtkAction *action);
 static GtkWidget *create_tool_item    (GtkAction *action);
-static void       connect_proxy       (GtkAction *action,
-				       GtkWidget *proxy);
+static void       connect_proxy       (GtkAction     *action,
+				       GtkWidget     *proxy);
 static void       disconnect_proxy    (GtkAction *action,
 				       GtkWidget *proxy);
+static void       closure_accel_activate (GClosure     *closure,
+					  GValue       *return_value,
+					  guint         n_param_values,
+					  const GValue *param_values,
+					  gpointer      invocation_hint,
+					  gpointer      marshal_data);
 
 static GObjectClass *parent_class = NULL;
 static guint         action_signals[LAST_SIGNAL] = { 0 };
@@ -260,7 +269,17 @@ gtk_action_init (GtkAction *action)
   action->private_data->label_set = FALSE;
   action->private_data->short_label_set = FALSE;
 
+  action->private_data->accel_count = 0;
+  action->private_data->accel_closure = 
+    g_closure_new_object (sizeof (GClosure), G_OBJECT (action));
+  g_closure_set_marshal (action->private_data->accel_closure, 
+			 closure_accel_activate);
+  g_closure_ref (action->private_data->accel_closure);
+  g_closure_sink (action->private_data->accel_closure);
+
   action->private_data->accel_quark = 0;
+  action->private_data->accel_count = 0;
+  action->private_data->accel_group = NULL;
 
   action->private_data->proxies = NULL;
 }
@@ -277,6 +296,10 @@ gtk_action_finalize (GObject *object)
   g_free (action->private_data->short_label);
   g_free (action->private_data->tooltip);
   g_free (action->private_data->stock_id);
+
+  g_object_unref (action->private_data->accel_closure);
+  if (action->private_data->accel_group)
+    g_object_unref (action->private_data->accel_group);
 }
 
 static void
@@ -441,7 +464,7 @@ remove_proxy (GtkWidget *proxy, 
 	      GtkAction *action)
 {
   if (GTK_IS_MENU_ITEM (proxy))
-    gtk_menu_item_set_accel_path (GTK_MENU_ITEM (proxy), NULL);
+    gtk_action_disconnect_accelerator (action);
 
   action->private_data->proxies = g_slist_remove (action->private_data->proxies, proxy);
 }
@@ -524,8 +547,8 @@ gtk_action_create_menu_proxy (GtkToolIte
 }
 
 static void
-connect_proxy (GtkAction *action, 
-	       GtkWidget *proxy)
+connect_proxy (GtkAction     *action, 
+	       GtkWidget     *proxy)
 {
   g_object_ref (action);
   g_object_set_data_full (G_OBJECT (proxy), "gtk-action", action,
@@ -552,6 +575,13 @@ connect_proxy (GtkAction *action, 
       GtkWidget *label;
       /* menu item specific synchronisers ... */
       
+      if (action->private_data->accel_quark)
+ 	{
+	  gtk_action_connect_accelerator (action);
+ 	  gtk_menu_item_set_accel_path (GTK_MENU_ITEM (proxy),
+ 					g_quark_to_string (action->private_data->accel_quark));
+ 	}
+      
       label = GTK_BIN (proxy)->child;
 
       /* make sure label is a label */
@@ -567,7 +597,7 @@ connect_proxy (GtkAction *action, 
 				"xalign", 0.0,
 				"visible", TRUE,
 				"parent", proxy,
-				"accel_widget", proxy,
+				"accel_closure", action->private_data->accel_closure,
 				NULL);
 	}
       gtk_label_set_label (GTK_LABEL (label), action->private_data->label);
@@ -599,15 +629,10 @@ connect_proxy (GtkAction *action, 
 				   proxy, 0);
 	}
 
-      if (action->private_data->accel_quark)
-	{
-	  gtk_menu_item_set_accel_path (GTK_MENU_ITEM (proxy),
-					g_quark_to_string (action->private_data->accel_quark));
-	}
-
       g_signal_connect_object (proxy, "activate",
 			       G_CALLBACK (gtk_action_activate), action,
 			       G_CONNECT_SWAPPED);
+
     }
   else if (GTK_IS_TOOL_BUTTON (proxy))
     {
@@ -923,6 +948,21 @@ gtk_action_unblock_activate_from (GtkAct
 				     action);
 }
 
+static void
+closure_accel_activate (GClosure     *closure,
+                        GValue       *return_value,
+                        guint         n_param_values,
+                        const GValue *param_values,
+                        gpointer      invocation_hint,
+                        gpointer      marshal_data)
+{
+  if (GTK_ACTION (closure->data)->private_data->sensitive)
+    g_signal_emit (closure->data, action_signals[ACTIVATE], 0);
+
+  /* we handled the accelerator */
+  g_value_set_boolean (return_value, TRUE);
+}
+
 /**
  * gtk_action_set_accel_path:
  * @action: the action object
@@ -938,5 +978,60 @@ void
 gtk_action_set_accel_path (GtkAction   *action, 
 			   const gchar *accel_path)
 {
+  g_return_if_fail (GTK_IS_ACTION (action));
+
   action->private_data->accel_quark = g_quark_from_string (accel_path);
+}
+
+void
+gtk_action_set_accel_group (GtkAction     *action,
+			    GtkAccelGroup *accel_group)
+{
+  g_return_if_fail (GTK_IS_ACTION (action));
+  g_return_if_fail (accel_group == NULL || GTK_IS_ACCEL_GROUP (accel_group));
+  
+  if (accel_group)
+    g_object_ref (accel_group);
+  if (action->private_data->accel_group)
+    g_object_unref (action->private_data->accel_group);
+
+  action->private_data->accel_group = accel_group;
+}
+
+void 
+gtk_action_connect_accelerator (GtkAction *action)
+{
+  g_return_if_fail (GTK_IS_ACTION (action));
+
+  if (!action->private_data->accel_quark ||
+      !action->private_data->accel_group)
+    return;
+
+  if (action->private_data->accel_count == 0)
+    {
+      const gchar *accel_path = 
+	g_quark_to_string (action->private_data->accel_quark);
+      
+      gtk_accel_group_connect_by_path (action->private_data->accel_group,
+				       accel_path,
+				       action->private_data->accel_closure);
+    }
+
+  action->private_data->accel_count++;
+}
+
+void 
+gtk_action_disconnect_accelerator (GtkAction *action)
+{
+  g_return_if_fail (GTK_IS_ACTION (action));
+
+  if (!action->private_data->accel_quark ||
+      !action->private_data->accel_group)
+    return;
+
+  action->private_data->accel_count--;
+
+  if (action->private_data->accel_count == 0)
+    gtk_accel_group_disconnect (action->private_data->accel_group,
+				action->private_data->accel_closure);
 }
Index: gtkaction.h
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkaction.h,v
retrieving revision 1.1
diff -u -p -r1.1 gtkaction.h
--- gtkaction.h	24 Aug 2003 19:58:28 -0000	1.1
+++ gtkaction.h	8 Sep 2003 22:32:31 -0000
@@ -100,8 +100,11 @@ void       gtk_action_unblock_activate_f
 				             GtkWidget   *proxy);
 
 /* protected ... for use by action groups */
-void       gtk_action_set_accel_path        (GtkAction   *action,
-				             const gchar *accel_path);
-
+void       gtk_action_set_accel_path         (GtkAction     *action,
+				              const gchar   *accel_path);
+void       gtk_action_set_accel_group        (GtkAction     *action,
+					      GtkAccelGroup *accel_group);
+void       gtk_action_connect_accelerator    (GtkAction     *action);
+void       gtk_action_disconnect_accelerator (GtkAction     *action);
 
 #endif  /* __GTK_ACTION_H__ */
Index: gtkmenu.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkmenu.c,v
retrieving revision 1.139
diff -u -p -r1.139 gtkmenu.c
--- gtkmenu.c	3 Sep 2003 21:26:13 -0000	1.139
+++ gtkmenu.c	8 Sep 2003 22:32:32 -0000
@@ -28,6 +28,7 @@
 
 #include <string.h> /* memset */
 #include "gdk/gdkkeysyms.h"
+#include "gtkaccellabel.h"
 #include "gtkaccelmap.h"
 #include "gtkbindings.h"
 #include "gtklabel.h"
@@ -2030,6 +2031,41 @@ gtk_menu_button_release (GtkWidget      
   return GTK_WIDGET_CLASS (parent_class)->button_release_event (widget, event);
 }
 
+static const gchar *
+get_accel_path (GtkWidget *menu_item,
+		gboolean  *locked)
+{
+  const gchar *path;
+  GtkWidget *label;
+  GClosure *accel_closure;
+  GtkAccelGroup *accel_group;    
+
+  path = _gtk_widget_get_accel_path (menu_item, locked);
+  if (!path)
+    {
+      path = GTK_MENU_ITEM (menu_item)->accel_path;
+      
+      if (locked)
+	{
+	  *locked = TRUE;
+
+	  label = GTK_BIN (menu_item)->child;
+	  
+	  if (GTK_IS_ACCEL_LABEL (label))
+	    {
+	      g_object_get (label, 
+			    "accel_closure", &accel_closure, 
+			    NULL);
+	      accel_group = gtk_accel_group_from_accel_closure (accel_closure);
+	      
+	      *locked = accel_group->lock_count > 0;
+	    }
+	}
+    }
+
+  return path;
+}
+
 static gboolean
 gtk_menu_key_press (GtkWidget	*widget,
 		    GdkEventKey *event)
@@ -2124,7 +2160,7 @@ gtk_menu_key_press (GtkWidget	*widget,
       gboolean locked, replace_accels = TRUE;
       const gchar *path;
 
-      path = _gtk_widget_get_accel_path (menu_item, &locked);
+      path = get_accel_path (menu_item, &locked);
       if (!path || locked)
 	{
 	  /* can't change accelerators on menu_items without paths
Index: gtkuimanager.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkuimanager.c,v
retrieving revision 1.17
diff -u -p -r1.17 gtkuimanager.c
--- gtkuimanager.c	6 Sep 2003 23:52:00 -0000	1.17
+++ gtkuimanager.c	8 Sep 2003 22:32:32 -0000
@@ -56,6 +56,7 @@ typedef enum 
   NODE_TYPE_MENUITEM,
   NODE_TYPE_TOOLITEM,
   NODE_TYPE_SEPARATOR,
+  NODE_TYPE_ACCELERATOR
 } NodeType;
 
 
@@ -829,6 +830,24 @@ start_element_handler (GMarkupParseConte
 
   switch (element_name[0])
     {
+    case 'a':
+      if (ctx->state == STATE_ROOT && !strcmp (element_name, "accelerator"))
+	{
+	  ctx->state = STATE_ROOT;
+	  ctx->current = get_child_node (self, ctx->current,
+					 node_name, strlen (node_name),
+					 NODE_TYPE_ACCELERATOR,
+					 TRUE, FALSE);
+	  if (NODE_INFO (ctx->current)->action_name == 0)
+	    NODE_INFO (ctx->current)->action_name = action_quark;
+
+	  node_prepend_ui_reference (NODE_INFO (ctx->current),
+				     ctx->merge_id, action_quark);
+	  NODE_INFO (ctx->current)->dirty = TRUE;
+
+	  raise_error = FALSE;
+	}
+      break;
     case 'u':
       if (ctx->state == STATE_START && !strcmp (element_name, "ui"))
 	{
@@ -1303,6 +1322,9 @@ gtk_ui_manager_add_ui (GtkUIManager     
 	case GTK_UI_MANAGER_POPUP:
 	  node_type = NODE_TYPE_POPUP;
 	  break;
+	case GTK_UI_MANAGER_ACCELERATOR:
+	  node_type = NODE_TYPE_ACCELERATOR;
+	  break;
 	default: ;
 	  /* do nothing */
 	}
@@ -1597,6 +1619,9 @@ update_node (GtkUIManager *self, 
 	  goto recurse_children;
 	}
 
+      if (action)
+	gtk_action_set_accel_group (action, self->private_data->accel_group);
+
       /* If the widget already has a proxy and the action hasn't changed, then
        * we only have to update the tearoff menu items.
        */
@@ -1628,11 +1653,7 @@ update_node (GtkUIManager *self, 
 	  break;
 	case NODE_TYPE_POPUP:
 	  if (info->proxy == NULL) 
-	    {
-	      info->proxy = gtk_menu_new ();
-	      gtk_menu_set_accel_group (GTK_MENU (info->proxy), 
-					self->private_data->accel_group);
-	    }
+	    info->proxy = gtk_menu_new ();
 	  break;
 	case NODE_TYPE_MENU:
 	  {
@@ -1669,7 +1690,6 @@ update_node (GtkUIManager *self, 
 		    tearoff = gtk_tearoff_menu_item_new ();
 		    gtk_menu_shell_append (GTK_MENU_SHELL (menu), tearoff);
 		    gtk_menu_item_set_submenu (GTK_MENU_ITEM (info->proxy), menu);
-		    gtk_menu_set_accel_group (GTK_MENU (menu), self->private_data->accel_group);
 		    gtk_menu_shell_insert (GTK_MENU_SHELL (menushell), info->proxy, pos);
 		  }
 	      }
@@ -1874,6 +1894,9 @@ update_node (GtkUIManager *self, 
 		}
 	    }
 	  break;
+	case NODE_TYPE_ACCELERATOR:
+	  gtk_action_connect_accelerator (action);
+	  break;
 	}
 
       if (action)
@@ -1904,6 +1927,8 @@ update_node (GtkUIManager *self, 
 	   info->type == NODE_TYPE_TOOLBAR_PLACEHOLDER) &&
 	  info->extra)
 	gtk_widget_destroy (info->extra);
+      if (info->type == NODE_TYPE_ACCELERATOR)
+	gtk_action_disconnect_accelerator (info->action);
       g_chunk_free (info, merge_node_chunk);
       g_node_destroy (node);
     }
@@ -2004,7 +2029,8 @@ static const gchar *open_tag_format[] = 
   "%*s<popup name='%s' action=\"%s\">\n",
   "%*s<menuitem name=\"%s\" action=\"%s\"/>\n", 
   "%*s<toolitem name=\"%s\" action=\"%s\"/>\n", 
-  "%*s<separator/>\n",
+  "%*s<separator name=\"%s\"/>\n",
+  "%*s<accelerator name=\"%s\" action=\"%s\"/>\n",
 };
 
 static const gchar *close_tag_format[] = {
@@ -2016,6 +2042,7 @@ static const gchar *close_tag_format[] =
   "%*s</placeholder>\n",
   "%*s</placeholder>\n",
   "%*s</popup>\n",
+  "",
   "",
   "",
   "",
Index: gtkuimanager.h
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkuimanager.h,v
retrieving revision 1.10
diff -u -p -r1.10 gtkuimanager.h
--- gtkuimanager.h	4 Sep 2003 20:39:14 -0000	1.10
+++ gtkuimanager.h	8 Sep 2003 22:32:32 -0000
@@ -81,7 +81,8 @@ typedef enum {
   GTK_UI_MANAGER_POPUP,
   GTK_UI_MANAGER_MENUITEM,
   GTK_UI_MANAGER_TOOLITEM,
-  GTK_UI_MANAGER_SEPARATOR
+  GTK_UI_MANAGER_SEPARATOR,
+  GTK_UI_MANAGER_ACCELERATOR
 } GtkUIManagerItemType;
 
 GType          gtk_ui_manager_get_type            (void);