On Mon, 10 May 2010 00:12:47 +0200 (CEST), Colomban wrote:
Hi,
I just wondered why my plugin's name and description wasn't translated since every other part of my plugin was. But a little thinking made me realize it's completely "normal": of course bindtextdomain() wasn't already called for my plugin when plugin_set_info() is called, which explains it all.
And then, how to solve this? The easy and naive solution would be to call main_locale_init() in the top of plugin_set_info(), but it doesn't work since geany_functions isn't set at this time. A manual call to bindtextdomain() and friends of course solves the problem, but we lose the cool things that both main_locale_init() (simplicity, consistent Windows support, etc.) and PLUGIN_SET_INFO() brings.
I think something like PLUGIN_SET_TRANSLATABLE_INFO(localedir, package, name, description, version, author) (or any other better name) would be a solution: it would call main_locale_init() (or manually do the stuff main_locale_init() does) and then set the plugin infos. In facts, it would only need to set geany_functions & friends /before any/ plugin function call -- even plugin_set_info(). OTHO, it pollutes a bit gettext's "database" by binding (and perhaps loading?) almost useless translation domains -- since the plugin might not be loaded -- but it probably can't be avoided if we want translations at this point.
What do you think? Do you see (an)other solution(s) to fix this? Do you think it worth it (I definitely think so but still)?
I personally don't think it's strictly necessary, though of course it would be nice. But then, I personally also use en_GB as locale so I almost always get the correct string :).
The reason why I personally think it is important is both for consistency (all Geany is -- or may be -- translated), and usability: even though I think that most Geany users understands at least a bit of English, I don't think it should be a necessity. Then, proper translation of plugin description (name is less useful since it can be an arbitrary name that shouldn't be translated at all) seems somewhat "needed" to me (take it relative though). But I can easily understand that you don't think it is that important, and it is probably true that it is more an improvement than a need.
PLUGIN_SET_TRANSLATABLE_INFO is an interesting idea, are you sure that would work? I didn't test it at all, so I'm just curious. I guess the former question will make you testing it and once you had enough code to test it, you probably also will send a patch, I guess :). If so, cool. If not, also cool.
Haha, trying to delegate the work to the one that asks is always the way to go, I must admit! :D Hum, more seriously: I already tried (by using direct gettext calls) and yes -- I would say of course -- it works fine. Even better, you succeeded to make me hack Geany in order to implement it directly (see attached patch) and yes again, it works well (and don't need so much code change finally). In practice, I moved geany_functions and geany_data assignation from plugin_init() to plugin_new() for them to be set before the call to plugin_set_info(); and added the PLUGIN_SET_TRANSLATABLE_INFO() macro that calls main_locale_init() before setting the info fileds.
At least this topic raised a few times (I think to remember at least two times). Ah, just found the last discussion: http://lists.uvena.de/pipermail/geany-devel/2009-September/001281.html
Oops, I didn't even think to search the archive about this. I must confess that I didn't think this could have been raised and let apart -- and haha, I'll use the excuse that I'm not using my regular computer since it is broken for now. Yes I know, it isn't a so good excuse, but who cares? Sorry, back to pointful things.
Hum, well, I read the thread, and it raises the same thinks that I thought about: 1. Is this important? 2. How to implement this? 3. What are the impacts?
To 1, you and I already answered our POV, and Nick seems to think it is -- as far as I can read in the thread.
To 2, I think we would all agree that simply calling main_locale_init() in plugin_set_info() is probably the best solution. I think the idea of Eugene to add a plugin_loaded() plugin function is quite elegant, but I also think it is completely overkill for this -- and I don't see why a plugin would use it apart i18n initialization.
3 is the more complicated and probably controversial point:
As raised in the mentioned thread, the biggest point is that it would load the entire translation catalogue for each plugin, even if only the the name and descriptions have to be translated when the plugin isn't loaded. I see no obvious way to address this. The only way I see would be to have a separate catalogue for this, but I don't think it is a good idea: it would be harder to set up (two catalogues need manual calls to dgettext() and fine build system tweaking) and it may even be a worst solution if the plugin gets loaded finally. I personally think that the catalogues are probably not that big. Geany's French translation file is about 100kb, and my plugins' French translation file is about 7kb. Even without benchmarking actual memory usage, I think we can safely say that 100 (!) plugins would probably not use 1mo for translations. I might then say that it's probably acceptable. Moreover, with geany-plugins plugins it is even less a problem since they share their catalogue, which means that it is loaded anyway if one of the plugin is loaded.
The other thing is allowing plugin_set_info() to call Geany functions, and then have both geany_function and geany_data set. I'm pretty sure this doesn't change anything for memory usage (only 2 pointers are set in order to point to Geany's internal data), and I think that the computing overhead (two module lookups, tests and pointer affectations) is more than reasonable.
Eugene also suggested a way -- he qualified ugly -- to integrate this to the current API, based on checking if LOCALEDIR and GETTEXT_PACKAGE are set. I don't think this is that ugly: * Most plugins with translations probably already use these constants names since they are quite conventional; * If these constants are defined, it is very unlikely the plugin won't setup i18n. The only objection I see is if somebody won't use main_locale_init() but some manual stuff; e.g. if the main_locale_init() defaults don't fit her needs, but I don't see why it would be so.
And finally, on the thread I see that you (global) say that when the plugin gets loaded, the name and description are translated? I don't see this, and I must say that I don't see how this would be possible without re-setting plugin infos after setting up i18n. The only way I see for this to work would be that gettext takes care to bind the translation to the same pointer it would return if no catalogue is loaded. Hum, I don't think this is reasonably implementable or would be implemented by reasonable peoples :-' But of course I can be wrong, indeed if it actually happens (with some plugins?).
PS: Ah, and on a similar topic, why does main_locale_init() call textdomain()? I think it is useless (and probably pointless) since it
To be honest: because I found it somewhere else and used it as well. This happened as one of the first steps of Geany's lifetime long time ago when I had a lot less experience. And since then, I just didn't touch the code anymore since it worked :).
Oh, ok, fine then. But it can probably be removed the next time you touch the function -- even though it is harmless.
Regards, Colomban