Bug 20988

Summary: [OMNIBUS] Internationalization: wrap all translatable text inside t() calls
Product: Koha Reporter: Julian Maurice <julian.maurice>
Component: I18N/L10NAssignee: Julian Maurice <julian.maurice>
Status: BLOCKED --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: bdiazmunoz, black23, f.demians, fridolin.somers, jonathan.druart, katrin.fischer, m.de.rooy, martin.renvoize, mtj, pasi.kallinen, victor
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29602
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 26392, 15395    
Bug Blocks:    
Attachments: Bug 20988: Modify templates so that they can be parsed as HTML
Bug 20988: Allow to create po for all languages at once
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/includes
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/help
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/unimarc
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/acqui
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/admin
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/authorities
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/basket
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/catalogue
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/circ
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/clubs
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/labels
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/members
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/onboarding
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/patroncards
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/plugins
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/installer
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/reports
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/serials
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/tools
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/offline_circ
Bug 20988: koha-tmpl/opac-tmpl/bootstrap/en/includes
Bug 20988: koha-tmpl/opac-tmpl/bootstrap/en/modules/opac
Bug 20988: koha-tmpl/opac-tmpl/bootstrap/en/modules
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules
Bug 20988: Add a sample of a translated PO file
Bug 20988: Add template macros to ease template rewriting
Bug 20988: Add template macros to ease template rewriting

Description Julian Maurice 2018-06-25 07:17:16 UTC
Bug 15395's Koha::I18n is working well, but it is basically useless until we wrap all the translatable strings inside __*() calls (or t*() calls inside templates).
This bug aims to do that.

I wrote a little tool to automatically find and replace translatable text by the corresponding call. It can be found here : https://github.com/jajm/koha-i18n.

This bug will hold a few patches that will make koha-i18n work better and I'll try to attach all the changes it does.
Comment 1 Julian Maurice 2018-06-25 07:20:26 UTC
Created attachment 76333 [details] [review]
Bug 20988: Modify templates so that they can be parsed as HTML

This is required for koha-i18n [1] to work correctly

[1] https://github.com/jajm/koha-i18n
Comment 2 Julian Maurice 2018-06-25 07:20:50 UTC
Created attachment 76334 [details] [review]
Bug 20988: Allow to create po for all languages at once

