Bug 18641

Summary: Translatability: Get rid of template directives in translations for *reserves.tt files
Product: Koha Reporter: Marc Véron <veron>
Component: I18N/L10NAssignee: Marc Véron <veron>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: trivial    
Priority: P5 - low CC: f.demians, jonathan.druart, katrin.fischer, m.de.rooy, mtj
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: String patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 18641 - Translatability: Get rid of template directives in translations for *reserves.tt files
[SIGNED-OFF] Bug 18641 - Translatability: Get rid of template directives in translations for *reserves.tt files
Bug 18649: Translatability: Get rid of tt directive in translation for admin/categories.tt and onboardingstep2.tt
Bug 18641 - Translatability: Get rid of template directives in translations for *reserves.tt files
Bug 18641: (RM-followup) for waitingreserves.tt
Bug 18641: Followup for pendingreserves.tt (less %s)

Description Marc Véron 2017-05-20 06:54:32 UTC Comment hidden (obsolete)
Comment 1 Marc Véron 2017-05-20 07:01:15 UTC
Template directives should not be exposed in translation, but lines like the following pop up in the translation tool:
Cancel [% IF TransferWhenCancelAllWaitingHolds %]and Transfer [% END %] All
Comment 2 Marc Véron 2017-05-20 07:23:11 UTC
Created attachment 63584 [details] [review]
Bug 18641 - Translatability: Get rid of template directives in translations for *reserves.tt files

Template directives should not be exposed in translation, but lines like the following pop up in the translation tool:
Cancel [% IF TransferWhenCancelAllWaitingHolds %]and Transfer [% END %] All

This patch fixes it.

To test:
- Apply patch
- Verify that code changes make sense
- Verify that submit buttons of pendingreserves.tt and waitingreserves.tt
  still work and display the same string as before
Comment 3 Josef Moravec 2017-05-22 10:04:09 UTC
Created attachment 63608 [details] [review]
[SIGNED-OFF] Bug 18641 - Translatability: Get rid of template directives in translations for *reserves.tt files

Template directives should not be exposed in translation, but lines like the following pop up in the translation tool:
Cancel [% IF TransferWhenCancelAllWaitingHolds %]and Transfer [% END %] All

This patch fixes it.

To test:
- Apply patch
- Verify that code changes make sense
- Verify that submit buttons of pendingreserves.tt and waitingreserves.tt
  still work and display the same string as before

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 4 Marc Véron 2017-05-22 16:07:40 UTC
Created attachment 63625 [details] [review]
Bug 18649: Translatability: Get rid of tt directive in translation for admin/categories.tt and onboardingstep2.tt

Translation tool shows
[% IF category.enrolmentperiod %][% category.enrolmentperiod %][% END %]
for en/modules/admin/categories.tt and
koha-tmpl/intranet-tmpl/prog/en/modules/onboarding/onboardingstep2.tt

This patch fixes it.

To test:
- Apply patch
- Verify that code changes make sense
- Verify that template directive is no longer picked by translation
Comment 5 Marc Véron 2017-05-22 16:08:17 UTC
Comment on attachment 63625 [details] [review]
Bug 18649: Translatability: Get rid of tt directive in translation for admin/categories.tt and onboardingstep2.tt

Sorry for the noise, I mistaked in Bug number...
Comment 6 Marcel de Rooy 2017-06-02 09:15:10 UTC
Created attachment 63928 [details] [review]
Bug 18641 - Translatability: Get rid of template directives in translations for *reserves.tt files

Template directives should not be exposed in translation, but lines like the following pop up in the translation tool:
Cancel [% IF TransferWhenCancelAllWaitingHolds %]and Transfer [% END %] All

This patch fixes it.

To test:
- Apply patch
- Verify that code changes make sense
- Verify that submit buttons of pendingreserves.tt and waitingreserves.tt
  still work and display the same string as before

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Jonathan Druart 2017-06-05 14:41:55 UTC
And what not something more readable like

