Bug 38618 - Remove implicit template translations (in .tt and .inc), use only explicit macro invocations
Summary: Remove implicit template translations (in .tt and .inc), use only explicit ma...
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: I18N/L10N (show other bugs)
Version: Main
Hardware: All All
: P4 enhancement
Assignee: Bugs List
QA Contact: Testopia
URL: https://wiki.koha-community.org/wiki/...
Keywords:
Depends on:
Blocks:
 
Reported: 2024-12-04 11:13 UTC by Michał
Modified: 2024-12-19 09:54 UTC (History)
4 users (show)

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Julian Maurice 2024-12-09 10:09:20 UTC
What's the process for commenting on the RFC ? Should we use the wiki "discussion" page ?

See also bug 36357 where it was suggested to remove the existing macros
Comment 2 Michał 2024-12-18 13:11:30 UTC
I'm not sure about the process, but I think we'd be best of using the bug on Bugzilla, given people will get e-mails for replies here and on wiki I'm not sure if they would, so it's more straightforward.
Comment 3 Julian Maurice 2024-12-19 09:54:28 UTC
I completely agree with the goal, but I'm not sure about the multiplication of template macros.

> enforces more consistent ways in how translations are embedded
Does it really enforces more consistency ? To me it only provides new ways to
do the same thing (eg. t_html() VS t_raw() | html)

> performance: single call with full control over returning already escaped
> string allows introducing some type of caching if escaping at runtime every
> time is a concern, would be impossible with chained calls
Is HTML-escaping slower than retrieving a value from a memcache server ?

> an explicit call like t_html (but not t_html_attr) allows us to do very cool
> things like: optionally enabling a preference to display a button to open the
> translation in Weblate (insert such snippet only into HTML text, not entities
> or JS strings)
A cool idea indeed. But I guess this would require adding a DOM element like a span and in theory it could break style or functionality

> inserting quotes for HTML attrs and JS strings means the developer and
> translator is not to worry about which type of quotes should be used and can
> focus on more ambitious things
Just use double quotes for HTML attrs, that's what is used everywhere already.
For JS strings, there is nothing to worry about, as we can already write:
    const s = __('Heya');
(more about that below)

> <span data-test=[% t_html_attr('I am a loose attribute') %]>...</span>
I believe that if the HTML is malformed like that, it can confuses the
legacy translator (which will need to continue to work until no "implicitly
translatable strings" exist)
See https://wiki.koha-community.org/wiki/Coding_Guidelines#HTML1:_Template_Toolkit_markup_inside_HTML

> <span data-test="[% t('I am a loose attribute') | html %]">...</span><!-- it should be html_entity, yet many people use html, sigh!!! -->
According to the spec, the only character that needs to be encoded within a double-quoted attribute value is the double-quote character, which is encoded by the 'html' filter.
Why should we use html_entity ?

> Decide if t() should use single or double quotes by convention, since it's
> mix currently...
It doesn't matter. Use double quotes if the string contains single quotes, or use single quotes if the string contains double quotes, or whatever is more practical if they contain both (or none)

About the non-goal #1 (Migrating _() calls to __() runtime JS calls):
Migrating from _() to [% t_js_string() %] instead of __() feels a like an unneeded temporary transition

> would have to move said strings from messages to messages-js
It can be automated

> would have to check if nothing else still uses it in messages before removal
No need to. They will be removed automatically if not present in the related files

> would immediately inflate the client translations JSON 2-3x times
If the long-term goal is to use __() then the translations will end up to be
2-3x times bigger anyway. Why is it a bad thing if it happens now ?

I'd rather have something like this:

    // No mix of TT and JS syntax
    const string2 = __('Something: "%s"').format("yay");

    // Same filters applied whether it's text content or attribute value
    <span>[% 'I am a loose string' | t | html %]</span> (see bug 36357)
    <span data-test="[% 'I am a loose attribute' | t | html %]"></span>

My 2 cents