Created attachment 24859 [details] [review] Make i18n toolchain ignore useless strings There are several translatable strings which contain nothing but spaces and placeholders - these are useless for translators. Attached patch makes the translation toolchain ignore such strings.
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.