Re: new accelerator system

[Owen Taylor <otaylor gtk org>]

Tim Janik <timj@gtk.org> writes:

> hello fellow gtk hackers,
> 
> 
> i've reworked the accelerator mechanism in gtk to feature sane accelerator
> installation/removal interception and locking, as well as full GDK_* keysym
> support. this will cause source incompatibilities in the development branch
> and for this reason i recommend reading through (or even commenting on) this
> mail for anyone who wants his application to support accelerator bindings.
> because of the source incompatibilities, and because the new accelerator
> handling will be done through a hashtable and thus eliminate the need for
> character tables (which only featured the ASCII and ISO_8859_1 characters)
> the new accelerator "manager" is called GtkAccelGroup.
> this change goes along the already proposed rewrite of the GtkMenuFactory
> which is going to be superceeded by GtkItemFactory for reasons i already
> mentioned on this list a few weeks ago.
> the required changes, besides vaporizing ;) GtkAcceleratorTable and
> GtkMenuFactory, affect the GtkWidget::install_accelerator and
> GtkWidget::remove_accelerator signals (and thus gtk_widget_install_accelerator
> plus gtk_widget_remove_accelerator) and the GtkWindow functions
> gtk_window_add_accelerator_table and gtk_window_remove_accelerator_table.
> the last two will most likely be renamed to
> void       gtk_window_join_accel_group    (GtkWindow          *window,
>                                            GtkAccelGroup      *accel_group);
> and
> void       gtk_window_leave_accel_group   (GtkWindow          *window,
>                                            GtkAccelGroup      *accel_group);
> also GtkMenu and GtkMenuItem are affected, but that will be taken care of at
> a later point (the accelerator binding display code will most likely be moved
> out of GtkMenuItem into a new widget GtkAccelLabel).
> the proposed system has already been implemented and will be commited to the
> development branch in a few days.
> 
> 
> Accelerator bindings
> --------------------
> an object can have a certain keyvalue/modifier combination associated with the
> emission of a certain signal. those associations are called "accelerator
> bindings" and are handled by a GtkAccelGroup.
> an object to be subject of an accelerator binding has to meat certain
> requirements:
> it needs to introduce the "add-accelerator" and "remove-accelerator" signals
> in its class branch, fitting the following signatures:
> 
> void	(*add_accelerator)	(GtkObject	*object,
> 				 GtkAccelGroup	*ac_group,
> 				 guint		 activation_signal_id,
> 				 guint		 keyval,
> 				 guint		 modifiers,
> 				 GtkAccelFlags	 accel_flags);
> void	(*remove_accelerator)	(GtkObject	*object,
> 				 GtkAccelGroup	*ac_group,
> 				 guint		 activation_signal_id,
> 				 guint		 keyval,
> 				 guint		 modifiers);
> 
> the marshaller functions for these signals are provided in gtkaccelgroup.c.

So - these signals are not in GtkWidget and have to be separately
added to each widget that wants to be able to handle an accelerator???
(This seems like unecessary complexity ;-) Also, it is quite possible
that someone would want to add some accelerators that are not
associated with a "typical" accelerator widget.

 gtk_accel_group_add (accel_group, cancel_button, "clicked",
                      GDK_ESCAPE, 0, 0);

IMO, these signals should be provided by GtkWidget with default
handlers. If you want to make the default handlers public to
allow other GtkObjects to also handle adding accelerators - well
that would be OK...

