Re: 6 API-ish issues

[Owen Taylor <otaylor redhat com>]

Tim Janik <timj gtk org> writes:

> On Mon, 21 Jan 2002, Owen Taylor wrote:
> 
> >  * 66590 guint should be flag type
> > 
> >    This is probably right; we do _normally_ use GtkModifier type 
> >    for modifier flags field. (In gtk/, about 15 public uses of 
> >    GtkModifierType vs. 7 of uint.) But this breaks source compatibility 
> >    with C++, and I don't think there is a big benefit for fixing 
> >    it unless we fix all uses of uint for modifiers. (Small benefits
> >    are consistency in GtkAccelMap and not introducing new functions
> >    with things broken.)
> > 
> >      http://bugzilla.gnome.org/show_bug.cgi?id=66590
> 
> the args currently being uint instead of GdkModifierType should really be
> considered bugs, due to my personal dumbness/lazynees and should be fixed
> ASAP. doesn't seem its a real issue for the c++ team.

Well, my concern wasn't really gtk-- (which wraps GTK+, so can deal
with this pretty easily), as much as people using GTK+ from C++ directly;
(Abiword, Mozilla,etc.).

Still, it's pretty easy to fix up (add a cast), and I doubt too many of
these people have started to port yet, so I've gone ahead and made the 
cleanup.
 
> >  * 68527 gtk_item_factory_path_from_widget should be G_CONST_RETURN
> > 
> >    Certainly right, but would break source compatibility a bit.
> > 
> >      http://bugzilla.gnome.org/show_bug.cgi?id=68527
> 
> breaking the source compatibility in terms of issuing a warning, i think
> we should definitely make this change.

OK, I've made this change.
 
> >  * 69242 _gtk_widget_set_accel_path() should be exported
> > 
> >    The reason for doing this is support for accelerators on buttons; the
> >    reason for supporting accelerators (not mnemonics) on buttons is unclear 
> >    to me, but Tim thinks it makes sense as part of an action based
> >    menu/toolbar API. (If it is missing, we could certainly work around
> >    the problem.)
> 
> i don't quite see how you want to work around this.
> ideally we'll be able to stuff a new action-based menu system into 2.2,
> something like jamesh has started out on with gmenu already, and that
> should be possible to test/develop based on the 2.0 API already. what's
> missing here is support for accelerator paths on non menu item widgets,
> e.g. to support accels on toolbar buttons.

Well, you didn't convince me that proxying was the best way to do action
based menu systems, but anyways, I've added it as it seemed the 
simplest way forward.

> as a result, if gtk_widget_set_accel_path() is exported, it should be
> accompanied by the follwing doc changes:
> - gtk_widget_set_accel_path(): mention that gtk_menu_item_set_accel_path() is
>   easier to use/preferred for menu items.
> - gtk_menu_item_set_accel_path(): point out that this is a convenience front
>   end to gtk_widget_set_accel_path() which updates the accel group from the
>   parent menu automatically
> - gtk_widget_add_accelerator(): discourage use of this function in favour of
>   using gtk_accel_map_add_entry() and gtk_widget_set_accel_path().

Index: gtkmenuitem.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkmenuitem.c,v
retrieving revision 1.70
diff -u -p -r1.70 gtkmenuitem.c
--- gtkmenuitem.c	2002/01/30 01:02:43	1.70
+++ gtkmenuitem.c	2002/01/30 03:26:05
@@ -1092,6 +1092,10 @@ _gtk_menu_item_refresh_accel_path (GtkMe
  * gtk_accel_map_add_entry() with the same @accel_path.
  * See also gtk_accel_map_add_entry() on the specifics of accelerator paths,
  * and gtk_menu_set_accel_path() for a more convenient variant of this function.
+ *
+ * This function is basically a convenience wrapper that handles calling
+ * gtk_widget_set_accel_path() with the appropriate accelerator group for
+ * the menu item.
  */
 void
 gtk_menu_item_set_accel_path (GtkMenuItem *menu_item,
Index: gtkwidget.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkwidget.c,v
retrieving revision 1.292
diff -u -p -r1.292 gtkwidget.c
--- gtkwidget.c	2002/01/30 03:07:14	1.292
+++ gtkwidget.c	2002/01/30 03:26:05
@@ -2593,14 +2593,14 @@ widget_new_accel_closure (GtkWidget *wid
  * @accel_mods:   modifier key combination of the accelerator
  * @accel_flags:  flag accelerators, e.g. %GTK_ACCEL_VISIBLE
  *
- * Installs an accelerator for this @widget in @accel_group, that causes
+ * Installs an accelerator for this @widget in @accel_group that causes
  * @accel_signal to be emitted if the accelerator is activated.
  * The @accel_group needs to be added to the widget's toplevel via
  * gtk_window_add_accel_group(), and the signal must be of type %G_RUN_ACTION.
  * Accelerators added through this function are not user changeable during
  * runtime. If you want to support accelerators that can be changed by the
- * user, use gtk_accel_map_add_entry() and gtk_menu_item_set_accel_path()
- * instead.
+ * user, use gtk_accel_map_add_entry() and gtk_widget_set_accel_path() or
+ * gtk_menu_item_set_accel_path() instead.
  */
 void
 gtk_widget_add_accelerator (GtkWidget      *widget,
@@ -2743,15 +2743,35 @@ destroy_accel_path (gpointer data)
   g_free (apath);
 }
 
-/* accel_group: the accel group used to activate this widget
- * accel_path:  the accel path, associating the accelerator
- *              to activate this widget
- * set accel path through which this widget can be actiavated.
- */
+
+/**
+ * gtk_widget_set_accel_path:
+ * @widget: a #GtkWidget
+ * @accel_path: path used to look up the the accelerator
+ * @accel_group: a #GtkAccelGroup.
+ * 
+ * Given an accelerator group, @accel_group, and an accelerator path,
+ * @accel_path, sets up an accelerator in @accel_group so whenever the
+ * key binding that is defined for @accel_path is pressed, @widget
+ * will be activated.  This removes any accelerators (for any
+ * accelerator group) installed by previous calls to
+ * gtk_widget_set_accel_path(). Associating accelerators with
+ * paths allows them to be modified by the user and the modifications
+ * to be saved for future use. (See gtk_accel_map_save().)
+ *
+ * This function is a low level function that would most likely
+ * be used by a menu creation system like #GtkItemFactory. If you
+ * use #GtkItemFactory, setting up accelerator paths will be done
+ * automatically.
+ *
+ * Even when you you aren't using #GtkItemFactory, if you only want to
+ * set up accelerators on menu items gtk_menu_item_set_accel_path()
+ * provides a somewhat more convenient interface.
+ **/
 void
-_gtk_widget_set_accel_path (GtkWidget     *widget,
-			    const gchar   *accel_path,
-			    GtkAccelGroup *accel_group)
+gtk_widget_set_accel_path (GtkWidget     *widget,
+			   const gchar   *accel_path,
+			   GtkAccelGroup *accel_group)
 {
   AccelPath *apath;