PolicyKit currently requires translations to be in the XML, e.g.: <description>Install system color profiles</description> <description xml:lang="cs">Instalovat systémové profily barev</description> This makes it hard to modify translations after build-time and to provide only a set of translations (i.e. a language pack). It would be better to have: <description gettext-domain="gnome-color-manager">Install system color profiles</description> And use the standard gettext system.
Created attachment 37938 [details] [review] Use gettext for translations in .policy files
Created attachment 38163 [details] [review] Optionally use gettext for translations in .policy files Updated to only use gettext if gettext-domain is defined. Falls back to current behaviour if it is not.
Created attachment 38164 [details] [review] Optionally use gettext for translations in .policy files Fix accidental renaming of function parameter. Note there appears to be a copy-paste error where one function has "interactivee" instead of "locale". I had it in the original patch, I've removed it here for clarity.
Created attachment 38166 [details] [review] Optionally use gettext for translations in .policy files Uses setlocale inside _localize as it didn't appear to work at the higher level. I've confirmed this works with the current translations and gettext translation. The Ubuntu 10.10 package is running with this.
Hi, any feedback on this patch?
Ping :)
Thanks for the patch. The approach broadly makes sense, three points for now: * The documentation should be updated as well. * AFAICT _localize() is not handling the lang==NULL case correctly. * The calls to setlocale() are worrisome; we do use threads at least in the JavaScript runtime. It seems that gettext can work with per-thread locales (uselocale()), OTOH that might not be portable. (Honestly not sure about this. Colin?) (I haven’t reviewed the memory handling in detail
Comment on attachment 38166 [details] [review] Optionally use gettext for translations in .policy files Review of attachment 38166 [details] [review]: ----------------------------------------------------------------- It might be useful to also allow gettext-domain to be specified on the <policyconfig> element, to avoid repeating it on every single <description> in the file. ::: src/polkitbackend/polkitbackendactionpool.c @@ +1131,4 @@ > * _localize: > * @translations: a mapping from xml:lang to the value, e.g. 'da' -> 'Smadre', 'en_CA' -> 'Punch, Aye!' > * @untranslated: the untranslated value, e.g. 'Punch' > + * @domain: the gettext domain for this string. Make be NULL. s/Make/May/ @@ +1153,5 @@ > + { > + gchar *old_locale; > + > + old_locale = g_strdup (setlocale (LC_ALL, NULL)); > + setlocale (LC_ALL, lang); setlocale() is not thread safe. Since polkit could be being used from a thread, we *cannot* call this. We could use uselocale() instead, which only operates on the current thread’s locale. http://pubs.opengroup.org/onlinepubs/9699919799/functions/uselocale.html Unfortunately, as far as I know, there is no gettext function which allows the locale to be specified. The other approach I can think of would be to load up the gettext catalogues manually for the desired locale, but I don’t know if APIs exist for that.
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/polkit/polkit/issues/34.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.