Bug 15395 - Internationalization: plural forms, context, and more
Summary: Internationalization: plural forms, context, and more
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: I18N/L10N (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement with 10 votes (vote)
Assignee: Julian Maurice
QA Contact: Testopia
URL:
Whiteboard:
Keywords: dependency
Depends on:
Blocks: 15522 20988 21156 21789 21823
  Show dependency treegraph
 
Reported: 2015-12-18 00:31 UTC by Julian Maurice
Modified: 2018-11-16 13:56 UTC (History)
17 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 15395: Allow correct handling of plural translation (10.48 KB, patch)
2015-12-18 00:33 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Plural translations in templates (10.81 KB, patch)
2015-12-18 00:33 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Example usage of I18N Template::Toolkit plugin (2.42 KB, patch)
2015-12-18 00:33 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Add plural translation capabilities to JS files (107.28 KB, patch)
2016-01-06 16:46 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Allow correct handling of plural translation (10.48 KB, patch)
2016-04-07 13:24 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Plural translations in templates (10.81 KB, patch)
2016-04-07 13:24 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Example usage of I18N Template::Toolkit plugin (2.66 KB, patch)
2016-04-07 13:24 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Add plural translation capabilities to JS files (107.28 KB, patch)
2016-04-07 13:24 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Allow correct handling of plural translation (10.50 KB, patch)
2017-04-05 10:23 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Plural translations in templates (10.84 KB, patch)
2017-04-05 10:23 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Example usage of I18N Template::Toolkit plugin (2.69 KB, patch)
2017-04-05 10:23 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Add plural translation capabilities to JS files (107.30 KB, patch)
2017-04-05 10:23 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Exclude xslt from the list of themes when building locale_data.js (993 bytes, patch)
2017-04-06 08:13 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Fix PO header reading in po2json (1.65 KB, patch)
2017-04-06 08:13 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Fix output of msgcat (1.64 KB, patch)
2017-04-07 16:56 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Make Koha::I18N work with Plack + add tests (11.96 KB, patch)
2017-04-10 17:10 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Search .js files in more directories (1.36 KB, patch)
2017-04-11 12:56 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Allow correct handling of plural translation (10.61 KB, patch)
2017-04-18 19:41 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15395: Plural translations in templates (10.94 KB, patch)
2017-04-18 19:46 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15395: Add plural translation capabilities to JS files (107.39 KB, patch)
2017-04-18 19:47 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15395: Exclude xslt from the list of themes when building locale_data.js (1.09 KB, patch)
2017-04-18 19:48 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15395: Fix PO header reading in po2json (1.77 KB, patch)
2017-04-18 19:49 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15395: Fix output of msgcat (1.76 KB, patch)
2017-04-18 19:50 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15395: Make Koha::I18N work with Plack + add tests (12.08 KB, patch)
2017-04-18 19:51 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15395: Search .js files in more directories (1.49 KB, patch)
2017-04-18 19:53 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15395: Example usage of I18N Template::Toolkit plugin (2.92 KB, patch)
2017-04-18 19:57 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15395: Fix several issues in translation process (7.60 KB, patch)
2017-04-26 09:07 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Remove the use of Locale::Util's set_locale (1.62 KB, patch)
2017-08-11 10:44 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Example usage of I18N Template::Toolkit plugin (2.67 KB, patch)
2018-01-26 20:04 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 15395: Fix several issues in translation process (7.60 KB, patch)
2018-01-26 20:04 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 15395: Remove the use of Locale::Util's set_locale (1.62 KB, patch)
2018-01-26 20:04 UTC, Jesse Weaver
Details | Diff | Splinter Review
Bug 15395: Allow correct handling of plural translation (10.63 KB, patch)
2018-02-19 13:47 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Plural translations in templates (10.97 KB, patch)
2018-02-19 13:47 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Add plural translation capabilities to JS files (107.51 KB, patch)
2018-02-19 13:47 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Exclude xslt from the list of themes when building locale_data.js (1.10 KB, patch)
2018-02-19 13:48 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Fix PO header reading in po2json (1.78 KB, patch)
2018-02-19 13:48 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Fix output of msgcat (1.77 KB, patch)
2018-02-19 13:48 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Make Koha::I18N work with Plack + add tests (12.09 KB, patch)
2018-02-19 13:48 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Search .js files in more directories (1.49 KB, patch)
2018-02-19 13:48 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Example usage of I18N Template::Toolkit plugin (2.64 KB, patch)
2018-02-19 13:48 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Fix several issues in translation process (7.60 KB, patch)
2018-02-19 13:48 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Remove the use of Locale::Util's set_locale (1.62 KB, patch)
2018-02-19 13:48 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Improve TT plugin example (1.69 KB, patch)
2018-02-19 13:48 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Fix for libintl-perl >= 1.25 (2.92 KB, patch)
2018-02-19 13:49 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Allow correct handling of plural translation (10.63 KB, patch)
2018-04-10 06:26 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Plural translations in templates (10.97 KB, patch)
2018-04-10 06:26 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Add plural translation capabilities to JS files (107.55 KB, patch)
2018-04-10 06:26 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Exclude xslt from the list of themes when building locale_data.js (1.10 KB, patch)
2018-04-10 06:26 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Fix PO header reading in po2json (1.78 KB, patch)
2018-04-10 06:26 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Fix output of msgcat (1.77 KB, patch)
2018-04-10 06:27 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Make Koha::I18N work with Plack + add tests (12.09 KB, patch)
2018-04-10 06:27 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Search .js files in more directories (1.49 KB, patch)
2018-04-10 06:27 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Example usage of I18N Template::Toolkit plugin (2.64 KB, patch)
2018-04-10 06:27 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Fix several issues in translation process (7.60 KB, patch)
2018-04-10 06:27 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Remove the use of Locale::Util's set_locale (1.62 KB, patch)
2018-04-10 06:27 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Improve TT plugin example (1.69 KB, patch)
2018-04-10 06:27 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Fix for libintl-perl >= 1.25 (2.92 KB, patch)
2018-04-10 06:27 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Keep JS translations separated from others (5.04 KB, patch)
2018-06-25 06:33 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Extract strings from TT BLOCKs too (5.19 KB, patch)
2018-06-25 06:33 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Fix translation when text contains non-ASCII characters (2.81 KB, patch)
2018-06-25 06:33 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Allow correct handling of plural translation (29.96 KB, patch)
2018-08-03 15:24 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Example usage of I18N Template::Toolkit plugin (2.55 KB, patch)
2018-08-03 15:24 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Allow correct handling of plural translation (27.93 KB, patch)
2018-10-25 13:23 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Require Locale::Messages 1.24 (1.02 KB, patch)
2018-10-25 13:23 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Make msgmerge quiet (802 bytes, patch)
2018-10-25 13:23 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Do not use nl_putenv, use $ENV instead (1.41 KB, patch)
2018-10-25 13:23 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Add unit tests for extract_messages_from_templates (5.62 KB, patch)
2018-10-25 13:23 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Make QA test script happy (8.82 KB, patch)
2018-10-25 13:23 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Do not process template files that do not use i18n.inc (1.37 KB, patch)
2018-10-25 13:23 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 15395: Example usage of I18N Template::Toolkit plugin (2.58 KB, patch)
2018-10-25 13:24 UTC, Julian Maurice
Details | Diff | Splinter Review
Screenshot of Pootle with plural translations and context (55.32 KB, image/png)
2018-10-25 14:58 UTC, Julian Maurice
Details
Bug 15395: Allow correct handling of plural translation (28.94 KB, patch)
2018-10-31 15:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15395: Make msgmerge quiet (875 bytes, patch)
2018-10-31 15:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15395: Do not use nl_putenv, use $ENV instead (1.48 KB, patch)
2018-10-31 15:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15395: Add unit tests for extract_messages_from_templates (5.70 KB, patch)
2018-10-31 15:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15395: Make QA test script happy (8.92 KB, patch)
2018-10-31 15:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15395: Do not process template files that do not use i18n.inc (1.44 KB, patch)
2018-10-31 15:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15395: Example usage of I18N Template::Toolkit plugin (2.65 KB, patch)
2018-10-31 15:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15395: Use POSIX::setlocale (1.19 KB, patch)
2018-10-31 15:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15395: Fix QA failure - filter not found (765 bytes, patch)
2018-10-31 15:03 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2015-12-18 00:31:36 UTC
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
Comment 1 Julian Maurice 2015-12-18 00:33:15 UTC Comment hidden (obsolete)
Comment 2 Julian Maurice 2015-12-18 00:33:18 UTC Comment hidden (obsolete)
Comment 3 Julian Maurice 2015-12-18 00:33:21 UTC Comment hidden (obsolete)
Comment 4 Julian Maurice 2016-01-06 16:46:11 UTC Comment hidden (obsolete)
Comment 5 Bernardo Gonzalez Kriegel 2016-04-07 13:02:38 UTC
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
Comment 6 Julian Maurice 2016-04-07 13:24:04 UTC Comment hidden (obsolete)
Comment 7 Julian Maurice 2016-04-07 13:24:08 UTC Comment hidden (obsolete)
Comment 8 Julian Maurice 2016-04-07 13:24:11 UTC Comment hidden (obsolete)
Comment 9 Julian Maurice 2016-04-07 13:24:15 UTC Comment hidden (obsolete)
Comment 10 Julian Maurice 2016-04-07 13:25:07 UTC
Patches rebased on master
Comment 11 Frédéric Demians 2016-06-03 08:11:28 UTC
./translate update fr-FR raises this error:

system call failed: /usr/bin/xgettext -L JavaScript
Comment 12 Julian Maurice 2016-06-15 06:27:02 UTC
(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
Comment 13 Frédéric Demians 2016-06-15 06:50:18 UTC
(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.
Comment 14 Julian Maurice 2016-06-15 07:06:15 UTC
(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 ?
Comment 15 Frédéric Demians 2016-06-15 09:17:15 UTC
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.
Comment 16 Marc Véron 2017-04-05 09:10:15 UTC
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
Comment 17 Julian Maurice 2017-04-05 10:23:00 UTC Comment hidden (obsolete)
Comment 18 Julian Maurice 2017-04-05 10:23:04 UTC Comment hidden (obsolete)
Comment 19 Julian Maurice 2017-04-05 10:23:08 UTC Comment hidden (obsolete)
Comment 20 Julian Maurice 2017-04-05 10:23:13 UTC Comment hidden (obsolete)
Comment 21 Julian Maurice 2017-04-05 10:29:37 UTC
(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')
Comment 22 Marc Véron 2017-04-05 20:14:44 UTC
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?
Comment 23 Julian Maurice 2017-04-06 08:13:46 UTC Comment hidden (obsolete)
Comment 24 Julian Maurice 2017-04-06 08:13:50 UTC Comment hidden (obsolete)
Comment 25 Julian Maurice 2017-04-06 08:15:22 UTC
(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.
Comment 26 Marc Véron 2017-04-06 09:29:50 UTC
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"
---------------
Comment 27 Julian Maurice 2017-04-06 09:42:02 UTC
(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.
Comment 28 Julian Maurice 2017-04-07 16:56:16 UTC Comment hidden (obsolete)
Comment 29 Julian Maurice 2017-04-10 17:10:30 UTC Comment hidden (obsolete)
Comment 30 Marc Véron 2017-04-11 08:11:24 UTC
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.
Comment 31 Julian Maurice 2017-04-11 08:31:35 UTC
(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.
Comment 32 Marc Véron 2017-04-11 09:06:23 UTC
Ah, got it, thanks for the explanation :-)
Comment 33 Marc Véron 2017-04-11 10:22:22 UTC
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)
Comment 34 Julian Maurice 2017-04-11 11:51:10 UTC
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
Comment 35 Julian Maurice 2017-04-11 12:56:29 UTC Comment hidden (obsolete)
Comment 36 Marc Véron 2017-04-11 15:25:15 UTC
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)
Comment 37 Julian Maurice 2017-04-11 15:47:51 UTC
(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
Comment 38 Marc Véron 2017-04-18 19:33:54 UTC
Oh, reading helps...
Comment 39 Marc Véron 2017-04-18 19:41:05 UTC
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>
Comment 40 Marc Véron 2017-04-18 19:46:12 UTC
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>
Comment 41 Marc Véron 2017-04-18 19:47:28 UTC
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>
Comment 42 Marc Véron 2017-04-18 19:48:46 UTC
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>
Comment 43 Marc Véron 2017-04-18 19:49:41 UTC
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>
Comment 44 Marc Véron 2017-04-18 19:50:38 UTC
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>
Comment 45 Marc Véron 2017-04-18 19:51:42 UTC
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>
Comment 46 Marc Véron 2017-04-18 19:53:01 UTC
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>
Comment 47 Marc Véron 2017-04-18 19:57:19 UTC Comment hidden (obsolete)
Comment 48 Julian Maurice 2017-04-19 07:36:26 UTC
Thanks for testing Marc!
Have you also tested translation with context (regarding bug 11285) ?
Comment 49 Marc Véron 2017-04-19 12:19:06 UTC
(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!
Comment 50 Jonathan Druart 2017-04-25 19:23:12 UTC
[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?
Comment 51 Julian Maurice 2017-04-26 07:17:36 UTC
(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.
Comment 52 Julian Maurice 2017-04-26 09:07:24 UTC Comment hidden (obsolete)
Comment 53 Marcel de Rooy 2017-07-14 08:45:55 UTC
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.
Comment 54 Mirko Tietgen 2017-07-14 08:58:32 UTC
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.
Comment 55 Julian Maurice 2017-07-24 16:11:52 UTC
(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)
Comment 56 Julian Maurice 2017-08-11 10:44:27 UTC
Created attachment 65894 [details] [review]
Bug 15395: Remove the use of Locale::Util's set_locale
Comment 57 Jesse Weaver 2018-01-26 20:04:43 UTC
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>
Comment 58 Jesse Weaver 2018-01-26 20:04:46 UTC
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
Comment 59 Jesse Weaver 2018-01-26 20:04:50 UTC
Created attachment 70976 [details] [review]
Bug 15395: Remove the use of Locale::Util's set_locale
Comment 60 paxed 2018-02-19 06:50:01 UTC
(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.
Comment 61 Julian Maurice 2018-02-19 13:02:02 UTC
(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.
Comment 62 Julian Maurice 2018-02-19 13:47:32 UTC
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>
Comment 63 Julian Maurice 2018-02-19 13:47:42 UTC
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>
Comment 64 Julian Maurice 2018-02-19 13:47:56 UTC
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>
Comment 65 Julian Maurice 2018-02-19 13:48:02 UTC
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>
Comment 66 Julian Maurice 2018-02-19 13:48:09 UTC
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>
Comment 67 Julian Maurice 2018-02-19 13:48:15 UTC
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>
Comment 68 Julian Maurice 2018-02-19 13:48:23 UTC
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>
Comment 69 Julian Maurice 2018-02-19 13:48:30 UTC
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>
Comment 70 Julian Maurice 2018-02-19 13:48:37 UTC
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>
Comment 71 Julian Maurice 2018-02-19 13:48:45 UTC
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
Comment 72 Julian Maurice 2018-02-19 13:48:51 UTC
Created attachment 71950 [details] [review]
Bug 15395: Remove the use of Locale::Util's set_locale
Comment 73 Julian Maurice 2018-02-19 13:48:58 UTC
Created attachment 71951 [details] [review]
Bug 15395: Improve TT plugin example

Include the number in the translatable string
Comment 74 Julian Maurice 2018-02-19 13:49:04 UTC
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
Comment 75 Julian Maurice 2018-04-10 06:26:31 UTC
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>
Comment 76 Julian Maurice 2018-04-10 06:26:36 UTC
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>
Comment 77 Julian Maurice 2018-04-10 06:26:44 UTC
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>
Comment 78 Julian Maurice 2018-04-10 06:26:50 UTC
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>
Comment 79 Julian Maurice 2018-04-10 06:26:55 UTC
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>
Comment 80 Julian Maurice 2018-04-10 06:27:00 UTC
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>
Comment 81 Julian Maurice 2018-04-10 06:27:06 UTC
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>
Comment 82 Julian Maurice 2018-04-10 06:27:11 UTC
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>
Comment 83 Julian Maurice 2018-04-10 06:27:16 UTC
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>
Comment 84 Julian Maurice 2018-04-10 06:27:22 UTC
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
Comment 85 Julian Maurice 2018-04-10 06:27:27 UTC
Created attachment 73911 [details] [review]
Bug 15395: Remove the use of Locale::Util's set_locale
Comment 86 Julian Maurice 2018-04-10 06:27:32 UTC
Created attachment 73912 [details] [review]
Bug 15395: Improve TT plugin example

Include the number in the translatable string
Comment 87 Julian Maurice 2018-04-10 06:27:38 UTC
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
Comment 88 Julian Maurice 2018-04-10 06:27:54 UTC
Patches rebased on master
Comment 89 Julian Maurice 2018-06-25 06:33:05 UTC
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.
Comment 90 Julian Maurice 2018-06-25 06:33:15 UTC
Created attachment 76329 [details] [review]
Bug 15395: Extract strings from TT BLOCKs too
Comment 91 Julian Maurice 2018-06-25 06:33:25 UTC
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
Comment 92 Julian Maurice 2018-08-03 15:24:26 UTC
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>
Comment 93 Julian Maurice 2018-08-03 15:24:43 UTC
Created attachment 77476 [details] [review]
Bug 15395: Example usage of I18N Template::Toolkit plugin

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 94 Julian Maurice 2018-08-03 15:30:33 UTC
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).
Comment 95 Julian Maurice 2018-08-03 15:33:45 UTC
JS part is in bug 21156
Comment 96 Jonathan Druart 2018-10-23 16:37:58 UTC
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.
Comment 97 Jonathan Druart 2018-10-23 16:43:05 UTC
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);
Comment 98 Julian Maurice 2018-10-23 17:51:47 UTC
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.
Comment 99 Jonathan Druart 2018-10-23 18:06:24 UTC
(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!
Comment 100 Julian Maurice 2018-10-23 18:29:29 UTC
(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.
Comment 101 Jonathan Druart 2018-10-23 18:56:49 UTC
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)
Comment 102 Jonathan Druart 2018-10-23 19:21:54 UTC
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
Comment 103 Julian Maurice 2018-10-23 19:30:53 UTC
(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.
Comment 104 Jonathan Druart 2018-10-24 15:50:59 UTC
(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?
Comment 105 Julian Maurice 2018-10-25 08:33:35 UTC
(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
Comment 106 Julian Maurice 2018-10-25 08:35:12 UTC
(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
Comment 107 Jonathan Druart 2018-10-25 11:46:04 UTC
(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.
Comment 108 Julian Maurice 2018-10-25 13:23:08 UTC
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>
Comment 109 Julian Maurice 2018-10-25 13:23:16 UTC
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
Comment 110 Julian Maurice 2018-10-25 13:23:23 UTC
Created attachment 81154 [details] [review]
Bug 15395: Make msgmerge quiet
Comment 111 Julian Maurice 2018-10-25 13:23:29 UTC
Created attachment 81155 [details] [review]
Bug 15395: Do not use nl_putenv, use $ENV instead

nl_putenv is only useful on Windows systems
Comment 112 Julian Maurice 2018-10-25 13:23:36 UTC
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
Comment 113 Julian Maurice 2018-10-25 13:23:42 UTC
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
Comment 114 Julian Maurice 2018-10-25 13:23:53 UTC
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.
Comment 115 Julian Maurice 2018-10-25 13:24:00 UTC
Created attachment 81159 [details] [review]
Bug 15395: Example usage of I18N Template::Toolkit plugin

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 116 Julian Maurice 2018-10-25 13:26:16 UTC
(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]
Comment 117 Julian Maurice 2018-10-25 13:28:13 UTC
(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
Comment 118 Julian Maurice 2018-10-25 13:34:12 UTC
(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
Comment 119 Julian Maurice 2018-10-25 14:58:10 UTC
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
Comment 120 Katrin Fischer 2018-10-27 09:18:16 UTC
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?
Comment 121 Julian Maurice 2018-10-27 10:08:26 UTC
(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 ?
Comment 122 Katrin Fischer 2018-10-27 10:29:23 UTC
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?
Comment 123 Katrin Fischer 2018-10-27 10:38:41 UTC
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.
Comment 124 Julian Maurice 2018-10-29 08:51:30 UTC
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)
Comment 125 Julian Maurice 2018-10-29 08:54:12 UTC
Moved the wiki page to https://wiki.koha-community.org/wiki/Internationalization,_plural_forms,_context,_and_more_RFC (parentheses break URL)
Comment 126 Julian Maurice 2018-10-29 09:09:56 UTC
(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') %]
Comment 127 Katrin Fischer 2018-10-29 18:53:18 UTC
(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.
Comment 128 David Cook 2018-10-30 23:16:46 UTC
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.
Comment 129 Victor Grousset/tuxayo 2018-10-31 08:41:30 UTC
> 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
Comment 130 Jonathan Druart 2018-10-31 14:34:34 UTC
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.
Comment 131 Jonathan Druart 2018-10-31 15:01:32 UTC
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>
Comment 132 Jonathan Druart 2018-10-31 15:01:45 UTC
Created attachment 81735 [details] [review]
Bug 15395: Make msgmerge quiet

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 133 Jonathan Druart 2018-10-31 15:01:56 UTC
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>
Comment 134 Jonathan Druart 2018-10-31 15:02:07 UTC
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>
Comment 135 Jonathan Druart 2018-10-31 15:02:21 UTC
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>
Comment 136 Jonathan Druart 2018-10-31 15:02:33 UTC
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>
Comment 137 Jonathan Druart 2018-10-31 15:02:51 UTC
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>
Comment 138 Jonathan Druart 2018-10-31 15:03:05 UTC
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>
Comment 139 Jonathan Druart 2018-10-31 15:03:16 UTC
Created attachment 81743 [details] [review]
Bug 15395: Fix QA failure - filter not found

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 140 Nick Clemens 2018-11-08 12:44:11 UTC
This updates dependencies - will we be able to get these packaged and ready for release?
Comment 141 Julian Maurice 2018-11-08 12:50:50 UTC
(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.
Comment 142 Nick Clemens 2018-11-08 18:24:43 UTC
Awesome work all!

Pushed to master for 18.11
Comment 143 Katrin Fischer 2018-11-08 18:48:15 UTC
For the next one getting stuck:

sudo apt-get install libintl-perl
Comment 144 Martin Renvoize 2018-11-16 13:56:43 UTC
Enhancement, will not be backported to 18.05.x series.