[% IF %]
  input
[% ELSE %]
  other input
[% END %]

?
Comment 8 Marc Véron 2017-06-05 15:44:38 UTC
Created attachment 63965 [details] [review]
Bug 18641: (RM-followup) for waitingreserves.tt

Code changed for waitingreserves.tt as asked for in comment #7
Comment 9 Jonathan Druart 2017-06-05 16:31:07 UTC
Marcel, what do you think?
Comment 10 Marcel de Rooy 2017-06-06 10:34:06 UTC
(In reply to Jonathan Druart from comment #9)
> Marcel, what do you think?

Looks good to me
Comment 11 Jonathan Druart 2017-06-06 13:21:43 UTC
(In reply to Marcel de Rooy from comment #10)
> (In reply to Jonathan Druart from comment #9)
> > Marcel, what do you think?
> 
> Looks good to me

Yes I know :)
But do you think the BLOCK method is acceptable, or we should stick to the IF/ELSE usual style?
I am asking for the other bug reports.
Comment 12 Marcel de Rooy 2017-06-06 15:02:04 UTC
(In reply to Jonathan Druart from comment #11)
> (In reply to Marcel de Rooy from comment #10)
> > (In reply to Jonathan Druart from comment #9)
> > > Marcel, what do you think?
> > 
> > Looks good to me
> 
> Yes I know :)
> But do you think the BLOCK method is acceptable, or we should stick to the
> IF/ELSE usual style?
> I am asking for the other bug reports.

In most cases we may be able to avoid BLOCK just as we do here now, but it might not be that easy everywhere..
Comment 13 Jonathan Druart 2017-06-06 20:00:48 UTC
For the pendingreserves changes:

Before the patch, I got

#. %1$s:  HoldsToPullStartDate.
#. %2$s:  IF ( HoldsToPullEndDate ).
#. %3$s:  HoldsToPullEndDate.
#. %4$s:  ELSE.
#. %5$s:  END.
#: intranet-tmpl/prog/en/modules/circ/pendingreserves.tt:198
#, c-format
msgid ""
"(Inclusive, default is %s days ago to %s%s days ahead%stoday%s, set other "
"date ranges as needed. )"

now I get:

#. %1$s:  BLOCK range_msg.
#. %2$s:  HoldsToPullStartDate.
#. %3$s:  IF ( HoldsToPullEndDate ).
#. %4$s:  HoldsToPullEndDate.
#. %5$s:  ELSE.
#. %6$s:  END.
#. %7$s:  END.
#: intranet-tmpl/prog/en/modules/circ/pendingreserves.tt:197
#, fuzzy, c-format
msgid ""
"%s(Inclusive, default is %s days ago to %s%s days ahead%stoday%s, set other "
"date ranges as needed.)%s "

Seems worst...
Comment 14 Marc Véron 2017-06-07 12:54:26 UTC
Created attachment 64078 [details] [review]
Bug 18641: Followup for pendingreserves.tt (less %s)

Change as of comment #13 to reduce %s

After creating a new translation for a "language" aa-AA, we have
the following in the file aa-AA-staff-prog.po

msgid ""
"(Inclusive, default is %s days ago to %s days ahead. Set other date ranges "
"as needed.)"
msgstr ""

msgid ""
"(Inclusive, default is %s days ago to today. Set other date ranges as "
"needed.)"
msgstr ""
Comment 15 Marc Véron 2017-06-07 12:55:26 UTC
Hope it's OK to set this back to PQA
Comment 16 Jonathan Druart 2017-06-09 13:04:36 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 17 Jonathan Druart 2017-06-09 13:04:58 UTC
I have squashed the 3 patches for readability.
Comment 18 Katrin Fischer 2017-06-13 05:46:51 UTC
Squashed patch from 17.05 has been pushed to 16.11.x and will be in 16.11.09.
Comment 19 Mason James 2017-07-31 11:18:40 UTC
Pushed to 16.05.x, for 16.05.15 release