> the `binding_flags' argument specifies certain "behaviours" of an accelerator
> binding. possible values can be composed by or'ing several of the following
> values:
> 
> typedef enum
> {
>   GTK_ACCEL_VISIBLE		= 1 << 0	/* should the binding appear in
>   						 * the widget's display?
>   						 */,
>   GTK_ACCEL_SIGNAL_VISIBLE	= 1 << 1	/* should the signal associated
>   						 * with this binding be also
>   						 * visible?
>   						 */,
>   GTK_ACCEL_LOCKED		= 1 << 2	/* may this binding be removed
>   						 * again?
>   						 */,
>   GTK_ACCEL_MASK		= 0x07
> } GtkAccelFlags;

Could you provide examples for GTK_ACCEL_VISIBLE and GTK_ACCEL_SIGNAL_VISIBLE?

>
> the corresponding functions for adding/removing an accelerator binding for
> an object are:
> 
> void            gtk_accel_group_add            (GtkAccelGroup  *accel_group,
>                                                 GtkObject      *object,
>                                                 const gchar  *activation_signal,
>                                                 guint           accel_key,
>                                                 guint           accel_mods,
>                                                 GtkAccelFlags   accel_flags);
> void            gtk_accel_group_remove         (GtkAccelGroup  *accel_group,
>                                                 GtkObject      *object,
>                                                 guint           accel_key,
>                                                 guint           accel_mods);

Could you provide examples for GTK_ACCEL_VISIBLE and GTK_ACCEL_SIGNAL_VISIBLE?

> these two functions do in fact not really remove or add the binding, but emit
> the above mentioned signals on the specified object. the objects default
> handler is then responsible for the actual addition/removal of the accelerator
> and should do so just by calling either of
> 
> void            gtk_accel_group_handle_add     (GtkObject      *object,
>                                                 GtkAccelGroup  *accel_group,
>                                                 guint      activation_signal_id,
>                                                 guint           accel_key,
>                                                 guint           accel_mods,
>                                                 GtkAccelFlags   accel_flags);
> or
> void            gtk_accel_group_remove         (GtkAccelGroup  *accel_group,
>                                                 GtkObject      *object,
>                                                 guint           accel_key,
>                                                 guint           accel_mods);
> 
> this might appear as unneccessary complexity but isn't. an object actually
> introducing the add_accelerator signal, e.g.
> 
>   widget_signals[ADD_ACCELERATOR] =
>     gtk_signal_new ("add-accelerator",
>                     GTK_RUN_LAST,
>                     object_class->type,
>                     GTK_SIGNAL_OFFSET (GtkWidgetClass, add_accelerator),
> 		    gtk_accel_group_marshal_add,
>                     GTK_TYPE_NONE, 3,
>                     GTK_TYPE_BOXED,
>                     GTK_TYPE_UINT,
>                     GTK_TYPE_UINT,
>                     GTK_TYPE_UINT,
>                     GTK_TYPE_ACCEL_FLAGS);
> 
> can just do
> 
>   widget_class->add_accelerator = gtk_accel_group_handle_add;

Are you then saying that the Widget class does have these signals?
Or was this an hypothetical example...
 
> and is all set about adding accelerator bindings. this step has been made to
> keep the accelerator system fully independant from the widget concept, and
> thus even the signal emission functions are provided by gtkaccelgroup.c.

Do you have an example in mind where you would want to have an
accelerator on a GtkObject other than a widget? 

> [ GtkAcceleratorTables implement this separation as well, but they do so by
>   checking whether the object is_a GTK_TYPE_WIDGET prior to signal emissions,
>   which just doesn't carry this abstraction to its fullest extends ]
> also accelerator addition/removal can now be sanly intercepted via handlers
> for the *_accelerator signals, simply by doing
> gtk_signal_emit_stop_by_name (object, "add-accelerator"); or
> gtk_signal_emit_stop_by_name (object, "remove-accelerator");.
> alternatively, a certain accelerator binding can be locked/unlocked by:
> 
> void            gtk_accel_group_lock_binding   (GtkAccelGroup  *accel_group,
>                                                 guint           accel_key,
>                                                 guint           accel_mods);
> void            gtk_accel_group_unlock_binding (GtkAccelGroup  *accel_group,
>                                                 guint           accel_key,
>                                                 guint           accel_mods);
> 
> which will in fact just set/clear the GTK_ACCEL_LOCKED flag on the specified
> binding.
 
 
> GtkAccelGroup
> -------------
> an accelerator group consists of a set of accelerator bindings for several
> widgets, whereas any widget is allowed to have as many accelerator bindings as
> desired. an accelerator binding specifically consists of a keyvalue, its
> modifiers, a widget pointer and a signal introduced on the widgets class
> branch. the signal will be emitted for this widget if the accelerator group is
> "activated" for a specific keyvalue/modifier pair. within accelerator groups,
> a specific keyvalue/modifier combination is unique. however, installation
> of accelerator bindings for a GtkAccelGroup will either remove an already
> existing binding with the same keyval/modifiers or be ignored if such an
> existing binding is locked. 

> furthermore all GtkAccelGroups that are joined
> by any object that joined the GtkAccelGroup subject for the accelerator
> installation, will be requested to remove this certain keyval/modifier
> combination.
> objects that wish to activate certain accelerator groups (usually toplevel
> widgets e.g. a GtkWindow) are expected to announce their interest in an
> accelerator group prior to any activation of such. this announcement
> is perfomed via
> 
> void gtk_accel_group_join (GtkAccelGroup	*ac_group,
>                            GtkObject         	*object);
> 
> the counterpart function is
> 
> void gtk_accel_group_leave (GtkAccelGroup	*ac_group,
> 			    GtkObject		*object);
> 
> after an object has joined a GtkAccelGroup, it can "ativate" that group by
> calling

I guess this points out a problem with the AccelGroup name. 
It is a group of what? Not accelerators. The above implies
that it is a group of toplevel widgets that share common bindings
- which though true, is not really getting at what it does.
And is not obvious from the name.

Alternate names that come to mind:

 GtkAccel[erator]Table  (GtkAcceleratorTable is the best name, but
                         we probably want to pick something new)
 GtkAccel[erator]Binder
 GtkBinder
 GtkKeyBinder

> gboolean gtk_accel_group_activate (GtkAccelGroup	*ac_group,
> 			           guint		 keyval,
> 			           guint		 modifiers);
> 
> the return value specifies whether an "activation" of an object has
> actually occoured.
> 
> accelerator groups can be "locked" to prevent further addition or removal of
> accelerator bindings.
> 
> void            gtk_accel_group_lock           (GtkAccelGroup  *accel_group);
> void            gtk_accel_group_unlock         (GtkAccelGroup  *accel_group);
> 
> the behaviour is implemented as incremental locking, thus for a GtkAccelGroup
> that is locked, gtk_accel_group_unlock (accel_group) needs to be called for
> at least as many times as it has been locked before, to actually become
> unlocked again.
> 
> Destruction
> -----------
> 
> upon an object's destruction, all of its bindings will automatically be removed
> and membership in certain accelerator groups will be liquidated as well.
> the membership in an accelerator group as well as an existing accelerator
> binding for an object is reflected in a reference count on the GtkAccelGroup,
> but *not* in the objects structure (thus the accelerator group cares for an
> objects destruction by connecting to its "destroy" signal), so as to avoid
> reference circles.

After the above quibbles - a more sweeping ideas:

Could this mechanism be used to do the customizable bindings
for Entries and Text widgets? You would just attach an
AcceleratorGroup with the appropriate bindings to the widget,
and add a whole bunch of "move_backword_character" type signals.

(Even vi bindings can be handled - you keep a mode flag in the
 Text/Entry widget, add accelerators for "h", "j", "k", "l", etc
 but if the mode is "insertion" you handle insertion keypresses 
 before activating the bindings from the accelerator table)

The difficulty is one of efficiency. You would need to create
a separate accelerator table (with dozens of bindings) for
each Entry in the application. Which might or might not be
a problem. I don't have a feel yet as for how "lightweight"
an Accelerator group is. (but there would be a lot of
signal emission going on to set up the bindings)

One possibility -

gboolean gtk_accel_group_activate (GtkAccelGroup	*ac_group,
			           guint		 keyval,
			           guint		 modifiers,
+  			           gpointer	         accelerator_data);
 
gtk_entry_move_forward (GtkWidget *dummy_entry, gpointer accelerator_data)
{
  GtkEntry entry = GTK_ENTRY (accelerator_data);
  ...
}

And there is one per-application singleton Entry that does nothing
but forward key presses to the real entries. (it is never added to a
parent realized or shown.)

(Or you could just keep track of the currently focused entry on
a global basis, and dispatch events to there without the
accelerator_data argument

gtk_entry_move_forward (GtkWidget *dummy_entry, gpointer accelerator_data)
{
  GtkEntry entry = focused_entry;
  ...
}
)

Anyways, there is a lot of power in your proposal. A fair bit of
complexity too, though most of this stuff will be hidden from the
application programmer. The CANCEL example probably does need
to work...

Regards,
                                        Owen