|
Description
Julian Maurice
2018-06-25 07:17:16 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 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. Created attachment 76335 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/includes Created attachment 76336 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/help Created attachment 76337 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/unimarc Created attachment 76338 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder Created attachment 76339 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/acqui Created attachment 76340 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/admin Created attachment 76341 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/authorities Created attachment 76342 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/basket Created attachment 76343 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/catalogue Created attachment 76344 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing Created attachment 76345 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/circ Created attachment 76346 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/clubs Created attachment 76347 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves Created attachment 76348 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/labels Created attachment 76349 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/members Created attachment 76350 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/onboarding Created attachment 76351 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists Created attachment 76352 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/patroncards Created attachment 76353 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/plugins Created attachment 76354 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/installer Created attachment 76355 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/reports Created attachment 76356 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/rotating_collections Created attachment 76357 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/serials Created attachment 76358 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/tools Created attachment 76359 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves Created attachment 76360 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules/offline_circ Created attachment 76361 [details] [review] Bug 20988: koha-tmpl/opac-tmpl/bootstrap/en/includes Created attachment 76362 [details] [review] Bug 20988: koha-tmpl/opac-tmpl/bootstrap/en/modules/opac Created attachment 76363 [details] [review] Bug 20988: koha-tmpl/opac-tmpl/bootstrap/en/modules Created attachment 76364 [details] [review] Bug 20988: koha-tmpl/intranet-tmpl/prog/en/modules 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 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 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. 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. "
(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 %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. 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 %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 " > (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. Created attachment 76934 [details] [review] Bug 20988: Add template macros to ease template rewriting Created attachment 76935 [details] [review] Bug 20988: Add template macros to ease template rewriting 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 Any testplan? (In reply to Michal Denar from comment #43) > Any testplan? Yes, see comment 35 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
(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'") %] (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 ;) I lost track of this one.. not sure what the next steps are? At this point I think it's probably one for early in the 20.11 cycle.. 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. (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. +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? I opened bug 26392 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. I am getting an error: Template process failed: file error - recursion into 'i18n.inc' at /kohadevbox/koha/C4/Templates.pm line 121 (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 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
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. 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. (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.
(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. > 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 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 ? (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. 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? (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. |