This is required for the script that comes with koha-i18n, which creates
the new PO files and fill them with existing translations.
Comment 3 Julian Maurice 2018-06-25 07:21:11 UTC
Created attachment 76335 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/includes
Comment 4 Julian Maurice 2018-06-25 07:21:23 UTC
Created attachment 76336 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/help
Comment 5 Julian Maurice 2018-06-25 07:21:34 UTC
Created attachment 76337 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/unimarc
Comment 6 Julian Maurice 2018-06-25 07:21:40 UTC
Created attachment 76338 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder
Comment 7 Julian Maurice 2018-06-25 07:21:48 UTC
Created attachment 76339 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/acqui
Comment 8 Julian Maurice 2018-06-25 07:21:58 UTC
Created attachment 76340 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/admin
Comment 9 Julian Maurice 2018-06-25 07:22:03 UTC
Created attachment 76341 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/authorities
Comment 10 Julian Maurice 2018-06-25 07:22:08 UTC
Created attachment 76342 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/basket
Comment 11 Julian Maurice 2018-06-25 07:22:14 UTC
Created attachment 76343 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/catalogue
Comment 12 Julian Maurice 2018-06-25 07:22:20 UTC
Created attachment 76344 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing
Comment 13 Julian Maurice 2018-06-25 07:22:27 UTC
Created attachment 76345 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/circ
Comment 14 Julian Maurice 2018-06-25 07:22:32 UTC
Created attachment 76346 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/clubs
Comment 15 Julian Maurice 2018-06-25 07:22:36 UTC
Created attachment 76347 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves
Comment 16 Julian Maurice 2018-06-25 07:22:42 UTC
Created attachment 76348 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/labels
Comment 17 Julian Maurice 2018-06-25 07:22:48 UTC
Created attachment 76349 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/members
Comment 18 Julian Maurice 2018-06-25 07:22:52 UTC
Created attachment 76350 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/onboarding
Comment 19 Julian Maurice 2018-06-25 07:22:58 UTC
Created attachment 76351 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists
Comment 20 Julian Maurice 2018-06-25 07:23:03 UTC
Created attachment 76352 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/patroncards
Comment 21 Julian Maurice 2018-06-25 07:23:07 UTC
Created attachment 76353 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/plugins
Comment 22 Julian Maurice 2018-06-25 07:23:12 UTC
Created attachment 76354 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/installer
Comment 23 Julian Maurice 2018-06-25 07:23:18 UTC
Created attachment 76355 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/reports
Comment 24 Julian Maurice 2018-06-25 07:23:23 UTC
Created attachment 76356 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections
Comment 25 Julian Maurice 2018-06-25 07:23:31 UTC
Created attachment 76357 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/serials
Comment 26 Julian Maurice 2018-06-25 07:23:39 UTC
Created attachment 76358 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/tools
Comment 27 Julian Maurice 2018-06-25 07:23:43 UTC
Created attachment 76359 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves
Comment 28 Julian Maurice 2018-06-25 07:23:48 UTC
Created attachment 76360 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/offline_circ
Comment 29 Julian Maurice 2018-06-25 07:23:55 UTC
Created attachment 76361 [details] [review]
Bug 20988: koha-tmpl/opac-tmpl/bootstrap/en/includes
Comment 30 Julian Maurice 2018-06-25 07:24:07 UTC
Created attachment 76362 [details] [review]
Bug 20988: koha-tmpl/opac-tmpl/bootstrap/en/modules/opac
Comment 31 Julian Maurice 2018-06-25 07:24:14 UTC
Created attachment 76363 [details] [review]
Bug 20988: koha-tmpl/opac-tmpl/bootstrap/en/modules
Comment 32 Julian Maurice 2018-06-25 07:24:22 UTC
Created attachment 76364 [details] [review]
Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules
Comment 33 Julian Maurice 2018-06-25 07:45:24 UTC
Created attachment 76365 [details] [review]
Bug 20988: Add a sample of a translated PO file

