This simplifies translations by being less redundant.
Note: label variable is not freed because `keybindings_set_item()` currently assumes static strings but for plugins, so we need not free the label right away. It's not a real problem though, as this string is required during the whole process' lifetime, so it'd use just as much memory anyway.
Closes #953.
--- I'm not certain this is a great way to fix, but it works. Opinions welcome. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/1058
-- Commit Summary --
* Use a format string instead of repeating almost the same string 9 times
-- File Changes --
M src/keybindings.c (39)
-- Patch Links --
https://github.com/geany/geany/pull/1058.patch https://github.com/geany/geany/pull/1058.diff
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1058
Another way might be like:
```c #define ADD_KB_CUSTOM_COMMAND(n, key, mod) \ add_kb(group, GEANY_KEYS_FORMAT_SENTOCMD##n, NULL, key, mod, \ "edit_sentocmd"#n, _("Send to Custom Command "#n), NULL) ```
Not sure if that works for gettext scanning or translations though. Alternatively, we could use `g_intern_string()` which will copy the given string, then we can free our formatted one, to avoid "leaking" it like:
```c #define ADD_KB_CUSTOM_COMMAND(n, key, mod) \ do { \ gchar *label = g_strdup_printf(_("Send to Custom Command %d"), n); \ add_kb(group, GEANY_KEYS_FORMAT_SENDTOCMD##n, NULL, key, mod, \ "edit_sendtocmd"#n, g_intern_string(label), NULL); \ g_free(label); \ } while(0) ```
Or just leak them...
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1058#issuecomment-224773699
Not sure it's a worthwhile improvement. It replaces one line with another (although shorter) one but adds the macro. plus having to malloc.
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1058#issuecomment-230893365
Another way might be like:
#define ADD_KB_CUSTOM_COMMAND(n, key, mod) \ add_kb(group, GEANY_KEYS_FORMAT_SENTOCMD##n, NULL, key, mod, \ "edit_sentocmd"#n, _("Send to Custom Command "#n), NULL) ```
That's not acceptable, because translators need to be able to move the number if needed.
Alternatively, we could use `g_intern_string()` which will copy the given string, then we can free our formatted one, to avoid "leaking" it
Mmyeah, not really different, but indeed.
Not sure it's a worthwhile improvement. It replaces one line with another (although shorter) one but adds the macro. plus having to malloc.
Me neither. It actually looks kinda ugly to me, that's why I PRd it for opinions. The advantage is that ther's then only 1 translation instead of 9, but the code here isn't very nice, and the translations now already exist, so…
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1058#issuecomment-230904073
:-1: for the macro :)
--- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1058#issuecomment-230942521
I don't think we should leak memory without holding a static pointer root to it, otherwise Valgrind will report it as definitely lost (rather than possibly lost). So if we want this then either `g_intern_string` or this: ```c static const gchar *labels[10]; int n;
for (n = 1; n != 10; n++) labels[n] = g_strdup_printf(_("Send to Custom Command %d"), n);
ADD_KB_CUSTOM_COMMAND(1, GDK_1, GEANY_PRIMARY_MOD_MASK); ... ``` I think the macro is not so bad if it only wraps `add_kb` and allocation is done outside it. (It's a shame there's no `g_string_chunk_insert_printf`).
github-comments@lists.geany.org