Am 08.06.2014 00:35, schrieb 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.
I can't comment on the reviewability of Matthew's changes, but for my part is I was planning to starting keybindings.c anew, with a single commit (hence I called it "rewrite") because I feel doing it from scratch yields better results than incrementally changing keybindings.c (and it'd take a lot less time). That said, the fundament is Matthew's changes which change the non-keybindings.c-code to work with GtkAction. You can consider this the first 80%, and my proposal the last 20%.
Of course the goal is to maintain the current APIs where it is sensible, or create similar/familiar APIs when not.
BTW, if you say now that Matthew's changes are unlikely to get into master in the short term then I see no point of starting my proposed rewrite.
Best regards.