Bug 26392 - Wrap all translatable strings in opac-main.tt
Summary: Wrap all translatable strings in opac-main.tt
Status: Patch doesn't apply
Alias: None
Product: Koha
Classification: Unclassified
Component: I18N/L10N (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Julian Maurice
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 20988
  Show dependency treegraph
 
Reported: 2020-09-04 14:56 UTC by Julian Maurice
Modified: 2021-12-05 22:55 UTC (History)
8 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 26392: Wrap all translatable strings in opac-main.tt (20.81 KB, patch)
2020-09-04 14:57 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 26392: Wrap all translatable strings in opac-main.tt (21.27 KB, patch)
2020-09-11 13:31 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 26392: Fix opac news display issue when there is no author (1.87 KB, patch)
2020-09-11 13:31 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 26392: Wrap all translatable strings in opac-main.tt (21.35 KB, patch)
2020-09-11 14:23 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 26392: Fix opac news display issue when there is no author (1.92 KB, patch)
2020-09-11 14:23 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 26392: Wrap all translatable strings in opac-main.tt (22.82 KB, patch)
2021-03-05 09:14 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 26392: Fix opac news display issue when there is no author (2.27 KB, patch)
2021-03-05 09:14 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 2020-09-04 14:56:45 UTC

    
Comment 1 Julian Maurice 2020-09-04 14:57:26 UTC
Created attachment 109666 [details] [review]
Bug 26392: Wrap all translatable strings in opac-main.tt
Comment 2 Julian Maurice 2020-09-04 15:04:07 UTC
Test plan:
1. Apply patch
2. cd misc/translator && ./translate update fr-FR
3. Open and translate misc/translator/po/fr-FR-messages.po (don't forget plural forms)
4. cd misc/translator && ./translate install fr-FR
5. Go to http://OPAC/cgi-bin/koha/opac-main.pl and try to make all translated strings to show up (log in, log out, create news, do some checkouts, holds, ...)
Comment 3 Owen Leonard 2020-09-11 13:19:20 UTC
This looks good overall but I see one issue:

If I have the NewsAuthorDisplay set to "none" it looks like the template is still trying to show an author. I see something like this on each news item:

Published on 10.03.2019 by

I think the "author" variable is getting evaluated as true even if the [% IF %] block inside it evaluates as false. I think the "author" BLOCK belongs inside the "[% IF ( (newsdisp == 'opac'..."
Comment 4 Julian Maurice 2020-09-11 13:31:22 UTC Comment hidden (obsolete)
Comment 5 Julian Maurice 2020-09-11 13:31:27 UTC Comment hidden (obsolete)
Comment 6 Julian Maurice 2020-09-11 13:33:14 UTC
(In reply to Owen Leonard from comment #3)
> This looks good overall but I see one issue:
> 
> If I have the NewsAuthorDisplay set to "none" it looks like the template is
> still trying to show an author. I see something like this on each news item:
> 
> Published on 10.03.2019 by
> 
> I think the "author" variable is getting evaluated as true even if the [% IF
> %] block inside it evaluates as false. I think the "author" BLOCK belongs
> inside the "[% IF ( (newsdisp == 'opac'..."

You're 100% right :) Thanks
Fixed in the last patch.
Comment 7 Owen Leonard 2020-09-11 14:23:22 UTC
Created attachment 109947 [details] [review]
Bug 26392: Wrap all translatable strings in opac-main.tt

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 8 Owen Leonard 2020-09-11 14:23:28 UTC
Created attachment 109948 [details] [review]
Bug 26392: Fix opac news display issue when there is no author

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 9 Katrin Fischer 2020-09-12 22:10:16 UTC
This appears to work well, but it doesn't really make the templates easier to read...

- <li><a href="/cgi-bin/koha/opac-user.pl#opac-user-checkouts"><span class="user_checkouts_count count_label">[% checkouts | html %]</span> checkout(s)</a></li>

Turns into:

+                                        [% count = BLOCK %]<span class="user_checkouts_count count_label">[% checkouts | html %]</span>[% END %]
+                                        <li><a href="/cgi-bin/koha/opac-user.pl#opac-user-checkouts">[% tn('{count} checkout', '{count} checkouts', checkouts) | html | $Expand count = count %]</a></li>

Can you explain a bit how this works? What is the BLOCK and Expand needed for?

We add tons more of | html filters... could this impact performance? I remember when adding the filters in the first place it was an issue. It also adds to the text. I wonder if there would be another way to go about these.

Could it make sense to only use the new syntax when we need singular/pluaral or other more advanced methods?
Comment 10 Julian Maurice 2020-09-13 12:12:38 UTC
(In reply to Katrin Fischer from comment #9)
> This appears to work well, but it doesn't really make the templates easier
> to read...
> 
> - <li><a href="/cgi-bin/koha/opac-user.pl#opac-user-checkouts"><span
> class="user_checkouts_count count_label">[% checkouts | html %]</span>
> checkout(s)</a></li>
> 
> Turns into:
> 
> +                                        [% count = BLOCK %]<span
> class="user_checkouts_count count_label">[% checkouts | html %]</span>[% END
> %]
> +                                        <li><a
> href="/cgi-bin/koha/opac-user.pl#opac-user-checkouts">[% tn('{count}
> checkout', '{count} checkouts', checkouts) | html | $Expand count = count
> %]</a></li>
> 
> Can you explain a bit how this works? What is the BLOCK and Expand needed
> for?
Sure.

[% var = BLOCK %]...[% END %] puts the content of the block inside variable var, so here the 'count' variable will be equal to something like this:

    <span class="user_checkouts_count count_label">42</span>

The expand plugin replace text enclosed with braces by another string passed in parameter

    [% t("Hello {name}") | $Expand name = "World !" %]

Will result in "Hello World !".
It is the same as

    [% tx("Hello {name}", name = "World !") %]

But having this as a separate plugin allows to add other filters in between.

    [% t("Hello {name}") | html | $Expand name = "<strong>World</strong> !" %]

Which is needed if we want to prevent translations to add unwanted html (like a <script> element for instance) and if expanded variables contain html that we do not want to filter, like in the example in the patch.

> We add tons more of | html filters... could this impact performance? I
> remember when adding the filters in the first place it was an issue. It also
> adds to the text. I wonder if there would be another way to go about these.
If I remember correctly, the issue was not about HTML filtering in itself, but the module we used was doing a lot of expensive string concatenation, which was the real performance issue.

> Could it make sense to only use the new syntax when we need singular/pluaral
> or other more advanced methods?
It could.
I agree with you when you say the template is less easy to read, but there is a long term benefit if we manage to translate all the strings that way : it will be no longer needed to copy the templates using `./translate install`
Comment 11 Katrin Fischer 2020-10-12 09:31:14 UTC
Hi Julian, I wonder if we should postpone this to next release now maybe, but I hope we can get some more active discussion. What do you think?
Comment 12 Julian Maurice 2020-10-12 09:34:35 UTC
(In reply to Katrin Fischer from comment #11)
> Hi Julian, I wonder if we should postpone this to next release now maybe,
> but I hope we can get some more active discussion. What do you think?

Yes sure, there is no hurry, it will be a very long journey to translate all strings using the new system anyway :)
Comment 13 David Cook 2020-10-18 23:04:35 UTC
This does not look developer friendly... 

As an aside...

The following could be written better:
<li><a href="/cgi-bin/koha/opac-user.pl#opac-user-checkouts"><span class="user_checkouts_count count_label">[% checkouts | html %]</span> checkout(s)</a></li>

Following the suggestions from https://developer.atlassian.com/server/framework/atlassian-sdk/pluralising-internationalisation-strings/:
<li><a href="/cgi-bin/koha/opac-user.pl#opac-user-checkouts"><span class="user_checkouts_count count_label">Checkouts: [% checkouts | html %]</span></a></li>

Then you're just needing to translate "Checkouts" rather than singular and plural forms.
Comment 14 Katrin Fischer 2020-10-18 23:11:25 UTC
> Then you're just needing to translate "Checkouts" rather than singular and
> plural forms.

We can also translate using singular/plural forms now - just missing an example right now, but there is documentation on the wiki:

https://wiki.koha-community.org/wiki/Internationalization,_plural_forms,_context,_and_more_RFC

So we can make it read 1 checkout, 2 checkouts depending on the actual number.
Comment 15 David Cook 2020-10-18 23:39:19 UTC
(In reply to Katrin Fischer from comment #14)
> We can also translate using singular/plural forms now - just missing an
> example right now, but there is documentation on the wiki:
> 
> https://wiki.koha-community.org/wiki/Internationalization,_plural_forms,
> _context,_and_more_RFC
> 
> So we can make it read 1 checkout, 2 checkouts depending on the actual
> number.

Yeah, I saw that, but it does require the developer to know the syntax, and it puts more work on translators. I think there are certainly contexts where you need to do that, but not in my above example.

I was reading somewhere too about plurals being problematic in other languages like Chinese and Russian, but my language skills don't extend that far. 

I do see Slovak mentioned in https://wiki.koha-community.org/wiki/Internationalization,_plural_forms,_context,_and_more_RFC, so that's interesting! 

Perhaps I just need to read more before I comment too much.
Comment 16 Katrin Fischer 2020-10-18 23:40:50 UTC
I think for that prominent spot on the start page it would be good to put in the little more work. And syntax can be learned :)
Comment 17 Owen Leonard 2020-10-19 11:23:59 UTC
(In reply to David Cook from comment #13)
> This does not look developer friendly...

We've been saying this kind of thing would not be developer-friendly for years, and it's time to get over it!

- Developers are smart. Most of them are smarter than me and I can handle this fine. 

- It's not more work for translators. It's *easier* work for translators. Right now the translation files are a mess because we're trying to parse language strings out of a mess of HTML, CSS, and JavaScript. We humans are the ones who know which strings should be translated so we should tell the computer.
Comment 18 David Cook 2020-10-19 23:37:29 UTC
(In reply to Owen Leonard from comment #17)
> (In reply to David Cook from comment #13)
> > This does not look developer friendly...
> 
> We've been saying this kind of thing would not be developer-friendly for
> years, and it's time to get over it!
> 
> - Developers are smart. Most of them are smarter than me and I can handle
> this fine. 
> 

You probably do more front-end work than anyone else though? I don't do it very often, so I think I'd have to re-learn it every time I tried to make a change.

> - It's not more work for translators. It's *easier* work for translators.
> Right now the translation files are a mess because we're trying to parse
> language strings out of a mess of HTML, CSS, and JavaScript. We humans are
> the ones who know which strings should be translated so we should tell the
> computer.

That makes sense but I was referring to "Checkouts: <count>" versus translating "<count> checkout, <count> checkouts". After re-reading my comment, I realize that totally wasn't clear though. I was referring to translating 1 line rather than 2+ lines. That was just based on advice I saw about internationalization on other websites.

Overall, I'll do whatever other people decide needs to be done, but I just worry that we're trying to re-invent the wheel sometimes and do things a harder way, when other projects have already sorted out internationalization more gracefully. That being said, languages like Java really have a lot more history and tooling around internationalization than Perl does I reckon.
Comment 19 Jonathan Druart 2020-11-11 08:23:08 UTC
(In reply to David Cook from comment #18)
> Overall, I'll do whatever other people decide needs to be done, but I just
> worry that we're trying to re-invent the wheel sometimes and do things a
> harder way, when other projects have already sorted out internationalization
> more gracefully.

How could we do that better? Do you have any suggestions?
Comment 20 Jonathan Druart 2020-11-11 08:25:19 UTC Comment hidden (obsolete)
Comment 21 Owen Leonard 2020-12-10 17:18:38 UTC
(In reply to David Cook from comment #18)
> when other projects have already sorted out internationalization
> more gracefully.

If you're referring to the question of syntax, other projects I've seen do something similar. For instance in Drupal:

<?php print t('Skip to main content'); ?></a>

Translatable strings are wrapped in the t() function.
Comment 22 David Cook 2020-12-11 00:46:34 UTC
(In reply to Owen Leonard from comment #21)
> If you're referring to the question of syntax, other projects I've seen do
> something similar. For instance in Drupal:
> 
> <?php print t('Skip to main content'); ?></a>
> 
> Translatable strings are wrapped in the t() function.

Drupal is an old project as well, and that example looks worrying. Based on https://en.wikipedia.org/wiki/Drupal#Themes, it looks like that example must be one of those "hard-coded PHP" templates. I wonder what they do in Drupal 8+ with Twig.
Comment 23 David Cook 2020-12-11 00:53:08 UTC
(In reply to Jonathan Druart from comment #19)
> (In reply to David Cook from comment #18)
> > Overall, I'll do whatever other people decide needs to be done, but I just
> > worry that we're trying to re-invent the wheel sometimes and do things a
> > harder way, when other projects have already sorted out internationalization
> > more gracefully.
> 
> How could we do that better? Do you have any suggestions?

Suggestion 1:
Referring back to https://developer.atlassian.com/server/framework/atlassian-sdk/pluralising-internationalisation-strings/, I think that we could do easy things like "Simplify the Message into a Plural-Independent Phrase". 

Suggestion 2:
Use a token based i18n scheme like Java (which has a huge history in internationalization as it was the origin of ICU):
https://docs.oracle.com/javase/tutorial/i18n/intro/steps.html

Suggestion 3:
Look at what others have done in Perl:
https://metacpan.org/pod/Catalyst::Plugin::I18N
https://metacpan.org/pod/Mojolicious::Plugin::I18N
Comment 24 David Cook 2020-12-11 00:59:53 UTC
(In reply to David Cook from comment #22)
> (In reply to Owen Leonard from comment #21)
> > If you're referring to the question of syntax, other projects I've seen do
> > something similar. For instance in Drupal:
> > 
> > <?php print t('Skip to main content'); ?></a>
> > 
> > Translatable strings are wrapped in the t() function.
> 
> Drupal is an old project as well, and that example looks worrying. Based on
> https://en.wikipedia.org/wiki/Drupal#Themes, it looks like that example must
> be one of those "hard-coded PHP" templates. I wonder what they do in Drupal
> 8+ with Twig.

Admittedly, https://metacpan.org/pod/Catalyst::Plugin::I18N uses a similar method using l() instead of t().

I suppose I wouldn't mind t() that much (although the Java way of doing it is still better I think) if that were the only macro we were required to use, but Bug 26392 introduces t(), tx(), tnx(), txn(), tp(), tpx(), tnp(), tnpx(). 

In my opinion, it's a bit absurd to expect developers to use 8 different macros which look almost exactly the same. 

I think that it'll make front-end developments take 10x* as long, as QA will constantly need to be telling people to re-do their templates. 

But I could be wrong. Maybe we'll all get used to the 8 different macros. 

I was going to say that I'll probably just stop contributing template changes to Koha, but I guess I'm on the QA team these days, so I should probably memorize them. 

*arbitrary order of magnitude
Comment 25 David Cook 2020-12-11 01:03:50 UTC
Regarding the Java method, look at "Fetch the Text from the ResourceBundle" at https://docs.oracle.com/javase/tutorial/i18n/intro/steps.html. 

In my mind, we'd fetch the relevant translated string using a msgId in the Perl, and then we'd pass the translated string via a variable to the template.

I think it would make templates look much cleaner and I think that it would make it easier to handle translations on the backend too.

It could also potentially allow plugins to override the output to the template, but that's a whole other thing.
Comment 26 David Cook 2020-12-11 01:05:14 UTC
However, I defer to those people who actually *do* the translation work. I am not pretending that I know what is best.

These are just ideas I have encountered in my other non-Koha projects, and none of those projects support as many translations/translators as Koha. 

So feel free to tell me to shut up ;).
Comment 27 Julian Maurice 2020-12-11 08:06:15 UTC
(In reply to David Cook from comment #22) 
> Drupal is an old project as well, and that example looks worrying. Based on
> https://en.wikipedia.org/wiki/Drupal#Themes, it looks like that example must
> be one of those "hard-coded PHP" templates. I wonder what they do in Drupal
> 8+ with Twig.

They still do the same: use t() and formatPlural(). Context and variables to substitute are passed as optional arguments

(In reply to David Cook from comment #23)
> Suggestion 1:
> Referring back to
> https://developer.atlassian.com/server/framework/atlassian-sdk/pluralising-
> internationalisation-strings/, I think that we could do easy things like
> "Simplify the Message into a Plural-Independent Phrase". 
Yep, we can do that. No need to change the i18n framework for that. And we can do things differently too if needed, which is not a bad thing IMO.

(In reply to David Cook from comment #24)
> Admittedly, https://metacpan.org/pod/Catalyst::Plugin::I18N uses a similar
> method using l() instead of t().
> 
> I suppose I wouldn't mind t() that much (although the Java way of doing it
> is still better I think) if that were the only macro we were required to
> use, but Bug 26392 introduces t(), tx(), tnx(), txn(), tp(), tpx(), tnp(),
> tnpx(). 
> 
> In my opinion, it's a bit absurd to expect developers to use 8 different
> macros which look almost exactly the same.

If we can't expect developers to use 8 functions, what can we expect from them ?
We can probably reduce the number of functions to 2: t and tn, and transform the other ones into optional arguments of these 2 functions (the same as drupal). But developers would now have to remember the name of the options. (And the parser which extracts translatable strings would have to be modified too)

(In reply to David Cook from comment #25)
> Regarding the Java method, look at "Fetch the Text from the ResourceBundle"
> at https://docs.oracle.com/javase/tutorial/i18n/intro/steps.html. 
> 
> In my mind, we'd fetch the relevant translated string using a msgId in the
> Perl, and then we'd pass the translated string via a variable to the
> template.
> 
> I think it would make templates look much cleaner and I think that it would
> make it easier to handle translations on the backend too.
> 
> It could also potentially allow plugins to override the output to the
> template, but that's a whole other thing.

I really don't see how what you're describing is a better solution than what we already have. This is looking worse actually. Correct me if I'm wrong but are you're suggesting that

[% t('Hello, World!') %]

should be replaced by

$template->param('hello_world_str' => fetch('hello_world_str'))

in Perl script and

[% hello_world_str %]

in template ? I don't find that very clean.
Comment 28 Martin Renvoize 2020-12-11 09:12:21 UTC
Just for documentation, we had a good discussion around this on IRC this morning with a few options to think about.. discussion is around 8.13: http://irc.koha-community.org/koha/2020-12-11#i_2316124
Comment 29 Katrin Fischer 2020-12-12 10:43:43 UTC
Have to still read discussion, but I also am in favor of keeping stuff readable in templates to some extent. Using only variables in templates and having to look them up in a Perl script adds an extra step and will make it harder again for people to contribute simple changes. Also I foresee use of a lot of silly variable names with the amount of strings we have.
Comment 30 David Cook 2020-12-14 00:01:11 UTC
(In reply to Julian Maurice from comment #27)
> I really don't see how what you're describing is a better solution than what
> we already have. This is looking worse actually. Correct me if I'm wrong but
> are you're suggesting that
> 
> [% t('Hello, World!') %]
> 
> should be replaced by
> 
> $template->param('hello_world_str' => fetch('hello_world_str'))
> 
> in Perl script and
> 
> [% hello_world_str %]
> 
> in template ? I don't find that very clean.

I would say that the latter example is actually much *cleaner*. I think that you mean that it is less *convenient*. 

A different example could be:

<p>[% opac_access_denied %]</p>

vs 

<p>[% t('Sorry, the system doesn\'t think you have permission to access this page.') %]</p>

That being said... I suppose using t() does make it easier to automate string extraction from templates rather than forcing developers to put their strings into translation files and to reference them correctly in the Perl.
Comment 31 David Cook 2020-12-14 00:02:46 UTC
(In reply to Julian Maurice from comment #27)
> If we can't expect developers to use 8 functions, what can we expect from
> them ?

We can expect developers to follow consistent simple rules. 

> We can probably reduce the number of functions to 2: t and tn, and transform
> the other ones into optional arguments of these 2 functions (the same as
> drupal). But developers would now have to remember the name of the options.
> (And the parser which extracts translatable strings would have to be
> modified too)

This sounds better to me. It is easier to lookup options for 2 functions than to lookup, compare, and contrast 8 different functions.
Comment 32 David Cook 2020-12-14 00:05:27 UTC
(In reply to Katrin Fischer from comment #29)
> Have to still read discussion, but I also am in favor of keeping stuff
> readable in templates to some extent. Using only variables in templates and
> having to look them up in a Perl script adds an extra step and will make it
> harder again for people to contribute simple changes. 

Easier doesn't always mean better.

> Also I foresee use of
> a lot of silly variable names with the amount of strings we have.

With DSpace, you see keys like this: "jsp.dspace-admin.eperson-edit.emailexists".
Comment 33 David Cook 2020-12-14 00:24:29 UTC
(In reply to Martin Renvoize from comment #28)
> Just for documentation, we had a good discussion around this on IRC this
> morning with a few options to think about.. discussion is around 8.13:
> http://irc.koha-community.org/koha/2020-12-11#i_2316124

Looking at http://irc.koha-community.org/koha/2020-12-11#i_2316124, I'm thinking about separation of concerns...

On one hand, using t() could make it easier for a frontend web designer to do their design in English and offload translations to others without worrying about the backend. 

On the other hand, using t() is looking quite complex and not designer-friendly. It really looks like we're trying to shoehorn a solution into the templates. 

Take the following example:
[% tx('Published on {published_on} by {author}', { published_on = filter(date_var, 'KohaDates', options), author = filter(author, 'html') }) | html %]

Compare it with this much simpler Vue.js i18n example:

https://kazupon.github.io/vue-i18n/guide/formatting.html#named-formatting

A lot of the i18n systems out there work based on this token idea. 

Now of course in the Koha version we're doing a lot of extra things. I'd actually argue that we shouldn't be using KohaDates in the templates and really should format the date in the Perl and then push the correctly formatted date to the template (ie view). After all, if we were getting the date by an AJAX call (say using Vue.js), we'd want the correct data, since we're just rendering it in the presentation layer of our application. Regarding html escaping, we'd just have that on by default. To avoid double-encoding, we could turn off the html filter at the function level.
Comment 34 David Cook 2020-12-14 00:34:12 UTC
I reiterate though that I'm not an expert on this and I'm happy to defer to others.

I'm just saying that the current proposal looks complex and difficult to use. 

Since the OPAC is much smaller than the Staff Interface, it's probably reasonable to experiment there. 

As Owen said on https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20988#c50, it makes sense to try a proof-of-concept before trying to update everything, and it seems Julian opened this bug as that proof-of-concept (it probably would've been good to announce it as such but oh well). 

Maybe we'll even find that due to the complexity of the 8 functions that people try to make simpler text constructs so that they can just use t(). 

To quote Jonathan (from ages ago), let's focus on code rather than on talk. I'm sure we could endlessly debate this one back and forth and never get anywhere.

So why don't we just move forward and see how we go? (The only downside to that approach is that a new release isn't for half a year.)
Comment 35 Katrin Fischer 2020-12-14 22:18:00 UTC
(In reply to David Cook from comment #32)
> (In reply to Katrin Fischer from comment #29)
> > Have to still read discussion, but I also am in favor of keeping stuff
> > readable in templates to some extent. Using only variables in templates and
> > having to look them up in a Perl script adds an extra step and will make it
> > harder again for people to contribute simple changes. 
> 
> Easier doesn't always mean better.

Not always, but I think in this case it applies. Forcing people to invent tons of variables will not make things more readable, but much likely less so. Koha is a very string heavy application. Look at the numbers in our po files on Pootle - I am an active translator.

> 
> > Also I foresee use of
> > a lot of silly variable names with the amount of strings we have.
> 
> With DSpace, you see keys like this:
> "jsp.dspace-admin.eperson-edit.emailexists".

And you say that's easier?
Comment 36 Tomás Cohen Arazi 2020-12-14 22:38:45 UTC
(In reply to Katrin Fischer from comment #35)
> (In reply to David Cook from comment #32)
> > (In reply to Katrin Fischer from comment #29)
> > > Have to still read discussion, but I also am in favor of keeping stuff
> > > readable in templates to some extent. Using only variables in templates and
> > > having to look them up in a Perl script adds an extra step and will make it
> > > harder again for people to contribute simple changes. 
> > 
> > Easier doesn't always mean better.
> 
> Not always, but I think in this case it applies. Forcing people to invent
> tons of variables will not make things more readable, but much likely less
> so. Koha is a very string heavy application. Look at the numbers in our po
> files on Pootle - I am an active translator.
> 
> > 
> > > Also I foresee use of
> > > a lot of silly variable names with the amount of strings we have.
> > 
> > With DSpace, you see keys like this:
> > "jsp.dspace-admin.eperson-edit.emailexists".
> 
> And you say that's easier?

Yes!

intranet.admin.libraries.list_title
Comment 37 Katrin Fischer 2020-12-14 22:46:51 UTC
(In reply to Tomás Cohen Arazi from comment #36)
> (In reply to Katrin Fischer from comment #35)
> > (In reply to David Cook from comment #32)
> > > (In reply to Katrin Fischer from comment #29)
> > > > Have to still read discussion, but I also am in favor of keeping stuff
> > > > readable in templates to some extent. Using only variables in templates and
> > > > having to look them up in a Perl script adds an extra step and will make it
> > > > harder again for people to contribute simple changes. 
> > > 
> > > Easier doesn't always mean better.
> > 
> > Not always, but I think in this case it applies. Forcing people to invent
> > tons of variables will not make things more readable, but much likely less
> > so. Koha is a very string heavy application. Look at the numbers in our po
> > files on Pootle - I am an active translator.
> > 
> > > 
> > > > Also I foresee use of
> > > > a lot of silly variable names with the amount of strings we have.
> > > 
> > > With DSpace, you see keys like this:
> > > "jsp.dspace-admin.eperson-edit.emailexists".
> > 
> > And you say that's easier?
> 
> Yes!
> 
> intranet.admin.libraries.list_title

I really don't see the advantage of this. To me it adds an extra step with no visible win.
Comment 38 David Cook 2020-12-14 22:56:49 UTC
(In reply to Katrin Fischer from comment #35)
> Not always, but I think in this case it applies. Forcing people to invent
> tons of variables will not make things more readable, but much likely less
> so. 

I think that we'll just have to agree to disagree on that one. The variables would be formulaic so they shouldn't be difficult to create. It would also make templates easier to wrangle, as you wouldn't have as much line wrapping and thick blocks of text adding bulk to the templates.

> Koha is a very string heavy application. Look at the numbers in our po
> files on Pootle - I am an active translator.

I agree that the PO files are huge, but looking through them that are also lots of junk lines, duplicate translations, unnecessary translations, etc. Using tokens would cut down on a lot of that. 

I'm looking at ./misc/translator/po/fr-FR-staff-prog.po right now, and lots of these lines would be eliminated or reduced by using tokens. 

I think that we could work smarter rather than harder.

> > 
> > > Also I foresee use of
> > > a lot of silly variable names with the amount of strings we have.
> > 
> > With DSpace, you see keys like this:
> > "jsp.dspace-admin.eperson-edit.emailexists".
> 
> And you say that's easier?

Absolutely. I work with this style in DSpace and Keycloak and it's so easy.

Also, it also makes it much easier to customize strings for existing installations, since you can update "string files" rather than updating templates which have been hard-coded with English strings.
Comment 39 David Cook 2020-12-14 23:04:50 UTC
(In reply to Katrin Fischer from comment #37)
> I really don't see the advantage of this. To me it adds an extra step with
> no visible win.

Here are some random examples from DSpace and Koha.

https://github.com/DSpace/DSpace/blob/dspace-6.3/dspace-jspui/src/main/webapp/tools/edit-community.jsp
https://github.com/Koha-Community/Koha/blob/master/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/editor.tt

For DSpace, just do a Ctrl+F for "fmt:message".

Personally, I think that the DSpace template is cleaner (notwithstanding the JSP block near the top but DSpace isn't perfect either). 

The template is easy to read. The tokens are logical. 

You can lookup keys in https://github.com/DSpace/DSpace/blob/dspace-6.3/dspace-api/src/main/resources/Messages.properties. 

Admittedly DSpace is smaller than Koha, but the Java model used by DSpace is used far and wide.
Comment 40 David Cook 2020-12-14 23:10:09 UTC
But I reiterate... this is basically a philosophical discussion at this point.

I'm not planning to do any work on Java style token-based translations, and Julian is doing work on the t() style translations, so we can just do it that way.

It'll probably be fine. If there are problems later, then there are problems later. No system is perfect anyway. Plus, we're a decentralized community and don't have any roadmaps, so there's no point going in circles. Whoever is willing to do the work does the work, and if it works then it gets into Koha. 

The early coder gets the worm ;). 

At this point, I think that I'm going to stop commenting on this bug.
Comment 41 Julian Maurice 2021-03-05 09:14:15 UTC
Created attachment 117794 [details] [review]
Bug 26392: Wrap all translatable strings in opac-main.tt

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 42 Julian Maurice 2021-03-05 09:14:19 UTC
Created attachment 117795 [details] [review]
Bug 26392: Fix opac news display issue when there is no author

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 43 Katrin Fischer 2021-05-02 16:00:05 UTC
I feel like this might be better as "in discussion", but it should really be discussed.
Comment 44 Fridolin Somers 2021-05-18 08:05:58 UTC
Hi, super.

Just to be sure :

This system uses html filter :
  <a href="/cgi-bin/koha/opac-main.pl">[% t('Home') | html %]</a>

So strings must not escape HTML chars right ?
  Like : t('Foo & Bar') ?

This means we do not use &nbsp; anymore (which is OK for me).
Comment 45 Marcel de Rooy 2021-08-27 07:18:43 UTC
Please look at comment43 too? Katrin could you elaborate on what exactly should be discussed here?
Comment 46 Jonathan Druart 2021-08-27 08:07:34 UTC
I don't think it is a discussion for 21.11.
Comment 47 Victor Grousset/tuxayo 2021-12-05 20:30:41 UTC
About the discussion of t("Koha online catalogue") vs t("opac.homepage.title"), the current approach (1st) doesn't close the door for the 2nd (token based) since the strings would already be wrapped in calls. So it's not closing this door assuming the token based approach is chosen in the end.t It's even a common step, right?


About that choice:
(In reply to David Cook from comment #38)
> I agree that the PO files are huge, but looking through them that are also lots of junk lines, duplicate translations, unnecessary translations, etc. Using tokens would cut down on a lot of that. 

How would tokens allow us to get less junk, duplicates and unnecessary translations? Don't a smarter (manual or automated) use of t() already does all that?
Comment 48 Katrin Fischer 2021-12-05 22:55:03 UTC
I haven't heard any good argument for tokens yet tbh. I think they would just obscure the templates and make them harder for people to work with. Koha is a huge application by now, that has a lot of strings, we'd also end up with tons of tokens. 

For the discussion:

I think we need to discuss pros and cons of rewriting all our templates, because of the consequences:

* A ton of pending code would have to be rewritten
* Backporting patches would be harder for quite a while/versions
* It would add another 'not so obvious' thing for development that is an additional barrier for people starting out
* Template files will be bigger/contain more code.
* We need to investigate a possible performance issue.
* We could save 'space' as templates files won't need to be copied for each added language any longer.
* We could see if we could shorten some of the constructs a bit more - the BLOCK/EXPAND especially.
* We would need good documentation.
* We would need to carefully the change for translators, not only locally, but also in Pootle.
* We need to solve the issue where Pootle marks strings using {} placeholders as an error which makes things really hard for translators right now.

So far I totally see the pros for using the new techniques for plural forms and context where words can have different meaning/need different translations, and we have already started to introduce them to the templates. But I am not totally persuaded that we should do a total switch (yet)

I think this is not something that can go through dev/sign-off/QA/RM with only a few people involved in the discussion, it's something a lot more people need to be aware of and given the possibility to comment on and get onboard with, because a lot of people will be affected. So mailing list, dev meeting, consensus, coding guidelines, documentation.