Bug 11631 - Make translation toolchain ignore useless translatable strings
Summary: Make translation toolchain ignore useless 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: paxed
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-01-29 11:26 UTC by paxed
Modified: 2014-12-07 20:05 UTC (History)
6 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
Make i18n toolchain ignore useless strings (1019 bytes, patch)
2014-01-29 11:26 UTC, paxed
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 11631 - Make translation toolchain ignore useless "translatable" strings that contain nothing but spaces and/or placeholders. (1.52 KB, patch)
2014-03-20 11:28 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Useless to translation strings (5.30 KB, text/plain)
2014-03-20 19:19 UTC, Bernardo Gonzalez Kriegel
Details
Bug 11631 - Followup to extend and compile pattern (1.30 KB, patch)
2014-03-21 13:32 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 11631 - Followup to extend and compile pattern (1.44 KB, patch)
2014-03-22 18:53 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 11631 - Followup to extend pattern (1.41 KB, patch)
2014-04-05 17:46 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Make i18n toolchain ignore useless strings (818 bytes, patch)
2014-04-24 08:15 UTC, paxed
Details | Diff | Splinter Review
[SIGNED_OFF] Bug 11631: Make i18n toolchain ignore useless strings (967 bytes, patch)
2014-04-24 11:34 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[SIGNED_OFF] Bug 11631: Make i18n toolchain ignore useless strings (1.40 KB, patch)
2014-04-26 16:22 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
[PASSED QA] Bug 11631: Make i18n toolchain ignore useless strings (1.49 KB, patch)
2014-04-26 16:31 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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|\(|\)|\[|\]|\{|\}|\\|\.|\,|;|:|\||\-|\?|!|\'|\"|\#|\*|%%|\/|\=|\›)//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.