Bug 11631

Summary: Make translation toolchain ignore useless translatable strings
Product: Koha Reporter: paxed <pasi.kallinen>
Component: I18N/L10NAssignee: paxed <pasi.kallinen>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: bgkriegel, f.demians, fridolin.somers, gmcharlt, jonathan.druart, katrin.fischer
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Make i18n toolchain ignore useless strings
[SIGNED-OFF] Bug 11631 - Make translation toolchain ignore useless "translatable" strings that contain nothing but spaces and/or placeholders.
Useless to translation strings
Bug 11631 - Followup to extend and compile pattern
Bug 11631 - Followup to extend and compile pattern
Bug 11631 - Followup to extend pattern
Make i18n toolchain ignore useless strings
[SIGNED_OFF] Bug 11631: Make i18n toolchain ignore useless strings
[SIGNED_OFF] Bug 11631: Make i18n toolchain ignore useless strings
[PASSED QA] Bug 11631: Make i18n toolchain ignore useless strings

Description paxed 2014-01-29 11:26:26 UTC Comment hidden (obsolete)
Comment 1 Bernardo Gonzalez Kriegel 2014-03-20 04:15:14 UTC
Hi,
it works well, but only remove 74 strings

Using this monster

$tmpt =~ s/(%s|\s|\(|\)|\[|\]|\{|\}|\\|\.|\,|;|:|\||\-|\?|!|\'|\"|\#|\*|%%|\/|\=|\&rsaquo;)//g;

removes 314 strings.

But I don't know if that is considered good coding :-)
Comment 2 Bernardo Gonzalez Kriegel 2014-03-20 11:28:52 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2014-03-20 13:10:53 UTC
Hello paxed,
It is a very great improvement!
Looking at the code, what do you think about replacing the temporary variable '$tmpt' and the substitution with only a match like
  next if $t =~ /^(%s|\s)*$/;
?
Comment 4 Bernardo Gonzalez Kriegel 2014-03-20 19:19:21 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2014-03-21 08:42:35 UTC
(In reply to Bernardo Gonzalez Kriegel from comment #4)
> My question: Is there a better o more elegant way to write this?

I agree with yours! I don't think we can write it better.
We should compile the regex (using qr).

Who does the patch? :)
Comment 6 Bernardo Gonzalez Kriegel 2014-03-21 13:32:33 UTC Comment hidden (obsolete)
Comment 7 Bernardo Gonzalez Kriegel 2014-03-21 13:33:54 UTC
> Who does the patch? :)
Done!

Now this need a sign :)
Comment 8 Bernardo Gonzalez Kriegel 2014-03-22 18:53:42 UTC Comment hidden (obsolete)
Comment 9 Bernardo Gonzalez Kriegel 2014-04-05 17:46:18 UTC Comment hidden (obsolete)
Comment 10 paxed 2014-04-24 08:15:51 UTC Comment hidden (obsolete)
Comment 11 Bernardo Gonzalez Kriegel 2014-04-24 11:34:16 UTC Comment hidden (obsolete)
Comment 12 Katrin Fischer 2014-04-26 16:14:00 UTC
Please always add the commit message to the test plan.
Comment 13 Katrin Fischer 2014-04-26 16:14:14 UTC
... the test plan to the commit message.
Comment 14 Bernardo Gonzalez Kriegel 2014-04-26 16:22:01 UTC Comment hidden (obsolete)
Comment 15 Bernardo Gonzalez Kriegel 2014-04-26 16:22:39 UTC
(In reply to Katrin Fischer from comment #13)
> ... the test plan to the commit message.

Not my patch, but my test plan :)
Comment 16 Katrin Fischer 2014-04-26 16:31:02 UTC
Created attachment 27660 [details] [review]
[PASSED QA] Bug 11631: Make i18n toolchain ignore useless strings

Test:
1) Update PO files of some lang, xx-YY-*po
cd misc/translator
perl translate update xx-YY
2) Do it again, just in case
3) rm po/xx-YY*po~
4) Extract all msgid's, sorted
cat po/xx-YY*po | egrep "^msgid" | sort | uniq > xx-YY-pre
5) Apply the patch
6) Repeat 1-3
7) Repeat 4 again, other file
cat po/xx-YY*po | egrep "^msgid" | sort | uniq > xx-YY-post
8) Do a diff, inspect results, only strings with %s and \s
diff xx-YY-pre xx-YY-post | less

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Works as described, 380 strings less to 'translate'
No koha-qa errors.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Tested according to test plan, works as described.
Comment 17 Galen Charlton 2014-04-27 22:03:50 UTC
Pushed to master.  Thanks, Pasi!
Comment 18 Fridolin Somers 2014-05-16 12:07:10 UTC
Pushed to 3.14.x, will be in 3.14.07.