I18N/L10N support for plugins

[palgarvio]

I'm opening this ticket to get some opinions on how the localization of trac plugins should be done.

There are several options:

• Merge trac catalogs and plugin catalogs at runtime, but this might be a disadvantage because while merging catalogs, if 2 of them share the same msgid, the one being merged will override the existing one, ie, the trac one, allowing plugins to introduce translation errors which might make users create new ticket's for trac and not the plugin in question.
• Make use of msgctxt. Each plugin will have it's name as it's context but this might be abusing a bit of the msgctxt. Defenition ​here.
• Make use of domains. Each plugin will have it's own domain, and all localization calls must include the domain.

Now, some more problems:

• Babel's msgctxt support is close to None currently and it's meant for 1.0, only some minor work has been done.
• Genshi's msgctxt support is non-existant mostly because Babel does not yet support it.
• Regarding the domain option, while this is easy inside python files, it won't be that easy regarding templates; perhaps a i18n:domain attribute should be added to genshi allowing a template to define it's domain and thus, allowing genshi to pass the domain when translating the message.

I'm tending towards the domain option, nothing will have to change regarding Babel and changes regarding genshi shouldn't be that much.

Also, regarding plugin developers, this would just mean using another translation function other than gettext or _ for example, and passing an extra arg; perhaps one could even do some code magic to ease this.

A new extension point would probably also be created so that plugins can register their domain and respective catalogs into trac's translator.

[palgarvio]

The above patch allows plugins to provide their own translations.

Unlike other projects which just merge catalogs together possibly overriding a good translation for a bad one, this patch "detects" if the caller of the function has a domain variable defined on top of file, if it has, it switches to that domain, translates and then switches back to trac's domain.

So, plugin developers should start defining the domain. As an example, consider the following file my_module/web_ui.py:

from trac.core import *

domain = 'my_module_messages'

class MyModuleWebUi(Component):
    def __init__(self):
        print _("Foo")

So, imagine a request to the above component comes in using pt_PT for the locale. What happens is that the pt_PT translations on MyPackage/my_module/locale are loaded, "Foo" is translated and then the trac translations are loaded back.

[palgarvio]

Oh, the only thing is that plugins must not have their catalog's domain be messages, they should be for the above example, my_module_messages.

[palgarvio]

Updated patch.

[palgarvio]

This latest patch depends on ​ticket:137.

[palgarvio]

You can now disregard the support_genshi_i18n_choose.patch​, that one would only work for python code.

The really good patch is the plugin_i18n_support.patch​.

This one also depends on ​ticket:137 and ​ticket:129, both this tickets have patches not yet applied to their sources. Those patches reached good working point and are just waiting for cmlenz to review and check them in.

Do note that with all 3 patches applied to all sources, plugin developers will then have the change to provide translations on a separate domain for their plugins. This works for both python code and genshi templates.

[cboos]

In plugin_i18n_support.patch​, I don't like so much the sys._getframe(1).f_globals.get('domain') hack.

What about this alternative, e.g.:

(in myplugin/__init__.py)

from trac.util.translation import dgettext

def _(string, **kwargs):
    return dgettext('mydomain', string, **kwargs)

(in myplugin/backend.py)

from trac.core import Component
from myplugin import _

class MyPlugin(Component):
    def __init__(self):
        self.log.info(_("MyPlugin %(version)s loaded ", version="0.1"))

Also, what's the status of the dependencies? Is using ​Babel:milestone:0.9.4 and ​Genshi:r954 (trunk) enough for experimenting with the attachment:plugin_i18n_support.patch​?

[cboos]

Looks like ​Genshi:source:branches/experimental/advanced-i18n is needed.

I had to fix a few conflicts for the patch above, due to recent changes in trunk.

The add_domain could probably benefit from some locking, as it manipulates a global, otherwise this seems risky (IIUC, that add_domain has to be called from plugins in their __init__ method).

Finally, looking at ​Babel:r448, it seems that the way to add a domain to the translations has changed since you wrote the patch.

I'm posting a work-in-progress patch which at least applies cleanly, but I have yet to try the domains with a plugin.

[cboos]

Updated patch - applies cleanly on current trunk, normal translations are working but I've not yet tested add_domain

[cboos]

Ah yes, the patch also introduce a bug by using translation.get_translations() when creating the Translator object:

trac/web/chrome.py

