Hi, I'm thinking about changing the keybindings_send_command() / keybinding enums code, for two reasons: 1. We can't change the order of keybindings without breaking the plugin ABI. 2. We can't reorder/move keybindings in groups without breaking the plugin ABI and the API. 3. The keybindings_send_command(GEANY_KEY_GROUP_FOCUS, GEANY_KEYS_FOCUS_COMPILER) syntax seems redundant - why not just keybindings_send_command(GEANY_KEYS_FOCUS_COMPILER)?
The 3rd issue isn't that important, but it would be nice to avoid it, as it could be a source of bugs.
To solve this, we could have the GEANY_KEYS_ commands share a single enum, but unsorted so we can append items as needed. All the group enums could be moved into keybindings.c, renaming them so all the existing code works without changes. The only function that would need updating would be keybindings_send_command(), plus storing the GEANY_KEYS_ enum value in each keybinding struct, so that function can find the callback.
Does this sound a good idea?
Regards, Nick
On Fri, 4 Jul 2008 15:08:27 +0100 Nick Treleaven nick.treleaven@btinternet.com wrote:
...
To solve this, we could have the GEANY_KEYS_ commands share a single enum, but unsorted so we can append items as needed. All the group enums could be moved into keybindings.c, renaming them so all the existing code works without changes. The only function that would need updating would be keybindings_send_command(), plus storing the GEANY_KEYS_ enum value in each keybinding struct, so that function can find the callback.
To be clearer, user code would change, but not much of keybindings.c would require changes. As an example:
keybindings.h: enum CommonKeyIds {..., GEANY_KEYS_SOME_COMMAND, ...}
keybindings.c: enum KeyGroups {..., KEY_GROUP_SOME, ...} enum SomeGroupOrder {..., KEYS_SOME_COMMAND, ...}
keygroups[KEY_GROUP_SOME]->keys[KEYS_SOME_COMMAND]->id = GEANY_KEYS_SOME_COMMAND;
Then keybindings_send_command() would search for that id in keygroups[x]->keys[y]. Although that's a relatively slow search algorithm, it probably wouldn't matter here. If necessary it could be optimized to use a hash/lookup table.
Regards, Nick
On Fri, 4 Jul 2008 15:26:28 +0100, Nick Treleaven nick.treleaven@btinternet.com wrote:
On Fri, 4 Jul 2008 15:08:27 +0100 Nick Treleaven nick.treleaven@btinternet.com wrote:
...
To solve this, we could have the GEANY_KEYS_ commands share a single enum, but unsorted so we can append items as needed. All the group enums could be moved into keybindings.c, renaming them so all the existing code works without changes. The only function that would need updating would be keybindings_send_command(), plus storing the GEANY_KEYS_ enum value in each keybinding struct, so that function can find the callback.
To be clearer, user code would change, but not much of keybindings.c would require changes. As an example:
keybindings.h: enum CommonKeyIds {..., GEANY_KEYS_SOME_COMMAND, ...}
keybindings.c: enum KeyGroups {..., KEY_GROUP_SOME, ...} enum SomeGroupOrder {..., KEYS_SOME_COMMAND, ...}
keygroups[KEY_GROUP_SOME]->keys[KEYS_SOME_COMMAND]->id = GEANY_KEYS_SOME_COMMAND;
Then keybindings_send_command() would search for that id in keygroups[x]->keys[y]. Although that's a relatively slow search algorithm, it probably wouldn't matter here. If necessary it could be optimized to use a hash/lookup table.
My first thought was the search algorithm wouldn't matter at all as we don't have that many keybindings. But indeed, we have 110 (!) keybindings. Anyway, 100 integer comparisons aren't that expensive and should be ok at least for now. As you said, it could be improved later.
Regards, Enrico
On Sun, 6 Jul 2008 20:18:25 +0200 Enrico Tröger enrico.troeger@uvena.de wrote:
On Fri, 4 Jul 2008 15:26:28 +0100, Nick Treleaven nick.treleaven@btinternet.com wrote:
keygroups[KEY_GROUP_SOME]->keys[KEYS_SOME_COMMAND]->id = GEANY_KEYS_SOME_COMMAND;
Then keybindings_send_command() would search for that id in keygroups[x]->keys[y]. Although that's a relatively slow search algorithm, it probably wouldn't matter here. If necessary it could be optimized to use a hash/lookup table.
My first thought was the search algorithm wouldn't matter at all as we don't have that many keybindings. But indeed, we have 110 (!) keybindings. Anyway, 100 integer comparisons aren't that expensive and should be ok at least for now. As you said, it could be improved later.
I think keybindings_send_command() is only used as a single call, such as to hide the sidebar in response to a menu item (e.g. file browser plugin). keybindings_got_event() would still iterate through the keybinding groups as usual.
But anyway, I hadn't really thought about the implementation of keybindings_send_command(), it can easily be done efficiently with a separate array:
key_commands[GEANY_KEYS_SOME_COMMAND] = keygroups[KEY_GROUP_SOME]->keys [KEYS_SOME_COMMAND];
I don't think these core changes require too much work, but I'll work on the GeanyEditor* changes first, as these are more important.
Regards, Nick
On Fri, 4 Jul 2008 15:08:27 +0100, Nick Treleaven nick.treleaven@btinternet.com wrote:
Hi, I'm thinking about changing the keybindings_send_command() / keybinding enums code, for two reasons: [...]
To solve this, we could have the GEANY_KEYS_ commands share a single enum, but unsorted so we can append items as needed. All the group enums could be moved into keybindings.c, renaming them so all the existing code works without changes. The only function that would need updating would be keybindings_send_command(), plus storing the GEANY_KEYS_ enum value in each keybinding struct, so that function can find the callback.
Yes, sounds fine and sounds like another bunch of much code to rewrite :D.
Regards, Enrico