PO files are too big to be attached on Bugzilla, so here's a gzipped
version of misc/translator/po/fr-FR-messages.po as generated by
create-messages.sh (found in https://github.com/jajm/koha-i18n)

After applying the patch, run
  gunzip misc/translator/po/fr-FR-messages.po.gz

A few numbers about this PO file:
- Number of messages: 14178
- Number of translated messages: 13631
- Number of non-fuzzy translated messages: 9720
Comment 34 Julian Maurice 2018-06-25 07:48:49 UTC
All theses patches (except the first two) were generated with the following commands:

  koha-i18n --in-place koha-tmpl/**/en/**/*.tt koha-tmpl/**/en/**/*.inc  # might 
be a zsh'ism, haven't tried with bash
  cd misc/translator && $GOPATH/src/github.com/jajm/koha-i18n/scripts/create-messages.sh
Comment 35 Julian Maurice 2018-06-25 07:54:46 UTC
Test plan:

1. Apply bug 15395
2. Apply the first two patches of this bug
3. Install koha-i18n (see the README)
4. Run koha-i18n on all .tt and .inc files
5. cd to misc/translator and run $GOPATH/src/github.com/jajm/koha-i18n/scripts/create-messages.sh
6. Run ./translate install fr-FR  # or any available language
7. Reload starman
8. Go to Koha with your browser, switch language to fr-FR and check that the strings translated in misc/translator/fr-FR-messages.po are translated in the interface.
Comment 36 paxed 2018-06-26 06:14:56 UTC
Unfortunately quite a lot of the messages are split in multiple parts, making translation very hard:

[% t('Warning: This file is version') %] [% message.upload_version %][% t(', but I only know how to import version') %] [% message.current_version %][% t('. I\'ll try my best.') %]


Currently they're much better, with placeholders:

"Warning: This file is version %s, but I only know how to import version %s. "
Comment 37 Julian Maurice 2018-06-26 06:52:59 UTC
(In reply to paxed from comment #36)
> Unfortunately quite a lot of the messages are split in multiple parts,
> making translation very hard:
> 
> [% t('Warning: This file is version') %] [% message.upload_version %][% t(',
> but I only know how to import version') %] [% message.current_version %][%
> t('. I\'ll try my best.') %]
> 
> 
> Currently they're much better, with placeholders:
> 
> "Warning: This file is version %s, but I only know how to import version %s.
> "

I agree. There's still some manual work, the goal of this bug is to reduce it to a minimum.

This particular example should be

[% tx('Warning: This file is version {upload_version} but I only know how to import {current_version}. I\'ll try my best.', { upload_version = upload_version, current_version = current_version) %]

I'm not sure we can accomplish that easily with an automated tool. It's probably easier to run the tool and fix manually the problematic ones.

But please note that, unlike actual PO files, the one attached doesn't contain strings like this:

"%s %s %s &nbsp; %s %s - %s %s %s - %s%s %s- %s%s %s - %s%s %sEdition: %s%s "
"%s ; %s%s %s - %s%s %s ; %s%s %s "


Also, I attached all the changes to this one bug, but that doesn't mean we should do it all at once. We can take the safer way of changing one template file at a time (or one module at a time). It will certainly be easier to rebase/test/review.
Comment 38 paxed 2018-06-28 06:55:42 UTC
BTW, my take on reducing the number of TT placeholders is in bug 12221

(In reply to Julian Maurice from comment #37)
> 
> But please note that, unlike actual PO files, the one attached doesn't
> contain strings like this:
> 
> "%s %s %s &nbsp; %s %s - %s %s %s - %s%s %s- %s%s %s - %s%s %sEdition: %s%s "
> "%s ; %s%s %s - %s%s %s ; %s%s %s "
>
Comment 39 Julian Maurice 2018-06-28 07:09:02 UTC
(In reply to paxed from comment #38)
> BTW, my take on reducing the number of TT placeholders is in bug 12221

I wasn't aware of this bug, thanks.
Comment 40 Julian Maurice 2018-07-13 08:29:36 UTC
Created attachment 76934 [details] [review]
Bug 20988: Add template macros to ease template rewriting
Comment 41 Julian Maurice 2018-07-13 08:30:13 UTC
Created attachment 76935 [details] [review]
Bug 20988: Add template macros to ease template rewriting
Comment 42 Julian Maurice 2018-07-13 08:32:29 UTC
There's a new version of koha-i18n, written in Perl this time. It is available at the same URL (https://github.com/jajm/koha-i18n).
It tries to wrap longer strings, so it should address comment 36.

I obsoleted all generated patches, the result can be seen here: https://gitlab.com/jajm/Koha/commits/bug/20988
Comment 43 Michal Denar 2018-09-13 12:04:23 UTC
Any testplan?
Comment 44 Julian Maurice 2018-09-13 12:41:33 UTC
(In reply to Michal Denar from comment #43)
> Any testplan?

Yes, see comment 35
Comment 45 Fridolin Somers 2018-09-24 14:48:27 UTC
Is it possible to use double-quotes ?
  [% t("translate me") %]
Because actually translations are made for text with double quotes especially for JavaScript. Because some translated text can add a single quote like :
today => aujourd'hui
Comment 46 Julian Maurice 2018-09-24 15:01:17 UTC
(In reply to Fridolin SOMERS from comment #45)
> Is it possible to use double-quotes ?
>   [% t("translate me") %]
> Because actually translations are made for text with double quotes
> especially for JavaScript. Because some translated text can add a single
> quote like :
> today => aujourd'hui
You can use single quotes or double quotes. It doesn't matter since strings are translated at runtime (and not replaced in the source file like `./translate install` does).

So you can write
[% t('A string with "double quotes"') %]
and
[% t("A string with 'single quotes'") %]
Comment 47 Fridolin Somers 2018-11-16 15:15:37 UTC
(In reply to Julian Maurice from comment #46)
> (In reply to Fridolin SOMERS from comment #45)
> > Is it possible to use double-quotes ?
> >   [% t("translate me") %]
> > Because actually translations are made for text with double quotes
> > especially for JavaScript. Because some translated text can add a single
> > quote like :
> > today => aujourd'hui
> You can use single quotes or double quotes. It doesn't matter since strings
> are translated at runtime (and not replaced in the source file like
> `./translate install` does).
> 
> So you can write
> [% t('A string with "double quotes"') %]
> and
> [% t("A string with 'single quotes'") %]

Ah of course. Nice ;)
Comment 48 Martin Renvoize 2020-04-15 17:21:15 UTC
I lost track of this one.. not sure what the next steps are?
Comment 49 Martin Renvoize 2020-04-15 17:22:19 UTC
At this point I think it's probably one for early in the 20.11 cycle..
Comment 50 Owen Leonard 2020-07-07 14:38:35 UTC
I think it would be better to have some patches on this bug which are proof-of-concept rather than trying to update everything at once.
Comment 51 Julian Maurice 2020-07-07 14:57:41 UTC
(In reply to Owen Leonard from comment #50)
> I think it would be better to have some patches on this bug which are
> proof-of-concept rather than trying to update everything at once.

I was thinking maybe one template at a time in separate bugs ? Easier to test and QA.
Comment 52 Victor Grousset/tuxayo 2020-07-09 22:41:43 UTC
+1 for proof-of-concept

Maybe also one template at a time, I don't know. Maybe beginning with the proof-of-concepts and see if it's necessary to keep doing one by one?
Comment 53 Julian Maurice 2020-09-04 14:59:10 UTC
I opened bug 26392
Comment 54 Jonathan Druart 2020-11-11 08:25:45 UTC
Julian, I'd like to know if we need this one or not, for 21.05.

Could you send an email to koha-devel with a pro/con list and detail what is done here? Then a vote at a dev meeting will release the situation here.
Comment 55 Jonathan Druart 2021-12-01 10:08:58 UTC
I am getting an error:
Template process failed: file error - recursion into 'i18n.inc' at /kohadevbox/koha/C4/Templates.pm line 121
Comment 56 Jonathan Druart 2021-12-01 10:10:32 UTC
(In reply to Jonathan Druart from comment #55)
> I am getting an error:
> Template process failed: file error - recursion into 'i18n.inc' at
> /kohadevbox/koha/C4/Templates.pm line 121

ok, fixed.

bin/koha-i18n ~/path/to/koha/**/*.tt ~/path/to/koha/**/*.inc
then remove the changes generated to i18n.inc
Comment 57 Jonathan Druart 2021-12-01 10:16:36 UTC
1. Item search is broken, invalid JSON.

2. /members/members-home.pl

Template process failed: file error - parse error - /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/members/member.tt line 153: unexpected token (")
  [% " s_o | html %] at /kohadevbox/koha/C4/Templates.pm line 121

3. /members/moremember.pl?borrowernumber=51

Template process failed: file error - parse error - patron-title.inc line 38: unexpected end of directive
  [% t('\' %] at /kohadevbox/koha/C4/Templates.pm line 121
Comment 58 Victor Grousset/tuxayo 2021-12-05 18:35:06 UTC
In 29602 the question of performance came up.

Here will be moved some comments from there.

Here are results of the benchmarks of bug 15395 on my laptop with a 2012 CPU (i5-3320M @3.3GHz on mono thread loads)

Summary
Average for 10 strings without translation:	0.0001145 seconds
Average for 100 strings without translation:	0.0001114 seconds
Average for 200 strings without translation:	9.82e-05 seconds
Average for 500 strings without translation:	0.0001932 seconds
Average for 10 strings with translation:	0.0014077 seconds
Average for 100 strings with translation:	0.0076227 seconds
Average for 200 strings with translation:	0.0120828 seconds
Average for 500 strings with translation:	0.0306117 seconds

The same PC has a median time of 0.350 sec to generate the untranslated HTML for this page:
/cgi-bin/koha/admin/preferences.pl?tab=circulation
(using browser network devtools)
There are 200 sysprefs shown there. So it's surely one the of the pages with the most translatable strings we have.
(Well, searching "a" in sysprefs yields 740 of them ^^" in more than 0.950 sec)

git grep "circulation.pref#"  misc/translator/po/bg-Cyrl-pref.po | wc -l
=> 633
So it seems we have 633 strings in our circulation sysprefs.
if 500 strings took 0.031 sec in the benchmark
then 633 strings => 0.039 sec
0.039 is 11% of 0.350
So we should expected that using t() on all string on this page should add 11% response time to 0.350 sec.

Would there be another case where the overhead could be the more perceptible than this? (it's not really perceptible)
Is the methodology valid?

If so then we shouldn't expect issues about performance.
Comment 59 Victor Grousset/tuxayo 2021-12-05 18:36:55 UTC
Jonathan wrote (bug 29602 comment 17)
(In reply to Victor Grousset/tuxayo from comment #16)
> Is the methodology valid?

You need to count how many times you enter t() instead of guessing the
number of lines matching a pattern there are in a file.
Comment 60 Victor Grousset/tuxayo 2021-12-05 18:46:00 UTC
(In reply to Jonathan Druart)
> You need to count how many times you enter t() instead of guessing the
> number of lines matching a pattern there are in a file.

Won't we have one t() usage per string starting with `circulation.pref#` ?

Doesn't the above prefix matches all the translatable strings of the page with all the circulation sysprefs? Except the ones on the left and up parts of the UI but in the end the strings of the syspref descriptions and values(dropdowns) dominate the whole page.
Comment 61 Jonathan Druart 2021-12-08 11:24:09 UTC
(In reply to Victor Grousset/tuxayo from comment #60)
> (In reply to Jonathan Druart)
> > You need to count how many times you enter t() instead of guessing the
> > number of lines matching a pattern there are in a file.
> 
> Won't we have one t() usage per string starting with `circulation.pref#` ?
> 
> Doesn't the above prefix matches all the translatable strings of the page
> with all the circulation sysprefs? Except the ones on the left and up parts
> of the UI but in the end the strings of the syspref descriptions and
> values(dropdowns) dominate the whole page.

The exact reason I am suggesting to count how many times you enter the method is that I don't want to think about those questions. Create a global var $i, warn ++$i and you will know.
Comment 62 Victor Grousset/tuxayo 2021-12-12 12:11:43 UTC
> The exact reason I am suggesting to count how many times you enter the method is that I don't want to think about those questions.

Oh, I see.

> Create a global var $i, warn ++$i and you will know.

Would that be using the wrapping script on the syspref template and then adding the counter in t in Koha/Template/Plugin/I18N.pm
Comment 63 Victor Grousset/tuxayo 2022-01-05 19:26:42 UTC
Did you mean Joubu that in order to check, the wrapping script (https://github.com/jajm/koha-i18n) should be used on the syspref template. And then a global counter can be added in the t sub in Koha/Template/Plugin/I18N.pm ?
Comment 64 Jonathan Druart 2022-06-03 04:34:27 UTC
(In reply to Victor Grousset/tuxayo from comment #63)
> Did you mean Joubu that in order to check, the wrapping script
> (https://github.com/jajm/koha-i18n) should be used on the syspref template.
> And then a global counter can be added in the t sub in
> Koha/Template/Plugin/I18N.pm ?

Yes.
Comment 65 Martin Renvoize 2023-09-20 20:42:13 UTC
With the weblate move around the corner and discussions taking place around the different po files we now have and confusion around what does what.. is this worth revisiting?
Comment 66 Jonathan Druart 2023-09-21 06:27:12 UTC
(In reply to Martin Renvoize from comment #65)
> With the weblate move around the corner and discussions taking place around
> the different po files we now have and confusion around what does what.. is
> this worth revisiting?

Nope, that's not related IMO.