Re: Review of accelerator changes



OK, here is my patch that:

 * Allows unconnected accelerators in a GtkAccelGroup

 * Removes the gtk_accel_map_add/remove_notifier functionality

 * Adds a small bit of extra connectivity between GtkAccelGroup
   and GtkAccelMap with:

    - _gtk_accel_map_path_add/remove_group()
    - _gtk_accel_group_reconnect_path ()

 * Splits gtk_accel_group_connect() into gtk_accel_group_connect()
   and gtk_accel_group_connect_by_path()

 * Makes GtkWidget use connect_by_path() rather than notifiers.

The result is about 50 lines shorter than the original, which
is noticeable, if not a huge saving. But I think, more importantly,
the API becomes simpler, because the connection between GtkAccelMap
and GtkAccelGroup is automatic.

Some notes and questions:

 * The handling of coellescing duplicates in entry->groups
   in internal_change_entry() is a little expensive -- a
   faster way (but making the code less consistent/readable)
   would be to keep entry->groups in sorted order and just
   ignore runs of the same item.

 * _gtk_accel_group_reconnect() could probably be cleaned up some.
   The use of qsort() in it is admittedly lazy, but I think 
   probably not a bad code-size saving for an infrequent operation.
   I'm a little more concerned about the duplication of code
   between here and quick_remove() / quick_add(). Perhaps those
   functions could be just used with a bit of restructuring.

 * I think there would be a general cleanup if the code was
   standardized to:

    - Use char * for paths everywhere externally (add_entry(),
      connect_by_path())
    - Use quarks for paths everywhere internally.

 * Right now, the code ends up using g_quark_from_string("")
   for the detail quark if accel_key/accel_mods = 0. Using
   0 instead might be more elegant.

 * I'm wondering if it wouldn't be better to get rid of the
   "accel_activate" trick and simply invoke the closure
   directly; for one thing, we'd save some memory. Also,
   we currently don't automatically handle invalidated closures;
   shouldn't we do that?

 * Who takes care of calling gtk_accel_group_disconnect() for
   accelerators created with gtk_widget_add_accelerator()?

Let me know what you think. Does this approach work for you?

Regards,
                                        Owen

