Bug 18654 - Translatability: Get rid of tt directives starting with [%% in translation for itemsearch.tt
Summary: Translatability: Get rid of tt directives starting with [%% in translation fo...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: I18N/L10N (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Marc Véron
QA Contact: Testopia
URL:
Keywords:
Depends on: 18633
Blocks:
  Show dependency treegraph
 
Reported: 2017-05-23 07:02 UTC by Marc Véron
Modified: 2019-10-14 19:57 UTC (History)
5 users (show)

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


Attachments
Bug 18654: Translatability: Get rid of tt directives starting with [%% in translation for itemsearch.tt (5.81 KB, patch)
2017-05-23 07:08 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18654: Translatability: Get rid of tt directives starting with [%% in translation for itemsearch.tt (5.92 KB, patch)
2017-05-28 10:34 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 18654: Translatability: Get rid of tt directives starting with [%% in translation for itemsearch.tt (4.04 KB, patch)
2017-06-11 06:07 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18654 - Translatability: Get rid of tt directives starting with [%% in translation for itemsearch.tt (3.36 KB, patch)
2017-06-29 13:26 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18654 - Translatability: Get rid of tt directives starting with [%% in translation for itemsearch.tt (3.61 KB, patch)
2017-08-01 22:39 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 18654 - Translatability: Get rid of tt directives starting with [%% in translation for itemsearch.tt (3.69 KB, patch)
2017-08-30 19:34 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18654 - Translatability: Get rid of tt directives starting with [%% in translation for itemsearch.tt - followup (1.57 KB, patch)
2017-09-12 13:02 UTC, Fridolin Somers
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marc Véron 2017-05-23 07:02:35 UTC
Translation for file itemsearch.tt exposes tt directifes like:
%s %s %s %s %s %s %s %s %s [%% params = { f => f.$i q = q.$i op = op.$i } %%] %s %s %s %s %s %s %s %s %s %s %s 
or
[%% INCLUDE form_field_select name="homebranch" options = branches empty_option = "All libraries" %%] [%% INCLUDE form_field_select name="holdingbranch" options = branches empty_option = "All libraries" %%] %s [%% INCLUDE form_field_select name="location" options = locations empty_option = "All locations" %%] %s
Comment 1 Marc Véron 2017-05-23 07:08:06 UTC
Created attachment 63643 [details] [review]
Bug 18654: Translatability: Get rid of tt directives starting with [%% in translation for itemsearch.tt

Translation for file itemsearch.tt exposes tt directifes like:
%s %s %s %s %s %s %s %s %s [%% params = { f => f.$i q = q.$i op = op.$i } %%] %s %s %s %s %s %s %s %s %s %s %s
or
[%% INCLUDE form_field_select name="homebranch" options = branches empty_option = "All libraries" %%] [%% INCLUDE form_field_select name="holdingbranch" options = branches empty_option = "All libraries" %%] %s [%% INCLUDE form_field_select name="location" options = locations empty_option = "All locations" %%] %s

This patch fixes it.

To test:
- Apply patch
- Verify that itemsearch behaves as before
- Verify that code changes make sense
- Test a translatiion e.g. translator create aa-AA and
  verifiy that strings from itemsearch.tt starting with [%%
  no longer appear
Comment 2 Alex Buckley 2017-05-28 10:34:49 UTC
Created attachment 63773 [details] [review]
Bug 18654: Translatability: Get rid of tt directives starting with [%% in translation for itemsearch.tt

Translation for file itemsearch.tt exposes tt directifes like:
%s %s %s %s %s %s %s %s %s [%% params = { f => f.$i q = q.$i op = op.$i } %%] %s %s %s %s %s %s %s %s %s %s %s
or
[%% INCLUDE form_field_select name="homebranch" options = branches empty_option = "All libraries" %%] [%% INCLUDE form_field_select name="holdingbranch" options = branches empty_option = "All libraries" %%] %s [%% INCLUDE form_field_select name="location" options = locations empty_option = "All locations" %%] %s

This patch fixes it.

To test:
- Apply patch
- Verify that itemsearch behaves as before
- Verify that code changes make sense
- Test a translatiion e.g. translator create aa-AA and
  verifiy that strings from itemsearch.tt starting with [%%
  no longer appear

Followed test plan and patch worked as intended

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 3 Jonathan Druart 2017-06-09 15:40:07 UTC
+[% BLOCK fieldset_start %]<fieldset>[% END %]
+[% BLOCK fieldset_end %]</fieldset>[% END %]

This is not acceptable, why do you need that?
Comment 4 Marc Véron 2017-06-11 06:01:41 UTC
(In reply to Jonathan Druart from comment #3)
> +[% BLOCK fieldset_start %]<fieldset>[% END %]
> +[% BLOCK fieldset_end %]</fieldset>[% END %]
> 
> This is not acceptable, why do you need that?

You are right, it is not necessary to replace the fieldset tags.
Comment 5 Marc Véron 2017-06-11 06:07:14 UTC Comment hidden (obsolete)
Comment 6 Marc Véron 2017-06-29 13:26:42 UTC
Created attachment 64723 [details] [review]
Bug 18654 - Translatability: Get rid of tt directives starting with [%% in translation for itemsearch.tt

This patch removes entries like the following in translations of itemsearch.tt:
"[%% INCLUDE form_field_select name=\"homebranch\" options = branches "
"empty_option = \"All libraries\" %%] [%% INCLUDE form_field_select name="
"\"holdingbranch\" options = branches empty_option = \"All libraries\" %%] %s "
"[%% INCLUDE form_field_select name=\"location\" options = locations "
"empty_option = \"All locations\" %%] %s "

New patch on top of Bug 18633 that resolves parts of initial comment.

To test:
- Verify that in itemsearch.tt no tt directives are splitted by new lines
  (search for [% INCLUDE )
- Verify that itemsearch.tt works as before
Comment 7 Marc Véron 2017-07-20 08:06:22 UTC
Still applies
Comment 8 Alex Buckley 2017-08-01 22:39:32 UTC
Created attachment 65407 [details] [review]
Bug 18654 - Translatability: Get rid of tt directives starting with [%% in translation for itemsearch.tt

This patch removes entries like the following in translations of itemsearch.tt:
"[%% INCLUDE form_field_select name=\"homebranch\" options = branches "
"empty_option = \"All libraries\" %%] [%% INCLUDE form_field_select name="
"\"holdingbranch\" options = branches empty_option = \"All libraries\" %%] %s "
"[%% INCLUDE form_field_select name=\"location\" options = locations "
"empty_option = \"All locations\" %%] %s "

New patch on top of Bug 18633 that resolves parts of initial comment.

To test:
- Verify that in itemsearch.tt no tt directives are splitted by new lines
  (search for [% INCLUDE )
- Verify that itemsearch.tt works as before

Followed test plan and verified that tt directives are not split by new
lines, the changes to the fieldset tags in comment 3 have been removed
and itemtype.tt still works correctly as before

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 9 Jonathan Druart 2017-08-30 19:34:28 UTC
Created attachment 66640 [details] [review]
Bug 18654 - Translatability: Get rid of tt directives starting with [%% in translation for itemsearch.tt

This patch removes entries like the following in translations of itemsearch.tt:
"[%% INCLUDE form_field_select name=\"homebranch\" options = branches "
"empty_option = \"All libraries\" %%] [%% INCLUDE form_field_select name="
"\"holdingbranch\" options = branches empty_option = \"All libraries\" %%] %s "
"[%% INCLUDE form_field_select name=\"location\" options = locations "
"empty_option = \"All locations\" %%] %s "

New patch on top of Bug 18633 that resolves parts of initial comment.

To test:
- Verify that in itemsearch.tt no tt directives are splitted by new lines
  (search for [% INCLUDE )
- Verify that itemsearch.tt works as before

Followed test plan and verified that tt directives are not split by new
lines, the changes to the fieldset tags in comment 3 have been removed
and itemtype.tt still works correctly as before

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 10 Jonathan Druart 2017-08-30 19:58:48 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 11 Fridolin Somers 2017-09-12 13:02:14 UTC
Ah there is a typo in patch :
  [% INCLUDE form_field_select name="holdingbranch" options = branche
Its branche[s].

I create a follow-up patch
Comment 12 Fridolin Somers 2017-09-12 13:02:34 UTC
Created attachment 67082 [details] [review]
Bug 18654 - Translatability: Get rid of tt directives starting with [%% in translation for itemsearch.tt - followup

Followup correcting a typo of previous patch :
  name="holdingbranch" options = branche
it is branche[s]

Test plan :
- Look at 'Current location' in item search
=> Without patch you see only 'All libraries'
=> With patch you see 'All libraries' and each existing library, like in 'Home library'
Comment 13 Jonathan Druart 2017-09-12 15:15:26 UTC
(In reply to Fridolin SOMERS from comment #12)
> Created attachment 67082 [details] [review] [review]
> Bug 18654 - Translatability: Get rid of tt directives starting with [%% in
> translation for itemsearch.tt - followup

Last patch pushed to master, thanks Frido!
Comment 14 Fridolin Somers 2017-09-14 13:31:32 UTC
Pushed to 17.05.x, will be in 17.05.04.
Comment 15 Katrin Fischer 2017-09-16 06:56:01 UTC
These patches have been pushed to 16.11.x and will be in 16.11.12.
Comment 16 Mason James 2017-11-23 21:58:21 UTC
Pushed to 16.05.x, for 16.05.17 release