Description
Bernardo Gonzalez Kriegel
2020-05-14 11:32:14 UTC
Created attachment 104876 [details] [review] Bug 25501: use utf8 for open in tmpl_process3.pl Opening files using "":encoding(UTF-8)" removes the warning "wide character in print" installing translations To test: 1) Verify the error installing translations for any language 2) Apply the patch 3) Repeat 1), check warning is gone I am used to explicitly specify the encoding when opening the file, but I am fine with this approach as well. Created attachment 105043 [details] [review] Bug 25501: use utf8 for open in tmpl_process3.pl Opening files using "":encoding(UTF-8)" removes the warning "wide character in print" installing translations To test: 1) Verify the error installing translations for any language 2) Apply the patch 3) Repeat 1), check warning is gone Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Bernardo, there is an encoding issue in the translation process now. Translated interface in French: Périodiques Created attachment 105060 [details] [review] Bug 25501: Encode strings in utf8 for output - translation process This is a follow-up of bug 25305, all strings need to be utf8 encoded when printed to the file. Test plan: Test the whole translation process and translate a language you know. Test the translated interface for: XSLT (detail and result pages), syspref, and "normal" views You should not get encoding errors (or warning in logs), nowhere. Created attachment 105067 [details] [review] Bug 25501: Pass the encoding to Locale::PO Still not ready: misc/translator/po/fr-FR-installer-MARC21.po: GNU gettext message catalogue, ASCII text misc/translator/po/fr-FR-installer.po: GNU gettext message catalogue, ASCII text misc/translator/po/fr-FR-marc-MARC21.po: GNU gettext message catalogue, UTF-8 Unicode text misc/translator/po/fr-FR-marc-NORMARC.po: GNU gettext message catalogue, UTF-8 Unicode text misc/translator/po/fr-FR-marc-UNIMARC.po: GNU gettext message catalogue, UTF-8 Unicode text misc/translator/po/fr-FR-messages-js.po: GNU gettext message catalogue, UTF-8 Unicode text misc/translator/po/fr-FR-messages.po: GNU gettext message catalogue, UTF-8 Unicode text misc/translator/po/fr-FR-opac-bootstrap.po: GNU gettext message catalogue, UTF-8 Unicode text misc/translator/po/fr-FR-pref.po: GNU gettext message catalogue, UTF-8 Unicode text, with very long lines misc/translator/po/fr-FR-staff-prog.po: GNU gettext message catalogue, UTF-8 Unicode text, with very long lines Actually better than before, for message-js: misc/translator/po/fr-FR-messages-js.po: GNU gettext message catalogue, ASCII text the installer PO files are not translated yet, and so does not contain utf8 chars. Ready for testing. Created attachment 105073 [details] [review] Bug 25501: Encode strings in utf8 for output - translation process This is a follow-up of bug 25305, all strings need to be utf8 encoded when printed to the file. Test plan: Test the whole translation process and translate a language you know. Test the translated interface for: XSLT (detail and result pages), syspref, and "normal" views You should not get encoding errors (or warning in logs), nowhere. Created attachment 105074 [details] [review] Bug 25501: Pass the encoding to Locale::PO With my patch there were 624 files with double encoding after install :( With yours none One qa error, you need to remove line 380 in misc/translator/tmpl_process3.pl - local(*INPUT, *OUTPUT); * But now something new: * (cd misc/translator/; perl translate update es-ES; perl translate install es-ES) Wide character in print at /home/bgkriegel/koha/kohaclone/misc/translator/po2json line 171. this never ends... Created attachment 105080 [details] [review] Bug 25501: Encode strings in utf8 for output - translation process This is a follow-up of bug 25305, all strings need to be utf8 encoded when printed to the file. Test plan: Test the whole translation process and translate a language you know. Test the translated interface for: XSLT (detail and result pages), syspref, and "normal" views You should not get encoding errors (or warning in logs), nowhere. Created attachment 105081 [details] [review] Bug 25501: Pass the encoding to Locale::PO Created attachment 105084 [details] [review] Bug 25501: Remove local vars Created attachment 105085 [details] [review] Bug 25501: Remove 'Wide character in print' for po2json Here is a simpler counter patch : https://gitlab.com/jajm/Koha/-/commit/a8764d0b0add70acbf5d5b5ad6046c1571e2ec13 It suppresses all warnings for me and I haven't found any encoding issues in the interface yet Jonathan, Without patch, no double encoding but warnings With patches, no warnings but 635 files with double encoding With Julian's approach, no warnings nor double encoding Checked encoding issues using http://paste.koha-community.org/20552 Created attachment 105095 [details] [review] Bug 25501: Supress warnings on installing translation To test: 1) Verify the error installing translations for any language 2) Apply the patch 3) Repeat 1), check warning is gone Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Attributed to Julian No warnings, no double encoding, no errors. Created attachment 105099 [details] [review] Bug 25501: Supress warnings on installing translation To test: 1) Verify the error installing translations for any language 2) Apply the patch 3) Repeat 1), check warning is gone Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Attributed to Julian No warnings, no double encoding, no errors. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Tested full install with yml and js strings, everything works great! Lowering severity as it did not actually break things. Nice work everyone! Pushed to master for 20.05 (In reply to Jonathan Druart from comment #20) > Lowering severity as it did not actually break things. I'm not sure if you mean that encoding issues are minor issues, or that there wasn't any encoding issue in the first place, but the patch did fix a double encoding issue for me. (In reply to Julian Maurice from comment #22) > (In reply to Jonathan Druart from comment #20) > > Lowering severity as it did not actually break things. > > I'm not sure if you mean that encoding issues are minor issues, or that > there wasn't any encoding issue in the first place, but the patch did fix a > double encoding issue for me. On master? I got encoding issues with Bernardo's attempt, but did not notice something wrong on master, did you? I had noticed problems with XSLT not displaying and the wide character error for de-DE on master a few days ago. Can retest now with the patch. (In reply to Jonathan Druart from comment #23) > On master? > I got encoding issues with Bernardo's attempt, but did not notice something > wrong on master, did you? Yes. To reproduce: 1. git reset --hard 8994ded2c47d22fd46ea46f6e7855da52826b351 # the commit before 25501 2. cd misc/translator && ./translate install fr-FR 3. Open koha-tmpl/intranet-tmpl/prog/fr-FR/xslt/MARC21Languages.xsl, you should see the string "Hébreu" 4. git reset --hard c97921bb68849a2c7dd5f2f5ad8f5b97789df5c1 5. ./translate install fr-FR 6. Reopen koha-tmpl/intranet-tmpl/prog/fr-FR/xslt/MARC21Languages.xsl and see that the string is now "Hébreu" (In reply to Katrin Fischer from comment #24) > I had noticed problems with XSLT not displaying and the wide character error > for de-DE on master a few days ago. Can retest now with the patch. I just retested and things see to be ok now - thx for all the work here! not backported to 19.11.x due to missing dependencies |