Re: 6 API-ish issues

[Tim Janik <timj gtk org>]

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.

>  * 68174 GtkEntry focus/selection problem
> 
>    Bug wants gtk_widget_grab_focus (entry) not to select the text in the
>    entry. I think you generally want the selection, but should we
>    force apps to do that themselves? Is there ever a
>    reason for an _app_ to know that it should't select the text
>    in one particular case?

there've been comments on this already (and i prefer the non-selecting
variant), one thing to through in though is that for windows-alike themes,
auto-selcting the entries contents might be desirable, so we might want
to do something like:

typedef enum
{
  GTK_TEXT_SELECTION_NONE,
  GTK_TEXT_SELECTION_FOCUS,
  GTK_TEXT_SELECTION_THEMABLE
} GtkTextSelectionType;

  g_object_class_install_property (entry_class,
                                   PROP_TEXT_SELECTION,
                                   g_param_spec_enum ("text_selection",
                                                      _("Text Selection"),
                                                      _("Controls auto-text-selection behaviour upon focus-in"),
                                                      GTK_TYPE_TEXT_SELECTION,
                                                      GTK_TEXT_SELECTION_THEMABLE,
                                                      G_PARAM_READWRITE));
  settings_install_property_parser (class,
                                    g_param_spec_boolean ("gtk-text-auto-selection",
                                                          _("Text Auto Selection"),
                                                          _("Whether entries should automatically select text upon focus-in"),
                                                          FALSE,
                                                          G_PARAM_READWRITE),
                                    NULL);
 

>  * 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.

>  * 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.

>    The main problems with exporting this are:
> 
>     - It will confuse people that they have to use gtk_menu_item_set_accel_path()
>       instead for menu items.

they don't actually need to, gtk_menu_item_set_accel_path() basically just
causes gtk_widget_set_accel_path() to be called on behalf of the user once
it can figure the accel group from the parent menu. so it's more or less
a convenience function and its documentation can simply point that out.

>     - Typical problems of adding API in an API freeze.
> 
>    IMO, it could wait until 2.2.

i don't think waiting until 2.2 is a good idea here, because:
- gtk_widget_set_accel_path() was meant as a public frontend in the first
  place, to setup accelerators on widgets which are maintained through the
  global accel map. i just didn't export it initially, because i wasn't sure
  whether it was actually needed until i saw the updated gmenu code.
  (in fact, my email accompanying the accel map code change asked for
  feedback on exporting _gtk_widget_set_accel_path(), which i didn't
  receive back then)
- widget action abstraction code like gmenu can't be fully supported and
  tested prior to 2.2 which makes it much harder to integrate something like
  this early and moderately bug free into 2.2.
- programmers wanting to setup accelerators for non menu-items are forced into
  static setups via gtk_widget_add_accelerator() which doesn't allow the user
  to customize his settings via the saved accel maps (or a dialog therefore,
  once that is implemented). this is a big drawback especially for non-US users
  as accelerator setups designed for a US keyboard are rather often suboptimal
  in different locales.

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().

> 
>      http://bugzilla.gnome.org/show_bug.cgi?id=69242
> 

---
ciaoTJ