|
Description
paxed
2014-01-29 11:26:26 UTC
Comment hidden (obsolete)
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 :-)
Created attachment 26478 [details] [review] [SIGNED-OFF] Bug 11631 - Make translation toolchain ignore useless "translatable" strings that contain nothing but spaces and/or placeholders. 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> 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)*$/; ? Created attachment 26492 [details]
Useless to translation strings
Jonathan suggestion works.
Using
next if ( $t =~ /^(›|%%|%s|\s|\d|\.|,|;|:|\?|\!|\*|#|-|\+|x|=|~|<|>|\||\/|\\|\(|\)|\[|\]|\{|\}|"|')*$/ );
also catch more strings, see attached file.
My question: Is there a better o more elegant way to write this?
(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? :) Created attachment 26512 [details] [review] Bug 11631 - Followup to extend and compile pattern This followup extendes and compile the pattern to test for useless strings To test: Same procedure as signed patch
> Who does the patch? :)
Done!
Now this need a sign :)
Created attachment 26522 [details] [review] Bug 11631 - Followup to extend and compile pattern This followup extendes and compile the pattern to test for useless strings To test: Same procedure as signed patch Ammended patch, there is already a place for this check Created attachment 26834 [details] [review] Bug 11631 - Followup to extend pattern This followup extendes and compile the pattern. Moved to proper place To test: Same procedure as signed patch Ammended patch, change description Created attachment 27522 [details] [review] Make i18n toolchain ignore useless strings This is slightly improved version of bgkriegel's patch. -Punctuation is matched with POSIX character class [:punct:] instead of listing everything separately. -HTML entities are also matched - those are not meant to be translated anyway, and are mostly non-breaking spaces or quotes anyway. Test as per bgkriegel's patch above. Created attachment 27527 [details] [review] [SIGNED_OFF] Bug 11631: Make i18n toolchain ignore useless strings Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Works as described, 380 strings less to 'translate' No koha-qa errors. Please always add the commit message to the test plan. ... the test plan to the commit message. Created attachment 27659 [details] [review] [SIGNED_OFF] Bug 11631: Make i18n toolchain ignore useless strings Copy of test plan: 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. (In reply to Katrin Fischer from comment #13) > ... the test plan to the commit message. Not my patch, but my test plan :) 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. Pushed to master. Thanks, Pasi! Pushed to 3.14.x, will be in 3.14.07. |