Install accelerators on actions, not on proxies, support accelerator-only

2003-09-18  Matthias Clasen  <maclas@gmx.de>

	Install accelerators on actions, not on proxies, support
	accelerator-only actions:

	* gtk/gtkmenu.c (get_accel_path): New function to get the accel path
	and its lock status either via _gtk_widget_get_accel_path() or by
	looking at the accel_path stored in the menu item itself and determining
	its lock status by peeking into the contained accel label. This was
	already (accidentally) committed a week ago.

	* gtk/gtkaction.h (gtk_action_set_accel_group):
	(gtk_action_[dis]connect_accelerator): New functions.

	* gtk/gtkaction.c (struct _GtkActionPrivate): Add accel_group,
	accel_closure and accel_count. We must have a reference to the accel_group,
	since we need it in connect_proxy. The count is necessary to ensure
	that the accelerator isn't removed before the last proxy requesting
	it has been unmerged.
	(connect_proxy): Connect the accelerator to the
	action now, only set the accel_path on the menuitem.
	(remove_proxy): Disconnect the accelerator from the action, not from
	the menuitem.
	(gtk_action_set_accel_group): Set the accel group.
	(gtk_action_[dis]connect_accelerator): Count the number of times
	this functions have been called and install/remove the accelerator if
	the count leaves/reaches zero.

	* gtk/gtkuimanager.h (GtkUIManagerItemType): Add
	GTK_UI_MANAGER_ACCELERATOR.

	* gtk/gtkuimanager.c (NodeType): Add NODE_TYPE_ACCELERATOR.
	(start_element_handler): Create NODE_TYPE_ACCELERATOR nodes from
	<accelerator> elements.
	(gtk_ui_manager_add_ui): Create NODE_TYPE_ACCELERATOR nodes when
	type is GTK_UI_MANAGER_ACCELERATOR.
	(update_node): Set the accel group on actions before creating their
	proxies. Don't set the accel group on created menus. For
	NODE_TYPE_ACCELERATOR nodes, [dis]connect the actions' accelerator.
	(print_node): Also emit <accelerator> elements.

	* tests/testmerge.c (dump_accels): Add a "Dump Accels" button.

4 files changed, 218 insertions, 56 deletions

diff --git a/gtk/gtkaction.c b/gtk/gtkaction.c
index df4301450b..e4d96d88a6 100644
--- a/gtk/gtkaction.c
+++ b/gtk/gtkaction.c
@@ -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 (GObject         *object,
 
 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 (GtkToolItem *tool_item,
 }
 
 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 */
@@ -560,16 +590,20 @@ connect_proxy (GtkAction *action,
 	  gtk_container_remove (GTK_CONTAINER (proxy), label);
 	  label = NULL;
 	}
+
       if (!label)
-	{
-	  label = g_object_new (GTK_TYPE_ACCEL_LABEL,
-				"use_underline", TRUE,
-				"xalign", 0.0,
-				"visible", TRUE,
-				"parent", proxy,
-				"accel_widget", proxy,
-				NULL);
-	}
+	label = g_object_new (GTK_TYPE_ACCEL_LABEL,
+			      "use_underline", TRUE,
+			      "xalign", 0.0,
+			      "visible", TRUE,
+			      "parent", proxy,
+			      NULL);
+      
+      if (GTK_IS_ACCEL_LABEL (label) && action->private_data->accel_quark)
+	g_object_set (G_OBJECT (label),
+		      "accel_closure", action->private_data->accel_closure,
+		      NULL);
+
       gtk_label_set_label (GTK_LABEL (label), action->private_data->label);
       g_signal_connect_object (action, "notify::label",
 			       G_CALLBACK (gtk_action_sync_label), proxy, 0);
@@ -599,15 +633,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 +952,21 @@ gtk_action_unblock_activate_from (GtkAction *action,
 				     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 +982,93 @@ 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);
 }
+
+/**
+ * gtk_action_set_accel_group:
+ * @action: the action object
+ * @accel_group: a #GtkAccelGroup or %NULL
+ * 
+ * Sets the #GtkAccelGroup in which the accelerator for this action
+ * will be installed.
+ *
+ * Since: 2.4
+ **/
+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;
+}
+
+/**
+ * gtk_action_connect_accelerator:
+ * @action: a #GtkAction
+ * 
+ * Installs the accelerator for @action if @action has an
+ * accel path and group. See gtk_action_set_accel_path() and 
+ * gtk_action_set_accel_group()
+ *
+ * Since multiple proxies may independently trigger the installation
+ * of the accelerator, the @action counts the number of times this
+ * function has been called and doesn't remove the accelerator until
+ * gtk_action_disconnect_accelerator() has been called as many times.
+ *
+ * Since: 2.4
+ **/
+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++;
+}
+
+/**
+ * gtk_action_disconnect_accelerator:
+ * @action: a #GtkAction
+ * 
+ * Undoes the effect of one call to gtk_action_connect_accelerator().
+ *
+ * Since: 2.4
+ **/
+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);
+}
diff --git a/gtk/gtkaction.h b/gtk/gtkaction.h
index 5e6c05bc49..8832298360 100644
--- a/gtk/gtkaction.h
+++ b/gtk/gtkaction.h
@@ -78,31 +78,32 @@ struct _GtkActionClass
   void (*_gtk_reserved4) (void);
 };
 
