Bug 30733 - Simplify translatable strings
Summary: Simplify translatable strings
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: I18N/L10N (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Victor Grousset/tuxayo
QA Contact: Fridolin Somers
URL:
Keywords:
Depends on:
Blocks: 31069 31246 31718
  Show dependency treegraph
 
Reported: 2022-05-11 16:20 UTC by Victor Grousset/tuxayo
Modified: 2023-06-08 22:26 UTC (History)
11 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Large patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Cleanup of translatable text done by guiding the string extractor to make it do simpler strings for translators instead of large concatenation of long strings in the code with a lot of unnecessary %s placeholders.
Version(s) released in:
22.11.00,22.05.01


Attachments
Bug 30733: Simplify translatable strings (245.00 KB, patch)
2022-05-11 16:21 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Simplify translatable strings (244.35 KB, patch)
2022-05-11 23:41 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Simplify translatable strings (244.35 KB, patch)
2022-05-12 05:57 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Move all semicolons inside the translatable strings (9.89 KB, patch)
2022-05-29 23:47 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Simplify translatable strings (245.28 KB, patch)
2022-06-02 03:40 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Move all semicolons inside the translatable strings (9.89 KB, patch)
2022-06-02 03:40 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Fix string context lost due to being in two parts (7.01 KB, patch)
2022-06-02 03:41 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Undo bad handling of a plural case (1.24 KB, patch)
2022-06-02 03:41 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Wrap one more string in <span> (1.60 KB, patch)
2022-06-02 03:41 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Undo <span> in an ICS calendar file (1.21 KB, patch)
2022-06-02 03:43 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Simplify translatable strings (246.17 KB, patch)
2022-06-13 12:17 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Move all semicolons inside the translatable strings (9.89 KB, patch)
2022-06-13 12:17 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Fix string context lost due to being in two parts (7.01 KB, patch)
2022-06-13 12:17 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Undo bad handling of a plural case (1.24 KB, patch)
2022-06-13 12:17 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Wrap one more string in <span> (1.59 KB, patch)
2022-06-13 12:17 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Undo <span> in an ICS calendar file (1.21 KB, patch)
2022-06-13 12:17 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Simplify some singular cases (3.91 KB, patch)
2022-06-13 12:17 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Undo some <span> additions that don't yield good strings (5.14 KB, patch)
2022-06-13 12:17 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: fix url => URL to merge with existing string (1.16 KB, patch)
2022-06-13 12:18 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Split strings differently (19.82 KB, patch)
2022-06-13 12:18 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Simplify translatable strings (246.23 KB, patch)
2022-06-13 22:06 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 30733: Move all semicolons inside the translatable strings (9.95 KB, patch)
2022-06-13 22:06 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 30733: Fix string context lost due to being in two parts (7.07 KB, patch)
2022-06-13 22:07 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 30733: Undo bad handling of a plural case (1.30 KB, patch)
2022-06-13 22:08 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 30733: Wrap one more string in <span> (1.66 KB, patch)
2022-06-13 22:08 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 30733: Undo <span> in an ICS calendar file (1.27 KB, patch)
2022-06-13 22:08 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 30733: Simplify some singular cases (3.97 KB, patch)
2022-06-13 22:09 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 30733: Undo some <span> additions that don't yield good strings (5.20 KB, patch)
2022-06-13 22:09 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 30733: fix url => URL to merge with existing string (1.22 KB, patch)
2022-06-13 22:09 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 30733: Split strings differently (19.88 KB, patch)
2022-06-13 22:10 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 30733: Simplify translatable strings (246.13 KB, patch)
2022-06-15 17:30 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Move all semicolons inside the translatable strings (9.89 KB, patch)
2022-06-15 17:31 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Fix string context lost due to being in two parts (7.01 KB, patch)
2022-06-15 17:31 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Undo bad handling of a plural case (1.24 KB, patch)
2022-06-15 17:31 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Wrap one more string in <span> (1.59 KB, patch)
2022-06-15 17:31 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Undo <span> in an ICS calendar file (1.21 KB, patch)
2022-06-15 17:31 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Simplify some singular cases (3.91 KB, patch)
2022-06-15 17:31 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Undo some <span> additions that don't yield good strings (5.14 KB, patch)
2022-06-15 17:31 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: fix url => URL to merge with existing string (1.16 KB, patch)
2022-06-15 17:31 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Split strings differently (19.82 KB, patch)
2022-06-15 17:31 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Simplify translatable strings (246.19 KB, patch)
2022-06-15 17:34 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Move all semicolons inside the translatable strings (9.95 KB, patch)
2022-06-15 17:35 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Fix string context lost due to being in two parts (7.07 KB, patch)
2022-06-15 17:35 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Undo bad handling of a plural case (1.30 KB, patch)
2022-06-15 17:35 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Wrap one more string in <span> (1.66 KB, patch)
2022-06-15 17:36 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Undo <span> in an ICS calendar file (1.27 KB, patch)
2022-06-15 17:36 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Simplify some singular cases (3.97 KB, patch)
2022-06-15 17:36 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Undo some <span> additions that don't yield good strings (5.20 KB, patch)
2022-06-15 17:37 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: fix url => URL to merge with existing string (1.22 KB, patch)
2022-06-15 17:37 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Split strings differently (19.88 KB, patch)
2022-06-15 17:37 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Simplify translatable strings (245.27 KB, patch)
2022-06-16 16:01 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Move all semicolons inside the translatable strings (9.95 KB, patch)
2022-06-16 16:02 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Fix string context lost due to being in two parts (7.07 KB, patch)
2022-06-16 16:02 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Undo bad handling of a plural case (1.30 KB, patch)
2022-06-16 16:02 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Wrap one more string in <span> (1.64 KB, patch)
2022-06-16 16:03 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Undo <span> in an ICS calendar file (1.27 KB, patch)
2022-06-16 16:03 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Simplify some singular cases (3.97 KB, patch)
2022-06-16 16:03 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Undo some <span> additions that don't yield good strings (5.18 KB, patch)
2022-06-16 16:04 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: fix url => URL to merge with existing string (1.22 KB, patch)
2022-06-16 16:04 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Split strings differently (19.91 KB, patch)
2022-06-16 16:04 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 30733: Simplify translatable strings (245.33 KB, patch)
2022-06-16 17:08 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 30733: Move all semicolons inside the translatable strings (10.00 KB, patch)
2022-06-16 17:08 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 30733: Fix string context lost due to being in two parts (7.12 KB, patch)
2022-06-16 17:08 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 30733: Undo bad handling of a plural case (1.36 KB, patch)
2022-06-16 17:08 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 30733: Wrap one more string in <span> (1.70 KB, patch)
2022-06-16 17:09 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 30733: Undo <span> in an ICS calendar file (1.33 KB, patch)
2022-06-16 17:09 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 30733: Simplify some singular cases (4.03 KB, patch)
2022-06-16 17:09 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 30733: Undo some <span> additions that don't yield good strings (5.24 KB, patch)
2022-06-16 17:09 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 30733: fix url => URL to merge with existing string (1.28 KB, patch)
2022-06-16 17:09 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 30733: Split strings differently (19.97 KB, patch)
2022-06-16 17:09 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 30733: Simplify translatable strings (245.39 KB, patch)
2022-06-16 21:30 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 30733: Move all semicolons inside the translatable strings (10.07 KB, patch)
2022-06-16 21:31 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 30733: Fix string context lost due to being in two parts (7.19 KB, patch)
2022-06-16 21:31 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 30733: Undo bad handling of a plural case (1.42 KB, patch)
2022-06-16 21:31 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 30733: Wrap one more string in <span> (1.76 KB, patch)
2022-06-16 21:32 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 30733: Undo <span> in an ICS calendar file (1.39 KB, patch)
2022-06-16 21:32 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 30733: Simplify some singular cases (4.09 KB, patch)
2022-06-16 21:32 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 30733: Undo some <span> additions that don't yield good strings (5.30 KB, patch)
2022-06-16 21:32 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 30733: fix url => URL to merge with existing string (1.34 KB, patch)
2022-06-16 21:33 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 30733: Split strings differently (20.03 KB, patch)
2022-06-16 21:34 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 30733: [22.05] Simplify translatable strings (244.46 KB, patch)
2022-06-17 14:03 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Victor Grousset/tuxayo 2022-05-11 16:20:36 UTC
A.K.A "We must be nicer with translators 2" (follow-up of bug 29602)

It's the same simple idea that worked well without issues: Add <span> in templates to not have anymore translatable strings like
%sEnter a new purchase suggestion%s %sPurchase suggestions%s &rsaquo; %s%s%sKoha online%s catalog


git is a great help to review this patch. Don't use the bugzilla diff view. Apply the patch and use:
git show --color-words=.
Now you will see just the added <span>
You can search for <span> and go one by one to check that the opening and closing tags match and that there is no typo.
Then the same with </span>
I already did that though, so you might want to spare pressing the "n" (next) key 1300 times (684 x 2)


---

This patch has been made by searching in the translation platform for "%s" and inspecting the 2300 strings containing it to find those that are terrible to translate: those with tons of %s which are a concatenation of independent strings. Mainly the IFs in template produce these abominations.
bug 29602 focused on CASE statement. This is an attempt for all the rest. Except for CSV templates and page <title> tags because <span> can't be used there. Some less terrible strings have been skipped due to lack of time.

---

I tried to think about right to left languages in cases like <span>Confirm deletion of subfield [%- tagsubfield | html -%]</span> I included the variable tagsubfield in the <span> so there will be a %s in the translatable string which should allow to invert the order when translating to be RTL.

And also I included semicolons in the <span> when the context allows for a translated version with the semicolon on the left to make sense. If the context has a related thing just after on the same line (on the right) to the string, then I didn't include the semicolon to leave it "pointing" at the right thing. Even though that's wrong, but we can't swap the phrases.

caveat: I don't have any experience in translating RTL in Koha so I'm not sure if these things help.
And I just noticed we have an RTL CSS that might actually swap stuff. If that's the case, the fact that I left the semicolon outside the <span> might not be an issue because it should end up swapped right?


---

There are a few fixes like
<span>Payment</span>[% ELSE %]<span>Transaction</span>[% END %] type: </label>
↓
<span>Payment type</span>[% ELSE %]<span>Transaction type</span>[% END %] type: </label>
because having "type" separated removes the context and it locks the word order as same as English. Which makes this string not possible to correctly translate in French, Spanish and Esperanto for example.

---

There is a double <span>, it looks like:
<span><span>Author(s)
This is expected. There was already one <span> spanning (ha ha) a few lines and I needed another one just for the string on this line.
Comment 1 Victor Grousset/tuxayo 2022-05-11 16:21:39 UTC Comment hidden (obsolete)
Comment 2 Fridolin Somers 2022-05-11 20:37:46 UTC
(In reply to Victor Grousset/tuxayo from comment #0)
> ---
> 
> There are a few fixes like
> <span>Payment</span>[% ELSE %]<span>Transaction</span>[% END %] type:
> </label>
> ↓
> <span>Payment type</span>[% ELSE %]<span>Transaction type</span>[% END %]
> type: </label>
> because having "type" separated removes the context and it locks the word
> order as same as English. Which makes this string not possible to correctly
> translate in French, Spanish and Esperanto for example.

In text above you forgot to remove 'type' in second line, after [% END %].

But it is OK in patch :
-            <label for="[% type | html %]_type" class="required">[% IF type == 'payment' %]Payment[% ELSE %]Transaction[% END %] type: </label>
+            <label for="[% type | html %]_type" class="required">[% IF type == 'payment' %]<span>Payment type</span>[% ELSE %]<span>Transaction type</span>[% END %]: </label>

Still I think there is a problem.

This will lead to the translation of :
  "Payment type"
  "Transaction type"
  "%s: "
Usually the ":" is with translated text :
  "Payment type: "
  "Transaction type: "
For example french translation will add a space "Type de transaction : "
But this may generate strings with and without ending character :
  "Payment type"
  "Payment type: "

You may add a span for ending character alone :
+            <label for="[% type | html %]_type" class="required">[% IF type == 'payment' %]<span>Payment type</span>[% ELSE %]<span>Transaction type</span>[% END %]<span>: </span></label>
But the context may be hard to understand for translators.
And there are soooooo much places to change.

Great work BTW ;)
Comment 3 Victor Grousset/tuxayo 2022-05-11 23:41:05 UTC Comment hidden (obsolete)
Comment 4 Victor Grousset/tuxayo 2022-05-11 23:47:26 UTC
Some testing notes before I forget.

- it has been checked multiple times that no <span> has been added to a <title>
- attention was given to leave out CSV and JSON outputs
- each .inc was checked individually to ensure it's indeed HTML and not CSV, JSON or the content of a <title>
- via doing searches in the git log for <span> and </span> it's possible to be very confident that there is not typo in the tag and not mismatching of tags (same method used on bug 29602 that worked well to find little mistakes)
- one proofreading pass was especially focused in ensuring that IF and ELSE couldn't cause a tag mismatch
- QA test tools pass albeit some unusual Perl warnings outputted by the script:

root@kohadevbox:koha(simplify-translatable-strings)$ time /kohadevbox/qa-test-tools/koha-qa.pl -v 2 -c 1
testing 1 commit(s) (applied to 68990b9 '9a Bug 30722: Typo in overdue recalls')

Processing files before patches
File not found : lib/jquery/jquery-ui-1.13.1.css at /kohadevbox/koha/Koha/Template/Plugin/Asset.pm line 107.
Argument "" isn't numeric in numeric gt (>) at /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/includes/patron_messages.inc line 130.
Argument "" isn't numeric in numeric gt (>) at /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/includes/patron_messages.inc line 130.
Argument "" isn't numeric in numeric gt (>) at /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt line 236.
Argument "" isn't numeric in numeric gt (>) at /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/circ/request-article.tt line 343.
|========================>| 116 / 116 (100.00%)
Processing files after patches
File not found : lib/jquery/jquery-ui-1.13.1.css at /kohadevbox/koha/Koha/Template/Plugin/Asset.pm line 107.
Argument "" isn't numeric in numeric gt (>) at /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/includes/patron_messages.inc line 130.
Argument "" isn't numeric in numeric gt (>) at /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/includes/patron_messages.inc line 130.
Argument "" isn't numeric in numeric gt (>) at /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt line 236.
Argument "" isn't numeric in numeric gt (>) at /kohadevbox/koha/koha-tmpl/intranet-tmpl/prog/en/modules/circ/request-article.tt line 343.
|========================>| 116 / 116 (100.00%)
Comment 5 Bernardo Gonzalez Kriegel 2022-05-12 00:43:04 UTC
Victor, great work!

In https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134908&action=diff#a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/overdue.tt_sec1
you enclose only "Items" between <span>..</span>, but not "Item" and "overdue as of", that's deliberate?

The original string was
  msgid "%s %sItems%sItem%s overdue as of %s%s "
and the new one
  msgid "%sItem%s overdue as of %s%s "
Comment 6 Victor Grousset/tuxayo 2022-05-12 01:26:18 UTC
(In reply to Fridolin Somers from comment #2)
> In text above you forgot to remove 'type' in second line, after [% END %].
> 
> But it is OK in patch :

Indeed I mess up the manual example, sorry for the confusion.


> Still I think there is a problem.
> 
> This will lead to the translation of :
>   "Payment type"
>   "Transaction type"
>   "%s: "

A diff between updated strings on master and updated strings with the patch shows by searching for the location comment "transaction_types.inc" that
"Payment type" is created
"Transaction type" already exists and the location "transaction_types.inc" is added in it's list
No other "transaction_types.inc" to be found that could show that "%s: " is created or linked to an existing one


> Usually the ":" is with translated text :
>   "Payment type: "
>   "Transaction type: "

ok, shall I make all the strings include the ":" ?
In your above example I see a trailing space after the semicolon but we don't aim for this in source strings right?

 
> You may add a span for ending character alone :
> +            <label for="[% type | html %]_type" class="required">[% IF type
> == 'payment' %]<span>Payment type</span>[% ELSE %]<span>Transaction
> type</span>[% END %]<span>: </span></label>
> But the context may be hard to understand for translators.

Translators probably will just copy it as it without deducing they can there decide their punctuation spacing style. Assuming the string extraction script keeps it. But it seems not from a check above.
Comment 7 Victor Grousset/tuxayo 2022-05-12 01:34:41 UTC
Hola Bernardo :D

(In reply to Bernardo Gonzalez Kriegel from comment #5)
> In
> https://bugs.koha-community.org/bugzilla3/attachment.
> cgi?id=134908&action=diff#a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/
> overdue.tt_sec1
> you enclose only "Items" between <span>..</span>, but not "Item" and
> "overdue as of", that's deliberate?
> 
> The original string was
>   msgid "%s %sItems%sItem%s overdue as of %s%s "
> and the new one
>   msgid "%sItem%s overdue as of %s%s "

Yes that's the intended result. To still keep some context, I hesitated and didn't have a strong opinion about what is better. Now that I reread it there would be no ambiguity. So better isolate every piece, right?

Now that you are here I can ask what should I do about semicolons? Should I always include them in the string? I left part of them out because I guess I would help RTL languages:

> And also I included semicolons in the <span> when the context allows for a translated version with the semicolon on the left to make sense. If the context has a related thing just after on the same line (on the right) to the string, then I didn't include the semicolon to leave it "pointing" at the right thing. Even though that's wrong, but we can't swap the phrases.

> caveat: I don't have any experience in translating RTL in Koha so I'm not sure if these things help.
> And I just noticed we have an RTL CSS that might actually swap stuff. If that's the case, the fact that I left the semicolon outside the <span> might not be an issue because it should end up swapped right?
Comment 8 Victor Grousset/tuxayo 2022-05-12 04:23:34 UTC
Updating the .po files of a lang on master, then doing "git add" on the .po and then updating with the patches should allow to see the difference.

git diff | grep "\-msgid" | wc -l 
182
git diff | grep "\+msgid" | wc -l
337

That makes sense, ~680 <span> used to remove 182 strings means that I split in 3.7 on average. I had the impression that on the 2300 search results on Pootle, I had touched more than 182 though. But if I "only" added 680 <span> than makes sense.
As for the 337 string added, with 680 <span> and assuming half of the simpler strings being duplicates with existing or between themselves, that makes sense to have only 337 new strings.

A quick skimming in the updated staff .po searching for %s shows that the terrible strings remaining are mostly those that make the <title> of each Koha page. So that's still quite a number of strings to clean. And then a few templates outputting CSV.

Doing git diff and there searching for «\-msgid "» and «\+msgid "» allows to 
see the complexity of the removed strings and the simplicity of the added ones.


@Fridolin here is your count method, I don't know what it means but here it is so you can interpret it:
#current master
msggrep -K -e '%s' misc/translator/po/fr-FR-staff-prog.po | grep -c "^msgid"
1749
#after patch
msggrep -K -e '%s' misc/translator/po/fr-FR-staff-prog.po | grep -c "^msgid"
1708
Comment 9 Victor Grousset/tuxayo 2022-05-12 04:27:35 UTC
After today's dev meeting: if we get enough confidence after reviewing the patch, Fridolin could still merge it in the next two days albeit the string freeze. The string enhancements will compensate the two days lost for the translators.
Comment 10 Victor Grousset/tuxayo 2022-05-12 05:57:06 UTC
Created attachment 134912 [details] [review]
Bug 30733: Simplify translatable strings

And a few minor fixes when they where causing issues for
translatability.

Simplified via wrapping strings with <span> to split to huge
concatenated strings with a lot of %s everywhere.

== Test plan ==
This patch needs mainly proof reading. Still it's possible to do some
basic testing to demonstrate that adding a <span> in an IF doesn't
break anything.
Pick in one of the 110 modified templates a string that you know how to
display. Otherwise:
1. acquisitions => vendor => basket => add to basket =>
   search "from existing record" => add order
2. Cancel the order
3. You see without issue "Bibliographic record will not be deleted"
4. administration => Patron categories
5. Try to delete a used and unused category
6. You see as expected
   Category XXXX is in use. Deletion not possible!
   and
   Confirm deletion of category XXXX
Comment 11 Victor Grousset/tuxayo 2022-05-12 06:17:33 UTC
More tests

1. Acquisitions => go to a basket
2. make an order
3. receive it and get and invoice
4. Home > Acquisitions > Invoices 
5. close and open it and check the status column say the status ok
6. Search for a shipment and billing date range that doesn't match your invoice
7. Check message looks okay
     Search was:
     Shipment date: From 05/02/2022 To 05/31/2022
     Billing date: From 05/02/2022 To 05/31/2022



1. Create a subscription with a "Subscription length"
2. Acquisitions => go to a basket
3. make an order from a subscription
4. pick your subscription
5. In Accounting details you should see something like
   "Frequency: 2/day | Number of weeks: 6"
Comment 12 Jonathan Druart 2022-05-12 07:28:18 UTC
(In reply to Victor Grousset/tuxayo from comment #9)
> After today's dev meeting: if we get enough confidence after reviewing the
> patch, Fridolin could still merge it in the next two days albeit the string
> freeze. The string enhancements will compensate the two days lost for the
> translators.

Aren't you creating new strings? I guess you are modifying old strings that are translated already, so actually you are adding more work for languages having those strings translated.
Comment 13 Fridolin Somers 2022-05-12 08:42:52 UTC
(In reply to Victor Grousset/tuxayo from comment #8)
> 
> @Fridolin here is your count method, I don't know what it means but here it
> is so you can interpret it:
> #current master
> msggrep -K -e '%s' misc/translator/po/fr-FR-staff-prog.po | grep -c "^msgid"
> 1749
> #after patch
> msggrep -K -e '%s' misc/translator/po/fr-FR-staff-prog.po | grep -c "^msgid"
> 1708

Adding this count for fr-FR-opac-bootstrap.po:
  current master : 390
  after patch    : 360

This counts how many strings to translate contains a %s.

So your changes adds strings to translate but lower down the number of strings containing a %s by at least 41+30=71.

This is great help for translators.

An example :

msgid ""
"%s Fulfilled %s Cancelled %s (%s) %s %s Waiting %s Processing %s In transit "
"%s Pending %s "
msgid ""
"%s In transit to %s %s Ready for pickup %s Expired on %s %s Cancelled on %s "
"%s Overdue to be returned %s Fulfilled %s Requested %s "

Are splitted in :
msgid "Fulfilled"
msgid "Cancelled"
...
Comment 14 Katrin Fischer 2022-05-12 09:27:49 UTC
This will create a lot of string changes for translators to completely retranslate. I think it's too late right now, there is already a lot of translations to make and this would be better for .1

Btw, we should announye string freeze properly on koha-translate etc.!
Comment 15 Bernardo Gonzalez Kriegel 2022-05-12 11:00:13 UTC
(In reply to Katrin Fischer from comment #14)
> This will create a lot of string changes for translators to completely
> retranslate. I think it's too late right now, there is already a lot of
> translations to make and this would be better for .1
> 
> Btw, we should announye string freeze properly on koha-translate etc.!

I agree with Katrin, this could wait until .1 release.

Strings have been updated yesterday.
Comment 16 Victor Grousset/tuxayo 2022-05-12 14:19:04 UTC
(In reply to Jonathan Druart from comment #12)
> Aren't you creating new strings? I guess you are modifying old strings that
> are translated already, 

That's a mix. Part of the long strings lists related to long IF ELSIF that are to be retranslated when a new release brings a new element in the list. Like with the recalls patch that added an element in a number of the long complex strings.
Part of the rest is the simpler elements of broke don't long strings that match with existing ones.
And part is actual new string additions.
I added 680 <span>s and IIUC that turned into 337 new strings in the .po files. So the last part of new string is half of the total.

The idea is that these are easier or not much work to translate than the long complex strings that have had a new element added in this release (or code change that adds a %s). Which require to retranslate with at best the previous translation and having counting of the %s and the elements to do.

> so actually you are adding more work for languages
> having those strings translated.

In case of a language that rushed translations and completed them in the 1st 24h, it will be a complete setback then. That's the worst case though.
https://translate.koha-community.org/projects/22.05/
We can see that all languages have still a big number of words to translate. And the number of words from the top translators doesn't show that someone managed to do a significant fraction of new 22.05 work.
Let's take Italian and German, the most complete langs.
1834 and 1898 strings to do. Around 9200 words and at best 700 worked on since a week when look for German.
So we are not in the worst case. There are 182 strings that will disappear with this patch. And part of those have to be retranslated for 22.05 and Italian and German translators can't have done a big share of that considering their overall remaining number of strings to do. For other language the gap is more and more large.
So the setback isn't significant yet.


> I guess you are modifying old strings that are translated already

To rephrase, only the part of the 182 strings (removed by this patch) that have to be retranslated for 22.05 count for this question. And actually it's the sub part that have already been translated since yesterday.

«old strings that are translated already» If they are still translated in 22.05 they aren't the issue. If this patch had been submitted and merged 3 month ago, translators still "lose" the rest of the 182 that were not altered for 22.05 and were already translated.
Only what matters are the strings already translated since yesterday that are part of the 22.05 updated strings that are part of the 182 removed by this patch.

So it really look like as of now it would help more the translators than hurt them.
Comment 17 Victor Grousset/tuxayo 2022-05-12 14:44:36 UTC
(In reply to Katrin Fischer from comment #14)
> This will create a lot of string changes for translators to completely
> retranslate. 

If comment 16 is correct then no, it won't be a setback.

Katrin:
> I think it's too late right now, there is already a lot of
> translations to make and this would be better for .1

Bernardo:
> I agree with Katrin, this could wait until .1 release.

That's a good surprise that such string changes can be considered to backport.
However the setback considerably more and this time significant for any language that makes a significant effort to get a 22.05.00 close to 100% translated, close to all of the long complex strings to retranslate for 22.05 would have been translated for nothing. While in the next days only a fraction of it would be done and lost.

In the end it only matter for languages close to 100% on previous release and whose team have enough workforce to have any significant work done in the first days of the freeze (the only teams to loose work now albeit it's not significant IIUC) and that can hope for a close to 100% 22.05.00 (the only teams to loose something significant if we wait for 22.05.01 window)

Most languages have more strings to worry about than those changed for 22.05 and most with a high completion percentage still don't translate fast enough now and in the coming few weeks to be impacted by whether this patch is merged around now or for .01.

So very happy to see that for this patch to make it for 22.05.x the possibility would be open for .01 but that doesn't look best for translating team (the most involved ones) compared to .00

> Btw, we should announye string freeze properly on koha-translate etc.!

Indeed!
Comment 18 Fridolin Somers 2022-05-13 02:22:22 UTC
I've sent the message to mailing lists.
Version is frozen for translation 🥶
So hands-up until release.
Comment 19 Katrin Fischer 2022-05-13 20:54:54 UTC
My expectation would be, that most of those strings would not require any changes for 22.05, so not appear as new/fuzzy right now. But by breaking them up, there would only be a few that match existing strings in other places and there should be quite a bit of extra new strings with the patch.
Comment 20 Victor Grousset/tuxayo 2022-05-14 06:16:49 UTC
> My expectation would be, that most of those strings would not require any changes for 22.05, so not appear as new/fuzzy right now.

We shouldn't expect that for this patch. Because that would happen also if this patch would have been merged 1 month ago and it wouldn't have been a problem to add 300 new/fuzzy strings then. The expectation is that we should not loose too much work already done since the string freeze, otherwise that's a problem.

If the patch was merged on 11th of May, here are the number of the strings it would have lost:
14 strings for German
1 strings for Armenian
17 strings for Spanish
9 strings for Turkish

11th of May is the last Pootle sync. That's where most work has been done anyway compared to the last two days so it's still representative. And the actual work we have to care about is in the last two days and for now it's even less than the low above numbers.
That's fine if this patch is merged for 22.05.01 but the lost work will be higher than the current small one. That's still a good plan because that lost work will be the highest for the most active&complete (yes, both criteria) languages in the next two weeks. And these very active languages will easily recover from that loss.


Calculations:

Using poedit here are some numbers of empty + fuzzy strings with the .po of master updated against master code. To simulate how it has been when 22.05 was created on the translation platform with the most complete languages:
German: staff: 947 opac: 168 = 1115 total strings needing work
Italian: staff: 2208 opac: 347 = 2555 (this is weird, Italian is at 100% for 21.11 but the unupdated .po in master aren't 100% complete before string update, as opposed other langs)
Armenian: staff: 1116 opac: 168 = 1284
Spanish: staff: 1311 opac: 170 = 1481
Turkish: staff: 1315 opac: 237 = 1552

After applying this patch and updating the strings:
German: staff: 1126 opac: 300 = 1426
Italian: staff: 2360 opac: 447 = 2807
Armenian: staff: 1295 opac: 300 = 1595
Spanish: staff: 1484 opac: 302 = 1786
Turkish: staff: 1488 opac: 365 = 1853
If the patch was merged 1 month ago, it would have added a net:
311 strings for German
252 strings for Italian (unreliable due to previous inconsistency)
311 strings for Armenian
305 strings for Spanish
301 strings for Turkish

I downloaded the .po from the translation platform as of 11th of May then I put them in my local Koha to get the translation work of the week preceding the string freeze. And I updated the strings.
Using poedit here are the numbers of empty + fuzzy strings:

German: staff: 713 opac: 29 = 742
Italian: staff: 808 opac: 107 = 915 (the .po in Pootle is okay though. But I can't compare to master then. The recent activity in Pootle is lower than for German so there can't have been 1500 strings done to bridge the gap from the .po in master. So I have to drop Italian in this analysis)
Armenian: staff: 839 opac: 115 = 954
Spanish: staff: 826 opac: 111 = 937
Turkish: staff: 998 opac: 116 = 1114

After applying this patch and updating the strings:
German: staff: 899 opac: 168 = 1067
Italian: staff: 996 opac: 240 = 1236
Armenian: staff: 1019 opac: 247 = 1266
Spanish: staff: 1016 opac: 243 = 1259
Turkish: staff: 1176 opac: 248 = 1424
If the patch was merged on 11th of May it would have added
325 strings for German
321 strings for Italian
312 strings for Armenian
322 strings for Spanish
310 strings for Turkish

The final result is the number of strings added in the 11th of May simulation minus those from the simulation from the creation of the 22.05 translation (with the .po from master)
Comment 21 Victor Grousset/tuxayo 2022-05-14 21:56:16 UTC
(The final result is what I posted at the beginning of the comment "the number of the strings it would have lost")
Comment 22 Victor Grousset/tuxayo 2022-05-29 23:47:34 UTC
Created attachment 135444 [details] [review]
Bug 30733: Move all semicolons inside the translatable strings

In the 1st patch, sometimes they where left out
Comment 23 Fridolin Somers 2022-05-31 00:56:48 UTC
Looks great !

I've walked into every change.
Found some issues and have some changes to propose.
Comment 24 Fridolin Somers 2022-05-31 01:03:14 UTC
Some breadcrumbs have a IF/ELSE on action (like add/modify) on an object.
We need to keep the action and the object together because of some languages like French.

For example :

koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt
-        <legend><h1>[% IF ( budget_id ) %]Modify[% ELSE %]Add[% END %] Fund
+        <legend><h1>[% IF ( budget_id ) %]<span>Modify</span>[% ELSE %]<span>Add</span>[% END %] <span>Fund</span>

Will generate :
  Modify
  Add
  Fund

But we need to have :
  Modify Fund
  Add Fund

For french translation :
  Modifier le poste budgétaire
  Ajouter un Poste budgétaire
"un/le" must be "une/la" when object is a she.

Same in those files :
acqbudgets.tt
authtypes.tt
cities.tt
debit_types.tt
edit-batcht.tt
edit-layout.tt
Comment 25 Fridolin Somers 2022-05-31 01:10:34 UTC
In koha-tmpl/intranet-tmpl/prog/en/modules/labels/label-bib-search.tt :

-[% INCLUDE 'doc-head-open.inc' %]Koha &rsaquo; Tools &rsaquo; Labels &rsaquo;: Search[% INCLUDE 'doc-head-close.inc' %]
+[% INCLUDE 'doc-head-open.inc' %]<span>Koha</span> &rsaquo; <span>Tools</span> &rsaquo; <span>Labels</span> &rsaquo;: <span>Search</span>[% INCLUDE 'doc-head-close.inc' %]
 </head>

I think the template is wrong even before this change.
This should be wrapped in a "title" tag.
Since it is the title tag, HTML tags are not allowed.
Maybe we can create another bug report for that.

Idem in opac-account-pay-error.tt
Comment 26 Fridolin Somers 2022-05-31 01:13:54 UTC
In koha-tmpl/intranet-tmpl/prog/en/modules/circ/overdue.tt
-  <h2>[% nnoverdue | html %] [% IF ( noverdue_is_plural ) %]Items[% ELSE %]Item[% END %] overdue as of [% todaysdate | html %][% IF ( isfiltered ) %] <span style="font-size:70%;">(Filtered. <a href="/cgi-bin/koha/circ/overdue.pl">Reset filter</a>)</span>[% END %]</h2>
+  <h2>[% nnoverdue | html %] [% IF ( noverdue_is_plural ) %]<span>Items</span>[% ELSE %]Item[% END %] overdue as of [% todaysdate | html %][% IF ( isfiltered ) %] <span style="font-size:70%;">(Filtered. <a href="/cgi-bin/koha/circ/overdue.pl">Reset filter</a>)</span>[% END %]</h2>

Why this small change ?
It is actually a IF/ELSE to use singular or plural Item or Items.
"overdue" will need to know the context.

I'd say do not touch.
Comment 27 Fridolin Somers 2022-05-31 01:15:11 UTC
This is a suggestion :

In koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt

                                             [% IF loopro.recall.item_level_recall %]
-                                                Item-level recall on <a href="/cgi-bin/koha/catalogue/moredetail.pl?item=[% loopro.recall.itemnumber | uri %]&amp;biblionumber=[% loopro.recall.biblionumber | uri %]&amp;bi=[% loopro.recall.item.biblioitemnumber | uri %]">item</a>
+                                                <span>Item-level recall on <a href="/cgi-bin/koha/catalogue/moredetail.pl?item=[% loopro.recall.itemnumber | uri %]&amp;biblionumber=[% loopro.recall.biblionumber | uri %]&amp;bi=[% loopro.recall.item.biblioitemnumber | uri %]">item</a></span>
                                             [% ELSE %]
                                                 Biblio-level recall on <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% loopro.recall.biblionumber | uri %]">biblio</a>
                                             [% END %]

Maybe add span also around "Biblio-level recall on..."
Comment 28 Fridolin Somers 2022-05-31 01:17:45 UTC
This is a suggestion :

koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-ics.tt

This is for ICal calendar export, maybe it should not contain HTML
Comment 29 Victor Grousset/tuxayo 2022-06-02 03:40:44 UTC
Created attachment 135564 [details] [review]
Bug 30733: Simplify translatable strings

And a few minor fixes when they where causing issues for
translatability.
And rephrased a string about password reset to have it identical to
other strings with the same meaning.

Simplified via wrapping strings with <span> to split to huge
concatenated strings with a lot of %s everywhere.

== Test plan ==
This patch needs mainly proof reading. Still it's possible to do some
basic testing to demonstrate that adding a <span> in an IF doesn't
break anything.
Pick in one of the 110 modified templates a string that you know how to
display. Otherwise:
1. acquisitions => vendor => basket => add to basket =>
   search "from existing record" => add order
2. Cancel the order
3. You see without issue "Bibliographic record will not be deleted"
4. administration => Patron categories
5. Try to delete a used and unused category
6. You see as expected
   Category XXXX is in use. Deletion not possible!
   and
   Confirm deletion of category XXXX
Comment 30 Victor Grousset/tuxayo 2022-06-02 03:40:58 UTC
Created attachment 135565 [details] [review]
Bug 30733: Move all semicolons inside the translatable strings

In the 1st patch, sometimes they where left out
Comment 31 Victor Grousset/tuxayo 2022-06-02 03:41:11 UTC
Created attachment 135566 [details] [review]
Bug 30733: Fix string context lost due to being in two parts

Due to conditions. Things like "Edit batch" need to be keept in one
strings.

Also removed some capitalization because it wasn't needed and wasn't
consistend with <title> and <h1> which had now capitalization.

Also fixed patron edit-batch.tt and edit-layout.tt because they were
incomplete in regard to their label counterparts. They had the condition
on description and batch_id but weren't displaying anything different
between the two cases.
Comment 32 Victor Grousset/tuxayo 2022-06-02 03:41:25 UTC
Created attachment 135567 [details] [review]
Bug 30733: Undo bad handling of a plural case
Comment 33 Victor Grousset/tuxayo 2022-06-02 03:41:40 UTC
Created attachment 135568 [details] [review]
Bug 30733: Wrap one more string in <span>
Comment 34 Victor Grousset/tuxayo 2022-06-02 03:43:44 UTC
Created attachment 135569 [details] [review]
Bug 30733: Undo <span> in an ICS calendar file
Comment 35 Victor Grousset/tuxayo 2022-06-02 04:00:31 UTC
Rebased.

---

(In reply to Fridolin Somers from multiple comments!)
> Some breadcrumbs have a IF/ELSE on action (like add/modify) on an object.
> We need to keep the action and the object together because of some languages
> like French.

Great catch thanks! Fixed.

---

> In koha-tmpl/intranet-tmpl/prog/en/modules/labels/label-bib-search.tt :
> [...]
> I think the template is wrong even before this change.
> This should be wrapped in a "title" tag.
> Since it is the title tag, HTML tags are not allowed.
> Maybe we can create another bug report for that.

> Idem in opac-account-pay-error.tt

After investigating these with you (thanks for the help :D) we are nearly sure they are dead code. I will open followup tickets. As the for changes of this patch they can stay there. They have no impact beside cleaning translation strings.

--- 

> In koha-tmpl/intranet-tmpl/prog/en/modules/circ/overdue.tt
> [...]
> Why this small change ?

It was so "[% ELSE %]Item[% END %] overdue as of" is extracted and the context is kept. But it doesn't look great on a second though to leave "Items" in the IF without context.

I undid it. And might handle it in a followup string cleanup patch using t() with proper plural handling.

---

> Maybe add span also around "Biblio-level recall on..."

Extracted string is «%s Biblio-level recall on» , I lacked time for those not really terrible strings. Done now.


---

> koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-ics.tt

> This is for ICal calendar export, maybe it should not contain HTML


Great catch! Undone.
Comment 36 Jonathan Druart 2022-06-02 06:52:19 UTC Comment hidden (obsolete)
Comment 37 Jonathan Druart 2022-06-02 06:53:25 UTC Comment hidden (obsolete)
Comment 38 Jonathan Druart 2022-06-02 07:19:50 UTC
My tests:
- checkout master
- run install (fr-FR)
- commit
- apply patches
- run install
- diff

The following changes look wrong:

* cataloguing/value_builder/marc21_linking_section.tt
-msgid "%sResults %s to %s of %s %sNo results found %s"
+#, fuzzy, c-format
+msgid "%sResults %s to %s of %s %s"
 msgstr "%sRésultats %s à %s sur %s %sAucun résultat %s"

Surround "Results with span
[% IF ( total ) %]<span>Results [% from | html %] to [% to | html %] of [% total | html %]</span>

* opac-account-pay-error.tt
It's not clear what's happening, isn't it be part of <title>?

* includes/holds-table.inc "Item in transit from"
You switched from:
 9591 -msgid "%s %s %s %s Item in transit from "
to:
 11135 +msgid "Item in transit from "

That's still not useful to translate. You just removed the %s, but translators still cannot translate the sentence correctly

* Same here, from:
 9712 -msgid "%s %stag%stags%s successfully added."
to:
 12611 +msgid "successfully added."

* Same: msgid "Added %s by", by who?
* Same: msgid "Waiting for patron at"
* Same: msgid "Recalled by patron expected at"


You want to help translators giving them sentences easy to translate. If you remove context you are make things worse :)
Comment 39 Julian Maurice 2022-06-02 07:40:59 UTC
Hey, have you heard about https://gitlab.com/koha-community/Koha/-/blob/master/docs/development/internationalization.md ? :-)
Comment 40 Jonathan Druart 2022-06-02 07:43:43 UTC
* You could also split this:
<span>Suspended [% IF ( HOLD.suspend_until ) %] until [% HOLD.suspend_until | $KohaDates %] [% END %]</span>

* msgid "If you have any questions, please contact the "

the what?

See on bug 29602 how we fixed this problem with KohaSpan

* msgid "Recalled by patron expected at"

* msgid "does not exist"
* msgid "does not match"

* msgid "%s Hold found for %s (" where is the closing parenthesis?


I am only at the middle of the diff, it's very boring, will continue later.
But I think you get it.
Comment 41 Jonathan Druart 2022-06-02 07:46:16 UTC
(In reply to Julian Maurice from comment #39)
> Hey, have you heard about
> https://gitlab.com/koha-community/Koha/-/blob/master/docs/development/
> internationalization.md ? :-)

Yes, and what Julian said, the i18n TT functions are the way to go for complicated strings/structures.
Comment 42 Victor Grousset/tuxayo 2022-06-03 03:42:45 UTC
Thanks for the feedback Julian and Jonathan :D

I'll 1st address the part about i18n TT functions.

(In reply to Julian Maurice from comment #39)
> Hey, have you heard about
> https://gitlab.com/koha-community/Koha/-/blob/master/docs/development/
> internationalization.md ? :-)

Of course I did :P , on Bug 29602 I pushed to use that instead of <span> but it couldn't be done (more on that bellow) and nothing changed on the situation of use of i18n TT functions so for this ticket I reluctantly used the same method as bug 29602. Which I find cluttering so I would be happy to make a 180° turn here and replace the ugly <span> with t().


(In reply to Jonathan Druart from comment #41)
> Yes, and what Julian said, the i18n TT functions are the way to go for
> complicated strings/structures.

Which couldn't be used for bug 29602 so I followed the same constraint here.

But you are the right person to clear the uncertainty about the used of i18n TT functions because you raised the concern about them on bug 29602 comment 7

> IIRC there were perf issues

That question was hard to settle on bug 20988. Current state is that I'm not sure what was the methodology you suggested to measure this accurately and asked for confirmation.

So what is your current position? I guess it's because of the not that high number of strings that performances aren't a worry here. But so was the case for bug 29602.
Comment 43 Jonathan Druart 2022-06-03 04:32:37 UTC
I would suggest then to continue the work and go with spans here.
Comment 44 Victor Grousset/tuxayo 2022-06-13 12:17:18 UTC
Created attachment 136012 [details] [review]
Bug 30733: Simplify translatable strings

And a few minor fixes when they where causing issues for
translatability.
And rephrased a string about password reset to have it identical to
other strings with the same meaning.

Simplified via wrapping strings with <span> to split to huge
concatenated strings with a lot of %s everywhere.

== Test plan ==
This patch needs mainly proof reading. Still it's possible to do some
basic testing to demonstrate that adding a <span> in an IF doesn't
break anything.
Pick in one of the 110 modified templates a string that you know how to
display. Otherwise:
1. acquisitions => vendor => basket => add to basket =>
   search "from existing record" => add order
2. Cancel the order
3. You see without issue "Bibliographic record will not be deleted"
4. administration => Patron categories
5. Try to delete a used and unused category
6. You see as expected
   Category XXXX is in use. Deletion not possible!
   and
   Confirm deletion of category XXXX
Comment 45 Victor Grousset/tuxayo 2022-06-13 12:17:23 UTC
Created attachment 136013 [details] [review]
Bug 30733: Move all semicolons inside the translatable strings

In the 1st patch, sometimes they where left out
Comment 46 Victor Grousset/tuxayo 2022-06-13 12:17:29 UTC
Created attachment 136014 [details] [review]
Bug 30733: Fix string context lost due to being in two parts

Due to conditions. Things like "Edit batch" need to be keept in one
strings.

Also removed some capitalization because it wasn't needed and wasn't
consistend with <title> and <h1> which had now capitalization.

Also fixed patron edit-batch.tt and edit-layout.tt because they were
incomplete in regard to their label counterparts. They had the condition
on description and batch_id but weren't displaying anything different
between the two cases.
Comment 47 Victor Grousset/tuxayo 2022-06-13 12:17:34 UTC
Created attachment 136015 [details] [review]
Bug 30733: Undo bad handling of a plural case
Comment 48 Victor Grousset/tuxayo 2022-06-13 12:17:40 UTC
Created attachment 136016 [details] [review]
Bug 30733: Wrap one more string in <span>
Comment 49 Victor Grousset/tuxayo 2022-06-13 12:17:46 UTC
Created attachment 136017 [details] [review]
Bug 30733: Undo <span> in an ICS calendar file
Comment 50 Victor Grousset/tuxayo 2022-06-13 12:17:53 UTC
Created attachment 136018 [details] [review]
Bug 30733: Simplify some singular cases

To be sure translators don't think it's wrong and translate it plurally.
Comment 51 Victor Grousset/tuxayo 2022-06-13 12:17:58 UTC
Created attachment 136019 [details] [review]
Bug 30733: Undo some <span> additions that don't yield good strings

They need more complex solutions that would be better in a follow up ticket
to not drag this one too long.
Comment 52 Victor Grousset/tuxayo 2022-06-13 12:18:05 UTC
Created attachment 136020 [details] [review]
Bug 30733: fix url => URL to merge with existing string
Comment 53 Victor Grousset/tuxayo 2022-06-13 12:18:11 UTC
Created attachment 136021 [details] [review]
Bug 30733: Split strings differently
Comment 54 Victor Grousset/tuxayo 2022-06-13 12:23:44 UTC
(In reply to Jonathan Druart from comment #38)
> My tests:
> - checkout master
> - run install (fr-FR)
> - commit
> - apply patches
> - run install

you mean update?

---

> The following changes look wrong:
> 
> * cataloguing/value_builder/marc21_linking_section.tt
> [...]
> Surround "Results with span
> [% IF ( total ) %]<span>Results [% from | html %] to [% to | html %] of [%
> total | html %]</span>

Ok done, initially I skipped some strings that were not really terrible to keep time to process all the worst.

---
 
> * opac-account-pay-error.tt
> It's not clear what's happening, isn't it be part of <title>?

After investigating with Fridolin we are nearly sure it's dead code from the time of Paypal payment integrated into Koha. I will open a followup ticket. As the for changes of this patch they can stay there. They have no impact besides cleaning translation strings. Simpler strings never used are still less translation work than a complex string never used.


> * includes/holds-table.inc "Item in transit from"
> You switched from:
>  9591 -msgid "%s %s %s %s Item in transit from "
> to:
>  11135 +msgid "Item in transit from "
> 
> That's still not useful to translate. You just removed the %s, but
> translators still cannot translate the sentence correctly

Ok, I extracted the <strong> in a BLOCK, thanks for the advice to use that.

> * Same here, from:
>  9712 -msgid "%s %stag%stags%s successfully added."
> to:
>  12611 +msgid "successfully added."

It's a plural case. Reverted, it should be done using I18n TT functions.

----

> * Same: msgid "Added %s by", by who?

Previous string:
msgid "%s Added %s by you %s %s Added %s by %s %s %s %s %s %s %s %s %s %s %s %s %s %s %s %s %s "

That's an improvement right? Can't do better IIUC, the last word of the displayed string is from a SWITCH https://git.koha-community.org/Koha-community/Koha/src/commit/beb663836b45a77f45195774133cc9b78957fcd5/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-showreviews.tt#L94

----

> * Same: msgid "Waiting for patron at"
> * Same: msgid "Recalled by patron expected at"

Is that about about including a trailing %s to make clear that there is a variable? Done.


---


> You want to help translators giving them sentences easy to translate. If you
> remove context you are make things worse :)

Is there actually a confusion? Given the way phrases end.


----


(In reply to Jonathan Druart from comment #40)
> * You could also split this:
> <span>Suspended [% IF ( HOLD.suspend_until ) %] until [% HOLD.suspend_until
> | $KohaDates %] [% END %]</span>

Indeed, that's not a loss of context. I isolated Suspended and until.

-----

> * msgid "If you have any questions, please contact the "
> 
> the what?
> See on bug 29602 how we fixed this problem with KohaSpan

I don't get the need of KohaSpan. Because here are the different resulting HTMLs with and without the KohaSpan filter.
<span>If you have any questions, please contact the <span><a href="mailto:root@localhost">site administrator</a></span></span>
<span>If you have any questions, please contact the <a href="mailto:root@localhost">site administrator</a></span>

And in both cases, the string will not be as wished. Because in contrast to the examples in bug 29602, "site administrator" is in the <a>. I got a list of a some strings like this. Maybe there is a way to deal with this? I18n TT functions to provide context for the incomplete strings?


-------
 
> * msgid "Recalled by patron expected at"

Fixed.

> * msgid "does not exist"
> * msgid "does not match"

It's just a bunch of IF, not much we can do.
https://git.koha-community.org/Koha-community/Koha/src/commit/972330bc9e13e40564885bcf0ef1723abcc51772/koha-tmpl/intranet-tmpl/prog/en/modules/tools/marc_modification_templates.tt#L186

> * msgid "%s Hold found for %s (" where is the closing parenthesis?

It doesn't look like I changed this so I don't know why if came up in your diff. Anyway, added a few <span>

> I am only at the middle of the diff, it's very boring, will continue later.
> But I think you get it.

It took me a long time to understand not having an ending %s was the issue. (I hope it was indeed that! 😮 )

I hope I spotted all the cases after going though the whole diff. Everything should be fixed or addressed in the previous paragraphs.
Comment 55 Victor Grousset/tuxayo 2022-06-13 12:32:48 UTC
> <span>If you have any questions, please contact the <a href="mailto:root@localhost">site administrator</a></span>

> And in both cases, the string will not be as wished. Because in contrast to the examples in bug 29602, "site administrator" is in the <a>. I got a list of a some strings like this. Maybe there is a way to deal with this? I18n TT functions to provide context for the incomplete strings?

Meanwhile, changes have been reverted.
Comment 56 Fridolin Somers 2022-06-13 22:06:21 UTC
Created attachment 136045 [details] [review]
Bug 30733: Simplify translatable strings

And a few minor fixes when they where causing issues for
translatability.
And rephrased a string about password reset to have it identical to
other strings with the same meaning.

Simplified via wrapping strings with <span> to split to huge
concatenated strings with a lot of %s everywhere.

== Test plan ==
This patch needs mainly proof reading. Still it's possible to do some
basic testing to demonstrate that adding a <span> in an IF doesn't
break anything.
Pick in one of the 110 modified templates a string that you know how to
display. Otherwise:
1. acquisitions => vendor => basket => add to basket =>
   search "from existing record" => add order
2. Cancel the order
3. You see without issue "Bibliographic record will not be deleted"
4. administration => Patron categories
5. Try to delete a used and unused category
6. You see as expected
   Category XXXX is in use. Deletion not possible!
   and
   Confirm deletion of category XXXX

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 57 Fridolin Somers 2022-06-13 22:06:45 UTC
Created attachment 136046 [details] [review]
Bug 30733: Move all semicolons inside the translatable strings

In the 1st patch, sometimes they where left out

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 58 Fridolin Somers 2022-06-13 22:07:50 UTC
Created attachment 136047 [details] [review]
Bug 30733: Fix string context lost due to being in two parts

Due to conditions. Things like "Edit batch" need to be keept in one
strings.

Also removed some capitalization because it wasn't needed and wasn't
consistend with <title> and <h1> which had now capitalization.

Also fixed patron edit-batch.tt and edit-layout.tt because they were
incomplete in regard to their label counterparts. They had the condition
on description and batch_id but weren't displaying anything different
between the two cases.

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 59 Fridolin Somers 2022-06-13 22:08:06 UTC
Created attachment 136048 [details] [review]
Bug 30733: Undo bad handling of a plural case

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 60 Fridolin Somers 2022-06-13 22:08:22 UTC
Created attachment 136049 [details] [review]
Bug 30733: Wrap one more string in <span>

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 61 Fridolin Somers 2022-06-13 22:08:41 UTC
Created attachment 136050 [details] [review]
Bug 30733: Undo <span> in an ICS calendar file

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 62 Fridolin Somers 2022-06-13 22:09:05 UTC
Created attachment 136051 [details] [review]
Bug 30733: Simplify some singular cases

To be sure translators don't think it's wrong and translate it plurally.

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 63 Fridolin Somers 2022-06-13 22:09:21 UTC
Created attachment 136052 [details] [review]
Bug 30733: Undo some <span> additions that don't yield good strings

They need more complex solutions that would be better in a follow up ticket
to not drag this one too long.

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 64 Fridolin Somers 2022-06-13 22:09:46 UTC
Created attachment 136053 [details] [review]
Bug 30733: fix url => URL to merge with existing string

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 65 Fridolin Somers 2022-06-13 22:10:04 UTC
Created attachment 136054 [details] [review]
Bug 30733: Split strings differently

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 66 Tomás Cohen Arazi 2022-06-15 14:19:41 UTC
Please rebase so we can QA and push fast. Thanks, and great work.
Comment 67 Victor Grousset/tuxayo 2022-06-15 17:30:44 UTC
Created attachment 136098 [details] [review]
Bug 30733: Simplify translatable strings

And a few minor fixes when they where causing issues for
translatability.
And rephrased a string about password reset to have it identical to
other strings with the same meaning.

Simplified via wrapping strings with <span> to split to huge
concatenated strings with a lot of %s everywhere.

== Test plan ==
This patch needs mainly proof reading. Still it's possible to do some
basic testing to demonstrate that adding a <span> in an IF doesn't
break anything.
Pick in one of the 110 modified templates a string that you know how to
display. Otherwise:
1. acquisitions => vendor => basket => add to basket =>
   search "from existing record" => add order
2. Cancel the order
3. You see without issue "Bibliographic record will not be deleted"
4. administration => Patron categories
5. Try to delete a used and unused category
6. You see as expected
   Category XXXX is in use. Deletion not possible!
   and
   Confirm deletion of category XXXX
Comment 68 Victor Grousset/tuxayo 2022-06-15 17:31:02 UTC
Created attachment 136099 [details] [review]
Bug 30733: Move all semicolons inside the translatable strings

In the 1st patch, sometimes they where left out
Comment 69 Victor Grousset/tuxayo 2022-06-15 17:31:12 UTC
Created attachment 136100 [details] [review]
Bug 30733: Fix string context lost due to being in two parts

Due to conditions. Things like "Edit batch" need to be keept in one
strings.

Also removed some capitalization because it wasn't needed and wasn't
consistend with <title> and <h1> which had now capitalization.

Also fixed patron edit-batch.tt and edit-layout.tt because they were
incomplete in regard to their label counterparts. They had the condition
on description and batch_id but weren't displaying anything different
between the two cases.
Comment 70 Victor Grousset/tuxayo 2022-06-15 17:31:18 UTC
Created attachment 136101 [details] [review]
Bug 30733: Undo bad handling of a plural case
Comment 71 Victor Grousset/tuxayo 2022-06-15 17:31:24 UTC
Created attachment 136102 [details] [review]
Bug 30733: Wrap one more string in <span>
Comment 72 Victor Grousset/tuxayo 2022-06-15 17:31:29 UTC
Created attachment 136103 [details] [review]
Bug 30733: Undo <span> in an ICS calendar file
Comment 73 Victor Grousset/tuxayo 2022-06-15 17:31:35 UTC
Created attachment 136104 [details] [review]
Bug 30733: Simplify some singular cases

To be sure translators don't think it's wrong and translate it plurally.
Comment 74 Victor Grousset/tuxayo 2022-06-15 17:31:43 UTC
Created attachment 136105 [details] [review]
Bug 30733: Undo some <span> additions that don't yield good strings

They need more complex solutions that would be better in a follow up ticket
to not drag this one too long.
Comment 75 Victor Grousset/tuxayo 2022-06-15 17:31:51 UTC
Created attachment 136106 [details] [review]
Bug 30733: fix url => URL to merge with existing string
Comment 76 Victor Grousset/tuxayo 2022-06-15 17:31:58 UTC
Created attachment 136107 [details] [review]
Bug 30733: Split strings differently
Comment 77 Victor Grousset/tuxayo 2022-06-15 17:34:50 UTC
Created attachment 136108 [details] [review]
Bug 30733: Simplify translatable strings

And a few minor fixes when they where causing issues for
translatability.
And rephrased a string about password reset to have it identical to
other strings with the same meaning.

Simplified via wrapping strings with <span> to split to huge
concatenated strings with a lot of %s everywhere.

== Test plan ==
This patch needs mainly proof reading. Still it's possible to do some
basic testing to demonstrate that adding a <span> in an IF doesn't
break anything.
Pick in one of the 110 modified templates a string that you know how to
display. Otherwise:
1. acquisitions => vendor => basket => add to basket =>
   search "from existing record" => add order
2. Cancel the order
3. You see without issue "Bibliographic record will not be deleted"
4. administration => Patron categories
5. Try to delete a used and unused category
6. You see as expected
   Category XXXX is in use. Deletion not possible!
   and
   Confirm deletion of category XXXX

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 78 Victor Grousset/tuxayo 2022-06-15 17:35:11 UTC
Created attachment 136109 [details] [review]
Bug 30733: Move all semicolons inside the translatable strings

In the 1st patch, sometimes they where left out

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 79 Victor Grousset/tuxayo 2022-06-15 17:35:34 UTC
Created attachment 136110 [details] [review]
Bug 30733: Fix string context lost due to being in two parts

Due to conditions. Things like "Edit batch" need to be keept in one
strings.

Also removed some capitalization because it wasn't needed and wasn't
consistend with <title> and <h1> which had now capitalization.

Also fixed patron edit-batch.tt and edit-layout.tt because they were
incomplete in regard to their label counterparts. They had the condition
on description and batch_id but weren't displaying anything different
between the two cases.

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 80 Victor Grousset/tuxayo 2022-06-15 17:35:50 UTC
Created attachment 136111 [details] [review]
Bug 30733: Undo bad handling of a plural case

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 81 Victor Grousset/tuxayo 2022-06-15 17:36:11 UTC
Created attachment 136112 [details] [review]
Bug 30733: Wrap one more string in <span>

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 82 Victor Grousset/tuxayo 2022-06-15 17:36:28 UTC
Created attachment 136113 [details] [review]
Bug 30733: Undo <span> in an ICS calendar file

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 83 Victor Grousset/tuxayo 2022-06-15 17:36:44 UTC
Created attachment 136114 [details] [review]
Bug 30733: Simplify some singular cases

To be sure translators don't think it's wrong and translate it plurally.

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 84 Victor Grousset/tuxayo 2022-06-15 17:37:02 UTC
Created attachment 136115 [details] [review]
Bug 30733: Undo some <span> additions that don't yield good strings

They need more complex solutions that would be better in a follow up ticket
to not drag this one too long.

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 85 Victor Grousset/tuxayo 2022-06-15 17:37:21 UTC
Created attachment 136116 [details] [review]
Bug 30733: fix url => URL to merge with existing string

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 86 Victor Grousset/tuxayo 2022-06-15 17:37:40 UTC
Created attachment 136117 [details] [review]
Bug 30733: Split strings differently

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 87 Victor Grousset/tuxayo 2022-06-15 17:38:15 UTC
Rebased. Sorry for the double upload, I lost Fridolin's signoff the 1st time.
Comment 88 Victor Grousset/tuxayo 2022-06-16 16:01:43 UTC
Created attachment 136174 [details] [review]
Bug 30733: Simplify translatable strings

And a few minor fixes when they where causing issues for
translatability.
And rephrased a string about password reset to have it identical to
other strings with the same meaning.

Simplified via wrapping strings with <span> to split to huge
concatenated strings with a lot of %s everywhere.

== Test plan ==
This patch needs mainly proof reading. Still it's possible to do some
basic testing to demonstrate that adding a <span> in an IF doesn't
break anything.
Pick in one of the 110 modified templates a string that you know how to
display. Otherwise:
1. acquisitions => vendor => basket => add to basket =>
   search "from existing record" => add order
2. Cancel the order
3. You see without issue "Bibliographic record will not be deleted"
4. administration => Patron categories
5. Try to delete a used and unused category
6. You see as expected
   Category XXXX is in use. Deletion not possible!
   and
   Confirm deletion of category XXXX

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 89 Victor Grousset/tuxayo 2022-06-16 16:02:03 UTC
Created attachment 136175 [details] [review]
Bug 30733: Move all semicolons inside the translatable strings

In the 1st patch, sometimes they where left out

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 90 Victor Grousset/tuxayo 2022-06-16 16:02:23 UTC
Created attachment 136176 [details] [review]
Bug 30733: Fix string context lost due to being in two parts

Due to conditions. Things like "Edit batch" need to be keept in one
strings.

Also removed some capitalization because it wasn't needed and wasn't
consistend with <title> and <h1> which had now capitalization.

Also fixed patron edit-batch.tt and edit-layout.tt because they were
incomplete in regard to their label counterparts. They had the condition
on description and batch_id but weren't displaying anything different
between the two cases.

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 91 Victor Grousset/tuxayo 2022-06-16 16:02:46 UTC
Created attachment 136177 [details] [review]
Bug 30733: Undo bad handling of a plural case

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 92 Victor Grousset/tuxayo 2022-06-16 16:03:01 UTC
Created attachment 136178 [details] [review]
Bug 30733: Wrap one more string in <span>

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 93 Victor Grousset/tuxayo 2022-06-16 16:03:18 UTC
Created attachment 136179 [details] [review]
Bug 30733: Undo <span> in an ICS calendar file

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 94 Victor Grousset/tuxayo 2022-06-16 16:03:38 UTC
Created attachment 136180 [details] [review]
Bug 30733: Simplify some singular cases

To be sure translators don't think it's wrong and translate it plurally.

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 95 Victor Grousset/tuxayo 2022-06-16 16:04:01 UTC
Created attachment 136181 [details] [review]
Bug 30733: Undo some <span> additions that don't yield good strings

They need more complex solutions that would be better in a follow up ticket
to not drag this one too long.

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 96 Victor Grousset/tuxayo 2022-06-16 16:04:25 UTC
Created attachment 136182 [details] [review]
Bug 30733: fix url => URL to merge with existing string

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 97 Victor Grousset/tuxayo 2022-06-16 16:04:49 UTC
Created attachment 136183 [details] [review]
Bug 30733: Split strings differently

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 98 Victor Grousset/tuxayo 2022-06-16 16:05:59 UTC
Rebased.
Comment 99 Paul Derscheid 2022-06-16 16:34:56 UTC
I this intended?
---

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/basket/sendbasket.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/basket/sendbasket.tt
index 6af47706d0..a39f377914 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/basket/sendbasket.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/basket/sendbasket.tt
@@ -38,7 +38,7 @@ Your cart
 
             <p>
             [% IF ( BIBLIO_RESULT.HASAUTHORS ) %]
-            <span>Author(s): [% IF ( BIBLIO_RESULT.author ) %][% BIBLIO_RESULT.author | $raw %][% END %]
+            <span><span>Author(s):</span> [% IF ( BIBLIO_RESULT.author ) %][% BIBLIO_RESULT.author | $raw %][% END %]
 
             [% IF ( BIBLIO_RESULT.MARCAUTHORS ) %]
                 [% IF ( BIBLIO_RESULT.author ) %]; [% END %]

---

I think there's something wrong here..

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
index 905fd07bd3..917a7ef246 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
@@ -516,14 +516,15 @@ Note that permanent location is a code, and location may be an authval.
 
                         [% IF item.recalled %]
                             [% IF item.recall.waiting_date %]
-                                Waiting at [% Branches.GetName( item.recall.pickup_library_id ) | html %] since [% item.recall.waiting_date | $KohaDates %]
+                                <span>Waiting at [% Branches.GetName( item.recall.pickup_library_id ) | html %] since [% item.recall.waiting_date | $KohaDates %]</span>
                             [% ELSE %]
-                                Item recalled by <a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% item.recall.patron_id | uri %]">[% item.recall.patron.firstname | html %] [% item.recall.patron.surname | html %] ([% item.recall.patron.cardnumber | html %])</a> on [% item.recall.created_date | $KohaDates %]                          
+                                [% patron_link = BLOCK %]<a href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[% item.recall.patron_id | uri %]">[% item.recall.patron.firstname | html %] [% item.recall.patron.surname | html %] ([% item.recall.patron.cardnumber | html %])</a>[% END %]                                                        
+                                <span>Item recalled by [% patron_link| $raw %] on [% item.recall.created_date | $KohaDates %]</span>
                             [% END %]
                         [% END %]

---

diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-tags.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-tags.tt
index 8953297ab7..445ec01a03 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-tags.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-tags.tt

@@ -127,7 +127,7 @@
                                 <div class="dialog message">[% added_count | html %] [% IF ( added_count == 1 ) %]tag[% ELSE %]tags[% END %] successfully added.</div>
                             [% END %]
                             [% IF ( deleted_count ) %]
-                            <div class="dialog message">[% deleted_count | html %] [% IF ( deleted_count == 1 ) %]tag[% ELSE %]tags[% END %] successfully deleted.</div>
+                                <div class="dialog message">[% deleted_count | html %] [% IF ( deleted_count == 1 ) %]tag[% ELSE %]tags[% END %] successfully deleted.</div>
                             [% END %]
                         [% END # /add_op %]
Comment 100 Victor Grousset/tuxayo 2022-06-16 17:01:26 UTC
(In reply to Paul Derscheid from comment #99)
> I this intended?
> [...]
>              <p>
>              [% IF ( BIBLIO_RESULT.HASAUTHORS ) %]
> -            <span>Author(s): [% IF ( BIBLIO_RESULT.author ) %][%
> BIBLIO_RESULT.author | $raw %][% END %]
> +            <span><span>Author(s):</span> [% IF ( BIBLIO_RESULT.author )
> %][% BIBLIO_RESULT.author | $raw %][% END %]

This is expected. There was already one <span> spanning (ha ha) a few lines and I needed another one just for the string on this line.

------

> I think there's something wrong here..
> 
> diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
> [...]
> -                                Item recalled by <a
> href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[%
> item.recall.patron_id | uri %]">[% item.recall.patron.firstname | html %] [%
> item.recall.patron.surname | html %] ([% item.recall.patron.cardnumber |
> html %])</a> on [% item.recall.created_date | $KohaDates %]                 
> 
> +                                [% patron_link = BLOCK %]<a
> href="/cgi-bin/koha/members/moremember.pl?borrowernumber=[%
> item.recall.patron_id | uri %]">[% item.recall.patron.firstname | html %] [%
> item.recall.patron.surname | html %] ([% item.recall.patron.cardnumber |
> html %])</a>[% END %]                                                        
> +                                <span>Item recalled by [% patron_link| $raw
> %] on [% item.recall.created_date | $KohaDates %]</span>
>                              [% END %]
>                          [% END %]

I had to do something ugly (adding <span> everywhere arguably is already ^^") , extracting the <a> in a BLOCK. Because having the <a> in the middle of the phrase would split it without any recourse. But a BLOCK usage doesn't cause the extractor to split the string.

------

> diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-tags.tt
> b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-tags.tt
> index 8953297ab7..445ec01a03 100644
> --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-tags.tt
> +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-tags.tt
> [...]
> -                            <div class="dialog message">[% deleted_count |
> html %] [% IF ( deleted_count == 1 ) %]tag[% ELSE %]tags[% END %]
> successfully deleted.</div>
> +                                <div class="dialog message">[%
> deleted_count | html %] [% IF ( deleted_count == 1 ) %]tag[% ELSE %]tags[%
> END %] successfully deleted.</div>


Here I had split the string initially and fixed indentation but I ended up not splitting the string. It's a plural case. It should be done using I18n TT functions. But I let the indentation fix.
Comment 101 Paul Derscheid 2022-06-16 17:08:44 UTC
Created attachment 136188 [details] [review]
Bug 30733: Simplify translatable strings

And a few minor fixes when they where causing issues for
translatability.
And rephrased a string about password reset to have it identical to
other strings with the same meaning.

Simplified via wrapping strings with <span> to split to huge
concatenated strings with a lot of %s everywhere.

== Test plan ==
This patch needs mainly proof reading. Still it's possible to do some
basic testing to demonstrate that adding a <span> in an IF doesn't
break anything.
Pick in one of the 110 modified templates a string that you know how to
display. Otherwise:
1. acquisitions => vendor => basket => add to basket =>
   search "from existing record" => add order
2. Cancel the order
3. You see without issue "Bibliographic record will not be deleted"
4. administration => Patron categories
5. Try to delete a used and unused category
6. You see as expected
   Category XXXX is in use. Deletion not possible!
   and
   Confirm deletion of category XXXX

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 102 Paul Derscheid 2022-06-16 17:08:50 UTC
Created attachment 136189 [details] [review]
Bug 30733: Move all semicolons inside the translatable strings

In the 1st patch, sometimes they where left out

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 103 Paul Derscheid 2022-06-16 17:08:54 UTC
Created attachment 136190 [details] [review]
Bug 30733: Fix string context lost due to being in two parts

Due to conditions. Things like "Edit batch" need to be keept in one
strings.

Also removed some capitalization because it wasn't needed and wasn't
consistend with <title> and <h1> which had now capitalization.

Also fixed patron edit-batch.tt and edit-layout.tt because they were
incomplete in regard to their label counterparts. They had the condition
on description and batch_id but weren't displaying anything different
between the two cases.

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 104 Paul Derscheid 2022-06-16 17:08:59 UTC
Created attachment 136191 [details] [review]
Bug 30733: Undo bad handling of a plural case

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 105 Paul Derscheid 2022-06-16 17:09:05 UTC
Created attachment 136192 [details] [review]
Bug 30733: Wrap one more string in <span>

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 106 Paul Derscheid 2022-06-16 17:09:10 UTC
Created attachment 136193 [details] [review]
Bug 30733: Undo <span> in an ICS calendar file

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 107 Paul Derscheid 2022-06-16 17:09:15 UTC
Created attachment 136194 [details] [review]
Bug 30733: Simplify some singular cases

To be sure translators don't think it's wrong and translate it plurally.

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 108 Paul Derscheid 2022-06-16 17:09:20 UTC
Created attachment 136195 [details] [review]
Bug 30733: Undo some <span> additions that don't yield good strings

They need more complex solutions that would be better in a follow up ticket
to not drag this one too long.

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 109 Paul Derscheid 2022-06-16 17:09:25 UTC
Created attachment 136196 [details] [review]
Bug 30733: fix url => URL to merge with existing string

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 110 Paul Derscheid 2022-06-16 17:09:30 UTC
Created attachment 136197 [details] [review]
Bug 30733: Split strings differently

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 111 Fridolin Somers 2022-06-16 21:29:09 UTC
QA script is happy.

Just a warning with and without patches :
File not found : lib/jquery/jquery-ui-1.13.1.css at /kohadevbox/koha/Koha/Template/Plugin/Asset.pm line 107.

We should look at this in another bug.
Comment 112 Fridolin Somers 2022-06-16 21:30:14 UTC
Created attachment 136202 [details] [review]
Bug 30733: Simplify translatable strings

And a few minor fixes when they where causing issues for
translatability.
And rephrased a string about password reset to have it identical to
other strings with the same meaning.

Simplified via wrapping strings with <span> to split to huge
concatenated strings with a lot of %s everywhere.

== Test plan ==
This patch needs mainly proof reading. Still it's possible to do some
basic testing to demonstrate that adding a <span> in an IF doesn't
break anything.
Pick in one of the 110 modified templates a string that you know how to
display. Otherwise:
1. acquisitions => vendor => basket => add to basket =>
   search "from existing record" => add order
2. Cancel the order
3. You see without issue "Bibliographic record will not be deleted"
4. administration => Patron categories
5. Try to delete a used and unused category
6. You see as expected
   Category XXXX is in use. Deletion not possible!
   and
   Confirm deletion of category XXXX

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 113 Fridolin Somers 2022-06-16 21:31:19 UTC
Created attachment 136203 [details] [review]
Bug 30733: Move all semicolons inside the translatable strings

In the 1st patch, sometimes they where left out

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 114 Fridolin Somers 2022-06-16 21:31:34 UTC
Created attachment 136204 [details] [review]
Bug 30733: Fix string context lost due to being in two parts

Due to conditions. Things like "Edit batch" need to be keept in one
strings.

Also removed some capitalization because it wasn't needed and wasn't
consistend with <title> and <h1> which had now capitalization.

Also fixed patron edit-batch.tt and edit-layout.tt because they were
incomplete in regard to their label counterparts. They had the condition
on description and batch_id but weren't displaying anything different
between the two cases.

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 115 Fridolin Somers 2022-06-16 21:31:48 UTC
Created attachment 136205 [details] [review]
Bug 30733: Undo bad handling of a plural case

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 116 Fridolin Somers 2022-06-16 21:32:03 UTC
Created attachment 136206 [details] [review]
Bug 30733: Wrap one more string in <span>

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 117 Fridolin Somers 2022-06-16 21:32:18 UTC
Created attachment 136207 [details] [review]
Bug 30733: Undo <span> in an ICS calendar file

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 118 Fridolin Somers 2022-06-16 21:32:33 UTC
Created attachment 136208 [details] [review]
Bug 30733: Simplify some singular cases

To be sure translators don't think it's wrong and translate it plurally.

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 119 Fridolin Somers 2022-06-16 21:32:48 UTC
Created attachment 136209 [details] [review]
Bug 30733: Undo some <span> additions that don't yield good strings

They need more complex solutions that would be better in a follow up ticket
to not drag this one too long.

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 120 Fridolin Somers 2022-06-16 21:33:03 UTC
Created attachment 136210 [details] [review]
Bug 30733: fix url => URL to merge with existing string

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 121 Fridolin Somers 2022-06-16 21:34:00 UTC
Created attachment 136211 [details] [review]
Bug 30733: Split strings differently

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 122 Fridolin Somers 2022-06-16 21:34:54 UTC
Maybe add some nice text to go in the release notes
Comment 123 Fridolin Somers 2022-06-16 21:35:50 UTC
Careful when generating HTML release notes, <span> in commit messages must be escaped.
Comment 124 Victor Grousset/tuxayo 2022-06-16 21:45:09 UTC
> Maybe add some nice text to go in the release notes

Done, is it good?

> Careful when generating HTML release notes, <span> in commit messages must be escaped.

Wasn't the issue only for the ticket name? Commit don't end up in release notes.
Comment 125 Tomás Cohen Arazi 2022-06-17 13:29:54 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 126 Victor Grousset/tuxayo 2022-06-17 14:03:33 UTC
Created attachment 136286 [details] [review]
Bug 30733: [22.05] Simplify translatable strings

And a few minor fixes when they where causing issues for
translatability.
And rephrased a string about password reset to have it identical to
other strings with the same meaning.

Simplified via wrapping strings with <span> to split to huge
concatenated strings with a lot of %s everywhere.

== Test plan ==
This patch needs mainly proof reading. Still it's possible to do some
basic testing to demonstrate that adding a <span> in an IF doesn't
break anything.
Pick in one of the 110 modified templates a string that you know how to
display. Otherwise:
1. acquisitions => vendor => basket => add to basket =>
   search "from existing record" => add order
2. Cancel the order
3. You see without issue "Bibliographic record will not be deleted"
4. administration => Patron categories
5. Try to delete a used and unused category
6. You see as expected
   Category XXXX is in use. Deletion not possible!
   and
   Confirm deletion of category XXXX

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 127 Lucas Gass 2022-06-17 20:29:52 UTC
backported to 22.05.x for 22.05.01
Comment 128 Katrin Fischer 2022-06-18 09:36:17 UTC
Strings are available on Pootle now, just from this patch:

Critical 
91 

Unfinished
1,539

It's ok, just to show the effect.
Comment 129 Arthur Suzuki 2022-06-21 14:48:41 UTC
big patch, lots of conflict when trying to apply to 21.11.x.
Provide a patch for 21.11 if needed, thanks!
Comment 130 Arthur Suzuki 2022-06-22 14:31:15 UTC
According to conversation during IRC dev meeting, won't backport to 21.11. Thx!

<pre>
16:25 < ashimema> is it actualyl worthwhile backporting 30733 any further.. ?
16:26 < ashimema> ha.. great minds
16:26 < cait1> it creates a lot of work for translators
16:26 < tuxayo> ashimema totally not
16:26 < cait1> and the later the version the more unlikley people keep up
16:26 < tuxayo> It's a mess ^^
16:26 < liliputech> if it's not i can manage not to work on it ;)
16:26 < ashimema> I don't think we should.. it would do the oposite of what it tries to do.. get translators to re-translate a lot
16:26 < tuxayo> 30733 was only for 22.05
16:26 < cait1> yep, I think it's good there
16:26 < liliputech> ok, then i will resolve-fix that one :) thx!
</pre>