Index: gtk/gtkaccelgroup.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkaccelgroup.c,v
retrieving revision 1.31
diff -u -p -r1.31 gtkaccelgroup.c
--- gtk/gtkaccelgroup.c	2001/11/17 23:28:49	1.31
+++ gtk/gtkaccelgroup.c	2001/11/18 21:48:11
@@ -135,7 +135,19 @@ static void
 gtk_accel_group_finalize (GObject *object)
 {
   GtkAccelGroup *accel_group = GTK_ACCEL_GROUP (object);
+  gint i;
+  
+  for (i = 0; i < accel_group->n_accels; i++)
+    {
+      GtkAccelGroupEntry *entry = &accel_group->priv_accels[i];
 
+      if (entry->accel_path_quark)
+	{
+	  const gchar *accel_path = g_quark_to_string (entry[i].accel_path_quark);
+	  _gtk_accel_map_path_remove_group (accel_path, accel_group);
+	}
+    }
+
   g_free (accel_group->priv_accels);
 
   G_OBJECT_CLASS (parent_class)->finalize (object);
@@ -327,9 +339,15 @@ quick_accel_add (GtkAccelGroup  *accel_g
 		 GClosure       *closure,
 		 GQuark          path_quark)
 {
+  gchar *accel_name;
+  GQuark accel_quark;
   guint pos, i = accel_group->n_accels++;
   GtkAccelGroupEntry key;
 
+  accel_name = gtk_accelerator_name (accel_key, accel_mods);
+  accel_quark = g_quark_from_string (accel_name);
+  g_free (accel_name);
+
   key.key.accel_key = accel_key;
   key.key.accel_mods = accel_mods;
   for (pos = 0; pos < i; pos++)
@@ -347,6 +365,12 @@ quick_accel_add (GtkAccelGroup  *accel_g
 
   /* tag closure for backwards lookup */
   g_closure_add_invalidate_notifier (closure, accel_group, accel_tag_func);
+
+  /* setup handler */
+  g_signal_connect_closure_by_id (accel_group, signal_accel_activate, accel_quark, closure, FALSE);
+
+  /* and notify */
+  g_signal_emit (accel_group, signal_accel_changed, accel_quark, accel_key, accel_mods, closure);
 }
 
 static GtkAccelGroupEntry*
@@ -396,6 +420,12 @@ quick_accel_remove (GtkAccelGroup  *acce
     return NULL;
   for (i = 0; i < n; i++)
     {
+      if (entry[i].accel_path_quark)
+	{
+	  const gchar *accel_path = g_quark_to_string (entry[i].accel_path_quark);
+	  _gtk_accel_map_path_remove_group (accel_path, accel_group);
+	}
+      
       g_closure_remove_invalidate_notifier (entry[i].closure, accel_group, accel_tag_func);
       clist = g_slist_prepend (clist, entry[i].closure);
     }
@@ -414,44 +444,68 @@ quick_accel_remove (GtkAccelGroup  *acce
  * @accel_mods:       modifier combination of the accelerator
  * @accel_flags:      a flag mask to configure this accelerator
  * @closure:          closure to be executed upon accelerator activation
- * @accel_path_quark: accelerator path quark from GtkAccelMapNotify
  *
  * Install an accelerator in this group. When @accel_group is being activated
  * in response to a call to gtk_accel_groups_activate(), @closure will be
  * invoked if the @accel_key and @accel_mods from gtk_accel_groups_activate()
  * match those of this connection.
  * The signature used for the @closure is that of #GtkAccelGroupActivate.
- * If this connection is made in response to an accelerator path change (see
- * gtk_accel_map_change_entry()) from a #GtkAccelMapNotify notifier,
- * @accel_path_quark must be passed on from the notifier into this function,
- * it should be 0 otherwise.
  */
 void
 gtk_accel_group_connect (GtkAccelGroup	*accel_group,
 			 guint		 accel_key,
 			 GdkModifierType accel_mods,
 			 GtkAccelFlags	 accel_flags,
-			 GClosure	*closure,
-			 GQuark          accel_path_quark)
+			 GClosure	*closure)
 {
-  gchar *accel_name;
-  GQuark accel_quark;
-
   g_return_if_fail (GTK_IS_ACCEL_GROUP (accel_group));
   g_return_if_fail (closure != NULL);
   g_return_if_fail (accel_key > 0);
 
-  accel_name = gtk_accelerator_name (accel_key, accel_mods);
-  accel_quark = g_quark_from_string (accel_name);
-  g_free (accel_name);
+  quick_accel_add (accel_group, accel_key, accel_mods, accel_flags, closure, 0);
+}
 
-  quick_accel_add (accel_group, accel_key, accel_mods, accel_flags, closure, accel_path_quark);
+/**
+ * gtk_accel_group_connect_by_path
+ * @accel_group:      the ccelerator group to install an accelerator in
+ * @accel_path:       path used for determining key and modifiers.
+ * @accel_flags:      a flag mask to configure this accelerator
+ * @closure:          closure to be executed upon accelerator activation
+ *
+ * Install an accelerator in this group, using a accelerator path to look
+ * up the appropriate key and modifiers. (See gtk_accel_map_add_entry())
+ * When @accel_group is being activated in response to a call to
+ * gtk_accel_groups_activate(), @closure will be invoked if the @accel_key and
+ * @accel_mods from gtk_accel_groups_activate() match the key and modifiers
+ * for the path.
+ * The signature used for the @closure is that of #GtkAccelGroupActivate.
+ */
+void
+gtk_accel_group_connect_by_path (GtkAccelGroup	*accel_group,
+				 const gchar    *accel_path,
+				 GtkAccelFlags	 accel_flags,
+				 GClosure	*closure)
+{
+  GQuark accel_path_quark;
+  guint accel_key = 0;
+  GdkModifierType accel_mods = 0;
+  GtkAccelKey key;
 
-  /* setup handler */
-  g_signal_connect_closure_by_id (accel_group, signal_accel_activate, accel_quark, closure, FALSE);
+  g_return_if_fail (GTK_IS_ACCEL_GROUP (accel_group));
+  g_return_if_fail (closure != NULL);
+  g_return_if_fail (accel_path != NULL);
 
-  /* and notify */
-  g_signal_emit (accel_group, signal_accel_changed, accel_quark, accel_key, accel_mods, closure);
+  accel_path_quark = g_quark_from_string (accel_path);
+
+  if (gtk_accel_map_lookup_entry (accel_path, &key))
+    {
+      accel_key = key.accel_key;
+      accel_mods = key.accel_mods;
+    }
+
+  _gtk_accel_map_path_add_group (accel_path, accel_group);
+  
+  quick_accel_add (accel_group, accel_key, accel_mods, accel_flags, closure, accel_path_quark);
 }
 
 static gboolean
@@ -1033,4 +1087,70 @@ guint
 gtk_accelerator_get_default_mod_mask (void)
 {
   return default_accel_mod_mask;
+}
+
+void
+_gtk_accel_group_reconnect_path (GtkAccelGroup  *accel_group,
+				 const gchar    *accel_path,
+				 guint           accel_key,
+				 GdkModifierType accel_mods)
+{
+  GQuark path_quark = g_quark_from_string (accel_path);
+  gchar *accel_name;
+  GQuark accel_quark;
+  guint old_accel_key = 0;	      /* Quiet GCC */
+  GdkModifierType old_accel_mods = 0; /*      "    */
+  GQuark old_accel_quark;
+  GSList *clist , *slist;
+  gint i;
+
+  accel_name = gtk_accelerator_name (accel_key, accel_mods);
+  accel_quark = g_quark_from_string (accel_name);
+  g_free (accel_name);
+
+  /* Change all instances of the path */
+  clist = NULL;
+  for (i = 0; i < accel_group->n_accels; i++)
+    {
+      GtkAccelGroupEntry *entry = &accel_group->priv_accels[i];
+      
+      if (entry->accel_path_quark == path_quark)
+	{
+	  old_accel_key = entry->key.accel_key;
+	  old_accel_mods = entry->key.accel_mods;
+      
+	  clist = g_slist_prepend (clist, g_closure_ref (entry->closure));
+
+	  g_signal_handlers_disconnect_matched (accel_group, G_SIGNAL_MATCH_CLOSURE | G_SIGNAL_MATCH_ID,
+						signal_accel_activate, 0,
+						entry->closure, NULL, NULL);
+	  
+	  entry->key.accel_key = accel_key;
+	  entry->key.accel_mods = accel_mods;
+	  
+	  g_signal_connect_closure_by_id (accel_group, signal_accel_activate, accel_quark, entry->closure, FALSE);
+	}
+    }
+
+  qsort (accel_group->priv_accels, accel_group->n_accels, sizeof (GtkAccelGroupEntry), bsearch_compare_accels);
+  
+  /* Now notify on each one */
+  if (clist)
+    {
+      accel_name = gtk_accelerator_name (old_accel_key, old_accel_mods);
+      old_accel_quark = g_quark_from_string (accel_name);
+      g_free (accel_name);
+
+      for (slist = clist; slist; slist = slist->next)
+	{
+	  GClosure *closure = slist->data;
+
+	  g_signal_emit (accel_group, signal_accel_changed, old_accel_quark, old_accel_key, old_accel_mods, closure);
+	  g_signal_emit (accel_group, signal_accel_changed, accel_quark, accel_key, accel_mods, closure);
+
+	  g_closure_unref (closure);
+	}
+
+      g_slist_free (clist);
+    }
 }
Index: gtk/gtkaccelgroup.h
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkaccelgroup.h,v
retrieving revision 1.11
diff -u -p -r1.11 gtkaccelgroup.h
--- gtk/gtkaccelgroup.h	2001/11/13 00:53:36	1.11
+++ gtk/gtkaccelgroup.h	2001/11/18 21:48:11
@@ -96,8 +96,11 @@ void		gtk_accel_group_connect		(GtkAccel
 						 guint		 accel_key,
 						 GdkModifierType accel_mods,
 						 GtkAccelFlags	 accel_flags,
-						 GClosure	*closure,
-						 GQuark		 accel_path_quark);
+						 GClosure	*closure);
+void            gtk_accel_group_connect_by_path (GtkAccelGroup	*accel_group,
+						 const gchar    *accel_path,
+						 GtkAccelFlags	 accel_flags,
+						 GClosure	*closure);
 gboolean	gtk_accel_group_disconnect	(GtkAccelGroup	*accel_group,
 						 guint		 accel_key,
 						 GdkModifierType accel_mods);
@@ -138,6 +141,12 @@ GtkAccelGroupEntry*	gtk_accel_group_quer
 						 guint		 accel_key,
 						 GdkModifierType accel_mods,
 						 guint          *n_entries);
+
+void _gtk_accel_group_reconnect_path (GtkAccelGroup  *accel_group,
+				      const gchar    *accel_path,
+				      guint           accel_key,
+				      GdkModifierType accel_mods);
+
 struct _GtkAccelGroupEntry
 {
   GtkAccelKey  key;
Index: gtk/gtkaccelmap.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkaccelmap.c,v
retrieving revision 1.1
diff -u -p -r1.1 gtkaccelmap.c
--- gtk/gtkaccelmap.c	2001/11/13 00:53:36	1.1
+++ gtk/gtkaccelmap.c	2001/11/18 21:48:11
@@ -34,7 +34,7 @@ typedef struct {
   guint	       std_accel_key;
   guint	       std_accel_mods;
   guint        changed : 1;
-  GHookList   *hooks;
+  GSList      *groups;
 } AccelEntry;
 
 
@@ -154,124 +154,6 @@ gtk_accel_map_add_entry (const gchar *ac
   return g_quark_try_string (entry->accel_path);
 }
 
-typedef struct {
-  GHook          hook;
-  GtkAccelGroup *accel_group;
-} AccelHook;
-
-static void
-accel_hook_finalize (GHookList *hook_list,
-		     GHook     *hook)
-{
-  GDestroyNotify destroy = hook->destroy;
-  AccelHook *ahook = (AccelHook*) hook;
-
-  if (ahook->accel_group)
-    g_object_unref (ahook->accel_group);
-
-  if (destroy)
-    {
-      hook->destroy = NULL;
-      destroy (hook->data);
-    }
-}
-
-/**
- * GtkAccelMapNotify
- * @data:             notifier user data
- * @accel_path_quark: accelerator path (as #GQuark) which has just changed
- * @accel_key:        new accelerator key
- * @accel_mods:       new accelerator modifiers
- * @accel_group:      accelerator group of this notifier
- * @old_accel_key:    former accelerator key
- * @old_accel_mods):  former accelerator modifiers
- *
- * #GtkAccelMapNotify is the signature of user callbacks, installed via
- * gtk_accel_map_add_notifier(). Once the accel path of the notifier changes,
- * the notifier is invoked with this signature, where @accel_path_quark
- * indicates the accel path that changed, and @data and @accel_group are
- * the notifier's arguments as passed into gtk_accel_map_add_notifier().
- */
-
-/**
- * gtk_accel_map_add_notifer
- * @accel_path:  valid accelerator path
- * @notify_data: data argument to the notifier
- * @notify_func: the notifier function
- * @accel_group: accelerator group used by the notifier function
- *
- * Install a notifier function to be called if an accelerator
- * map entry changes. @accel_group has to be the accel group
- * that is being affected (gets an accelerator removed or added,
- * when the notifier function is executed).
- */
-void
-gtk_accel_map_add_notifer (const gchar      *accel_path,
-			   gpointer          notify_data,
-			   GtkAccelMapNotify notify_func,
-			   GtkAccelGroup    *accel_group)
-{
-  AccelEntry *entry;
-  AccelHook *ahook;
-  GHook *hook;
-
-  g_return_if_fail (gtk_accel_path_is_valid (accel_path));
-  g_return_if_fail (notify_func != NULL);
-  if (accel_group)
-    g_return_if_fail (GTK_IS_ACCEL_GROUP (accel_group));
-
-  entry = accel_path_lookup (accel_path);
-  if (!entry)
-    {
-      gtk_accel_map_add_entry (accel_path, 0, 0);
-      entry = accel_path_lookup (accel_path);
-    }
-  if (!entry->hooks)
-    {
-      entry->hooks = g_new (GHookList, 1);
-      g_hook_list_init (entry->hooks, sizeof (AccelHook));
-      entry->hooks->finalize_hook = accel_hook_finalize;
-    }
-  hook = g_hook_alloc (entry->hooks);
-  hook->data = notify_data;
-  hook->func = notify_func;
-  hook->destroy = NULL;
-  ahook = (AccelHook*) hook;
-  ahook->accel_group = accel_group ? g_object_ref (accel_group) : NULL;
-  g_hook_append (entry->hooks, hook);
-}
-
-/**
- * gtk_accel_map_remove_notifer
- * @accel_path:  valid accelerator path
- * @notify_data: data argument to the notifier
- * @notify_func: the notifier function
- *
- * Remove a notifier function, previously installed through
- * gtk_accel_map_add_notifer().
- */
-void
-gtk_accel_map_remove_notifer (const gchar      *accel_path,
-			      gpointer          notify_data,
-			      GtkAccelMapNotify notify_func)
-{
-  AccelEntry *entry;
-
-  g_return_if_fail (gtk_accel_path_is_valid (accel_path));
-  g_return_if_fail (notify_func != NULL);
-
-  entry = accel_path_lookup (accel_path);
-  if (entry && entry->hooks)
-    {
-      GHook *hook = g_hook_find_func_data (entry->hooks, TRUE, notify_func, notify_data);
-
-      if (hook && g_hook_destroy (entry->hooks, hook->hook_id))
-	return; /* successfully removed */
-    }
-  g_warning (G_STRLOC ": no notifier %p(%p) installed for accel path \"%s\"",
-	     notify_func, notify_data, accel_path);
-}
-
 /**
  * gtk_accel_map_lookup_entry
  * @accel_path:  valid accelerator path
@@ -332,9 +214,8 @@ internal_change_entry (const gchar    *a
 {
   GSList *node, *slist, *win_list, *group_list, *replace_list = NULL;
   GHashTable *group_hm, *win_hm;
-  gboolean change_accel, removable, can_change = TRUE, seen_accel = FALSE, hooks_may_recurse = TRUE;
+  gboolean change_accel, removable, can_change = TRUE, seen_accel = FALSE;
   GQuark entry_quark;
-  GHook *hook;
   AccelEntry *entry = accel_path_lookup (accel_path);
 
   /* not much todo if there's no entry yet */
@@ -356,7 +237,7 @@ internal_change_entry (const gchar    *a
     return FALSE;
 
   /* nobody's interested, easy going */
-  if (!entry->hooks)
+  if (!entry->groups)
     {
       if (!simulate)
 	{
@@ -371,16 +252,10 @@ internal_change_entry (const gchar    *a
   entry_quark = g_quark_try_string (entry->accel_path);
   group_hm = g_hash_table_new (NULL, NULL);
   win_hm = g_hash_table_new (NULL, NULL);
-  hook = g_hook_first_valid (entry->hooks, hooks_may_recurse);
-  while (hook)
-    {
-      AccelHook *ahook = (AccelHook*) hook;
-      
-      if (ahook->accel_group)
-	g_hash_table_insert (group_hm, ahook->accel_group, ahook->accel_group);
-      hook = g_hook_next_valid (entry->hooks, hook, hooks_may_recurse);
-    }
 
+  for (slist = entry->groups; slist; slist = slist->next)
+    g_hash_table_insert (group_hm, slist->data, slist->data);
+
   /* 2) collect acceleratables affected */
   group_list = g_hash_table_slist_values (group_hm);
   for (slist = group_list; slist; slist = slist->next)
@@ -467,25 +342,9 @@ internal_change_entry (const gchar    *a
       entry->accel_key = accel_key;
       entry->accel_mods = accel_mods;
       entry->changed = TRUE;
-      hook = g_hook_first_valid (entry->hooks, hooks_may_recurse);
-      while (hook)
-	{
-	  gboolean was_in_call, need_destroy = FALSE;
-	  GtkAccelMapNotify hook_func = hook->func;
-	  AccelHook *ahook = (AccelHook*) hook;
-	  
-	  was_in_call = G_HOOK_IN_CALL (hook);
-	  hook->flags |= G_HOOK_FLAG_IN_CALL;
-	  /* need_destroy = */ hook_func (hook->data, g_quark_try_string (entry->accel_path),
-					  entry->accel_key, entry->accel_mods,
-					  ahook->accel_group,
-					  old_accel_key, old_accel_mods);
-	  if (!was_in_call)
-	    hook->flags &= ~G_HOOK_FLAG_IN_CALL;
-	  if (need_destroy)
-	    g_hook_destroy_link (entry->hooks, hook);
-	  hook = g_hook_next_valid (entry->hooks, hook, hooks_may_recurse);
-	}
+      for (slist = group_list; slist; slist = slist->next)
+	_gtk_accel_group_reconnect_path (slist->data, entry->accel_path,
+					 entry->accel_key, entry->accel_mods);
     }
   g_slist_free (replace_list);
   g_slist_free (group_list);
@@ -505,12 +364,9 @@ internal_change_entry (const gchar    *a
  * Change the @accel_key and @accel_mods currently associated with @accel_path.
  * Due to conflicts with other accelerators, a change may not alwys be possible,
  * @replace indicates whether other accelerators may be deleted to resolve such
- * conflicts. A change will only occour if all conflicts could be resolved (which
- * might not be the case if conflicting accelerators are locked). Succesfull
+ * conflicts. A change will only occur if all conflicts could be resolved (which
+ * might not be the case if conflicting accelerators are locked). Succesful
  * changes are indicated by a %TRUE return value.
- * Changes occouring are also indicated by invocation of notifiers attached to
- * @accel_path (see gtk_accel_map_add_notifer() on this) and other accelerators
- * that are being deleted.
  */
 gboolean
 gtk_accel_map_change_entry (const gchar    *accel_path,
@@ -855,4 +711,35 @@ gtk_accel_map_add_filter (const gchar *f
 	return;
       }
   accel_filters = g_slist_prepend (accel_filters, pspec);
+}
+
+void
+_gtk_accel_map_path_add_group (const gchar   *accel_path,
+			       GtkAccelGroup *accel_group)
+{
+  AccelEntry *entry;
+
+  g_return_if_fail (gtk_accel_path_is_valid (accel_path));
+  g_return_if_fail (GTK_IS_ACCEL_GROUP (accel_group));
+
+  entry = accel_path_lookup (accel_path);
+  if (!entry)
+    {
+      gtk_accel_map_add_entry (accel_path, 0, 0);
+      entry = accel_path_lookup (accel_path);
+    }
+  entry->groups = g_slist_prepend (entry->groups, accel_group);
+}
+
+void
+_gtk_accel_map_path_remove_group (const gchar   *accel_path,
+				  GtkAccelGroup *accel_group)
+{
+  AccelEntry *entry;
+
+  entry = accel_path_lookup (accel_path);
+  g_return_if_fail (entry != NULL);
+  g_return_if_fail (g_slist_find (entry->groups, accel_group));
+
+  entry->groups = g_slist_remove (entry->groups, accel_group);
 }
Index: gtk/gtkaccelmap.h
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkaccelmap.h,v
retrieving revision 1.1
diff -u -p -r1.1 gtkaccelmap.h
--- gtk/gtkaccelmap.h	2001/11/13 00:53:36	1.1
+++ gtk/gtkaccelmap.h	2001/11/18 21:48:11
@@ -26,13 +26,6 @@ G_BEGIN_DECLS
 
 
 /* --- notifier --- */
-typedef void (*GtkAccelMapNotify)		(gpointer	 data,
-						 GQuark          accel_path_quark,
-						 guint           accel_key,
-						 guint           accel_mods,
-						 GtkAccelGroup  *accel_group,
-						 guint           old_accel_key,
-						 guint           old_accel_mods);
 typedef void (*GtkAccelMapForeach)		(gpointer	 data,
 						 const gchar	*accel_path,
 						 guint           accel_key,
@@ -44,13 +37,6 @@ typedef void (*GtkAccelMapForeach)		(gpo
 GQuark	   gtk_accel_map_add_entry	(const gchar		*accel_path,
 					 guint			 accel_key,
 					 guint			 accel_mods);
-void	   gtk_accel_map_add_notifer	(const gchar		*accel_path,
-					 gpointer		 notify_data,
-					 GtkAccelMapNotify	 notify_func,
-					 GtkAccelGroup		*accel_group);
-void	   gtk_accel_map_remove_notifer	(const gchar		*accel_path,
-					 gpointer		 notify_data,
-					 GtkAccelMapNotify	 notify_func);
 GQuark     gtk_accel_map_lookup_entry	(const gchar		*accel_path,
 					 GtkAccelKey		*key);
 gboolean   gtk_accel_map_change_entry	(const gchar		*accel_path,
@@ -75,6 +61,10 @@ void	gtk_accel_map_foreach_unfilterd	(gp
 /* --- internal API --- */
 void		_gtk_accel_map_init		(void);
 
+void            _gtk_accel_map_path_add_group    (const gchar   *accel_path,
+						  GtkAccelGroup *accel_group);
+void            _gtk_accel_map_path_remove_group (const gchar   *accel_path,
+						  GtkAccelGroup *accel_group);
 
 G_END_DECLS
 
Index: gtk/gtkitemfactory.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkitemfactory.c,v
retrieving revision 1.51
diff -u -p -r1.51 gtkitemfactory.c
--- gtk/gtkitemfactory.c	2001/11/13 00:53:36	1.51
+++ gtk/gtkitemfactory.c	2001/11/18 21:48:11
@@ -293,7 +293,7 @@ gtk_item_factory_add_foreign (GtkWidget 
    */
   if (gtk_signal_lookup ("activate", GTK_OBJECT_TYPE (accel_widget)))
     {
-      if (accel_key && accel_group)
+      if (accel_group)
 	{
 	  gtk_accel_map_add_entry (full_path, accel_key, accel_mods);
 	  _gtk_widget_set_accel_path (accel_widget, full_path, accel_group);
Index: gtk/gtkwidget.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkwidget.c,v
retrieving revision 1.266
diff -u -p -r1.266 gtkwidget.c
--- gtk/gtkwidget.c	2001/11/17 23:28:51	1.266
+++ gtk/gtkwidget.c	2001/11/18 21:48:11
@@ -2622,8 +2622,7 @@ gtk_widget_add_accelerator (GtkWidget   
 			   accel_key,
 			   accel_mods,
 			   accel_flags | GTK_ACCEL_LOCKED,
-			   closure,
-			   0);
+			   closure);
 
   g_signal_emit (widget, widget_signals[ACCEL_CLOSURES_CHANGED], 0);
 }
@@ -2688,57 +2687,14 @@ typedef struct {
 } AccelPath;
 
 static void
-accel_path_changed (gpointer        data,
-		    GQuark	    accel_path_quark,
-		    guint           accel_key,
-		    guint           accel_mods,
-		    GtkAccelGroup  *accel_group,
-		    guint           old_accel_key,
-		    guint           old_accel_mods)
-{
-  AccelPath *apath = data;
-  gboolean notify = FALSE;
-  
-  if (apath->closure)
-    {
-      /* the closure might have been removed already (due to replacements) */
-      gtk_accel_groups_disconnect_closure (apath->closure);
-      g_closure_unref (apath->closure);
-      apath->closure = NULL;
-      notify = TRUE;
-    }
-  if (accel_key)
-    {
-      apath->closure = widget_new_accel_closure (apath->widget, GTK_WIDGET_GET_CLASS (apath->widget)->activate_signal);
-      g_closure_ref (apath->closure);
-      /* need to specify path to get an unlocked accelerator */
-      gtk_accel_group_connect (apath->accel_group,
-			       accel_key,
-			       accel_mods,
-			       GTK_ACCEL_VISIBLE,
-			       apath->closure,
-			       accel_path_quark);
-      notify = TRUE;
-    }
-  if (notify)
-    g_signal_emit (apath->widget, widget_signals[ACCEL_CLOSURES_CHANGED], 0);
-}
-
-static void
 destroy_accel_path (gpointer data)
 {
   AccelPath *apath = data;
-
-  /* stop notification */
-  gtk_accel_map_remove_notifer (apath->path, apath, accel_path_changed);
 
-  /* if the closure is currently connected, get rid of that connection */
-  if (apath->closure)
-    {
-      gtk_accel_groups_disconnect_closure (apath->closure);
-      g_closure_unref (apath->closure);
-    }
+  gtk_accel_groups_disconnect_closure (apath->closure);
+  /* closures_destroy takes care of unrefing the closure */
   g_object_unref (apath->accel_group);
+  
   g_free (apath);
 }
 
@@ -2753,7 +2709,6 @@ _gtk_widget_set_accel_path (GtkWidget   
 			    GtkAccelGroup *accel_group)
 {
   AccelPath *apath;
-  GtkAccelKey key;
 
   g_return_if_fail (GTK_IS_WIDGET (widget));
   g_return_if_fail (GTK_WIDGET_GET_CLASS (widget)->activate_signal != 0);
@@ -2771,7 +2726,7 @@ _gtk_widget_set_accel_path (GtkWidget   
       apath->widget = widget;
       apath->accel_group = g_object_ref (accel_group);
       apath->path = g_quark_to_string (quark_path);
-      apath->closure = NULL;
+      apath->closure = widget_new_accel_closure (apath->widget, GTK_WIDGET_GET_CLASS (apath->widget)->activate_signal);
     }
   else
     apath = NULL;
@@ -2780,14 +2735,9 @@ _gtk_widget_set_accel_path (GtkWidget   
   g_object_set_qdata_full (G_OBJECT (widget), quark_accel_path, apath, destroy_accel_path);
 
   if (apath)
-    {
-      /* setup accel path hooks to react to changes */
-      gtk_accel_map_add_notifer (apath->path, apath, accel_path_changed, apath->accel_group);
+    gtk_accel_group_connect_by_path (apath->accel_group, apath->path, GTK_ACCEL_VISIBLE, apath->closure);
 
-      /* install accelerators for this path */
-      if (gtk_accel_map_lookup_entry (apath->path, &key))
-	accel_path_changed (apath, g_quark_try_string (apath->path), key.accel_key, key.accel_mods, NULL, 0, 0);
-    }
+  g_signal_emit (widget, widget_signals[ACCEL_CLOSURES_CHANGED], 0);
 }
 
 const gchar*


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]