Bug 36357 - Add Template::Toolkit filters for internationalization (I18N)
Summary: Add Template::Toolkit filters for internationalization (I18N)
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: I18N/L10N (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Julian Maurice
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-03-19 08:27 UTC by Julian Maurice
Modified: 2024-07-21 20:58 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 36357: Add Template::Toolkit filters for internationalization (I18N) (6.54 KB, patch)
2024-03-19 08:28 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 36357: Add Template::Toolkit filters for internationalization (I18N) (6.54 KB, patch)
2024-03-19 08:28 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 36357: Add Template::Toolkit filters for internationalization (I18N) (6.60 KB, patch)
2024-03-20 12:10 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 36357: Tidy xgettext-tt2.t (3.13 KB, patch)
2024-03-20 12:11 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 36357: Tidy I18N.pm (2.61 KB, patch)
2024-03-20 12:11 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 36357: Add POD to Filters/I18N.pm (2.82 KB, patch)
2024-03-20 12:11 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 36357: Fix Template::Filters error (657 bytes, patch)
2024-03-21 10:18 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 36357: Add Template::Toolkit filters for internationalization (I18N) (6.65 KB, patch)
2024-07-15 03:16 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 36357: Tidy xgettext-tt2.t (3.19 KB, patch)
2024-07-15 03:17 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 36357: Tidy I18N.pm (2.66 KB, patch)
2024-07-15 03:17 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 36357: Add POD to Filters/I18N.pm (2.88 KB, patch)
2024-07-15 03:17 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 36357: Fix Template::Filters error (713 bytes, patch)
2024-07-15 03:17 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 36357: Fix typos (1.02 KB, patch)
2024-07-15 07:08 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 36357: Use "use" instead of "require" (996 bytes, patch)
2024-07-15 07:08 UTC, Julian Maurice
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2024-03-19 08:27:17 UTC

    
Comment 1 Julian Maurice 2024-03-19 08:28:01 UTC
Created attachment 163380 [details] [review]
Bug 36357: Add Template::Toolkit filters for internationalization (I18N)

This patch makes it possible to write in templates the following:

[% 'simple text' | t %]
[% 'text with {var}' | tx(var = 'value') %]
[% 'singular' | tn('plural', count) %]
[% 'singular with {var}' | tnx('plural with {var}', cnt, var = 'val') %]

These filters are an alternative to the macros defined in i18n.inc.
Some advantages:
* They do not conflict with variables (see bug 36927)
* They are always defined (no need to USE a plugin or PROCESS a file)

i18n functions that takes a context argument (tp, tpx, tnp, tnpx)
are deliberately not defined as filters.
Because context is the first argument, this would require writing:
  [% 'context' | tp('message') %]
Whereas we'd prefer to write:
  [% 'message' | tp('context') %]
There are two ways to fix this:
1. Change the order of arguments in Koha::Template::Plugin::I18N and
   change the -k options in xgettext-tt2 (the msgid must be first), or
2. Use different keywords, and add them as -k options in xgettext-tt2

Test plan:
1. Find a template file that you can edit and where you can easily see
   the changes (for instance intranet-main.tt)
2. Add the following code:
   [% 'simple text' | t %]
   [% 'text with {var}' | tx(var = 'value') %]
   [% 'singular' | tn('plural', 1) %]
   [% 'singular with {var}' | tnx('plural with {var}', 2, var = 'val') %]
3. Run misc/translator/translate update fr-FR
4. Open misc/translator/po/fr-FR-messages.po, make sure you find those 4
   new strings. Translate them.
5. Run misc/translator/translate install fr-FR
6. Restart Koha
7. Change the language in the interface. Verify that the strings are
   translated
Comment 2 Julian Maurice 2024-03-19 08:28:42 UTC
Created attachment 163381 [details] [review]
Bug 36357: Add Template::Toolkit filters for internationalization (I18N)

This patch makes it possible to write in templates the following:

[% 'simple text' | t %]
[% 'text with {var}' | tx(var = 'value') %]
[% 'singular' | tn('plural', count) %]
[% 'singular with {var}' | tnx('plural with {var}', cnt, var = 'val') %]

These filters are an alternative to the macros defined in i18n.inc.
Some advantages:
* They do not conflict with variables (see bug 36297)
* They are always defined (no need to USE a plugin or PROCESS a file)

i18n functions that takes a context argument (tp, tpx, tnp, tnpx)
are deliberately not defined as filters.
Because context is the first argument, this would require writing:
  [% 'context' | tp('message') %]
Whereas we'd prefer to write:
  [% 'message' | tp('context') %]
There are two ways to fix this:
1. Change the order of arguments in Koha::Template::Plugin::I18N and
   change the -k options in xgettext-tt2 (the msgid must be first), or
2. Use different keywords, and add them as -k options in xgettext-tt2

Test plan:
1. Find a template file that you can edit and where you can easily see
   the changes (for instance intranet-main.tt)
2. Add the following code:
   [% 'simple text' | t %]
   [% 'text with {var}' | tx(var = 'value') %]
   [% 'singular' | tn('plural', 1) %]
   [% 'singular with {var}' | tnx('plural with {var}', 2, var = 'val') %]
3. Run misc/translator/translate update fr-FR
4. Open misc/translator/po/fr-FR-messages.po, make sure you find those 4
   new strings. Translate them.
5. Run misc/translator/translate install fr-FR
6. Restart Koha
7. Change the language in the interface. Verify that the strings are
   translated
Comment 3 Kyle M Hall 2024-03-20 12:10:57 UTC
Created attachment 163521 [details] [review]
Bug 36357: Add Template::Toolkit filters for internationalization (I18N)

This patch makes it possible to write in templates the following:

[% 'simple text' | t %]
[% 'text with {var}' | tx(var = 'value') %]
[% 'singular' | tn('plural', count) %]
[% 'singular with {var}' | tnx('plural with {var}', cnt, var = 'val') %]

These filters are an alternative to the macros defined in i18n.inc.
Some advantages:
* They do not conflict with variables (see bug 36297)
* They are always defined (no need to USE a plugin or PROCESS a file)

i18n functions that takes a context argument (tp, tpx, tnp, tnpx)
are deliberately not defined as filters.
Because context is the first argument, this would require writing:
  [% 'context' | tp('message') %]
Whereas we'd prefer to write:
  [% 'message' | tp('context') %]
There are two ways to fix this:
1. Change the order of arguments in Koha::Template::Plugin::I18N and
   change the -k options in xgettext-tt2 (the msgid must be first), or
2. Use different keywords, and add them as -k options in xgettext-tt2

Test plan:
1. Find a template file that you can edit and where you can easily see
   the changes (for instance intranet-main.tt)
2. Add the following code:
   [% 'simple text' | t %]
   [% 'text with {var}' | tx(var = 'value') %]
   [% 'singular' | tn('plural', 1) %]
   [% 'singular with {var}' | tnx('plural with {var}', 2, var = 'val') %]
3. Run misc/translator/translate update fr-FR
4. Open misc/translator/po/fr-FR-messages.po, make sure you find those 4
   new strings. Translate them.
5. Run misc/translator/translate install fr-FR
6. Restart Koha
7. Change the language in the interface. Verify that the strings are
   translated

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 4 Kyle M Hall 2024-03-20 12:11:03 UTC
Created attachment 163522 [details] [review]
Bug 36357: Tidy xgettext-tt2.t

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 5 Kyle M Hall 2024-03-20 12:11:05 UTC
Created attachment 163523 [details] [review]
Bug 36357: Tidy I18N.pm

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 6 Kyle M Hall 2024-03-20 12:11:07 UTC
Created attachment 163524 [details] [review]
Bug 36357: Add POD to Filters/I18N.pm

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Kyle M Hall 2024-03-20 19:12:38 UTC
Getting
Can't locate object method "new" via package "Template::Filters" (perhaps you forgot to load "Template::Filters"?) at /kohadevbox/koha/C4/Templates.pm line 74
Comment 8 Julian Maurice 2024-03-21 07:39:41 UTC
(In reply to Kyle M Hall from comment #7)
> Getting
> Can't locate object method "new" via package "Template::Filters" (perhaps
> you forgot to load "Template::Filters"?) at /kohadevbox/koha/C4/Templates.pm
> line 74

How do you get that ? I can't reproduce
Comment 9 Kyle M Hall 2024-03-21 10:18:58 UTC
Created attachment 163597 [details] [review]
Bug 36357: Fix Template::Filters error

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 10 Kyle M Hall 2024-03-21 10:21:13 UTC
(In reply to Julian Maurice from comment #8)
> (In reply to Kyle M Hall from comment #7)
> > Getting
> > Can't locate object method "new" via package "Template::Filters" (perhaps
> > you forgot to load "Template::Filters"?) at /kohadevbox/koha/C4/Templates.pm
> > line 74
> 
> How do you get that ? I can't reproduce

Odd. I was able to fix the error by adding "use Template::Filters" to Templates.pm!
Comment 11 Victor Grousset/tuxayo 2024-07-15 03:16:44 UTC
Created attachment 168955 [details] [review]
Bug 36357: Add Template::Toolkit filters for internationalization (I18N)

This patch makes it possible to write in templates the following:

[% 'simple text' | t %]
[% 'text with {var}' | tx(var = 'value') %]
[% 'singular' | tn('plural', count) %]
[% 'singular with {var}' | tnx('plural with {var}', cnt, var = 'val') %]

These filters are an alternative to the macros defined in i18n.inc.
Some advantages:
* They do not conflict with variables (see bug 36297)
* They are always defined (no need to USE a plugin or PROCESS a file)

i18n functions that takes a context argument (tp, tpx, tnp, tnpx)
are deliberately not defined as filters.
Because context is the first argument, this would require writing:
  [% 'context' | tp('message') %]
Whereas we'd prefer to write:
  [% 'message' | tp('context') %]
There are two ways to fix this:
1. Change the order of arguments in Koha::Template::Plugin::I18N and
   change the -k options in xgettext-tt2 (the msgid must be first), or
2. Use different keywords, and add them as -k options in xgettext-tt2

Test plan:
1. Find a template file that you can edit and where you can easily see
   the changes (for instance intranet-main.tt)
2. Add the following code:
   [% 'simple text' | t %]
   [% 'text with {var}' | tx(var = 'value') %]
   [% 'singular' | tn('plural', 1) %]
   [% 'singular with {var}' | tnx('plural with {var}', 2, var = 'val') %]
3. Run misc/translator/translate update fr-FR
4. Open misc/translator/po/fr-FR-messages.po, make sure you find those 4
   new strings. Translate them.
5. Run misc/translator/translate install fr-FR
6. Restart Koha
7. Change the language in the interface. Verify that the strings are
   translated

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 12 Victor Grousset/tuxayo 2024-07-15 03:17:04 UTC
Created attachment 168956 [details] [review]
Bug 36357: Tidy xgettext-tt2.t

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 13 Victor Grousset/tuxayo 2024-07-15 03:17:07 UTC
Created attachment 168957 [details] [review]
Bug 36357: Tidy I18N.pm

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 14 Victor Grousset/tuxayo 2024-07-15 03:17:11 UTC
Created attachment 168958 [details] [review]
Bug 36357: Add POD to Filters/I18N.pm

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 15 Victor Grousset/tuxayo 2024-07-15 03:17:14 UTC
Created attachment 168959 [details] [review]
Bug 36357: Fix Template::Filters error

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 16 Victor Grousset/tuxayo 2024-07-15 03:22:47 UTC
What's the reason to choose `require Koha::Template::Filters::I18N;` vs `use`?
See https://wiki.koha-community.org/wiki/Coding_Guidelines#PERL31:_Prefer_.22use.22_than_.22require.22

----

typos found:
«Usesdf different»
«Instanticate»

---

Besides that, all seems to comply with coding guidelines :)

Additional testing was done to double check the plural and single cases.
To complement the test plan, I tested in the OPAC instead of staff interface. I don't see what else would be missing from test plan.

I don't know enough on the functional side and about eventual unwanted side effects but at least here is a partial QA on the boring steps ^^
Comment 17 Julian Maurice 2024-07-15 07:08:14 UTC
Created attachment 168962 [details] [review]
Bug 36357: Fix typos
Comment 18 Julian Maurice 2024-07-15 07:08:17 UTC
Created attachment 168963 [details] [review]
Bug 36357: Use "use" instead of "require"
Comment 19 Julian Maurice 2024-07-15 07:12:13 UTC
(In reply to Victor Grousset/tuxayo from comment #16)
> What's the reason to choose `require Koha::Template::Filters::I18N;` vs
> `use`?
It was only to avoid loading the module when not needed. I changed it since it isn't allowed by the new guideline.

Thanks for the review
Comment 20 Victor Grousset/tuxayo 2024-07-21 20:58:39 UTC
> It was only to avoid loading the module when not needed. I changed it since it isn't allowed by the new guideline.

If the loading when not needed is heavy enough, there would be a case to use require.


Anyway, thanks for the fixes :)