In response to Frédéric's comment on bug 15206 (http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15206#c33) I wanted to see what could be done to handle translation of plural in Koha. This is my attempt to manage translation in both Perl code and templates
Created attachment 45797 [details] [review] Bug 15395: Allow correct handling of plural translation Locale::Maketext does not allow correct handling of plural translation for languages that have more than one plural forms. Locale::Messages does. So Koha::I18N is now a wrapper around Locale::Messages, just like Locale::TextDomain, and export the same symbols as Locale::TextDomain. You can refer to documentation of Locale::TextDomain to know how to use exported subroutines. The PO file moves from misc/translator/po/xx-XX-messages.po to misc/translator/po/xx_XX/LC_MESSAGES/Koha.po and now needs to be compiled to MO in order to be used by Koha. Compilation of PO file is done by running: ./translate install xx-XX Remove dependency to Locale::Maketext and Locale::Maketext::Lexicon Add dependency to Locale::Messages Test plan: 1. Open a .pl script or .pm module with your favorite text editor 2. Add 'use Koha::I18N;' in the beginning of file 3. Use one of the subroutines exported by Koha::I18N and be sure to have a way to visualize the result (pass result to the template for example, or simply warn and watch the log file) 4. cd misc/translator && ./translate update fr-FR # try other languages 5. Open misc/translator/po/fr_FR/LC_MESSAGES/Koha.po and translate your string(s) You will need to change the "Plural-Forms" header. See https://localization-guide.readthedocs.org/en/latest/l10n/pluralforms.html 6. ./translate install fr-FR 7. Use your web browser to go to the page that should display the translation, change language and verify the translation is correct Example usage: __("Hi") __x("Hi {name}", name => 'Bob') __n("item", "items", $num_items) __nx("one item", "{count} items", $num_items, count => $num_items) __p("Bibliographic record", "item")
Created attachment 45798 [details] [review] Bug 15395: Plural translations in templates Provides a way to handle translation of plural forms in templates Add Template::Toolkit plugin for Koha::I18N Use like this: [%# USE the plugin and define some macros %] [% PROCESS 'i18n.inc' %] [%# tn is the equivalent of __n %] [%# macro names can't start with underscore, t is for "translate" %] [% tn('item', 'items', num_items) %] Extraction of strings from templates is a bit complicated and use Template::Parser and PPI. Template is compiled into Perl code and then analyzed by PPI. It is slow, but should be correct even with complex constructions. Add dependency to PPI Test plan: 1. Open a template file (.tt or .inc) with your favorite text editor 2. Add the PROCESS directive mentioned above in the beginning of file 3. Use one of the t* macros defined in i18n.inc. They are used like their "__" equivalent, with one difference: the 'x' variants take a hashref instead of a hash as last parameter 4. cd misc/translator && ./translate update fr-FR 5. Open misc/translator/po/fr_FR/LC_MESSAGES/Koha.po and translate your string(s) 6. ./translate install fr-FR 7. Use your web browser to go to the page that should display the translation, change language and verify the translation is correct
Created attachment 45799 [details] [review] Bug 15395: Example usage of I18N Template::Toolkit plugin
Created attachment 46337 [details] [review] Bug 15395: Add plural translation capabilities to JS files It adds Javascript equivalent of Koha::I18N's exported subroutines, and they are used the same way. String extraction is done only on *.js files and require gettext 0.19 (available in Debian stable, and also in wheezy-backports) It adds Javascript library Gettext.js for handling translation and a Perl script po2json to transform PO file into JSON. Gettext.js and po2json both come from Locale::Simple. There are several tools named po2json. It's simpler to integrate this one into Koha than to check if the good one is installed on the system. Locale::Simple is not needed. To avoid polluting the global namespace too much, this patch also introduce a global JS object named Koha and add some stuff in Koha.i18n Test plan: 1. Add a translatable string in a JS file. For example, add this: alert(__nx("There is one item", "There are {count} items", 3, {count: 3})); to staff-global.js 2. cd misc/translator && ./translate update fr-FR 3. Open misc/translator/po/fr_FR/LC_MESSAGES/Koha.po, verify that your string is present, and translate it 4. cd misc/translator && ./translate install fr-FR 5. (Optional) Verify that koha-tmpl/intranet-tmpl/prog/fr-FR/js/locale_data.js exists and contains your translation 6. Open your browser on the staff main page, change language and verify that the message is translated 7. Repeat 1-6 on OPAC side
Applying: Bug 15395: Example usage of I18N Template::Toolkit plugin Using index info to reconstruct a base tree... M C4/Search.pm M koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt
Created attachment 50023 [details] [review] Bug 15395: Allow correct handling of plural translation Locale::Maketext does not allow correct handling of plural translation for languages that have more than one plural forms. Locale::Messages does. So Koha::I18N is now a wrapper around Locale::Messages, just like Locale::TextDomain, and export the same symbols as Locale::TextDomain. You can refer to documentation of Locale::TextDomain to know how to use exported subroutines. The PO file moves from misc/translator/po/xx-XX-messages.po to misc/translator/po/xx_XX/LC_MESSAGES/Koha.po and now needs to be compiled to MO in order to be used by Koha. Compilation of PO file is done by running: ./translate install xx-XX Remove dependency to Locale::Maketext and Locale::Maketext::Lexicon Add dependency to Locale::Messages Test plan: 1. Open a .pl script or .pm module with your favorite text editor 2. Add 'use Koha::I18N;' in the beginning of file 3. Use one of the subroutines exported by Koha::I18N and be sure to have a way to visualize the result (pass result to the template for example, or simply warn and watch the log file) 4. cd misc/translator && ./translate update fr-FR # try other languages 5. Open misc/translator/po/fr_FR/LC_MESSAGES/Koha.po and translate your string(s) You will need to change the "Plural-Forms" header. See https://localization-guide.readthedocs.org/en/latest/l10n/pluralforms.html 6. ./translate install fr-FR 7. Use your web browser to go to the page that should display the translation, change language and verify the translation is correct Example usage: __("Hi") __x("Hi {name}", name => 'Bob') __n("item", "items", $num_items) __nx("one item", "{count} items", $num_items, count => $num_items) __p("Bibliographic record", "item")
Created attachment 50024 [details] [review] Bug 15395: Plural translations in templates Provides a way to handle translation of plural forms in templates Add Template::Toolkit plugin for Koha::I18N Use like this: [%# USE the plugin and define some macros %] [% PROCESS 'i18n.inc' %] [%# tn is the equivalent of __n %] [%# macro names can't start with underscore, t is for "translate" %] [% tn('item', 'items', num_items) %] Extraction of strings from templates is a bit complicated and use Template::Parser and PPI. Template is compiled into Perl code and then analyzed by PPI. It is slow, but should be correct even with complex constructions. Add dependency to PPI Test plan: 1. Open a template file (.tt or .inc) with your favorite text editor 2. Add the PROCESS directive mentioned above in the beginning of file 3. Use one of the t* macros defined in i18n.inc. They are used like their "__" equivalent, with one difference: the 'x' variants take a hashref instead of a hash as last parameter 4. cd misc/translator && ./translate update fr-FR 5. Open misc/translator/po/fr_FR/LC_MESSAGES/Koha.po and translate your string(s) 6. ./translate install fr-FR 7. Use your web browser to go to the page that should display the translation, change language and verify the translation is correct
Created attachment 50025 [details] [review] Bug 15395: Example usage of I18N Template::Toolkit plugin
Created attachment 50026 [details] [review] Bug 15395: Add plural translation capabilities to JS files It adds Javascript equivalent of Koha::I18N's exported subroutines, and they are used the same way. String extraction is done only on *.js files and require gettext 0.19 (available in Debian stable, and also in wheezy-backports) It adds Javascript library Gettext.js for handling translation and a Perl script po2json to transform PO file into JSON. Gettext.js and po2json both come from Locale::Simple. There are several tools named po2json. It's simpler to integrate this one into Koha than to check if the good one is installed on the system. Locale::Simple is not needed. To avoid polluting the global namespace too much, this patch also introduce a global JS object named Koha and add some stuff in Koha.i18n Test plan: 1. Add a translatable string in a JS file. For example, add this: alert(__nx("There is one item", "There are {count} items", 3, {count: 3})); to staff-global.js 2. cd misc/translator && ./translate update fr-FR 3. Open misc/translator/po/fr_FR/LC_MESSAGES/Koha.po, verify that your string is present, and translate it 4. cd misc/translator && ./translate install fr-FR 5. (Optional) Verify that koha-tmpl/intranet-tmpl/prog/fr-FR/js/locale_data.js exists and contains your translation 6. Open your browser on the staff main page, change language and verify that the message is translated 7. Repeat 1-6 on OPAC side
Patches rebased on master
./translate update fr-FR raises this error: system call failed: /usr/bin/xgettext -L JavaScript
(In reply to Frédéric Demians from comment #11) > ./translate update fr-FR raises this error: > > system call failed: /usr/bin/xgettext -L JavaScript This error was due to a too old version of gettext, 0.19 is needed
(In reply to Julian Maurice from comment #12) > This error was due to a too old version of gettext, 0.19 is needed Yes. I can confirm that the whole set of patches works with the proper gettext version.
(In reply to Frédéric Demians from comment #13) > Yes. I can confirm that the whole set of patches works with the proper > gettext version. Hi Frédéric, Did you follow all 3 test plans ? If so, could you signoff these patches ?
I'm not over-confident with gettext.js. It seems quiet old and unmaintained. Have you ever considered this: https://slexaxton.github.io/Jed/ I'd be also interested in feedback from Bernardo, since this patch will modify the translation workflow.
Patch does no longer apply. How would disambiguation work? e.g. for the 3 meanings of "Title" mentioned in https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11285#c25
Created attachment 61873 [details] [review] Bug 15395: Allow correct handling of plural translation Locale::Maketext does not allow correct handling of plural translation for languages that have more than one plural forms. Locale::Messages does. So Koha::I18N is now a wrapper around Locale::Messages, just like Locale::TextDomain, and export the same symbols as Locale::TextDomain. You can refer to documentation of Locale::TextDomain to know how to use exported subroutines. The PO file moves from misc/translator/po/xx-XX-messages.po to misc/translator/po/xx_XX/LC_MESSAGES/Koha.po and now needs to be compiled to MO in order to be used by Koha. Compilation of PO file is done by running: ./translate install xx-XX Remove dependency to Locale::Maketext and Locale::Maketext::Lexicon Add dependency to Locale::Messages Test plan: 1. Open a .pl script or .pm module with your favorite text editor 2. Add 'use Koha::I18N;' in the beginning of file 3. Use one of the subroutines exported by Koha::I18N and be sure to have a way to visualize the result (pass result to the template for example, or simply warn and watch the log file) 4. cd misc/translator && ./translate update fr-FR # try other languages 5. Open misc/translator/po/fr_FR/LC_MESSAGES/Koha.po and translate your string(s) You will need to change the "Plural-Forms" header. See https://localization-guide.readthedocs.org/en/latest/l10n/pluralforms.html 6. ./translate install fr-FR 7. Use your web browser to go to the page that should display the translation, change language and verify the translation is correct Example usage: __("Hi") __x("Hi {name}", name => 'Bob') __n("item", "items", $num_items) __nx("one item", "{count} items", $num_items, count => $num_items) __p("Bibliographic record", "item")
Created attachment 61874 [details] [review] Bug 15395: Plural translations in templates Provides a way to handle translation of plural forms in templates Add Template::Toolkit plugin for Koha::I18N Use like this: [%# USE the plugin and define some macros %] [% PROCESS 'i18n.inc' %] [%# tn is the equivalent of __n %] [%# macro names can't start with underscore, t is for "translate" %] [% tn('item', 'items', num_items) %] Extraction of strings from templates is a bit complicated and use Template::Parser and PPI. Template is compiled into Perl code and then analyzed by PPI. It is slow, but should be correct even with complex constructions. Add dependency to PPI Test plan: 1. Open a template file (.tt or .inc) with your favorite text editor 2. Add the PROCESS directive mentioned above in the beginning of file 3. Use one of the t* macros defined in i18n.inc. They are used like their "__" equivalent, with one difference: the 'x' variants take a hashref instead of a hash as last parameter 4. cd misc/translator && ./translate update fr-FR 5. Open misc/translator/po/fr_FR/LC_MESSAGES/Koha.po and translate your string(s) 6. ./translate install fr-FR 7. Use your web browser to go to the page that should display the translation, change language and verify the translation is correct
Created attachment 61875 [details] [review] Bug 15395: Example usage of I18N Template::Toolkit plugin
Created attachment 61876 [details] [review] Bug 15395: Add plural translation capabilities to JS files It adds Javascript equivalent of Koha::I18N's exported subroutines, and they are used the same way. String extraction is done only on *.js files and require gettext 0.19 (available in Debian stable, and also in wheezy-backports) It adds Javascript library Gettext.js for handling translation and a Perl script po2json to transform PO file into JSON. Gettext.js and po2json both come from Locale::Simple. There are several tools named po2json. It's simpler to integrate this one into Koha than to check if the good one is installed on the system. Locale::Simple is not needed. To avoid polluting the global namespace too much, this patch also introduce a global JS object named Koha and add some stuff in Koha.i18n Test plan: 1. Add a translatable string in a JS file. For example, add this: alert(__nx("There is one item", "There are {count} items", 3, {count: 3})); to staff-global.js 2. cd misc/translator && ./translate update fr-FR 3. Open misc/translator/po/fr_FR/LC_MESSAGES/Koha.po, verify that your string is present, and translate it 4. cd misc/translator && ./translate install fr-FR 5. (Optional) Verify that koha-tmpl/intranet-tmpl/prog/fr-FR/js/locale_data.js exists and contains your translation 6. Open your browser on the staff main page, change language and verify that the message is translated 7. Repeat 1-6 on OPAC side
(In reply to Marc Véron from comment #16) > Patch does no longer apply. > > How would disambiguation work? > > e.g. for the 3 meanings of "Title" mentioned in > https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11285#c25 To attach context to a string, you have to use one of the 'p' variants (__p, __px, __np, __npx). This context will appear in the PO file to help the translator and gettext will use this information to choose the right translation https://www.gnu.org/software/gettext/manual/html_node/Contexts.html Eg. __p('Patron', 'Title') __p('Bibliographic record', 'Title')
I tried to test "Example usage of I18N Template::Toolkit plugin" , using the test plan from comment #7 After perl translate update fr-FR, I edited: koha/misc/translator/po/fr_FR/LC_MESSAGES/Koha.po translated the strings and changed the line: "Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\n" to: "Plural-Forms: nplurals=2; plural=(n > 1);\n" However, perl translate install fr-FR complains with following warnings: Use of uninitialized value $pref_name in concatenation (.) or string at LangInstaller.pm line 202. Use of uninitialized value $pref_name in concatenation (.) or string at LangInstaller.pm line 244. NO PLURAL FORM HEADER FOUND - DEFAULTING TO 2 print() on closed filehandle $fh at LangInstaller.pm line 689. What did I miss?
Created attachment 61934 [details] [review] Bug 15395: Exclude xslt from the list of themes when building locale_data.js
Created attachment 61935 [details] [review] Bug 15395: Fix PO header reading in po2json Since version 0.24 Locale::PO do not escape newline characters in string returned by dequote() This patch set the required version of Locale::PO to 0.24 and fix po2json https://rt.cpan.org/Public/Bug/Display.html?id=96016
(In reply to Marc Véron from comment #22) > NO PLURAL FORM HEADER FOUND - DEFAULTING TO 2 This was caused by an incompatibility between po2json and the newer version of Locale::PO. This is fixed in the last patch.
Hi Julian, I can not yet make it work. I think I missed something perl translate install fr-FR Use of uninitialized value $pref_name in concatenation (.) or string at LangInstaller.pm line 202. Use of uninitialized value $pref_name in concatenation (.) or string at LangInstaller.pm line 244. SKIPPING ERROR MARKER IN HEADER: #-#-#-#-# Koha-perl.pot (PACKAGE VERSION) #-#-#-#-# SKIPPING ERROR MARKER IN HEADER: #-#-#-#-# Koha-js.pot (PACKAGE VERSION) #-#-#-#-# Vales are not replaced in .tt file: [% SEARCH_RESULT.items_count %] [% tn('item', 'items', SEARCH_RESULT.items_count) %](...) I would expect: [% SEARCH_RESULT.items_count %] [% tn('exemplaire', 'exemplaires', SEARCH_RESULT.items_count) %](...) This is the .po file after editing Plural forms: ------------ # FIRST AUTHOR <EMAIL@ADDRESS>, YEAR. # #, fuzzy msgid "" msgstr "" "Project-Id-Version: PACKAGE VERSION\n" "Report-Msgid-Bugs-To: \n" "POT-Creation-Date: 2017-04-06 10:52+0200\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" "Language-Team: LANGUAGE <LL@li.org>\n" "Language: \n" "MIME-Version: 1.0\n" "Content-Type: text/plain; charset=UTF-8\n" "Content-Transfer-Encoding: 8bit\n" "#-#-#-#-# Koha-perl.pot (PACKAGE VERSION) #-#-#-#-#\n" "Plural-Forms: nplurals=2; plural=(n > 1);\n" "#-#-#-#-# Koha-js.pot (PACKAGE VERSION) #-#-#-#-#\n" #: koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt:1 msgid "item" msgid_plural "items" msgstr[0] "exemplaire" msgstr[1] "exemplaires" ---------------
(In reply to Marc Véron from comment #26) > Hi Julian, I can not yet make it work. I think I missed something > > perl translate install fr-FR > Use of uninitialized value $pref_name in concatenation (.) or string at > LangInstaller.pm line 202. > Use of uninitialized value $pref_name in concatenation (.) or string at > LangInstaller.pm line 244. > SKIPPING ERROR MARKER IN HEADER: #-#-#-#-# Koha-perl.pot (PACKAGE VERSION) > #-#-#-#-# > SKIPPING ERROR MARKER IN HEADER: #-#-#-#-# Koha-js.pot (PACKAGE VERSION) > #-#-#-#-# I also have these messages, but they are only warnings, we can fix them later > > Vales are not replaced in .tt file: > [% SEARCH_RESULT.items_count %] [% tn('item', 'items', > SEARCH_RESULT.items_count) %](...) > I would expect: > [% SEARCH_RESULT.items_count %] [% tn('exemplaire', 'exemplaires', > SEARCH_RESULT.items_count) %](...) > This result is expected. Messages are not translated directly in template files, but tn() will return the correct translation at runtime.
Created attachment 61977 [details] [review] Bug 15395: Fix output of msgcat By default, msgcat tries to merge translations when two or more files have the same msgid, even for the header (msgid ""). This resulted in an invalid PO file ("#-#-#-#-#" markers in header). This patch tells msgcat to use the first available translation instead. Also, this patch set correctly the Project-Id-Version field
Created attachment 62008 [details] [review] Bug 15395: Make Koha::I18N work with Plack + add tests Language detection was done in a BEGIN block so it was not working correctly with Plack. This is now done when the first translation is requested. This patch also fixes a bug when set_locale(LC_ALL, ...) fails for some reasons (locale is not installed system-wide for example). And finally, add some unit tests Test plan: 1. prove t/Koha/I18N.t 2. Replay the previous test plans with plack enabled
Hi Julian I tried again with the tt example from patch 'Bug 15395: Example usage of I18N Template::Toolkit plugin' Followed the test plan (with fr-FR and de-DE), the translations for item/items showed up as expected (exemplaire/exemplaires and Exemplar/Exemplare). Great! However it seems that the second part of line 630 in results.tt is no longer picked for translation or does not translate properly (n available rsp. None available). I have the impression that the rest of the line after the plural stuff is skipped.
(In reply to Marc Véron from comment #30) > However it seems that the second part of line 630 in results.tt is no longer > picked for translation or does not translate properly (n available rsp. None > available). I have the impression that the rest of the line after the plural > stuff is skipped. This is expected, quite annoying, but expected. Due to the way the template strings extractor works (not the new one brought by this bug but the current one), almost every change in a template will result in a change in the PO file. For instance, msgid "%s %s %sitems%sitem%s%s, %s available:%s, None available%s %s " will be changed to msgid "%s %s %s%s, %s available:%s, None available%s %s " and will be marked as "fuzzy" so it will not be used. But you can fix the translation, remove the fuzzy flag, and run ./translate install. Text will be translated again.
Ah, got it, thanks for the explanation :-)
Hmm, I can not make js files translate. What I did: Change staff-global.js as suggested in comment #9 function openHelp(){ alert(__nx("There is one thing", "There are {count} things", 3, {count: 3})); The alert shows up fine in English version (Hit the 'Help' link) However after translate update, I find nothing in Koha.po (only the strings from the tt example)
Ah, you found a bug! :-) Actually the string extractor only search in koha-tmpl/**/en/ to avoid picking strings from already translated files (when I wrote the test plan, staff-global.js was inside /en/ I think). I guess I need to refine the file search to also search in koha-tmpl/intranet-tmpl/prog/js and koha-tmpl/opac-tmpl/bootstrap/js (anything else ?). Anyway, to test it right now you could modify a .js file in the /en/ directory
Created attachment 62041 [details] [review] Bug 15395: Search .js files in more directories Specifically, koha-tmpl/intranet-tmpl/prog/js and koha-tmpl/opac-tmpl/bootstrap/js
OK, I tested with 3 files: koha-tmpl/intranet-tmpl/prog/en/js/categories.js koha-tmpl/intranet-tmpl/prog/js/staff-global.js koha-tmpl/opac-tmpl/bootstrap/js/global.js All files were picked up in Koha.po I translated them and did a translate install Example for JavaScript used (added as line 1 of koha-tmpl/intranet-tmpl/prog/en/js/categories.js): alert(__nx("There is one category", "There are {count} categories", 4, {count: 4})); Translation in misc/translator/po/de_DE/LC_MESSAGES/Koha.po: #: koha-tmpl/intranet-tmpl/prog/en/js/categories.js:1 msgid "There is one category" msgid_plural "There are {count} categories" msgstr[0] "Es gibt {count} Kategorien" msgstr[1] "Es gibt nur eine Kategorie..." English page displays "There are 4 categories" (OK) German page displays "Es gibt nur eine Kategorie..." (WRONG)
(In reply to Marc Véron from comment #36) > German page displays "Es gibt nur eine Kategorie..." (WRONG) It depends on how you configured the Plural-Forms header, but by convention, msgstr[0] is the singular form and msgstr[1] is the plural form, so it looks ok to me
Oh, reading helps...
Created attachment 62326 [details] [review] Bug 15395: Allow correct handling of plural translation Locale::Maketext does not allow correct handling of plural translation for languages that have more than one plural forms. Locale::Messages does. So Koha::I18N is now a wrapper around Locale::Messages, just like Locale::TextDomain, and export the same symbols as Locale::TextDomain. You can refer to documentation of Locale::TextDomain to know how to use exported subroutines. The PO file moves from misc/translator/po/xx-XX-messages.po to misc/translator/po/xx_XX/LC_MESSAGES/Koha.po and now needs to be compiled to MO in order to be used by Koha. Compilation of PO file is done by running: ./translate install xx-XX Remove dependency to Locale::Maketext and Locale::Maketext::Lexicon Add dependency to Locale::Messages Test plan: 1. Open a .pl script or .pm module with your favorite text editor 2. Add 'use Koha::I18N;' in the beginning of file 3. Use one of the subroutines exported by Koha::I18N and be sure to have a way to visualize the result (pass result to the template for example, or simply warn and watch the log file) 4. cd misc/translator && ./translate update fr-FR # try other languages 5. Open misc/translator/po/fr_FR/LC_MESSAGES/Koha.po and translate your string(s) You will need to change the "Plural-Forms" header. See https://localization-guide.readthedocs.org/en/latest/l10n/pluralforms.html 6. ./translate install fr-FR 7. Use your web browser to go to the page that should display the translation, change language and verify the translation is correct Example usage: __("Hi") __x("Hi {name}", name => 'Bob') __n("item", "items", $num_items) __nx("one item", "{count} items", $num_items, count => $num_items) __p("Bibliographic record", "item") Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 62327 [details] [review] Bug 15395: Plural translations in templates Provides a way to handle translation of plural forms in templates Add Template::Toolkit plugin for Koha::I18N Use like this: [%# USE the plugin and define some macros %] [% PROCESS 'i18n.inc' %] [%# tn is the equivalent of __n %] [%# macro names can't start with underscore, t is for "translate" %] [% tn('item', 'items', num_items) %] Extraction of strings from templates is a bit complicated and use Template::Parser and PPI. Template is compiled into Perl code and then analyzed by PPI. It is slow, but should be correct even with complex constructions. Add dependency to PPI Test plan: 1. Open a template file (.tt or .inc) with your favorite text editor 2. Add the PROCESS directive mentioned above in the beginning of file 3. Use one of the t* macros defined in i18n.inc. They are used like their "__" equivalent, with one difference: the 'x' variants take a hashref instead of a hash as last parameter 4. cd misc/translator && ./translate update fr-FR 5. Open misc/translator/po/fr_FR/LC_MESSAGES/Koha.po and translate your string(s) 6. ./translate install fr-FR 7. Use your web browser to go to the page that should display the translation, change language and verify the translation is correct Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 62328 [details] [review] Bug 15395: Add plural translation capabilities to JS files It adds Javascript equivalent of Koha::I18N's exported subroutines, and they are used the same way. String extraction is done only on *.js files and require gettext 0.19 (available in Debian stable, and also in wheezy-backports) It adds Javascript library Gettext.js for handling translation and a Perl script po2json to transform PO file into JSON. Gettext.js and po2json both come from Locale::Simple. There are several tools named po2json. It's simpler to integrate this one into Koha than to check if the good one is installed on the system. Locale::Simple is not needed. To avoid polluting the global namespace too much, this patch also introduce a global JS object named Koha and add some stuff in Koha.i18n Test plan: 1. Add a translatable string in a JS file. For example, add this: alert(__nx("There is one item", "There are {count} items", 3, {count: 3})); to staff-global.js 2. cd misc/translator && ./translate update fr-FR 3. Open misc/translator/po/fr_FR/LC_MESSAGES/Koha.po, verify that your string is present, and translate it 4. cd misc/translator && ./translate install fr-FR 5. (Optional) Verify that koha-tmpl/intranet-tmpl/prog/fr-FR/js/locale_data.js exists and contains your translation 6. Open your browser on the staff main page, change language and verify that the message is translated 7. Repeat 1-6 on OPAC side Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 62329 [details] [review] Bug 15395: Exclude xslt from the list of themes when building locale_data.js Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 62330 [details] [review] Bug 15395: Fix PO header reading in po2json Since version 0.24 Locale::PO do not escape newline characters in string returned by dequote() This patch set the required version of Locale::PO to 0.24 and fix po2json https://rt.cpan.org/Public/Bug/Display.html?id=96016 Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 62331 [details] [review] Bug 15395: Fix output of msgcat By default, msgcat tries to merge translations when two or more files have the same msgid, even for the header (msgid ""). This resulted in an invalid PO file ("#-#-#-#-#" markers in header). This patch tells msgcat to use the first available translation instead. Also, this patch set correctly the Project-Id-Version field Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 62332 [details] [review] Bug 15395: Make Koha::I18N work with Plack + add tests Language detection was done in a BEGIN block so it was not working correctly with Plack. This is now done when the first translation is requested. This patch also fixes a bug when set_locale(LC_ALL, ...) fails for some reasons (locale is not installed system-wide for example). And finally, add some unit tests Test plan: 1. prove t/Koha/I18N.t 2. Replay the previous test plans with plack enabled Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 62333 [details] [review] Bug 15395: Search .js files in more directories Specifically, koha-tmpl/intranet-tmpl/prog/js and koha-tmpl/opac-tmpl/bootstrap/js Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 62335 [details] [review] Bug 15395: Example usage of I18N Template::Toolkit plugin Moved this patch for example usage to teh end of patch set and resolved minor conflict while applyin Signed-off-by: Marc Véron <veron@veron.ch>
Thanks for testing Marc! Have you also tested translation with context (regarding bug 11285) ?
(In reply to Julian Maurice from comment #48) > Thanks for testing Marc! > Have you also tested translation with context (regarding bug 11285) ? Just tested: - Inserted the following in a template file: <p>AAAA: [% tp("Biblio","Title") %]</p> <p>BBBB: [% tp("Person","Title") %]</p> - translate update de-DE - Edit misc/translator/po/de_DE/LC_MESSAGES/Koha.po #: koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt:1 msgctxt "Biblio" msgid "Title" msgstr "Überschrift" #: koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt:2 msgctxt "Person" msgid "Title" msgstr "Anrede" - translate install de-DE Went to page, displys in English: AAAA: Title BBBB: Title ...and in German (de-DE): AAAA: Überschrift BBBB: Anrede Works great!
[kohadev-koha@kohadevbox:/home/vagrant/kohaclone/misc/translator (bug_15395) ✭]% perl translate install fr-FR /bin/cp: cannot stat ‘Koha.pot’: No such file or directory /usr/bin/msgfmt: error while opening "/home/vagrant/kohaclone/misc/translator/po/fr_FR/LC_MESSAGES/Koha.po" for reading: No such file or directory /home/vagrant/kohaclone/misc/translator/po2json {-p} {file.po} > {outputfile.json} -p : do pretty-printing of json data What did I miss?
(In reply to Jonathan Druart from comment #50) > What did I miss? For now you need to run `translate update fr-FR` first to create the PO file, then you need to change the Plural-Forms header, and only after that you can run `translate install` to compile the PO into MO. I think we can avoid the manual step of setting Plural-Forms by using msginit. I'll give it a try.
Created attachment 62709 [details] [review] Bug 15395: Fix several issues in translation process - Change location of generated PO files to ease its integration into Pootle (po/xx-XX-messages.po instead of xx_XX/LC_MESSAGES/Koha.po) - Use msginit for PO creation to try to set correctly Plural-Forms header. If it fails, set a default one - 'install' command now creates the PO file if it doesn't already exist - Fix locale name when language code is different from ll-CC (where ll is the language code and CC is the country code). Test plan: 1. `perl translate install ar-Arab` should results in po/ar/LC_MESSAGES/Koha.mo and po/ar-Arab-messages.po 2. `perl translate install gl` should results in po/gl/LC_MESSAGES/Koha.mo and po/gl-messages.po 3. `perl translate install zh-Hans-CN` should results in po/zh_CN/LC_MESSAGES/Koha.mo and po/zh-Hans-CN-messages.po 4. `perl translate install fr-FR` should results in po/fr_FR/LC_MESSAGES/Koha.mo and po/fr-FR-messages.po
This patchset includes a new perl dep: Locale::Util. You call set_locale two times. I am not sure if it is packaged for Debian; could not find it so quickly. Should be added to PerlDependencies? Also do we really need it? Can we use POSIX instead, or do without it, construct it some other way ?? Glancing through CPAN, I note: It is not the recommended way to set the program locale for a regular application. Changing status to reflect need for feedback.
Regarding packages, I found the following: > $ apt-file search Locale::Util > libdatetime-locale-perl: /usr/share/man/man3/DateTime::Locale::Util.3pm.gz > libintl-perl: /usr/share/man/man3/Locale::Util.3pm.gz > libsharyanto-utils-perl: /usr/share/man/man3/SHARYANTO::Locale::Util.3pm.gz > > $ rmadison libintl-perl > debian: > libintl-perl | 1.20-1 | squeeze | source, all > libintl-perl | 1.20-1 | wheezy | source, all > libintl-perl | 1.23-1 | jessie-kfreebsd | source, all > libintl-perl | 1.23-1+deb8u1 | jessie-kfreebsd-security | source, all > libintl-perl | 1.23-1+deb8u1 | jessie-security | source, all > libintl-perl | 1.23-1+deb8u1 | jessie | source, all > libintl-perl | 1.26-2 | stretch | source, all > libintl-perl | 1.26-2 | buster | source, all > libintl-perl | 1.26-2 | sid | source, all If that is the correct package and the version in Jessie is ok, I don't see a problem. This is only about availability of the dependency, not about the other questions Marcel brought up.
(In reply to Marcel de Rooy from comment #53) > Also do we really need it? Can we use POSIX instead, or do without it, > construct it some other way ?? Glancing through CPAN, I note: It is not the > recommended way to set the program locale for a regular application. When I searched for the recommended way to set the program locale, it appeared to be with Locale::Util's set_locale. But after playing with this patchset a little, I noticed some small differences between using a locale that is installed system-wide and using a locale that is not installed (when set_locale returns false). So it might be better to not use set_locale or POSIX's setlocale, and just set the environment variables LANGUAGE, LANG and OUTPUT_CHARSET (never use the system locale, even if installed)
Created attachment 65894 [details] [review] Bug 15395: Remove the use of Locale::Util's set_locale
Created attachment 70974 [details] [review] Bug 15395: Example usage of I18N Template::Toolkit plugin Moved this patch for example usage to teh end of patch set and resolved minor conflict while applyin Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 70975 [details] [review] Bug 15395: Fix several issues in translation process - Change location of generated PO files to ease its integration into Pootle (po/xx-XX-messages.po instead of xx_XX/LC_MESSAGES/Koha.po) - Use msginit for PO creation to try to set correctly Plural-Forms header. If it fails, set a default one - 'install' command now creates the PO file if it doesn't already exist - Fix locale name when language code is different from ll-CC (where ll is the language code and CC is the country code). Test plan: 1. `perl translate install ar-Arab` should results in po/ar/LC_MESSAGES/Koha.mo and po/ar-Arab-messages.po 2. `perl translate install gl` should results in po/gl/LC_MESSAGES/Koha.mo and po/gl-messages.po 3. `perl translate install zh-Hans-CN` should results in po/zh_CN/LC_MESSAGES/Koha.mo and po/zh-Hans-CN-messages.po 4. `perl translate install fr-FR` should results in po/fr_FR/LC_MESSAGES/Koha.mo and po/fr-FR-messages.po
Created attachment 70976 [details] [review] Bug 15395: Remove the use of Locale::Util's set_locale
(In reply to Jesse Weaver from comment #57) > Created attachment 70974 [details] [review] [review] > Bug 15395: Example usage of I18N Template::Toolkit plugin > > Moved this patch for example usage to teh end of patch set > and resolved minor conflict while applyin > Signed-off-by: Marc Véron <veron@veron.ch> I'm confused by this example. How does the msgid look like in the po-file? msgid "item" msgid_plural "items" Like that? If so, then that's a bad example, as it's basically the same as concatenating number + "item" The number placeholder should be included in the translatable string, so the msgid looks like this: msgid "one item" msgid_plural "%s items" Because the translator can then actually make it sound natural in their own language - perhaps the language needs an article before the number, or a suffix to the number, both of which would be impossible to do with the original example.
(In reply to paxed from comment #60) > (In reply to Jesse Weaver from comment #57) > > Created attachment 70974 [details] [review] [review] [review] > > Bug 15395: Example usage of I18N Template::Toolkit plugin > > > > Moved this patch for example usage to teh end of patch set > > and resolved minor conflict while applyin > > Signed-off-by: Marc Véron <veron@veron.ch> > > I'm confused by this example. How does the msgid look like in the po-file? > > msgid "item" > msgid_plural "items" > > Like that? > > If so, then that's a bad example, as it's basically the same as concatenating > number + "item" > The number placeholder should be included in the translatable string, so the > msgid looks like this: > > msgid "one item" > msgid_plural "%s items" > > Because the translator can then actually make it sound natural in their own > language - perhaps the language needs an article before the number, or a > suffix to the number, both of which would be impossible to do with the > original example. The problem solved by this example is not the concatenation of the number and the "item/items" string, but the fact that it allows only two forms (one singular, one plural), which is ok for some languages, but not for all. But I agree that it can be improved. I will submit another patch.
Created attachment 71940 [details] [review] Bug 15395: Allow correct handling of plural translation Locale::Maketext does not allow correct handling of plural translation for languages that have more than one plural forms. Locale::Messages does. So Koha::I18N is now a wrapper around Locale::Messages, just like Locale::TextDomain, and export the same symbols as Locale::TextDomain. You can refer to documentation of Locale::TextDomain to know how to use exported subroutines. The PO file moves from misc/translator/po/xx-XX-messages.po to misc/translator/po/xx_XX/LC_MESSAGES/Koha.po and now needs to be compiled to MO in order to be used by Koha. Compilation of PO file is done by running: ./translate install xx-XX Remove dependency to Locale::Maketext and Locale::Maketext::Lexicon Add dependency to Locale::Messages Test plan: 1. Open a .pl script or .pm module with your favorite text editor 2. Add 'use Koha::I18N;' in the beginning of file 3. Use one of the subroutines exported by Koha::I18N and be sure to have a way to visualize the result (pass result to the template for example, or simply warn and watch the log file) 4. cd misc/translator && ./translate update fr-FR # try other languages 5. Open misc/translator/po/fr_FR/LC_MESSAGES/Koha.po and translate your string(s) You will need to change the "Plural-Forms" header. See https://localization-guide.readthedocs.org/en/latest/l10n/pluralforms.html 6. ./translate install fr-FR 7. Use your web browser to go to the page that should display the translation, change language and verify the translation is correct Example usage: __("Hi") __x("Hi {name}", name => 'Bob') __n("item", "items", $num_items) __nx("one item", "{count} items", $num_items, count => $num_items) __p("Bibliographic record", "item") Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 71941 [details] [review] Bug 15395: Plural translations in templates Provides a way to handle translation of plural forms in templates Add Template::Toolkit plugin for Koha::I18N Use like this: [%# USE the plugin and define some macros %] [% PROCESS 'i18n.inc' %] [%# tn is the equivalent of __n %] [%# macro names can't start with underscore, t is for "translate" %] [% tn('item', 'items', num_items) %] Extraction of strings from templates is a bit complicated and use Template::Parser and PPI. Template is compiled into Perl code and then analyzed by PPI. It is slow, but should be correct even with complex constructions. Add dependency to PPI Test plan: 1. Open a template file (.tt or .inc) with your favorite text editor 2. Add the PROCESS directive mentioned above in the beginning of file 3. Use one of the t* macros defined in i18n.inc. They are used like their "__" equivalent, with one difference: the 'x' variants take a hashref instead of a hash as last parameter 4. cd misc/translator && ./translate update fr-FR 5. Open misc/translator/po/fr_FR/LC_MESSAGES/Koha.po and translate your string(s) 6. ./translate install fr-FR 7. Use your web browser to go to the page that should display the translation, change language and verify the translation is correct Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 71942 [details] [review] Bug 15395: Add plural translation capabilities to JS files It adds Javascript equivalent of Koha::I18N's exported subroutines, and they are used the same way. String extraction is done only on *.js files and require gettext 0.19 (available in Debian stable, and also in wheezy-backports) It adds Javascript library Gettext.js for handling translation and a Perl script po2json to transform PO file into JSON. Gettext.js and po2json both come from Locale::Simple. There are several tools named po2json. It's simpler to integrate this one into Koha than to check if the good one is installed on the system. Locale::Simple is not needed. To avoid polluting the global namespace too much, this patch also introduce a global JS object named Koha and add some stuff in Koha.i18n Test plan: 1. Add a translatable string in a JS file. For example, add this: alert(__nx("There is one item", "There are {count} items", 3, {count: 3})); to staff-global.js 2. cd misc/translator && ./translate update fr-FR 3. Open misc/translator/po/fr_FR/LC_MESSAGES/Koha.po, verify that your string is present, and translate it 4. cd misc/translator && ./translate install fr-FR 5. (Optional) Verify that koha-tmpl/intranet-tmpl/prog/fr-FR/js/locale_data.js exists and contains your translation 6. Open your browser on the staff main page, change language and verify that the message is translated 7. Repeat 1-6 on OPAC side Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 71943 [details] [review] Bug 15395: Exclude xslt from the list of themes when building locale_data.js Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 71944 [details] [review] Bug 15395: Fix PO header reading in po2json Since version 0.24 Locale::PO do not escape newline characters in string returned by dequote() This patch set the required version of Locale::PO to 0.24 and fix po2json https://rt.cpan.org/Public/Bug/Display.html?id=96016 Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 71945 [details] [review] Bug 15395: Fix output of msgcat By default, msgcat tries to merge translations when two or more files have the same msgid, even for the header (msgid ""). This resulted in an invalid PO file ("#-#-#-#-#" markers in header). This patch tells msgcat to use the first available translation instead. Also, this patch set correctly the Project-Id-Version field Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 71946 [details] [review] Bug 15395: Make Koha::I18N work with Plack + add tests Language detection was done in a BEGIN block so it was not working correctly with Plack. This is now done when the first translation is requested. This patch also fixes a bug when set_locale(LC_ALL, ...) fails for some reasons (locale is not installed system-wide for example). And finally, add some unit tests Test plan: 1. prove t/Koha/I18N.t 2. Replay the previous test plans with plack enabled Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 71947 [details] [review] Bug 15395: Search .js files in more directories Specifically, koha-tmpl/intranet-tmpl/prog/js and koha-tmpl/opac-tmpl/bootstrap/js Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 71948 [details] [review] Bug 15395: Example usage of I18N Template::Toolkit plugin Moved this patch for example usage to teh end of patch set and resolved minor conflict while applyin Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 71949 [details] [review] Bug 15395: Fix several issues in translation process - Change location of generated PO files to ease its integration into Pootle (po/xx-XX-messages.po instead of xx_XX/LC_MESSAGES/Koha.po) - Use msginit for PO creation to try to set correctly Plural-Forms header. If it fails, set a default one - 'install' command now creates the PO file if it doesn't already exist - Fix locale name when language code is different from ll-CC (where ll is the language code and CC is the country code). Test plan: 1. `perl translate install ar-Arab` should results in po/ar/LC_MESSAGES/Koha.mo and po/ar-Arab-messages.po 2. `perl translate install gl` should results in po/gl/LC_MESSAGES/Koha.mo and po/gl-messages.po 3. `perl translate install zh-Hans-CN` should results in po/zh_CN/LC_MESSAGES/Koha.mo and po/zh-Hans-CN-messages.po 4. `perl translate install fr-FR` should results in po/fr_FR/LC_MESSAGES/Koha.mo and po/fr-FR-messages.po
Created attachment 71950 [details] [review] Bug 15395: Remove the use of Locale::Util's set_locale
Created attachment 71951 [details] [review] Bug 15395: Improve TT plugin example Include the number in the translatable string
Created attachment 71952 [details] [review] Bug 15395: Fix for libintl-perl >= 1.25 Since 1.25, LANGUAGE environment variable is ignored unless LANG is set to a valid locale. This patch sets LANG to a valid locale and calls setlocale. If no valid locale can be found, a warning message is written in logs. See https://rt.cpan.org/Public/Bug/Display.html?id=81315 and also https://github.com/gflohr/libintl-perl/commit/5910980b6e
Created attachment 73901 [details] [review] Bug 15395: Allow correct handling of plural translation Locale::Maketext does not allow correct handling of plural translation for languages that have more than one plural forms. Locale::Messages does. So Koha::I18N is now a wrapper around Locale::Messages, just like Locale::TextDomain, and export the same symbols as Locale::TextDomain. You can refer to documentation of Locale::TextDomain to know how to use exported subroutines. The PO file moves from misc/translator/po/xx-XX-messages.po to misc/translator/po/xx_XX/LC_MESSAGES/Koha.po and now needs to be compiled to MO in order to be used by Koha. Compilation of PO file is done by running: ./translate install xx-XX Remove dependency to Locale::Maketext and Locale::Maketext::Lexicon Add dependency to Locale::Messages Test plan: 1. Open a .pl script or .pm module with your favorite text editor 2. Add 'use Koha::I18N;' in the beginning of file 3. Use one of the subroutines exported by Koha::I18N and be sure to have a way to visualize the result (pass result to the template for example, or simply warn and watch the log file) 4. cd misc/translator && ./translate update fr-FR # try other languages 5. Open misc/translator/po/fr_FR/LC_MESSAGES/Koha.po and translate your string(s) You will need to change the "Plural-Forms" header. See https://localization-guide.readthedocs.org/en/latest/l10n/pluralforms.html 6. ./translate install fr-FR 7. Use your web browser to go to the page that should display the translation, change language and verify the translation is correct Example usage: __("Hi") __x("Hi {name}", name => 'Bob') __n("item", "items", $num_items) __nx("one item", "{count} items", $num_items, count => $num_items) __p("Bibliographic record", "item") Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 73902 [details] [review] Bug 15395: Plural translations in templates Provides a way to handle translation of plural forms in templates Add Template::Toolkit plugin for Koha::I18N Use like this: [%# USE the plugin and define some macros %] [% PROCESS 'i18n.inc' %] [%# tn is the equivalent of __n %] [%# macro names can't start with underscore, t is for "translate" %] [% tn('item', 'items', num_items) %] Extraction of strings from templates is a bit complicated and use Template::Parser and PPI. Template is compiled into Perl code and then analyzed by PPI. It is slow, but should be correct even with complex constructions. Add dependency to PPI Test plan: 1. Open a template file (.tt or .inc) with your favorite text editor 2. Add the PROCESS directive mentioned above in the beginning of file 3. Use one of the t* macros defined in i18n.inc. They are used like their "__" equivalent, with one difference: the 'x' variants take a hashref instead of a hash as last parameter 4. cd misc/translator && ./translate update fr-FR 5. Open misc/translator/po/fr_FR/LC_MESSAGES/Koha.po and translate your string(s) 6. ./translate install fr-FR 7. Use your web browser to go to the page that should display the translation, change language and verify the translation is correct Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 73903 [details] [review] Bug 15395: Add plural translation capabilities to JS files It adds Javascript equivalent of Koha::I18N's exported subroutines, and they are used the same way. String extraction is done only on *.js files and require gettext 0.19 (available in Debian stable, and also in wheezy-backports) It adds Javascript library Gettext.js for handling translation and a Perl script po2json to transform PO file into JSON. Gettext.js and po2json both come from Locale::Simple. There are several tools named po2json. It's simpler to integrate this one into Koha than to check if the good one is installed on the system. Locale::Simple is not needed. To avoid polluting the global namespace too much, this patch also introduce a global JS object named Koha and add some stuff in Koha.i18n Test plan: 1. Add a translatable string in a JS file. For example, add this: alert(__nx("There is one item", "There are {count} items", 3, {count: 3})); to staff-global.js 2. cd misc/translator && ./translate update fr-FR 3. Open misc/translator/po/fr_FR/LC_MESSAGES/Koha.po, verify that your string is present, and translate it 4. cd misc/translator && ./translate install fr-FR 5. (Optional) Verify that koha-tmpl/intranet-tmpl/prog/fr-FR/js/locale_data.js exists and contains your translation 6. Open your browser on the staff main page, change language and verify that the message is translated 7. Repeat 1-6 on OPAC side Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 73904 [details] [review] Bug 15395: Exclude xslt from the list of themes when building locale_data.js Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 73905 [details] [review] Bug 15395: Fix PO header reading in po2json Since version 0.24 Locale::PO do not escape newline characters in string returned by dequote() This patch set the required version of Locale::PO to 0.24 and fix po2json https://rt.cpan.org/Public/Bug/Display.html?id=96016 Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 73906 [details] [review] Bug 15395: Fix output of msgcat By default, msgcat tries to merge translations when two or more files have the same msgid, even for the header (msgid ""). This resulted in an invalid PO file ("#-#-#-#-#" markers in header). This patch tells msgcat to use the first available translation instead. Also, this patch set correctly the Project-Id-Version field Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 73907 [details] [review] Bug 15395: Make Koha::I18N work with Plack + add tests Language detection was done in a BEGIN block so it was not working correctly with Plack. This is now done when the first translation is requested. This patch also fixes a bug when set_locale(LC_ALL, ...) fails for some reasons (locale is not installed system-wide for example). And finally, add some unit tests Test plan: 1. prove t/Koha/I18N.t 2. Replay the previous test plans with plack enabled Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 73908 [details] [review] Bug 15395: Search .js files in more directories Specifically, koha-tmpl/intranet-tmpl/prog/js and koha-tmpl/opac-tmpl/bootstrap/js Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 73909 [details] [review] Bug 15395: Example usage of I18N Template::Toolkit plugin Moved this patch for example usage to teh end of patch set and resolved minor conflict while applyin Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 73910 [details] [review] Bug 15395: Fix several issues in translation process - Change location of generated PO files to ease its integration into Pootle (po/xx-XX-messages.po instead of xx_XX/LC_MESSAGES/Koha.po) - Use msginit for PO creation to try to set correctly Plural-Forms header. If it fails, set a default one - 'install' command now creates the PO file if it doesn't already exist - Fix locale name when language code is different from ll-CC (where ll is the language code and CC is the country code). Test plan: 1. `perl translate install ar-Arab` should results in po/ar/LC_MESSAGES/Koha.mo and po/ar-Arab-messages.po 2. `perl translate install gl` should results in po/gl/LC_MESSAGES/Koha.mo and po/gl-messages.po 3. `perl translate install zh-Hans-CN` should results in po/zh_CN/LC_MESSAGES/Koha.mo and po/zh-Hans-CN-messages.po 4. `perl translate install fr-FR` should results in po/fr_FR/LC_MESSAGES/Koha.mo and po/fr-FR-messages.po
Created attachment 73911 [details] [review] Bug 15395: Remove the use of Locale::Util's set_locale
Created attachment 73912 [details] [review] Bug 15395: Improve TT plugin example Include the number in the translatable string
Created attachment 73913 [details] [review] Bug 15395: Fix for libintl-perl >= 1.25 Since 1.25, LANGUAGE environment variable is ignored unless LANG is set to a valid locale. This patch sets LANG to a valid locale and calls setlocale. If no valid locale can be found, a warning message is written in logs. See https://rt.cpan.org/Public/Bug/Display.html?id=81315 and also https://github.com/gflohr/libintl-perl/commit/5910980b6e
Created attachment 76328 [details] [review] Bug 15395: Keep JS translations separated from others It saves the browser from downloading a whole ~1Mib JSON file where 99% of it will only be used on the server side. The downside of this is that if the same strings appear in templates and in JS files, they will have to be translated twice.
Created attachment 76329 [details] [review] Bug 15395: Extract strings from TT BLOCKs too
Created attachment 76330 [details] [review] Bug 15395: Fix translation when text contains non-ASCII characters msgid, msgid_plural and msgctxt need to be utf8-encoded Also, remove call to Locale::Messages->select_package which is not needed anymore
Created attachment 77475 [details] [review] Bug 15395: Allow correct handling of plural translation Locale::Maketext does not allow correct handling of plural translation for languages that have more than one plural forms. Locale::Messages does. So Koha::I18N is now a wrapper around Locale::Messages, just like Locale::TextDomain, and export the same symbols as Locale::TextDomain. You can refer to documentation of Locale::TextDomain to know how to use exported subroutines. Example usage: __("Hi") __x("Hi {name}", name => 'Bob') __n("item", "items", $num_items) __nx("one item", "{count} items", $num_items, count => $num_items) __p("Bibliographic record", "item") This patch also brings Koha::I18N power to Template::Toolkit templates by adding a TT plugin. This plugin can be used like this: [%# USE the plugin and define some macros %] [% PROCESS 'i18n.inc' %] [%# tn is the equivalent of __n %] [%# macro names can't start with underscore, t is for "translate" %] [% tn('item', 'items', num_items) %] Extraction of strings from templates is a bit complicated and use Template::Parser and PPI. Template is compiled into Perl code and then analyzed by PPI. It is slow, but should be correct even with complex constructions. Remove dependency to Locale::Maketext and Locale::Maketext::Lexicon Add dependency to Locale::Messages and PPI Test plan for translation in Perl code: 1. Open a .pl script or .pm module with your favorite text editor 2. Add 'use Koha::I18N;' in the beginning of file 3. Use one of the subroutines exported by Koha::I18N and be sure to have a way to visualize the result (pass result to the template for example, or simply warn and watch the log file) 4. cd misc/translator && ./translate update fr-FR # try other languages 5. Open misc/translator/po/fr-FR-messages.po and translate your string(s) You may need to change the "Plural-Forms" header. See https://localization-guide.readthedocs.org/en/latest/l10n/pluralforms.html 6. ./translate install fr-FR 7. Use your web browser to go to the page that should display the translation, change language and verify the translation is correct 8. prove t/Koha/I18N.t Test plan for translation in templates: 1. Open a template file (.tt or .inc) with your favorite text editor 2. Add the PROCESS directive mentioned above in the beginning of file 3. Use one of the t* macros defined in i18n.inc. They are used like their "__" equivalent, with one difference: the 'x' variants take a hashref instead of a hash as last parameter 4. cd misc/translator && ./translate update fr-FR 5. Open misc/translator/po/fr-FR-messages.po and translate your string(s) 6. ./translate install fr-FR 7. Use your web browser to go to the page that should display the translation, change language and verify the translation is correct Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 77476 [details] [review] Bug 15395: Example usage of I18N Template::Toolkit plugin Signed-off-by: Marc Véron <veron@veron.ch>
To ease testing I squashed all the patches, merged the test plans, and removed the JS part (I will re-add it in another bug).
JS part is in bug 21156
Sorry but I still do not manage to make it works. Naive test, following the test plan: 0/ do not apply the patch 1/ perl translate update es-ES && perl translate install es-ES 2/ git commit -a -m"init po's" # to see the diff later 3/ yes|git bz apply 21156 4/ perl translate update es-ES 5/ Edit misc/translator/po/es-ES-messages.po 6/ #: koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt:1 #, perl-brace-format msgid "{count} item" msgid_plural "XXX {count} ejemplares XXX" msgstr[0] "No ejemplar" msgstr[1] "Solo uno ejemplar" 4/ perl translate install es-ES 5/ I see: English version: https://snag.gy/91kXtN.jpg Spanish version: https://snag.gy/HD7OQN.jpg I have added msgstr[2] "XXX {count} ejemplares XXX" But it does not change the result.
Also I should note that I had to amend the code, the tests were failing # Failed test 'use Koha::I18N;' # at t/Koha/I18N.t line 10. # Tried to use 'Koha::I18N'. # Error: "setlocale" is not exported by the Locale::Messages module I did, in Koha/I18N.pm -use Locale::Messages qw(:locale_h nl_putenv setlocale LC_MESSAGES); +use Locale::Messages qw(:locale_h nl_putenv LC_MESSAGES); +use POSIX qw (setlocale);
Hi Jonathan, and thanks for testing. There are several problems here: - You must not modify msgid_plural in the PO file, it's a string identifier just like msgid. - According to https://localization-guide.readthedocs.io/en/latest/l10n/pluralforms.html Spanish has 2 "plural forms" (one of which is the singular form), so msgstr[0] should contain the singular form and msgstr[1] the plural form. - Locale::Messages does export setlocale but only since version 1.24. It should be ok since Ubuntu xenial/16.04 and Debian stretch/stable. Which version of this package are you using ? - bug 21156 is not needed here. Please don't apply it.
(In reply to Julian Maurice from comment #98) > Hi Jonathan, and thanks for testing. > > There are several problems here: > - You must not modify msgid_plural in the PO file, it's a string identifier > just like msgid. Erk! ok retesting! > - According to > https://localization-guide.readthedocs.io/en/latest/l10n/pluralforms.html > Spanish has 2 "plural forms" (one of which is the singular form), so > msgstr[0] should contain the singular form and msgstr[1] the plural form. Did you try if these changes will behave correctly in pootle? i.e. Does the pootle's interface will let translators filled these forms according to the language, etc.? > - Locale::Messages does export setlocale but only since version 1.24. It > should be ok since Ubuntu xenial/16.04 and Debian stretch/stable. Which > version of this package are you using ? 1.23 I guess it will not hurt to have it, right? > - bug 21156 is not needed here. Please don't apply it. ok!
(In reply to Jonathan Druart from comment #99) > > - According to > > https://localization-guide.readthedocs.io/en/latest/l10n/pluralforms.html > > Spanish has 2 "plural forms" (one of which is the singular form), so > > msgstr[0] should contain the singular form and msgstr[1] the plural form. > > Did you try if these changes will behave correctly in pootle? > i.e. Does the pootle's interface will let translators filled these forms > according to the language, etc.? I haven't tried but the documentation says that pootle support this feature http://docs.translatehouse.org/projects/translate-toolkit/en/latest/formats/po.html#po and it's a basic feature of gettext/PO files. I would be very surprised if Pootle doesn't handle it.
It works better instead. Just noting: == es-ES - nplurals=2; plural=(n != 1); == msgid "{count} item" msgid_plural "{count} items" msgstr[0] "Uno ejemplar" msgstr[1] "{count} ejemplares" will output: 1: Uno ejemplar 2: 2 ejemplares but also: 0: 0 ejemplares == fr-FR - nplurals=2; plural=(n > 1); == msgid "{count} item" msgid_plural "{count} items" msgstr[0] "Un exemplaire" msgstr[1] "{count} exemplaires" will output: 1: Un examplaire 2: 2 exemplaires but also: 0: Un exemplaire So, from what I understand the third form (for 0) will be not supported, unless we change the Plural-Forms (are we really supposed to change that?). With: "Plural-Forms: nplurals=3; plural=(n==0 ? 0 : n==1 ? 1 : 2);\n" #: koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt:1 #: koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt:2 #, perl-brace-format msgid "{count} item" msgid_plural "{count} items" msgstr[0] "Pas d'exemplaire" msgstr[1] "Un exemplaire" msgstr[2] "{count} exemplaires" I get what I expected: 1: Un exemplaire 2: 2 exemplaires 0: Pas d'exemplaire (I have moved the tnx call outside before the SEARCH_RESULT.items_count)
Code review: 1. Not important but the Koha.po file provided for the tests are not taken into account. Maybe it could be better to generate the .mo from the tests? 2. No tests provided for misc/translator/LangInstaller.pm - I would not consider it as blocker as it is the current state. 3. The content from the .inc files are added to the .po because they contain TT blocks on several lines. Maybe we should have a list of files to skip and copy as it (?) 4. What is done in Koha::I18N::init is weird, but I trust you if you think everythink is needed. However: pod Locale::Messages, nl_putenv: "The sole purpose of this function is to work around some ideosyncrasies in the environment processing of Windows systems. If you want to portably set or unset environment variables, use this function instead of directly manipulating %ENV." So maybe we should use ENV, it will make the line clear about what it does, and 'git grep $ENV' will work
(In reply to Jonathan Druart from comment #101) > So, from what I understand the third form (for 0) will be not supported, > unless we change the Plural-Forms (are we really supposed to change that?). I don't think we are supposed to. The simple solution would be to always use the {count} so we would have "0 exemplaire" and "1 exemplaire". Or we could handle this in the code: [% IF count %] [% tnx('{count} item', '{count} items', count, { count = count }) %] [% ELSE %] [% t('Not a single item!') %] [% END %] I think there is no best solution here and both can be used. Sometimes we want to display a different message when there are 0 "things", and sometimes not.
(In reply to Julian Maurice from comment #92) > Extraction of strings from templates is a bit complicated and use > Template::Parser and PPI. Template is compiled into Perl code and then > analyzed by PPI. It is slow, but should be correct even with complex > constructions. Indeed, it is super slow compared. I would consider it acceptable compared to the enhancement it brings. (master) % time perl translate update fr-FR perl translate update fr-FR 10.02s user 5.16s system 84% cpu 17.868 total (master) % time perl translate install fr-FR perl translate install fr-FR 12.88s user 3.05s system 79% cpu 19.988 total (bug_21156) % time perl translate install fr-FR . done. perl translate update fr-FR 102.00s user 11.62s system 97% cpu 1:56.00 total (bug_21156) % time perl translate install fr-FR perl translate install fr-FR 12.49s user 2.56s system 79% cpu 18.940 total BTW do you know what is the ". done." outputted by install?
(In reply to Jonathan Druart from comment #102) > Code review: > 1. Not important but the Koha.po file provided for the tests are not taken > into account. Maybe it could be better to generate the .mo from the tests? Or maybe we can simply remove the .po file as it can be easily generated from the .mo file with msgunfmt ? > 2. No tests provided for misc/translator/LangInstaller.pm - I would not > consider it as blocker as it is the current state. Thanks for not considering it a blocker :) Testing a module that modifies the filesystem can be a huge pain, especially in this case where paths are hardcoded and where it modifies files tracked by git. But I can certainly try to write a test for extract_messages_from_templates since it doesn't have too many external dependencies and is the most complicated thing of the whole patch. > 3. The content from the .inc files are added to the .po because they contain > TT blocks on several lines. Maybe we should have a list of files to skip and > copy as it (?) I don't understand. My .po file is clean after an 'update'. Why would we want to skip .inc files ? Can you give an example of what gets wrongly added to the .po ? > 4. What is done in Koha::I18N::init is weird, but I trust you if you think > everythink is needed. I agree that it's weird but I'm pretty sure everything is needed > However: pod Locale::Messages, nl_putenv: "The sole purpose of this function > is to work around some ideosyncrasies in the environment processing of > Windows systems. If you want to portably set or unset environment variables, > use this function instead of directly manipulating %ENV." > So maybe we should use ENV, it will make the line clear about what it does, > and 'git grep $ENV' will work Ok I'll try without nl_putenv and see if it works
(In reply to Jonathan Druart from comment #104) > BTW do you know what is the ". done." outputted by install? It is a message outputted by msgmerge, I will make it quiet
(In reply to Julian Maurice from comment #105) > > 3. The content from the .inc files are added to the .po because they contain > > TT blocks on several lines. Maybe we should have a list of files to skip and > > copy as it (?) > I don't understand. My .po file is clean after an 'update'. Why would we > want to skip .inc files ? Can you give an example of what gets wrongly added > to the .po ? #: opac-tmpl/bootstrap/en/includes/i18n.inc:1 #, c-format msgid "" "[%% USE I18N; MACRO t(msgid) BLOCK; I18N.t(msgid); END; MACRO tx(msgid, " "vars) BLOCK; I18N.tx(msgid, vars); END; MACRO tn(msgid, msgid_plural, count) " "BLOCK; I18N.tn(msgid, msgid_plural, count); END; MACRO tnx(msgid, " "msgid_plural, count, vars) BLOCK; I18N.tnx(msgid, msgid_plural, count, " "vars); END; MACRO txn(msgid, msgid_plural, count, vars) BLOCK; I18N." "txn(msgid, msgid_plural, count, vars); END; MACRO tp(msgctxt, msgid) BLOCK; " "I18N.tp(msgctxt, msgid); END; MACRO tpx(msgctxt, msgid, vars) BLOCK; I18N." "tpx(msgctxt, msgid, vars); END; MACRO tnp(msgctxt, msgid, msgid_plural, " "count) BLOCK; I18N.tnp(msgctxt, msgid, msgid_plural, count); END; MACRO " "tnpx(msgctxt, msgid, msgid_plural, count, vars) BLOCK; I18N.tnpx(msgctxt, " "msgid, msgid_plural, count, vars); END; %%] " msgstr "" And there is nothing to translate in this file, there are just TT tags. It happens when TT tags are split on several lines.
Created attachment 81152 [details] [review] Bug 15395: Allow correct handling of plural translation Locale::Maketext does not allow correct handling of plural translation for languages that have more than one plural forms. Locale::Messages does. So Koha::I18N is now a wrapper around Locale::Messages, just like Locale::TextDomain, and export the same symbols as Locale::TextDomain. You can refer to documentation of Locale::TextDomain to know how to use exported subroutines. Example usage: __("Hi") __x("Hi {name}", name => 'Bob') __n("item", "items", $num_items) __nx("one item", "{count} items", $num_items, count => $num_items) __p("Bibliographic record", "item") This patch also brings Koha::I18N power to Template::Toolkit templates by adding a TT plugin. This plugin can be used like this: [%# USE the plugin and define some macros %] [% PROCESS 'i18n.inc' %] [%# tn is the equivalent of __n %] [%# macro names can't start with underscore, t is for "translate" %] [% tn('item', 'items', num_items) %] Extraction of strings from templates is a bit complicated and use Template::Parser and PPI. Template is compiled into Perl code and then analyzed by PPI. It is slow, but should be correct even with complex constructions. Remove dependency to Locale::Maketext and Locale::Maketext::Lexicon Add dependency to Locale::Messages and PPI Test plan for translation in Perl code: 1. Open a .pl script or .pm module with your favorite text editor 2. Add 'use Koha::I18N;' in the beginning of file 3. Use one of the subroutines exported by Koha::I18N and be sure to have a way to visualize the result (pass result to the template for example, or simply warn and watch the log file) 4. cd misc/translator && ./translate update fr-FR # try other languages 5. Open misc/translator/po/fr-FR-messages.po and translate your string(s) You may need to change the "Plural-Forms" header. See https://localization-guide.readthedocs.org/en/latest/l10n/pluralforms.html 6. ./translate install fr-FR 7. Use your web browser to go to the page that should display the translation, change language and verify the translation is correct 8. prove t/Koha/I18N.t Test plan for translation in templates: 1. Open a template file (.tt or .inc) with your favorite text editor 2. Add the PROCESS directive mentioned above in the beginning of file 3. Use one of the t* macros defined in i18n.inc. They are used like their "__" equivalent, with one difference: the 'x' variants take a hashref instead of a hash as last parameter 4. cd misc/translator && ./translate update fr-FR 5. Open misc/translator/po/fr-FR-messages.po and translate your string(s) 6. ./translate install fr-FR 7. Use your web browser to go to the page that should display the translation, change language and verify the translation is correct Signed-off-by: Marc Véron <veron@veron.ch>
Created attachment 81153 [details] [review] Bug 15395: Require Locale::Messages 1.24 1.21 fixes a bug in the way environment variables LANGUAGE and LANG are interpreted https://rt.cpan.org/Public/Bug/Display.html?id=81315 1.24 brings its own setlocale which "does the right thing" https://metacpan.org/pod/release/GUIDO/libintl-perl-1.29/lib/Locale/Messages.pm#setlocale
Created attachment 81154 [details] [review] Bug 15395: Make msgmerge quiet
Created attachment 81155 [details] [review] Bug 15395: Do not use nl_putenv, use $ENV instead nl_putenv is only useful on Windows systems
Created attachment 81156 [details] [review] Bug 15395: Add unit tests for extract_messages_from_templates Because LangInstaller use FindBin to define some paths, the test produces warnings about a missing directory. I'm not sure how we can avoid that. Test plan: 1. prove t/LangInstaller.t
Created attachment 81157 [details] [review] Bug 15395: Make QA test script happy It is not totally happy because of the template file used in tests. It has line breaks inside TT directives and QA test script complains because line breaks in TT directives confuse the current translatable strings extractor. However this patchset (along with bug 20988) will hopefully make the current extractor obsolete and thus make the 'no-line-breaks' rule useless
Created attachment 81158 [details] [review] Bug 15395: Do not process template files that do not use i18n.inc It should make the string extraction process a little faster For the record, I timed some parts of the process to see why it was so slow, and without surprises the most time-consuming task is the Perl code analysis by PPI with an average time of 50ms by "template block" on my machine. Multiply that by the number of template blocks (approximatively 900), that gives us 45 seconds just for this task.
Created attachment 81159 [details] [review] Bug 15395: Example usage of I18N Template::Toolkit plugin Signed-off-by: Marc Véron <veron@veron.ch>
(In reply to Jonathan Druart from comment #107) > (In reply to Julian Maurice from comment #105) > > > 3. The content from the .inc files are added to the .po because they contain > > > TT blocks on several lines. Maybe we should have a list of files to skip and > > > copy as it (?) > > I don't understand. My .po file is clean after an 'update'. Why would we > > want to skip .inc files ? Can you give an example of what gets wrongly added > > to the .po ? > > #: opac-tmpl/bootstrap/en/includes/i18n.inc:1 > #, c-format > msgid "" > "[%% USE I18N; MACRO t(msgid) BLOCK; I18N.t(msgid); END; MACRO tx(msgid, " > "vars) BLOCK; I18N.tx(msgid, vars); END; MACRO tn(msgid, msgid_plural, > count) " > "BLOCK; I18N.tn(msgid, msgid_plural, count); END; MACRO tnx(msgid, " > "msgid_plural, count, vars) BLOCK; I18N.tnx(msgid, msgid_plural, count, " > "vars); END; MACRO txn(msgid, msgid_plural, count, vars) BLOCK; I18N." > "txn(msgid, msgid_plural, count, vars); END; MACRO tp(msgctxt, msgid) BLOCK; > " > "I18N.tp(msgctxt, msgid); END; MACRO tpx(msgctxt, msgid, vars) BLOCK; I18N." > "tpx(msgctxt, msgid, vars); END; MACRO tnp(msgctxt, msgid, msgid_plural, " > "count) BLOCK; I18N.tnp(msgctxt, msgid, msgid_plural, count); END; MACRO " > "tnpx(msgctxt, msgid, msgid_plural, count, vars) BLOCK; I18N.tnpx(msgctxt, " > "msgid, msgid_plural, count, vars); END; %%] " > msgstr "" > > And there is nothing to translate in this file, there are just TT tags. > It happens when TT tags are split on several lines. Ah I thought you were talking about xx-XX-messages.po but this is in xx-XX-opac-bootstrap.po right ? This should be fixed by attachment 81157 [details] [review]
(In reply to Jonathan Druart from comment #104) > (In reply to Julian Maurice from comment #92) > > Extraction of strings from templates is a bit complicated and use > > Template::Parser and PPI. Template is compiled into Perl code and then > > analyzed by PPI. It is slow, but should be correct even with complex > > constructions. > > Indeed, it is super slow compared. I would consider it acceptable compared > to the enhancement it brings. > attachment 81158 [details] [review] should make it a little more acceptable
(In reply to Julian Maurice from comment #105) > (In reply to Jonathan Druart from comment #102) > > Code review: > > 1. Not important but the Koha.po file provided for the tests are not taken > > into account. Maybe it could be better to generate the .mo from the tests? > Or maybe we can simply remove the .po file as it can be easily generated from > the .mo file with msgunfmt ? I removed it from the first patch and added a comment in the test script to tell how to modify this MO file
Created attachment 81167 [details] Screenshot of Pootle with plural translations and context (In reply to Julian Maurice from comment #100) > (In reply to Jonathan Druart from comment #99) > > Did you try if these changes will behave correctly in pootle? > > i.e. Does the pootle's interface will let translators filled these forms > > according to the language, etc.? > I haven't tried but the documentation says that pootle support this feature > http://docs.translatehouse.org/projects/translate-toolkit/en/latest/formats/ > po.html#po > and it's a basic feature of gettext/PO files. I would be very surprised if > Pootle doesn't handle it. And it handle it very well. See screenshot attached
Hi all, really happy to see this one moving! Maybe time to think about a few things: - When is the best moment to push this to master? Should we aim to before string freeze? In this case it would be nice to get an OK from Bernardo before we do that. The scheduled time for translating is not very long this time, so we should avoid issues. I think plural forms might not be set in all our files. Some languages like Japanese don't even have proper plural. - Does it affect performance? I see you discussed times for generating the .po/.mo files already. What about page loads? I am not sure if I understood the method correctly here, are we still creating translated templates or is this more 'on the fly'? - Can we have some dev documentation for the wiki with explanations and examples?
(In reply to Katrin Fischer from comment #120) > Hi all, > > really happy to see this one moving! Really happy too! > > Maybe time to think about a few things: > > - When is the best moment to push this to master? Should we aim to before > string freeze? In this case it would be nice to get an OK from Bernardo > before we do that. The scheduled time for translating is not very long this > time, so we should avoid issues. I think plural forms might not be set in > all our files. Some languages like Japanese don't even have proper plural. Apart from the "Example usage of I18N Template::Toolkit plugin" patch, this whole patchset won't affect existing translations at all. I'm thinking that maybe I can provide an other example in a more hidden place, so if there is problem with it at release it won't be noticed (or maybe just don't push the example, to be totally safe) > > - Does it affect performance? I see you discussed times for generating the > .po/.mo files already. What about page loads? I am not sure if I understood > the method correctly here, are we still creating translated templates or is > this more 'on the fly'? We'll have to continue to create translated templates until all translatable messages are "wrapped" (see bug 20988 which aims to do that) and translated. But once it is done we should stop, as all translations would be retrieved on the fly. So it will affect performance, but it should be negligible. I did some performance tests some time ago and noticed an increase in page load time of ~3ms for 100 strings translated in a template file. I'm currently trying to write another performance test script. I'll publish it as soon as it is finished so other people can test. > > - Can we have some dev documentation for the wiki with explanations and > examples? I can certainly do that. What kind of explanations would you expect from such documentation ?
Hi Julian, it's Saturday! ;) I think some best practice examples like this one from the comments would be good: [% IF count %] [% tnx('{count} item', '{count} items', count, { count = count }) %] [% ELSE %] [% t('Not a single item!') %] [% END %] Also still not sure about the "context and more" - seeing examples for that if possible would be great too. An example in German for where context would be useful is "subject". It can be an email subject, a topic subject on a bibliographic record or just the subject (topic) something talks about... and it would all be different words in German. And maybe some explanation about how this changes the translation process technically?
Hm, looking at the example, it seems quite complicated and not sure I understand it. Do we really want to do that to all our strings? I feel like we should discuss this change at a general meeting if the long term goal is to change the way we write templates fundamentally outside of some cases where the extra features are useful.
I wrote https://wiki.koha-community.org/wiki/Internationalization,_plural_forms,_context,_and_more_(RFC) and initiated a thread on Framavox to gather opinions: https://framavox.org/d/2juWqIMz/internationalization-plural-forms-context-and-more (feel free to not use it but please give your opinion either on the wiki page or directly here on the bug report)
Moved the wiki page to https://wiki.koha-community.org/wiki/Internationalization,_plural_forms,_context,_and_more_RFC (parentheses break URL)
(In reply to Katrin Fischer from comment #123) > Hm, looking at the example, it seems quite complicated and not sure I > understand it. Do we really want to do that to all our strings? It really isn't that complicated once you get used to it, and all other softwares I have worked on do that in one way or another. So yes, I think we should to that to all our strings. But please note that the example is demonstrating how plural forms will work, and the majority of strings will be in the simpler form: [% t('translatable strings') %]
(In reply to Julian Maurice from comment #124) > I wrote > https://wiki.koha-community.org/wiki/Internationalization,_plural_forms, > _context,_and_more_(RFC) > > and initiated a thread on Framavox to gather opinions: > https://framavox.org/d/2juWqIMz/internationalization-plural-forms-context- > and-more > (feel free to not use it but please give your opinion either on the wiki > page or directly here on the bug report) Can you please send out to the maliing list too? More devs to see it then.
I don't love the sound of these patches... (or the implementation of Koha::I18N). What's performance like for handling all the translations at run time? (Might not be a huge issue with template caching?) That said, the current system for generating translations is also painful. I have often wished that we could do something like Messages.Properties in Java or resources in Android Java development, which seem a more flexible for developers and easier for translators. Although I guess those might suffer from pluralization problems too.
> I don't love the sound of these patches... (or the implementation of Koha::I18N). What do you mean? > What's performance like for handling all the translations at run time? (Might not be a huge issue with template caching?) See this section and if you can, try on your machine so we have more data about performance impact. https://wiki.koha-community.org/wiki/Internationalization,_plural_forms,_context,_and_more_RFC#Downsides
I am in favor of these patches as they will help us to handle plurals much better than we do so far. I have been working (test/review) on these patch for a long time and I think we should include them into 18.11 to make translation improvements backportable. The long term goal (bug 20988) can be discussed later.
Created attachment 81734 [details] [review] Bug 15395: Allow correct handling of plural translation Locale::Maketext does not allow correct handling of plural translation for languages that have more than one plural forms. Locale::Messages does. So Koha::I18N is now a wrapper around Locale::Messages, just like Locale::TextDomain, and export the same symbols as Locale::TextDomain. You can refer to documentation of Locale::TextDomain to know how to use exported subroutines. Example usage: __("Hi") __x("Hi {name}", name => 'Bob') __n("item", "items", $num_items) __nx("one item", "{count} items", $num_items, count => $num_items) __p("Bibliographic record", "item") This patch also brings Koha::I18N power to Template::Toolkit templates by adding a TT plugin. This plugin can be used like this: [%# USE the plugin and define some macros %] [% PROCESS 'i18n.inc' %] [%# tn is the equivalent of __n %] [%# macro names can't start with underscore, t is for "translate" %] [% tn('item', 'items', num_items) %] Extraction of strings from templates is a bit complicated and use Template::Parser and PPI. Template is compiled into Perl code and then analyzed by PPI. It is slow, but should be correct even with complex constructions. Remove dependency to Locale::Maketext and Locale::Maketext::Lexicon Add dependency to Locale::Messages and PPI Test plan for translation in Perl code: 1. Open a .pl script or .pm module with your favorite text editor 2. Add 'use Koha::I18N;' in the beginning of file 3. Use one of the subroutines exported by Koha::I18N and be sure to have a way to visualize the result (pass result to the template for example, or simply warn and watch the log file) 4. cd misc/translator && ./translate update fr-FR # try other languages 5. Open misc/translator/po/fr-FR-messages.po and translate your string(s) You may need to change the "Plural-Forms" header. See https://localization-guide.readthedocs.org/en/latest/l10n/pluralforms.html 6. ./translate install fr-FR 7. Use your web browser to go to the page that should display the translation, change language and verify the translation is correct 8. prove t/Koha/I18N.t Test plan for translation in templates: 1. Open a template file (.tt or .inc) with your favorite text editor 2. Add the PROCESS directive mentioned above in the beginning of file 3. Use one of the t* macros defined in i18n.inc. They are used like their "__" equivalent, with one difference: the 'x' variants take a hashref instead of a hash as last parameter 4. cd misc/translator && ./translate update fr-FR 5. Open misc/translator/po/fr-FR-messages.po and translate your string(s) 6. ./translate install fr-FR 7. Use your web browser to go to the page that should display the translation, change language and verify the translation is correct Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 81735 [details] [review] Bug 15395: Make msgmerge quiet Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 81736 [details] [review] Bug 15395: Do not use nl_putenv, use $ENV instead nl_putenv is only useful on Windows systems Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 81737 [details] [review] Bug 15395: Add unit tests for extract_messages_from_templates Because LangInstaller use FindBin to define some paths, the test produces warnings about a missing directory. I'm not sure how we can avoid that. Test plan: 1. prove t/LangInstaller.t Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 81738 [details] [review] Bug 15395: Make QA test script happy It is not totally happy because of the template file used in tests. It has line breaks inside TT directives and QA test script complains because line breaks in TT directives confuse the current translatable strings extractor. However this patchset (along with bug 20988) will hopefully make the current extractor obsolete and thus make the 'no-line-breaks' rule useless Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 81739 [details] [review] Bug 15395: Do not process template files that do not use i18n.inc It should make the string extraction process a little faster For the record, I timed some parts of the process to see why it was so slow, and without surprises the most time-consuming task is the Perl code analysis by PPI with an average time of 50ms by "template block" on my machine. Multiply that by the number of template blocks (approximatively 900), that gives us 45 seconds just for this task. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 81741 [details] [review] Bug 15395: Example usage of I18N Template::Toolkit plugin Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 81742 [details] [review] Bug 15395: Use POSIX::setlocale To keep compatibility with jessie (that have Message::Locales 1.23, so without setlocale) Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 81743 [details] [review] Bug 15395: Fix QA failure - filter not found Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This updates dependencies - will we be able to get these packaged and ready for release?
(In reply to Nick Clemens from comment #140) > This updates dependencies - will we be able to get these packaged and ready > for release? The added dependencies are already packaged since Debian oldoldstable. I think they won't be a problem.
Awesome work all! Pushed to master for 18.11
For the next one getting stuck: sudo apt-get install libintl-perl
Enhancement, will not be backported to 18.05.x series.
Woohoo! I did my first contextualization (locally)! I was wondering what is the process to add a contextualization to the community file? Do I just submit the .tt file in a bug? It probably wouldn't be relevant to all languages...
(In reply to Caroline Cyr La Rose from comment #145) > Woohoo! I did my first contextualization (locally)! > > I was wondering what is the process to add a contextualization to the > community file? Do I just submit the .tt file in a bug? It probably wouldn't > be relevant to all languages... Glad to hear it! :) You should create a new bug report and submit your changes in the form of a .patch file. See https://wiki.koha-community.org/wiki/Submitting_A_Patch for more informations.