New features on tmpl_process3.pl are needed to fulfill some tasks a) Cleaning of PO files ----------------------- When doing an update of translation files, old strings are preserved as comments (^#~). As a normal part of update this are removed with a script, and backup files removed. Translators that use to do offline translations remove those comments I propose a patch to remove obsoleted strings and backup files. (see Bug 8257) b) Multiple source dirs ----------------------- Currently only files below one source dir are scanned for strings. At this moment we have 3 opac files (prog/ccsr/bootstrap), but they share a lot of common strings (this date on master): bootstrap 2282 strings prog 2274 strings ccsr 109 strings But there are only 382 different strings between bootstrap/prog, and only 8 between prog/ccsr When we update translation files we need to do the same translation on each file, not a big problem for near complete translations, but a big task for new languages. An hypothetical unified PO file for all opac themes would have 2476 strings, and we need to translate only once each string. I know that prog is programmed to be phased out soon, but each bugfix needs to be ported to each current theme, so the need to translate each file. I propose a patch to add the ability to scan multiple source dirs to build a translation file, to make for example and unified opac PO file. This has not been requested by nobody yet, but is in my mind every time I translate. c) Include/Exclude scanned files by name ---------------------------------------- Currently we can exclude "dir" names, which was used for Bug 7934. But certain requests needs the ability to include/exclude files, for example we have a request to separate translations for each MARC dialect. It's a valid request. Normally a language will only use one MARC dialect, and only one will use NORMARC :) To do that we need a way to select which files to scan, or not scan (and also the previous feature, multiple source dirs) I propose a patch to add this feature, choosing which files to include/exclude given some strings to match with filenames
Created attachment 26943 [details] [review] Bug 12061 - tmpl_process3.pl - Remove obsoleted strings/backups This patch modifies the update process for PO files, removing obsoleted strings. Currently they are removed manually as part of the standar procedure of strings update. Also backup files (*~) need to be removed manually. To test: 1. Update translation files for any language, ej. fi-FI (cd misc/translation; perl translate update fi-FI) 2. Check existence of obsoleted strings egrep "#~ msg" misc/translator/po/fi-FI-* 3. Check existence of backup files ls misc/translator/po/fi-FI-*~ 4. Reset to HEAD and clean 5. Apply the patch 6. Update again, check there are no more obsoleted strings nor backup files
Created attachment 26944 [details] [review] Bug 12061 - tmpl_process3.pl - Multiple source dir This patch adds the option to use multiple source dirs for tmpl_process3.pl on create/update actions. To test after patch: A) Old 1. install cd misc/translator perl tmpl_process3.pl create -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s bootstrap.po -r 2. update perl tmpl_process3.pl update -i ../../koha-tmpl/opac-tmpl/bootstrap -s bootstrap.po -r 3. install mkdir test perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/bootstrap -s bootstrap.po -r -o ./test -q - check same number of files ls -lR ../../koha-tmpl/opac-tmpl/bootstrap | wc -l ls -lR ./test | wc -l rm -f bootstrap.po B) New 4. create from multiple sources perl tmpl_process3.pl create -i ../../koha-tmpl/opac-tmpl/prog/en -i ../../koha-tmpl/opac-tmpl/bootstrap/en -i ../../koha-tmpl/opac-tmpl/ccsr/en -s opac.po -r 5. update from multiple source perl tmpl_process3.pl update -i ../../koha-tmpl/opac-tmpl/prog/en -i ../../koha-tmpl/opac-tmpl/bootstrap/en -i ../../koha-tmpl/opac-tmpl/ccsr/en -s opac.po -r 6. install (must give an error) perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/prog/en -i ../../koha-tmpl/opac-tmpl/bootstrap -s opac.po -r -o ./test 7. updated help perl tmpl_process3.pl --help Any other functionality must not be affected
Created attachment 26945 [details] [review] Bug 12061 - tmpl_process3.pl - Include/exclude file by name This patch adds two options to tmpl_process3.pl for include/exclude some files by matching their names. To test after patch: A) Include only 'normarc' 1. create cd misc/translator perl tmpl_process3.pl create -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s normarc.po -r -m normarc - check provenance of strings egrep "^#:" normarc.po | cut -d":" -f2 | sort | uniq - only files with normarc in their names must be present 2. update perl tmpl_process3.pl update -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s normarc.po -r -m normarc - repeat check 3. install mkdir test perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/bootstrap/en -o ./test -s normarc.po -r -m normarc - check name of created files rm -rf test normarc.po B) Exclude 4. create perl tmpl_process3.pl create -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s xnormarc.po -r -n normarc - check provenance egrep "^#:" xnormarc.po | cut -d":" -f2 | sort | uniq | grep -i normarc - there must be no results 5. update perl tmpl_process3.pl update -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s xnormarc.po -r -n normarc - check provenance 6. install mkdir test perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/bootstrap/en -o ./test -s xnormarc.po -r -n normarc - check files find test | grep -i normarc - there must be no results You can also try another combination, use for example -m/-n "patron|user|bottom" or mixed "-m marc -n normarc", do create/install and look filenames
Applying: Bug 12061 - tmpl_process3.pl - Multiple source dir fatal: sha1 information is lacking or useless (misc/translator/tmpl_process3.pl).
Created attachment 28443 [details] [review] Bug 12061 - tmpl_process3.pl - Remove obsoleted strings/backups This patch modifies the update process for PO files, removing obsoleted strings. Currently they are removed manually as part of the standar procedure of strings update. Also backup files (*~) need to be removed manually. To test: 1. Update translation files for any language, ej. fi-FI (cd misc/translation; perl translate update fi-FI) 2. Check existence of obsoleted strings egrep "#~ msg" misc/translator/po/fi-FI-* 3. Check existence of backup files ls misc/translator/po/fi-FI-*~ 4. Reset to HEAD and clean 5. Apply the patch 6. Update again, check there are no more obsoleted strings nor backup files
Created attachment 28444 [details] [review] Bug 12061 - tmpl_process3.pl - Multiple source dir This patch adds the option to use multiple source dirs for tmpl_process3.pl on create/update actions. To test after patch: A) Old 1. install cd misc/translator perl tmpl_process3.pl create -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s bootstrap.po -r 2. update perl tmpl_process3.pl update -i ../../koha-tmpl/opac-tmpl/bootstrap -s bootstrap.po -r 3. install mkdir test perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/bootstrap -s bootstrap.po -r -o ./test -q - check same number of files ls -lR ../../koha-tmpl/opac-tmpl/bootstrap | wc -l ls -lR ./test | wc -l rm -f bootstrap.po B) New 4. create from multiple sources perl tmpl_process3.pl create -i ../../koha-tmpl/opac-tmpl/prog/en -i ../../koha-tmpl/opac-tmpl/bootstrap/en -i ../../koha-tmpl/opac-tmpl/ccsr/en -s opac.po -r 5. update from multiple source perl tmpl_process3.pl update -i ../../koha-tmpl/opac-tmpl/prog/en -i ../../koha-tmpl/opac-tmpl/bootstrap/en -i ../../koha-tmpl/opac-tmpl/ccsr/en -s opac.po -r 6. install (must give an error) perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/prog/en -i ../../koha-tmpl/opac-tmpl/bootstrap -s opac.po -r -o ./test 7. updated help perl tmpl_process3.pl --help Any other functionality must not be affected
Created attachment 28445 [details] [review] Bug 12061 - tmpl_process3.pl - Include/exclude file by name This patch adds two options to tmpl_process3.pl for include/exclude some files by matching their names. To test after patch: A) Include only 'normarc' 1. create cd misc/translator perl tmpl_process3.pl create -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s normarc.po -r -m normarc - check provenance of strings egrep "^#:" normarc.po | cut -d":" -f2 | sort | uniq - only files with normarc in their names must be present 2. update perl tmpl_process3.pl update -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s normarc.po -r -m normarc - repeat check 3. install mkdir test perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/bootstrap/en -o ./test -s normarc.po -r -m normarc - check name of created files rm -rf test normarc.po B) Exclude 4. create perl tmpl_process3.pl create -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s xnormarc.po -r -n normarc - check provenance egrep "^#:" xnormarc.po | cut -d":" -f2 | sort | uniq | grep -i normarc - there must be no results 5. update perl tmpl_process3.pl update -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s xnormarc.po -r -n normarc - check provenance 6. install mkdir test perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/bootstrap/en -o ./test -s xnormarc.po -r -n normarc - check files find test | grep -i normarc - there must be no results You can also try another combination, use for example -m/-n "patron|user|bottom" or mixed "-m marc -n normarc", do create/install and look filenames
Strange, for me they apply. Rebased anyway.
Comment on attachment 28445 [details] [review] Bug 12061 - tmpl_process3.pl - Include/exclude file by name Review of attachment 28445 [details] [review]: ----------------------------------------------------------------- ::: misc/translator/tmpl_process3.pl @@ +200,5 @@ > -s, --str-file=FILE Specify FILE as the translation (po) file > for input (install) or output (create, update) > + -x, --exclude=REGEXP Exclude dirs matching the given REGEXP > + -m, --match=STRING Pipe (|) separated list. Include only files > + with names matching the given REGEXP It looks to be the same behavior as the -f option.
Created attachment 28450 [details] [review] Bug 12061 - tmpl_process3.pl - Include/exclude file by name This patch adds an option to tmpl_process3.pl for exclude some files by matching their names. Also modifies current code to include only selected files to check only filenames. Checking is case insensitive !! To test after patch: A) Include only 'normarc' 1. create cd misc/translator perl tmpl_process3.pl create -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s normarc.po -r -f normarc - check provenance of strings egrep "^#:" normarc.po | cut -d":" -f2 | sort | uniq - only files with normarc in their names must be present 2. update perl tmpl_process3.pl update -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s normarc.po -r -f normarc - repeat check 3. install mkdir test perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/bootstrap/en -o ./test -s normarc.po -r -f normarc - check name of created files rm -rf test normarc.po B) Exclude 4. create perl tmpl_process3.pl create -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s xnormarc.po -r -n normarc - check provenance egrep "^#:" xnormarc.po | cut -d":" -f2 | sort | uniq | grep -i normarc - there must be no results 5. update perl tmpl_process3.pl update -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s xnormarc.po -r -n normarc - check provenance 6. install mkdir test perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/bootstrap/en -o ./test -s xnormarc.po -r -n normarc - check files find test | grep -i normarc - there must be no results You can also try another combination, use for example "-f patron -f user -f bottom" (or use -n) or mixed "-f marc -n normarc", do create/install and look filenames Changed to adapt current functionality (-f) http://bugs.koha-community.org/show_bug.cgi?id=12292
(In reply to Jonathan Druart from comment #9) > > It looks to be the same behavior as the -f option. Yes... almost the same. Uploaded a revised version. I chsnged '-f' to only look on filenames, currently it can match a dir name (try -f help), also did the check case insensitive Removed -m, same -f function! -n stay, same input as -f changed usage description
Created attachment 28451 [details] [review] Bug 12061 - tmpl_process3.pl - Include/exclude file by name This patch adds an option to tmpl_process3.pl for exclude some files by matching their names. Also modifies current code to include only selected files to check only filenames. Checking is case insensitive !! To test after patch: A) Include only 'normarc' 1. create cd misc/translator perl tmpl_process3.pl create -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s normarc.po -r -f normarc - check provenance of strings egrep "^#:" normarc.po | cut -d":" -f2 | sort | uniq - only files with normarc in their names must be present 2. update perl tmpl_process3.pl update -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s normarc.po -r -f normarc - repeat check 3. install mkdir test perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/bootstrap/en -o ./test -s normarc.po -r -f normarc - check name of created files rm -rf test normarc.po B) Exclude 4. create perl tmpl_process3.pl create -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s xnormarc.po -r -n normarc - check provenance egrep "^#:" xnormarc.po | cut -d":" -f2 | sort | uniq | grep -i normarc - there must be no results 5. update perl tmpl_process3.pl update -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s xnormarc.po -r -n normarc - check provenance 6. install mkdir test perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/bootstrap/en -o ./test -s xnormarc.po -r -n normarc - check files find test | grep -i normarc - there must be no results You can also try another combination, use for example "-f patron -f user -f bottom" (or use -n) or mixed "-f marc -n normarc", do create/install and look filenames Changed to adapt current functionality (-f) http://bugs.koha-community.org/show_bug.cgi?id=12292
Nice job. I'm testing. I think there is an error in test plan of "tmpl_process3.pl - Multiple source dir ": 3. install mkdir test perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/bootstrap -s bootstrap.po -r -o ./test -q - check same number of files ls -lR ../../koha-tmpl/opac-tmpl/bootstrap | wc -l ls -lR ./test | wc -l It should translate only "en" folder no ? otherwise there is not the same number of files : 3. install mkdir test perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s bootstrap.po -r -o ./test -q - check same number of files ls -lR ../../koha-tmpl/opac-tmpl/bootstrap/en | wc -l ls -lR ./test | wc -l Best regards.
(In reply to Fridolin SOMERS from comment #13) > Nice job. > > I'm testing. > I think there is an error in test plan of "tmpl_process3.pl - Multiple > source dir ": > ... > > It should translate only "en" folder no ? otherwise there is not the same > number of files : > Hi Fridolin, thanks for testing :) No, tmpl_process3.pl do not look for 'en' dir. On create it looks (recursively -r) for valid files to pick strings from, on install it reproduces (translated or not, but all) files and dirs from input to target dir. That test is only to check for regressions (all files present), because I touch the handling of source dir. Just tested (with patches): ~/kohaclone/misc/translator$ ./tmpl_process3.pl create -i ../../koha-tmpl/opac-tmpl/bootstrap -s test.po -r -q Warning: Can't determine original templates' charset, defaulting to UTF-8 Warning: Charset Out defaulting to UTF-8 Removing empty file test.po I UTF-8 O UTF-8 at ./tmpl_process3.pl line 339. ~/kohaclone/misc/translator$ mkdir test ~/kohaclone/misc/translator$ ./tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/bootstrap -s test.po -r -q -o ./test ~/kohaclone/misc/translator$ ls -lR ../../koha-tmpl/opac-tmpl/bootstrap/ | wc -l ; ls -lR ./test | wc -l 1951 1951
(In reply to Bernardo Gonzalez Kriegel from comment #14) > (In reply to Fridolin SOMERS from comment #13) > > Nice job. > > > > I'm testing. > > I think there is an error in test plan of "tmpl_process3.pl - Multiple > > source dir ": > > > ... > > > > It should translate only "en" folder no ? otherwise there is not the same > > number of files : > > > > Hi Fridolin, thanks for testing :) > > No, tmpl_process3.pl do not look for 'en' dir. On create it looks > (recursively -r) for valid files to pick strings from, on install it > reproduces (translated or not, but all) files and dirs from input to target > dir. > That test is only to check for regressions (all files present), because I > touch the handling of source dir. > > Just tested (with patches): > ~/kohaclone/misc/translator$ ./tmpl_process3.pl create -i > ../../koha-tmpl/opac-tmpl/bootstrap -s test.po -r -q > Warning: Can't determine original templates' charset, defaulting to UTF-8 > Warning: Charset Out defaulting to UTF-8 > Removing empty file test.po > I UTF-8 O UTF-8 at ./tmpl_process3.pl line 339. > > ~/kohaclone/misc/translator$ mkdir test > > ~/kohaclone/misc/translator$ ./tmpl_process3.pl install -i > ../../koha-tmpl/opac-tmpl/bootstrap -s test.po -r -q -o ./test > > ~/kohaclone/misc/translator$ ls -lR ../../koha-tmpl/opac-tmpl/bootstrap/ | > wc -l ; ls -lR ./test | wc -l > 1951 > 1951 My fault, I think I add not remove some previous tests. Now it works.
Created attachment 28643 [details] [review] Bug 12061 - tmpl_process3.pl - Remove obsoleted strings/backups This patch modifies the update process for PO files, removing obsoleted strings. Currently they are removed manually as part of the standar procedure of strings update. Also backup files (*~) need to be removed manually. To test: 1. Update translation files for any language, ej. fi-FI (cd misc/translation; perl translate update fi-FI) 2. Check existence of obsoleted strings egrep "#~ msg" misc/translator/po/fi-FI-* 3. Check existence of backup files ls misc/translator/po/fi-FI-*~ 4. Reset to HEAD and clean 5. Apply the patch 6. Update again, check there are no more obsoleted strings nor backup files Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Created attachment 28644 [details] [review] Bug 12061 - tmpl_process3.pl - Multiple source dir This patch adds the option to use multiple source dirs for tmpl_process3.pl on create/update actions. To test after patch: A) Old 1. install cd misc/translator perl tmpl_process3.pl create -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s bootstrap.po -r 2. update perl tmpl_process3.pl update -i ../../koha-tmpl/opac-tmpl/bootstrap -s bootstrap.po -r 3. install mkdir test perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/bootstrap -s bootstrap.po -r -o ./test -q - check same number of files ls -lR ../../koha-tmpl/opac-tmpl/bootstrap | wc -l ls -lR ./test | wc -l rm -f bootstrap.po B) New 4. create from multiple sources perl tmpl_process3.pl create -i ../../koha-tmpl/opac-tmpl/prog/en -i ../../koha-tmpl/opac-tmpl/bootstrap/en -i ../../koha-tmpl/opac-tmpl/ccsr/en -s opac.po -r 5. update from multiple source perl tmpl_process3.pl update -i ../../koha-tmpl/opac-tmpl/prog/en -i ../../koha-tmpl/opac-tmpl/bootstrap/en -i ../../koha-tmpl/opac-tmpl/ccsr/en -s opac.po -r 6. install (must give an error) perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/prog/en -i ../../koha-tmpl/opac-tmpl/bootstrap -s opac.po -r -o ./test 7. updated help perl tmpl_process3.pl --help Any other functionality must not be affected Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Created attachment 28645 [details] [review] Bug 12061 - tmpl_process3.pl - Include/exclude file by name This patch adds an option to tmpl_process3.pl for exclude some files by matching their names. Also modifies current code to include only selected files to check only filenames. Checking is case insensitive !! To test after patch: A) Include only 'normarc' 1. create cd misc/translator perl tmpl_process3.pl create -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s normarc.po -r -f normarc - check provenance of strings egrep "^#:" normarc.po | cut -d":" -f2 | sort | uniq - only files with normarc in their names must be present 2. update perl tmpl_process3.pl update -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s normarc.po -r -f normarc - repeat check 3. install mkdir test perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/bootstrap/en -o ./test -s normarc.po -r -f normarc - check name of created files rm -rf test normarc.po B) Exclude 4. create perl tmpl_process3.pl create -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s xnormarc.po -r -n normarc - check provenance egrep "^#:" xnormarc.po | cut -d":" -f2 | sort | uniq | grep -i normarc - there must be no results 5. update perl tmpl_process3.pl update -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s xnormarc.po -r -n normarc - check provenance 6. install mkdir test perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/bootstrap/en -o ./test -s xnormarc.po -r -n normarc - check files find test | grep -i normarc - there must be no results You can also try another combination, use for example "-f patron -f user -f bottom" (or use -n) or mixed "-f marc -n normarc", do create/install and look filenames Changed to adapt current functionality (-f) http://bugs.koha-community.org/show_bug.cgi?id=12292 Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
All test plans work. Perfect job.
Comment on attachment 28645 [details] [review] Bug 12061 - tmpl_process3.pl - Include/exclude file by name Review of attachment 28645 [details] [review]: ----------------------------------------------------------------- ::: misc/translator/tmpl_process3.pl @@ +134,5 @@ > > sub listfiles { > + my($dir, $type, $action) = @_; > + my $filenames = join ('|', @filenames); > + my $nofilenames = join ('|', @nofilenames); Don't you think it is better to pass filenames and nofilenames in parameter rather than using global vars?
Comment on attachment 28645 [details] [review] Bug 12061 - tmpl_process3.pl - Include/exclude file by name Review of attachment 28645 [details] [review]: ----------------------------------------------------------------- ::: misc/translator/tmpl_process3.pl @@ +146,4 @@ > || (defined $exclude_regex && $dirent =~ /^(?:$exclude_regex)$/)) { > ; > } elsif (-f $path) { > + my $basename = fileparse( $path ); hum, not sure to understand the change here.
Comment on attachment 28643 [details] [review] Bug 12061 - tmpl_process3.pl - Remove obsoleted strings/backups Review of attachment 28643 [details] [review]: ----------------------------------------------------------------- ::: misc/translator/tmpl_process3.pl @@ +377,4 @@ > if ( @filenames ) { > my ($tmph3, $tmpfile3) = tmpnam(); > $st = system("msgcat $str_file $tmpfile2 > $tmpfile3"); > + $st = system("msgmerge ".($quiet?'-q':'')." -s $str_file $tmpfile3 -o - | msgattrib --no-obsolete -o $str_file") Could you detail why this change is useful please? Maybe a small comment would be helpful.
(In reply to Jonathan Druart from comment #20) > Comment on attachment 28645 [details] [review] [review] > Bug 12061 - tmpl_process3.pl - Include/exclude file by name > > Review of attachment 28645 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: misc/translator/tmpl_process3.pl > @@ +134,5 @@ > > > > sub listfiles { > > + my($dir, $type, $action) = @_; > > + my $filenames = join ('|', @filenames); > > + my $nofilenames = join ('|', @nofilenames); > > Don't you think it is better to pass filenames and nofilenames in parameter > rather than using global vars? May be. What we can pass is a reference, so we are using them (glob vars) anyway. My thinking was that we pass \@filenames, later use @$filenames, why not directly use @filenames? If you think it's more clear to pass them explicitly, no problem from me, i'll rewrite that.
(In reply to Jonathan Druart from comment #21) > Comment on attachment 28645 [details] [review] [review] > Bug 12061 - tmpl_process3.pl - Include/exclude file by name > > Review of attachment 28645 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: misc/translator/tmpl_process3.pl > @@ +146,4 @@ > > || (defined $exclude_regex && $dirent =~ /^(?:$exclude_regex)$/)) { > > ; > > } elsif (-f $path) { > > + my $basename = fileparse( $path ); > > hum, not sure to understand the change here. Currently $basename is declared and initialized, but not used. My idea was to use this variable to match filenames, instead of using $path. Current code could match a dir name if used with a generic term, e.g. "-f help" will match all help 'dirs' on staff, and is supposed to match filenames. But, if I understand correctly [1], it's better to use fileparse() for that instead of basename(). [1] http://perldoc.perl.org/File/Basename.html
(In reply to Jonathan Druart from comment #22) > Comment on attachment 28643 [details] [review] [review] > Bug 12061 - tmpl_process3.pl - Remove obsoleted strings/backups > > Review of attachment 28643 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: misc/translator/tmpl_process3.pl > @@ +377,4 @@ > > if ( @filenames ) { > > my ($tmph3, $tmpfile3) = tmpnam(); > > $st = system("msgcat $str_file $tmpfile2 > $tmpfile3"); > > + $st = system("msgmerge ".($quiet?'-q':'')." -s $str_file $tmpfile3 -o - | msgattrib --no-obsolete -o $str_file") > > Could you detail why this change is useful please? Maybe a small comment > would be helpful. There is a comment in bug description, point a), But no problem :) With every 'translate update ...' a msgid that is no more present is marked as obsolete '#~' and moved to the bottom. They have value if on time the same string is back again, then old translation could be used. As translator I normally remove by hand those entries, and as TM I do the same for all files as part for the 'normal' procedure I inherited ([1], 'From Koha to Pootle'). On Bug [1] http://wiki.koha-community.org/wiki/Translation_Server
> On Bug ... Sorry, was sent before finished. There is also a comment about this on Bug 8257 (one of the open i18n bugs) The idea then is save a little work and trim the files a bit removing obsolete strings. Thanks for your comments :)
(In reply to Bernardo Gonzalez Kriegel from comment #25) > > Could you detail why this change is useful please? Maybe a small comment > > would be helpful. > > There is a comment in bug description, point a), But no problem :) I was thinking a comment in the code. But it could be retrieve from this bug report :) (In reply to Bernardo Gonzalez Kriegel from comment #26) > Thanks for your comments :) Thanks for your answers, all look good and nothing is blocker in my opinion. Marked as Passed QA.
Created attachment 28735 [details] [review] Bug 12061 - tmpl_process3.pl - Remove obsoleted strings/backups This patch modifies the update process for PO files, removing obsoleted strings. Currently they are removed manually as part of the standar procedure of strings update. Also backup files (*~) need to be removed manually. To test: 1. Update translation files for any language, ej. fi-FI (cd misc/translation; perl translate update fi-FI) 2. Check existence of obsoleted strings egrep "#~ msg" misc/translator/po/fi-FI-* 3. Check existence of backup files ls misc/translator/po/fi-FI-*~ 4. Reset to HEAD and clean 5. Apply the patch 6. Update again, check there are no more obsoleted strings nor backup files Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 28736 [details] [review] Bug 12061 - tmpl_process3.pl - Multiple source dir This patch adds the option to use multiple source dirs for tmpl_process3.pl on create/update actions. To test after patch: A) Old 1. install cd misc/translator perl tmpl_process3.pl create -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s bootstrap.po -r 2. update perl tmpl_process3.pl update -i ../../koha-tmpl/opac-tmpl/bootstrap -s bootstrap.po -r 3. install mkdir test perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/bootstrap -s bootstrap.po -r -o ./test -q - check same number of files ls -lR ../../koha-tmpl/opac-tmpl/bootstrap | wc -l ls -lR ./test | wc -l rm -f bootstrap.po B) New 4. create from multiple sources perl tmpl_process3.pl create -i ../../koha-tmpl/opac-tmpl/prog/en -i ../../koha-tmpl/opac-tmpl/bootstrap/en -i ../../koha-tmpl/opac-tmpl/ccsr/en -s opac.po -r 5. update from multiple source perl tmpl_process3.pl update -i ../../koha-tmpl/opac-tmpl/prog/en -i ../../koha-tmpl/opac-tmpl/bootstrap/en -i ../../koha-tmpl/opac-tmpl/ccsr/en -s opac.po -r 6. install (must give an error) perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/prog/en -i ../../koha-tmpl/opac-tmpl/bootstrap -s opac.po -r -o ./test 7. updated help perl tmpl_process3.pl --help Any other functionality must not be affected Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 28737 [details] [review] Bug 12061 - tmpl_process3.pl - Include/exclude file by name This patch adds an option to tmpl_process3.pl for exclude some files by matching their names. Also modifies current code to include only selected files to check only filenames. Checking is case insensitive !! To test after patch: A) Include only 'normarc' 1. create cd misc/translator perl tmpl_process3.pl create -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s normarc.po -r -f normarc - check provenance of strings egrep "^#:" normarc.po | cut -d":" -f2 | sort | uniq - only files with normarc in their names must be present 2. update perl tmpl_process3.pl update -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s normarc.po -r -f normarc - repeat check 3. install mkdir test perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/bootstrap/en -o ./test -s normarc.po -r -f normarc - check name of created files rm -rf test normarc.po B) Exclude 4. create perl tmpl_process3.pl create -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s xnormarc.po -r -n normarc - check provenance egrep "^#:" xnormarc.po | cut -d":" -f2 | sort | uniq | grep -i normarc - there must be no results 5. update perl tmpl_process3.pl update -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s xnormarc.po -r -n normarc - check provenance 6. install mkdir test perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/bootstrap/en -o ./test -s xnormarc.po -r -n normarc - check files find test | grep -i normarc - there must be no results You can also try another combination, use for example "-f patron -f user -f bottom" (or use -n) or mixed "-f marc -n normarc", do create/install and look filenames Changed to adapt current functionality (-f) http://bugs.koha-community.org/show_bug.cgi?id=12292 Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
(In reply to Bernardo Gonzalez Kriegel from comment #24) > But, if I understand correctly [1], it's better to use fileparse() for that > instead of basename(). I always used basename! Thank for pointing that out to me.
Found a problem, do not push
Created attachment 30148 [details] [review] Bug 12061 - tmpl_process3.pl - Include/exclude file by name This patch adds options to include/exclude files by matching their names. Also modifies current code to check only filenames (not dirs) Checking is case insensitive !! NOTE: The difference between -f and -m is subtle, but important. They differs mainly on update,: -f do a merge, -m a replace To test after patch: A) Include only 'normarc' 1. create cd misc/translator perl tmpl_process3.pl create -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s normarc.po -r -m normarc - check provenance of strings egrep "^#:" normarc.po | cut -d":" -f2 | sort | uniq - only files with normarc in their names must be present 2. update perl tmpl_process3.pl update -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s normarc.po -r -m normarc - repeat check 3. install mkdir test perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/bootstrap/en -o ./test -s normarc.po -r -m normarc - check name of created files rm -rf test normarc.po B) Exclude 4. create perl tmpl_process3.pl create -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s xnormarc.po -r -n normarc - check provenance egrep "^#:" xnormarc.po | cut -d":" -f2 | sort | uniq | grep -i normarc - there must be no results 5. update perl tmpl_process3.pl update -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s xnormarc.po -r -n normarc - check provenance 6. install mkdir test perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/bootstrap/en -o ./test -s xnormarc.po -r -n normarc - check files find test | grep -i normarc - there must be no results You can also try another combination, use for example "-m patron -m user -m bottom" (or use -n) or mixed "-m marc -n normarc", do create/install and look filenames http://bugs.koha-community.org/show_bug.cgi?id=12292 Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(In reply to Jonathan Druart from comment #9) ... > It looks to be the same behavior as the -f option. After all, it is not the same behavior! This code on 'update' is the main difference if ( @filenames ) { my ($tmph3, $tmpfile3) = tmpnam(); $st = system("msgcat $str_file $tmpfile2 > $tmpfile3"); $st = system("msgmerge ".($quiet?'-q':'')." -s $str_file $tmpfile3 -o - | msgattrib --no-obsolete -o $str_file") unless $st; } else { $st = system("msgmerge ".($quiet?'-q':'')." -s $str_file $tmpfile2 -o - | msgattrib --no-obsolete -o $str_file"); } '-f' is the right choice to use when a file is modified and one updates translation file to reflect that. But that is not the behavior that I want, PO file must reflect exactly the contents of source dir on create & update. I retained Fridolin sign, he signed a previous version with both (-n and -m) new options, the code is slightly different but closer to the version that you QAed. Left as signed for your consideration, but only changed the last patch.
Created attachment 30966 [details] [review] Bug 12061 - tmpl_process3.pl - Include/exclude file by name This patch adds options to include/exclude files by matching their names. Also modifies current code to check only filenames (not dirs) Checking is case insensitive !! NOTE: The difference between -f and -m is subtle, but important. They differs mainly on update,: -f do a merge, -m a replace To test after patch: A) Include only 'normarc' 1. create cd misc/translator perl tmpl_process3.pl create -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s normarc.po -r -m normarc - check provenance of strings egrep "^#:" normarc.po | cut -d":" -f2 | sort | uniq - only files with normarc in their names must be present 2. update perl tmpl_process3.pl update -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s normarc.po -r -m normarc - repeat check 3. install mkdir test perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/bootstrap/en -o ./test -s normarc.po -r -m normarc - check name of created files rm -rf test normarc.po B) Exclude 4. create perl tmpl_process3.pl create -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s xnormarc.po -r -n normarc - check provenance egrep "^#:" xnormarc.po | cut -d":" -f2 | sort | uniq | grep -i normarc - there must be no results 5. update perl tmpl_process3.pl update -i ../../koha-tmpl/opac-tmpl/bootstrap/en -s xnormarc.po -r -n normarc - check provenance 6. install mkdir test perl tmpl_process3.pl install -i ../../koha-tmpl/opac-tmpl/bootstrap/en -o ./test -s xnormarc.po -r -n normarc - check files find test | grep -i normarc - there must be no results You can also try another combination, use for example "-m patron -m user -m bottom" (or use -n) or mixed "-m marc -n normarc", do create/install and look filenames Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Patches pushed to master. Thanks Bernardo!