-GType        gtk_action_get_type              (void);
-const gchar* gtk_action_get_name              (GtkAction   *action);
-void         gtk_action_activate              (GtkAction   *action);
-GtkWidget *  gtk_action_create_icon           (GtkAction   *action,
-					       GtkIconSize  icon_size);
-GtkWidget *  gtk_action_create_menu_item      (GtkAction   *action);
-GtkWidget *  gtk_action_create_tool_item      (GtkAction   *action);
-void         gtk_action_connect_proxy         (GtkAction   *action,
-					       GtkWidget   *proxy);
-void         gtk_action_disconnect_proxy      (GtkAction   *action,
-					       GtkWidget   *proxy);
-GSList    *  gtk_action_get_proxies           (GtkAction   *action);
-
+GType        gtk_action_get_type               (void);
+const gchar* gtk_action_get_name               (GtkAction     *action);
+void         gtk_action_activate               (GtkAction     *action);
+GtkWidget*   gtk_action_create_icon            (GtkAction     *action,
+						GtkIconSize    icon_size);
+GtkWidget*   gtk_action_create_menu_item       (GtkAction     *action);
+GtkWidget*   gtk_action_create_tool_item       (GtkAction     *action);
+void         gtk_action_connect_proxy          (GtkAction     *action,
+						GtkWidget     *proxy);
+void         gtk_action_disconnect_proxy       (GtkAction     *action,
+						GtkWidget     *proxy);
+GSList*      gtk_action_get_proxies            (GtkAction     *action);
+void         gtk_action_connect_accelerator    (GtkAction     *action);
+void         gtk_action_disconnect_accelerator (GtkAction     *action);
 
 /* protected ... for use by child actions */
-void         gtk_action_block_activate_from   (GtkAction   *action,
-					       GtkWidget   *proxy);
-void         gtk_action_unblock_activate_from (GtkAction   *action,
-					       GtkWidget   *proxy);
-
+void         gtk_action_block_activate_from    (GtkAction     *action,
+						GtkWidget     *proxy);
+void         gtk_action_unblock_activate_from  (GtkAction     *action,
+						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);
 
 
 #endif  /* __GTK_ACTION_H__ */
diff --git a/gtk/gtkuimanager.c b/gtk/gtkuimanager.c
index 838a896e2a..203604712f 100644
--- a/gtk/gtkuimanager.c
+++ b/gtk/gtkuimanager.c
@@ -56,6 +56,7 @@ typedef enum
   NODE_TYPE_MENUITEM,
   NODE_TYPE_TOOLITEM,
   NODE_TYPE_SEPARATOR,
+  NODE_TYPE_ACCELERATOR
 } NodeType;
 
 
@@ -861,6 +862,24 @@ start_element_handler (GMarkupParseContext *context,
 
   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"))
 	{
@@ -1131,7 +1150,6 @@ static GMarkupParser ui_parser = {
   cleanup
 };
 
-
 static guint
 add_ui_from_string (GtkUIManager *self,
 		    const gchar  *buffer, 
@@ -1205,8 +1223,8 @@ gtk_ui_manager_add_ui_from_string (GtkUIManager *self,
   const gchar *p;
   const gchar *end;
 
-  g_return_val_if_fail (GTK_IS_UI_MANAGER (self), FALSE);
-  g_return_val_if_fail (buffer != NULL, FALSE);
+  g_return_val_if_fail (GTK_IS_UI_MANAGER (self), 0);
+  g_return_val_if_fail (buffer != NULL, 0);
 
   if (length < 0)
     length = strlen (buffer);
@@ -1246,6 +1264,8 @@ gtk_ui_manager_add_ui_from_file (GtkUIManager *self,
   gint length;
   guint res;
 
+  g_return_val_if_fail (GTK_IS_UI_MANAGER (self), 0);
+
   if (!g_file_get_contents (filename, &buffer, &length, error))
     return 0;
 
@@ -1365,6 +1385,9 @@ gtk_ui_manager_add_ui (GtkUIManager        *self,
 	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 */
 	}
@@ -1734,6 +1757,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.
        */
@@ -1765,11 +1791,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:
 	  {
@@ -1806,7 +1828,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);
 		  }
 	      }
@@ -2045,6 +2066,9 @@ update_node (GtkUIManager *self,
 		}
 	    }
 	  break;
+	case NODE_TYPE_ACCELERATOR:
+	  gtk_action_connect_accelerator (action);
+	  break;
 	}
 
       if (action)
@@ -2065,10 +2089,10 @@ update_node (GtkUIManager *self,
       child = current->next;
       update_node (self, current, add_tearoffs && (info->type != NODE_TYPE_POPUP));
     }
-  
-  if (info->proxy)
+
+  if (info->proxy) 
     {
-      if (info->type == NODE_TYPE_MENU)
+      if (info->type == NODE_TYPE_MENU) 
 	update_smart_separators (gtk_menu_item_get_submenu (GTK_MENU_ITEM (info->proxy)));
       else if (info->type == NODE_TYPE_TOOLBAR)
 	update_smart_separators (info->proxy);
@@ -2081,6 +2105,8 @@ update_node (GtkUIManager *self,
 	gtk_widget_destroy (info->proxy);
       if (info->extra)
 	gtk_widget_destroy (info->extra);
+      if (info->type == NODE_TYPE_ACCELERATOR)
+	gtk_action_disconnect_accelerator (info->action);
       free_node (node);
       g_node_destroy (node);
     }
@@ -2181,7 +2207,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[] = {
@@ -2196,6 +2223,7 @@ static const gchar *close_tag_format[] = {
   "",
   "",
   "",
+  "",
 };
 
 static void
diff --git a/gtk/gtkuimanager.h b/gtk/gtkuimanager.h
index 844f4f0510..1694159414 100644
--- a/gtk/gtkuimanager.h
+++ b/gtk/gtkuimanager.h
@@ -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);