Hello,
based up on Matthew's fine GtkActions branch [1] 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).
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.
Is such a rewrite desirable, and would it have a realistic chance of getting merged? I'm asking because I don't want to spend time on this and never get it merged. Otherwise I would volunteer to do this.
Best regards.
I think it better to port to GAction instead of GtkAction (GtkAction has been deprecated since version 3.10 and will be removed in GTK+ 4), so it will be ease to port geany to GTK+ 4 in the future.
Also, IIRC, there is a problem with GtkAction, wich cause to some accelactors to work only when the keywoard layout on Englisg. For example, most of the accelactors in epiphany dosn't work when the keywoard layout is Hebrew, and it will be solved when someone will port epiphany from GtkAction GAction.
Also, I think it will be good idea to port all geany to GApplication & GtkApplication, GAction and GOptionContext and other new GTK+ API.
Regards, Yosef Or Boczko
בתאריך ו', יונ 6, 2014 בשעה 5:17 PM, Thomas Martitz kugel@rockbox.org כתב:
Hello,
based up on Matthew's fine GtkActions branch [1] 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).
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.
Is such a rewrite desirable, and would it have a realistic chance of getting merged? I'm asking because I don't want to spend time on this and never get it merged. Otherwise I would volunteer to do this.
Best regards.
Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Am 06.06.2014 16:24, schrieb Yosef Or Boczko:
I think it better to port to GAction instead of GtkAction (GtkAction has been deprecated since version 3.10 and will be removed in GTK+ 4), so it will be ease to port geany to GTK+ 4 in the future.
We have elaborated GAction. The problem is that it is only recent in Glib/Gtk versions. Geany is committed to support at least the last gtk2 release for some time to come.
I think the transition from GtkAction to GAction isn't actually that terrible (mostly search & replace) so we can do it at some point in the future.
PS: Please do not top-post on this list.
Best regards
On 14-06-06 01:35 PM, Thomas Martitz wrote:
Am 06.06.2014 16:24, schrieb Yosef Or Boczko:
I think it better to port to GAction instead of GtkAction (GtkAction has been deprecated since version 3.10 and will be removed in GTK+ 4), so it will be ease to port geany to GTK+ 4 in the future.
We have elaborated GAction. The problem is that it is only recent in Glib/Gtk versions. Geany is committed to support at least the last gtk2 release for some time to come.
Also, unless I missed it in the docs, GAction/GSimpleAction do not appear to support associating labels, tooltips, icons, etc with the action so would require duplicating these all over the place (similar to the current way we do it).
I think the transition from GtkAction to GAction isn't actually that terrible (mostly search & replace) so we can do it at some point in the future.
As GtkAction is so useful and what it does is quite simple, IMO we could also create our own replacement class for it in the future, or just take the relevant source files from the GTK+ tree.
Cheers, Matthew Brush
Le 07/06/2014 02:01, Matthew Brush a écrit :
On 14-06-06 01:35 PM, Thomas Martitz wrote:
Am 06.06.2014 16:24, schrieb Yosef Or Boczko:
I think it better to port to GAction instead of GtkAction (GtkAction has been deprecated since version 3.10 and will be removed in GTK+ 4), so it will be ease to port geany to GTK+ 4 in the future.
We have elaborated GAction. The problem is that it is only recent in Glib/Gtk versions. Geany is committed to support at least the last gtk2 release for some time to come.
Also, unless I missed it in the docs, GAction/GSimpleAction do not appear to support associating labels, tooltips, icons, etc with the action so would require duplicating these all over the place (similar to the current way we do it).
The action not having the UI details may not be the worst thing ever, basically actions ways to interact with the application ("commands" if you will), not necessarily a UI element.
But yeah, defining the action appearance and using this all over the place is super handy.
Le 06/06/2014 16:24, Yosef Or Boczko a écrit :
I think it better to port to GAction instead of GtkAction (GtkAction has been deprecated since version 3.10 and will be removed in GTK+ 4), so it will be ease to port geany to GTK+ 4 in the future.
Also, IIRC, there is a problem with GtkAction, wich cause to some accelactors to work only when the keywoard layout on Englisg. For example, most of the accelactors in epiphany dosn't work when the keywoard layout is Hebrew, and it will be solved when someone will port epiphany from GtkAction GAction.
…or fix the bug in GtkAction. And is this really a bug in GtkAction that was never discovered or fixed in the GTK2 days, or is it a specific issue with Epiphany? Anyway, as Thomas said, it's too recent to be a candidate for Geany right now. But if it really doesn't work, we may not want to use it then…
Also, I think it will be good idea to port all geany to GApplication & GtkApplication,
Basically GTK3 only (or we re-do almost everything ourselves, plus depend on a post-GTK2.24 glib), and although I like those classes, porting it for the sake of porting it doesn't sound sensible. Yes they probably simplify some of our code, but the cost of porting and retaining same behavior might not be worth it.
GAction and GOptionContext and other new GTK+ API.
We use GOptionContext for ages.
Am 06.06.2014 16:24, schrieb Yosef Or Boczko:
I think it better to port to GAction instead of GtkAction (GtkAction has been deprecated since version 3.10 and will be removed in GTK+ 4), so it will be ease to port geany to GTK+ 4 in the future.
Also, IIRC, there is a problem with GtkAction, wich cause to some accelactors to work only when the keywoard layout on Englisg. For example, most of the accelactors in epiphany dosn't work when the keywoard layout is Hebrew, and it will be solved when someone will port epiphany from GtkAction GAction.
Can you check if that isn't a bug with Epiphany? And if it occurs in Geany as well? Geany's approach to keybindings is most trivial (at it's core it does if (keyval == $KEY && modifier == $MODIFIER)). Gtk cannot do worse, so I expect it to have something more smart (or be equally dumb). So it sounds unlikely that Gtk has a locale-related problem that Geany does not.
Best regards
Regards, Yosef Or Boczko
בתאריך ב', יונ 9, 2014 בשעה 11:30 PM, Thomas Martitz kugel@rockbox.org כתב:
Am 06.06.2014 16:24, schrieb Yosef Or Boczko:
I think it better to port to GAction instead of GtkAction (GtkAction has been deprecated since version 3.10 and will be removed in GTK+ 4), so it will be ease to port geany to GTK+ 4 in the future.
Also, IIRC, there is a problem with GtkAction, wich cause to some accelactors to work only when the keywoard layout on Englisg. For example, most of the accelactors in epiphany dosn't work when the keywoard layout is Hebrew, and it will be solved when someone will port epiphany from GtkAction GAction.
Can you check if that isn't a bug with Epiphany? And if it occurs in Geany as well? Geany's approach to keybindings is most trivial (at it's core it does if (keyval == $KEY && modifier == $MODIFIER)). Gtk cannot do worse, so I expect it to have something more smart (or be equally dumb). So it sounds unlikely that Gtk has a locale-related problem that Geany does not.
I see the same problem with part of the accelactors in totem (the accels totem handle by hand and not by GAction), and btw also somewhere in gnome-shell.
I'm not sure, but I think gtkkeyhash.c[1] relate to this.
[1] https://git.gnome.org/browse/gtk+/tree/gtk/gtkkeyhash.c
Regards, Yosef Or Boczko
Best regards _______________________________________________ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Am 09.06.2014 22:39, schrieb Yosef Or Boczko:
Regards, Yosef Or Boczko בתאריך ב', יונ 9, 2014 בשעה 11:30 PM, Thomas Martitz kugel@rockbox.org כתב:
Am 06.06.2014 16:24, schrieb Yosef Or Boczko:
I think it better to port to GAction instead of GtkAction (GtkAction has been deprecated since version 3.10 and will be removed in GTK+ 4), so it will be ease to port geany to GTK+ 4 in the future.
Also, IIRC, there is a problem with GtkAction, wich cause to some accelactors to work only when the keywoard layout on Englisg. For example, most of the accelactors in epiphany dosn't work when the keywoard layout is Hebrew, and it will be solved when someone will port epiphany from GtkAction GAction.
Can you check if that isn't a bug with Epiphany? And if it occurs in Geany as well? Geany's approach to keybindings is most trivial (at it's core it does if (keyval == $KEY && modifier == $MODIFIER)). Gtk cannot do worse, so I expect it to have something more smart (or be equally dumb). So it sounds unlikely that Gtk has a locale-related problem that Geany does not.
I see the same problem with part of the accelactors in totem (the accels totem handle by hand and not by GAction), and btw also somewhere in gnome-shell.
I'm not sure, but I think gtkkeyhash.c[1] relate to this.
So the keybings totem does by hand don't work but those that are handled by Gtk/Glib do work? And what about gnome-shell. It's all Gtk3 so I'd expect it to use GAction.
What about current Geany?
Best regards
Regards, Yosef Or Boczko
בתאריך ג', יונ 10, 2014 בשעה 12:54 AM, Thomas Martitz kugel@rockbox.org כתב:
Am 09.06.2014 22:39, schrieb Yosef Or Boczko:
Regards, Yosef Or Boczko בתאריך ב', יונ 9, 2014 בשעה 11:30 PM, Thomas Martitz kugel@rockbox.org כתב:
Am 06.06.2014 16:24, schrieb Yosef Or Boczko:
I think it better to port to GAction instead of GtkAction
(GtkAction
has been deprecated since version 3.10 and will be removed in
GTK+ 4),
so it will be ease to port geany to GTK+ 4 in the future.
Also, IIRC, there is a problem with GtkAction, wich cause to some accelactors to work only when the keywoard layout on Englisg. For example, most of the accelactors in epiphany dosn't work when the keywoard layout is Hebrew, and it will be solved when someone will port epiphany from GtkAction GAction.
Can you check if that isn't a bug with Epiphany? And if it occurs
in
Geany as well? Geany's approach to keybindings is most trivial (at it's core it does if (keyval == $KEY && modifier == $MODIFIER)).
Gtk
cannot do worse, so I expect it to have something more smart (or
be
equally dumb). So it sounds unlikely that Gtk has a locale-related problem that Geany does not.
I see the same problem with part of the accelactors in totem (the accels totem handle by hand and not by GAction), and btw also somewhere in gnome-shell.
I'm not sure, but I think gtkkeyhash.c[1] relate to this.
So the keybings totem does by hand don't work but those that are handled by Gtk/Glib do work? And what about gnome-shell. It's all Gtk3 so I'd expect it to use GAction.
I found for you an example in the code for totem.
- Ctrl+F to switch the search bar dosn't work when the keywoard layout in Hebrew (note you need in the code to by the hand both Ctrl+F and Ctrl+f): https://git.gnome.org/browse/totem/tree/src/totem-grilo.c#n1634
- Ctrl+A to select all work when the keywoard layout in Hebrew: https://git.gnome.org/browse/totem/tree/src/totem-grilo.c#n2206
You can't use GtkKeyHash yourself because is a private API (you can to copy it if you want).
About gnome-shell, it relate to mutter and clutter and not to GTK+ as I know. Four mont ago, part of this problem (such Super+L) was solved in bug 678001[1], but part of another accels, like Ctrl+A to select text in ClutterText (same to GtkEntry) still dosn't work with non-Latin layout, because the code in clutter not use GtkKeyHash yet (we needs to copy the code from gtk+ to clutter or to wait until ebassi will finish to merge clutter to gtk+ aka GTK+ 4).
[1] https://bugzilla.gnome.org/show_bug.cgi?id=678001
Regards, Yosef Or Boczko
What about current Geany?
Best regards _______________________________________________ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Am 10.06.2014 00:36, schrieb Yosef Or Boczko:
Regards, Yosef Or Boczko בתאריך ג', יונ 10, 2014 בשעה 12:54 AM, Thomas Martitz kugel@rockbox.org כתב:
Am 09.06.2014 22:39, schrieb Yosef Or Boczko:
Regards, Yosef Or Boczko בתאריך ב', יונ 9, 2014 בשעה 11:30 PM, Thomas Martitz >
kugel@rockbox.org כתב:
Am 06.06.2014 16:24, schrieb Yosef Or Boczko:
I think it better to port to GAction instead of GtkAction (GtkAction has been deprecated since version 3.10 and will be removed in
GTK+ 4),
so it will be ease to port geany to GTK+ 4 in the future.
Also, IIRC, there is a problem with GtkAction, wich cause to some accelactors to work only when the keywoard layout on Englisg. For example, most of the accelactors in epiphany dosn't work when the keywoard layout is Hebrew, and it will be solved when someone will port epiphany from GtkAction GAction.
Can you check if that isn't a bug with Epiphany? And if it occurs
in >> Geany as well? Geany's approach to keybindings is most trivial (at >> it's core it does if (keyval == $KEY && modifier == $MODIFIER)). Gtk >> cannot do worse, so I expect it to have something more smart (or be >> equally dumb). So it sounds unlikely that Gtk has a locale-related >> problem that Geany does not.
I see the same problem with part of the accelactors in totem (the >
accels totem handle
by hand and not by GAction), and btw also somewhere in gnome-shell.
I'm not sure, but I think gtkkeyhash.c[1] relate to this.
So the keybings totem does by hand don't work but those that are handled by Gtk/Glib do work? And what about gnome-shell. It's all Gtk3 so I'd expect it to use GAction.
I found for you an example in the code for totem.
- Ctrl+F to switch the search bar dosn't work when the keywoard layout
in Hebrew (note you need in the code to by the hand both Ctrl+F and Ctrl+f): https://git.gnome.org/browse/totem/tree/src/totem-grilo.c#n1634
- Ctrl+A to select all work when the keywoard layout in Hebrew:
https://git.gnome.org/browse/totem/tree/src/totem-grilo.c#n2206
So, what you are saying is that if you match keys manually, as totem does in the first example, you get bugs with non-latin keyboards. This is what Geany does currently.
The second example shows that if you use Gtk APIs/accelerators these problems can be avoided.
So, from my understanding, the proposed rewrite would make things work better (not worse) under non-latin keyboards.
What about current Geany?
Why do you avoid this question?
Best regards.
בתאריך ג', יונ 10, 2014 בשעה 2:06 PM, Thomas Martitz kugel@rockbox.org כתב:
Am 10.06.2014 00:36, schrieb Yosef Or Boczko:
Regards, Yosef Or Boczko בתאריך ג', יונ 10, 2014 בשעה 12:54 AM, Thomas Martitz kugel@rockbox.org כתב:
Am 09.06.2014 22:39, schrieb Yosef Or Boczko:
Regards, Yosef Or Boczko בתאריך ב', יונ 9, 2014 בשעה 11:30 PM, Thomas
Martitz >
kugel@rockbox.org כתב:
Am 06.06.2014 16:24, schrieb Yosef Or Boczko:
I think it better to port to GAction instead of GtkAction
(GtkAction
has been deprecated since version 3.10 and will be removed in
GTK+ 4),
so it will be ease to port geany to GTK+ 4 in the future.
Also, IIRC, there is a problem with GtkAction, wich cause to
some
accelactors to work only when the keywoard layout on Englisg. For example, most of the accelactors in epiphany dosn't work
when
the keywoard layout is Hebrew, and it will be solved when someone will port epiphany from GtkAction GAction.
Can you check if that isn't a bug with Epiphany? And if it
occurs
in >> Geany as well? Geany's approach to keybindings is most
trivial
(at >> it's core it does if (keyval == $KEY && modifier == $MODIFIER)). Gtk >> cannot do worse, so I expect it to have
something
more smart (or be >> equally dumb). So it sounds unlikely that Gtk has a locale-related >> problem that Geany does not.
I see the same problem with part of the accelactors in totem
(the >
accels totem handle
by hand and not by GAction), and btw also somewhere in
gnome-shell.
I'm not sure, but I think gtkkeyhash.c[1] relate to this.
So the keybings totem does by hand don't work but those that are handled by Gtk/Glib do work? And what about gnome-shell. It's all Gtk3 so I'd expect it to use GAction.
I found for you an example in the code for totem.
- Ctrl+F to switch the search bar dosn't work when the keywoard
layout
in Hebrew (note you need in the code to by the hand both Ctrl+F and Ctrl+f): https://git.gnome.org/browse/totem/tree/src/totem-grilo.c#n1634
- Ctrl+A to select all work when the keywoard layout in Hebrew:
https://git.gnome.org/browse/totem/tree/src/totem-grilo.c#n2206
So, what you are saying is that if you match keys manually, as totem does in the first example, you get bugs with non-latin keyboards. This is what Geany does currently.
The second example shows that if you use Gtk APIs/accelerators these problems can be avoided.
So, from my understanding, the proposed rewrite would make things work better (not worse) under non-latin keyboards.
Yeah, somthing like this. Just, as I understand, GtkAction not use GtkKeyHash, so it dosn't work on non-Lating layout.
What about current Geany?
Why do you avoid this question?
IIRC, from time to time I see some accels in Geany broken in non-Latin layout, but I not sure which accels, because part of them work currently. Also in epiphany, part of the accels still work in non-Latin layout. it very strange, I must to say.
Regards, Yosef Or Boczko
Best regards. _______________________________________________ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
On 14-06-06 07:17 AM, Thomas Martitz wrote:
Hello,
based up on Matthew's fine GtkActions branch [1] 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).
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.
We could alternatively add a GtkActionGroup to each GeanyPlugin(Private) structure so plugins could just add in their own actions and not have the core even care much. I can imagine something like this:
void plugin_init(GeanyData *unused) { GtkAction *action = gtk_action_new("foo", "Foo", "Does foo", NULL); gtk_action_group_add_action( plugin_get_action_group(geany_plugin), action); GtkMenuItem *item = gtk_action_create_menu_item(action); gtk_menu_shell_append( GTK_MENU_SHELL(geany_data.main_widgets.tools_menu), item); gtk_widget_show(item); // etc... }
The keybindings editor GUI would then be able to know which actions the plugin wants managed/edited without requiring any special API functions.
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.
Is such a rewrite desirable, and would it have a realistic chance of getting merged? I'm asking because I don't want to spend time on this and never get it merged. Otherwise I would volunteer to do this.
Calling it a re-write will mostly likely not help :)
Cheers, Matthew Brush
On 7 June 2014 10:32, Matthew Brush mbrush@codebrainz.ca wrote:
On 14-06-06 07:17 AM, Thomas Martitz wrote:
Hello,
based up on Matthew's fine GtkActions branch [1] 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).
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.
We could alternatively add a GtkActionGroup to each GeanyPlugin(Private) structure so plugins could just add in their own actions and not have the core even care much. I can imagine something like this:
void plugin_init(GeanyData *unused) { GtkAction *action = gtk_action_new("foo", "Foo", "Does foo", NULL); gtk_action_group_add_action( plugin_get_action_group(geany_plugin), action); GtkMenuItem *item = gtk_action_create_menu_item(action); gtk_menu_shell_append( GTK_MENU_SHELL(geany_data.main_widgets.tools_menu), item); gtk_widget_show(item); // etc... }
The keybindings editor GUI would then be able to know which actions the plugin wants managed/edited without requiring any special API functions.
And as Matthew explained on IRC this would allow Geany to destroy the action group(s) associated with the plugin when the plugin is unloaded, thus destroying the actions, so no leaks :)
The general idea looks ok to me.
Cheers Lex
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.
Is such a rewrite desirable, and would it have a realistic chance of getting merged? I'm asking because I don't want to spend time on this and never get it merged. Otherwise I would volunteer to do this.
Calling it a re-write will mostly likely not help :)
Cheers, Matthew Brush
Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
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
On 14-06-07 03:35 PM, Colomban Wendling wrote:
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.
IMO it's much better to use something declarative for this than to write it in code. Even though XML is kind of ugly, it's still more maintainable/easier/etc than C, especially since Glade supports creating/editing/assigning the actions through it's user interface.
Originally I was going to create a whole separate GtkBuilder file (ex. actions.xml or such) for actions, but then you loose the possibility to select the actions for the various widgets using Glade's user interface (unless I missed how to make it understand two files at once).
I don't really think it's a big deal having them in the main GtkBuilder file, I mean they are *UI* actions used to control the appearance and proxy the behaviour of *UI* widgets already in the same file with other stuff like list models for the UI, etc.
Cheers, Matthew Brush
Am 08.06.2014 01:34, schrieb Matthew Brush:
On 14-06-07 03:35 PM, Colomban Wendling wrote:
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.
IMO it's much better to use something declarative for this than to write it in code. Even though XML is kind of ugly, it's still more maintainable/easier/etc than C, especially since Glade supports creating/editing/assigning the actions through it's user interface.
Originally I was going to create a whole separate GtkBuilder file (ex. actions.xml or such) for actions, but then you loose the possibility to select the actions for the various widgets using Glade's user interface (unless I missed how to make it understand two files at once).
I don't really think it's a big deal having them in the main GtkBuilder file, I mean they are *UI* actions used to control the appearance and proxy the behaviour of *UI* widgets already in the same file with other stuff like list models for the UI, etc.
Cheers, Matthew Brush
I agree, the GtkActions should be in the glade file. It has the big advantage that you can use glade to assign buttons to actions. Whereas having it in the code brings no advantage in my book.
While GtkActions itself aren't widgets they are to be activated through widgets (buttons, menu items, etc.). Keybindings are an added feature. All actions have at least one activating widget, but not all have keybindings.
Best regards.
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.
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.
In reply to the removed-features part, and generally as an update, I wanted to tell that I basically suspended my work on this. I already converted keybindings half-way to GtkAction when I found that we allow keybindings to be mapped to multiple actions, and even do this by default for tab. Plus we allow those keybindings to be context sensitive in the sense that the handler function can tell that it didn't handle the keybinding in which case the next mapped keybinding-action will be run.
This is basically impossible to support with plain GtkActions, and therefore I stopped my work on this for the time being. If there's any interest in continuing and ideas how to resolve the above issue then please leave a comment.
Best regards
[...]
This is basically impossible to support with plain GtkActions, and therefore I stopped my work on this for the time being. If there's any interest in continuing and ideas how to resolve the above issue then please leave a comment.
Thanks for trying anyway.
Cheers Lex
Best regards
Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel