[Geany-Devel] RFC: Keybindings rewrite

[Colomban Wendling]

Le 06/06/2014 16:17, Thomas Martitz a écrit :
> Hello,
> 
> based up on Matthew's fine GtkActions branch [1]

We probably should not define the actions in the Glade file but rather
in the code, they aren't really a UI thing but should reflect what the
app can do basically.  Basically it's rather a replacement for our
send_command() rather than simply keybindings.

> I think we could
> realistically rewrite keybindings.c use GtkAction/accelerators properly.
> 
> Currently it re-implements lots of gtk stuff, such as the actual looping
> through the known keybindings for the callback when a keybinding
> pressed. When this would be rewritten keybindings.c could probably be
> half as large (in LOC terms).

Yes, current handmade keybinding is suboptimal, and GtkAction is
probably a good idea.  However, we need to make sure it doesn't remove
feature on the way, because we have some tricky handling in some cases
AFAIK.

> The advantage of doing so would be that plugins could register
> keybindings simply by providing a GtkAction instance (and optionally a
> GtkMenuItem) instead of a plain callback. This would enable to handle
> keybindings in a more natural (from a glib/gtk POV) form through
> signals. This is especially interesting for non-C plugins because it
> easier to support/implement a gobject-based API then a
> function-pointer-based one. This is the major reason I'm interested in
> this.
> 
> I think it is possible to do this without breaking the API or at least
> without actual damage because plugins don't use the fields of
> GeanyKeyGroup and GeanyKeyBinding so we can change these structs.

Mmmmmokay, great.

> Is such a rewrite desirable,

If it doesn't lose features on the way, yes.

> and would it have a realistic chance of
> getting merged? 

It depends on how it is done I guess.  If it's incrementally mergeable,
or if it's reasonably reviewable, yes.  If it changes 80% of Geany's
internals in weird commits dropped on the end, it probably will be
*really* hard to merge.
For example, Matthew's commits are nice in the sense they are
individual, but not in the sense each commit touches a lot of partially
unrelated stuff.  It'd be easier to review the removals of some
state-prefs, regardless of the action changes, and then the actions
changing less code.  If it of course unrealistic to anticipate all
changes before, but obvious ones could benefit from being merged first.
 And there is the hard balance between half-changed code and huge commits.
Anyway, basically it's a "yes, but if it's a commit bomb on the end".
But I'm not the sole developer (am I?), so other can work on that too
and answer that (e.g. maybe Matthew since he started a branch?).

Also, I'm again not sure we should put actions in the Glade file, but
probably rather define them in the code and simply use them in Glade.
This is GtkActions in both cases, but implemented differently, and this
needs to be agreed on before because it can change a great deal of code.


Regards,
Colomban