@@ -660 +662 @@
         """
         if not self.templates:
             def _template_loaded(template):
-                template.filters.insert(0, Translator(translation.gettext))
+                translator = Translator(translation.get_translations())
+                setup_i18n(template, translator)
 
             self.templates = TemplateLoader(self.get_all_templates_dirs(),
                                             auto_reload=self.auto_reload,

That will associate permanently the currently activated Translations object with the template as the TemplateLoader will only call the callback on first load. From then on, the template will be retrieved from the cache and the filter will keep using the original Translations object, not the currently activated one.

This can be easily verified, e.g. on the preference page itself, if you start in German, then switch to French: the translations coming from the html template will stay in german, those obtained from the code will be in french.

Suggested fix: we should instead pass a Translations "proxy" that has its translation methods delegating the call to the currently active Translations instance.

[palgarvio]

Sorry for not helping that much now, but I've started a clean install on my laptop yesterday and my development environment is not yet up and running. I also thought I had submited a reply which I'm now not seeing but I guess you did find the right branch :)

I'll test what you propose as soon as possible.

[cboos]

Version améliorée, well, improved version of the i18n support for plugin

[cboos]

In attachment:plugin_i18n_support-r7678.patch​, I did several refactorings:

• fixed the bug explained in comment:9 by introducing a TranslationsProxy class.
• removed the sys._getframe(1)... hack from the TranslationsProxy *gettext methods

Also, I tested the add_domain capability on the Mercurial plugin, along the way suggested in comment:7.

The tracext.hg.__init__.py contains the translation marker/substitution functions:

import pkg_resources

from trac.util.translation import dgettext, dtgettext, add_domain

domain = "tracmercurial"

def _(string, **kwargs):
    return dgettext(domain, string, **kwargs)

def tag_(string, **kwargs):
    return dtgettext(domain, string, **kwargs)

def N_(arg):
    return arg


def add_plugin_domain(env_path):
    locale_dir = pkg_resources.resource_filename(__name__, '../locale')
    add_domain(env_path, domain, locale_dir)

The adaptations to the tracext.hg.backend.py are minimal:

# ...

from tracext.hg import _, tag_, N_, add_plugin_domain

# only the add_plugin_domain() line is new:

class MercurialConnector(Component):

    implements(IRepositoryConnector, IWikiSyntaxProvider)

    def __init__(self):
        self._version = None
        self.ui = None
        add_plugin_domain(self.env.path)

Also the setup.cfg contains the domain spec:

[extract_messages]
add_comments = TRANSLATOR:
msgid_bugs_address = cboos@neuf.fr
output_file = tracext/locale/messages.pot
keywords = _ ngettext:1,2 N_ tag_
width = 72

[init_catalog]
input_file = tracext/locale/messages.pot
output_dir = tracext/locale
domain = tracmercurial

[compile_catalog]
directory = tracext/locale
domain = tracmercurial

[update_catalog]
input_file = tracext/locale/messages.pot
output_dir = tracext/locale
domain = tracmercurial

I have yet to test what happens for a conflict (i.e. when a translation in the tracmercurial catalog differs from one in the base trac message catalog), but other than that it seems to work fine.

[cboos]

Follow-up to comment:11

I have yet to test what happens for a conflict (i.e. when a translation in the tracmercurial catalog differs from one in the base trac message catalog),

Conflicts are handled correctly, domain translation is preferred, falling back to the main catalog if not found in the domain catalog. Nicely done on the Babel level!

Now, I came up with a helper function that simplifies the setup for domain specific translations functions.

One has simply the following to do in his/her plugin, e.g. for tracext/hg/backend.py:

from trac.util.translation import domain_functions

_, tag_, N_, add_domain = domain_functions('tracmercurial', 
    '_', 'tag_', 'N_', 'add_domain')

# use _, tag_ and N_ as usual, e.g. _("this is a diff")

class MercurialConnector(Component):

    implements(IRepositoryConnector, IWikiSyntaxProvider)

    def __init__(self):
        self._version = None
        self.ui = None
        # bind the 'tracmercurial' catalog to the specified locale directory
        locale_dir = pkg_resources.resource_filename(__name__, '../locale')
        add_domain(self.env.path, locale_dir)

There are a few things left to be done on the patch:

• add locking for add_domain (comment:8)
• add a dummy domain_functions for the case Babel is not present

See attachment:plugin_i18n_support-domain_functions.patch​

[cboos]

Same as previous patch, with a new helper function domain_functions to ease the setup of plugins.

[cboos]

As a side-note, I wanted to extract the messages using Babel 0.9.4 and Genshi's advanced-i18n branch, but then I lost several strings (e.g. from the macros.html).

Looking a bit around, I saw the uncomment below to extract messages without adding directives comment in the i18n.py Genshi filter. Can you tell us more about that? Are we supposed to add i18n:msg attributes everywhere in Trac?

In the meantime, I'm using Genshi 0.5.1 for the extraction.

[palgarvio]

Replying to cboos:

As a side-note, I wanted to extract the messages using Babel 0.9.4 and Genshi's advanced-i18n branch, but then I lost several strings (e.g. from the macros.html).

Looking a bit around, I saw the uncomment below to extract messages without adding directives comment in the i18n.py Genshi filter. Can you tell us more about that? Are we supposed to add i18n:msg attributes everywhere in Trac?

In the meantime, I'm using Genshi 0.5.1 for the extraction.

Well, when setting up Genshi I18N(on the advanced-i18n branch) one should now also install/setup the i18n directives which will handle perfectly the messages extraction, like those messages you've lost. Got any public test code to see what/how you're doin it?

You should now use genshi.filters.i18n.setup_i18n. Ie, trac should be doin' something like ​this.

[palgarvio]

Replying to cboos:

Are we supposed to add i18n:msg attributes everywhere in Trac?

Forgot to answer this one, No! Only if needed to handle html markup(<p i18n:msg="">Foo<b>Bar</b></p>) and/or to handle plurals(i18n:choose).

[cboos]

Replying to palgarvio:

… when setting up Genshi I18N(on the advanced-i18n branch) one should now also install/setup the i18n directives which will handle perfectly the messages extraction, like those messages you've lost. Got any public test code to see what/how you're doin it?

Well, I'm talking about the Trac codebase. I use latest trunk + the patch above, which seems to call setup_i18n as appropriate (but maybe not, I didn't touch that part of the original patch). That's on the usage side, though.

But I was talking about the message extraction phase and the content of the catalog, i.e. when one runs python setup.py extract_messages, with Babel 0.9.4 and Genshi's advanced-i18n branch. Then, the messages.pot file is updated, some new messages are effectively added, but also some others are lost, notably all the strings coming from trac/templates/macros.html (e.g. look for Case changes).

The tmpl.add_directives(Translator.NAMESPACE, translator) call seems to be done in extract, the Genshi plugin for Babel, so maybe it's another problem/bug at work here. Maybe you could have a look?

[palgarvio]

Replying to cboos:

But I was talking about the message extraction phase and the content of the catalog, i.e. when one runs python setup.py extract_messages, with Babel 0.9.4 and Genshi's advanced-i18n branch. Then, the messages.pot file is updated, some new messages are effectively added, but also some others are lost, notably all the strings coming from trac/templates/macros.html (e.g. look for Case changes).

The tmpl.add_directives(Translator.NAMESPACE, translator) call seems to be done in extract, the Genshi plugin for Babel, so maybe it's another problem/bug at work here. Maybe you could have a look?

I've added a testcase to genshi's advanced i18n branch(​have a look)with a code snippet grabbed from macros.html which succeeds, I'll see if I can catch the bug(lack of code/i18n setup) somehere else.

[palgarvio]

heh, failed the macros.html​ link :)

[palgarvio]

Ok, found the bug!!!

The issue happens because of the py:strip used, must be something failing on the genshi's advanced i18n branch.

Here's the ​failing testcase which reproduces the problem you noticed.

[palgarvio]

The issue is happening because currently I'm pop'ing the strip directive while extracting, something that was needed by some reason which I know don't remember now, but has something to do with the 3 failing testcases if I don't pop it ;)

I'll see what I can do regarding this issue.

[palgarvio]

Please update your genshi advanced i18n branch and see if latest changes fix the problems you found.

[cboos]

Works fine with ​Genshi:r966, thanks!

Now, any suggestion about the patch itself and domain_functions or can I finalize it as presented in comment:12?

[palgarvio]

Replying to cboos:

Works fine with ​Genshi:r966, thanks!

Now, any suggestion about the patch itself and domain_functions or can I finalize it as presented in comment:12?

Nope, no suggestions, your solution is much cleaner, go for it!

[cboos]

Finalized version committed as [7705]. Thanks pedro for doing all the ground work on this!

[rblank]

There's a problem with [7705], as it tries to import setup_i18n from genshi.filters, but this symbol is present neither in 0.5.1 nor in Genshi trunk, only in the advanced-i18n branch.

Maybe setup_i18n should be imported conditionally?

[palgarvio]

Replying to rblank:

There's a problem with [7705], as it tries to import setup_i18n from genshi.filters, but this symbol is present neither in 0.5.1 nor in Genshi trunk, only in the advanced-i18n branch.

Maybe setup_i18n should be imported conditionally?

Push cmlenz to review the branch and include those changes into trunk ;)

[palgarvio]

Replying to cboos:

Finalized version committed as [7705]. Thanks pedro for doing all the ground work on this!

Anytime!

[cboos]

Replying to rblank:

Maybe setup_i18n should be imported conditionally?

Right, done so in [7707].

[rblank]

Unfortunately, this was not enough, in the case where Babel is not installed:

• The various d(|n|t|tn)gettext() functions are missing.
• There's a typo in domain_functions().
• activate() is missing the second argument.
• There was a regression of the fix from [7480].

I fixed these in [7713]. All unit tests pass here, but it looks like Bitten is still choking on something. Ah, got it, the functools module was introduced in 2.5. Fixed in [7714].

[cboos]

Thanks for fixing the breakage, I'm focusing on the Python 2.5/Babel-0.9.4/Genshi advanced-i18n combination for now…

About partial: we have it in compat, i.e. from trac.util.compat import partial.

For the i18n ns error, I was looking in the wrong direction, thought we had to tweak something at the lxml level, I probably missed [7480].