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