[Bug 9859] New: C4::Charset -> nsb_clean() remove Â
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 Bug ID: 9859 Summary: C4::Charset -> nsb_clean() remove  Classification: Unclassified Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: minor Priority: P5 - low Component: Cataloging Assignee: gmcharlt@gmail.com Reporter: stephane.delaune@biblibre.com CC: m.de.rooy@rijksmuseum.nl the nsb_clean sub (from C4::Charset) that aims to remove Non Sorting Block from strings have an undesirable side effect : it remove this char :  -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 stephane.delaune@biblibre.com changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |stephane.delaune@biblibre.c | |om -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 --- Comment #1 from stephane.delaune@biblibre.com --- Created attachment 16465 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16465&action=edit proposed patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 --- Comment #2 from stephane.delaune@biblibre.com --- Created attachment 16469 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16469&action=edit to test the patch run this file : ( perl ./test9859.pl ) -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 stephane.delaune@biblibre.com changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 stephane.delaune@biblibre.com changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|stephane.delaune@biblibre.c |gmcharlt@gmail.com |om | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |stephane.delaune@biblibre.c | |om -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 mathieu saby <mathieu.saby@univ-rennes2.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mathieu.saby@univ-rennes2.f | |r --- Comment #3 from mathieu saby <mathieu.saby@univ-rennes2.fr> --- I will test it. I note that nsb_clean is only called in 3 files in Koha : authorities/ysearch.pl cataloguing/ysearch.pl C4/Biblio.pm::RemoveAllNsb authorities/ysearch.pl is called in 1 file : koha-tmpl/intranet-tmpl/prog/en/includes/auth-finder-search.inc: cataloguing/ysearch.pl is called in 1 file : koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/unimarc_field_210c_bis.tt RemoveAllNsb is called in 1 file : tools/export.pl If it is a usefull function, it would probably be usefull to use it more often. M. Saby -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 --- Comment #4 from mathieu saby <mathieu.saby@univ-rennes2.fr> --- Note : In tools/export.pl, RemoveAllNsb is called only if export.pl is used as in command line, with --clean option. It is not called if export.pl is called from export.tt or an other page. M. Saby -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 --- Comment #5 from mathieu saby <mathieu.saby@univ-rennes2.fr> --- My own test to proove the problem with  1 creating a record with 200$aMoyen Âge and 200$f = Pierre Reverdy 2 exporting with tools/export.pl --format=marc --record-type=bibs --filename=koha.mrc 3 exporting with tools/export.pl --format=marc --record-type=bibs --clean --filename=koha2.mrc 4 searching the record in koha1.mrc with yaz-marcdump koha1.mrc | grep Reverdy 200 1 $a Moyen ge $b Texte imprimé $e notes 1930-1936 $f Pierre Reverdy 700 1 $3 027096327 $a Reverdy $b Pierre $f 1889-1960 $4 070 $9 21567 5 searching the record in koha2.mrc with yaz-marcdump koha1.mrc | grep Reverdy 200 1 $a Moyen Âge $b Texte imprimé $e notes 1930-1936 $f Pierre Reverdy 700 1 $3 027096327 $a Reverdy $b Pierre $f 1889-1960 $4 070 $9 21567 In 1st file, the  is removed, because --clean option force the script to call RemoveAllNsb, which call nsb_clean. M. Saby -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 --- Comment #6 from mathieu saby <mathieu.saby@univ-rennes2.fr> --- The 2d little issue is caused by authorities/ysearch.pl : koha-tmpl/intranet-tmpl/prog/en/includes/auth-finder-search.inc calls ysearch.pl for building the dynamic suggestions (in ajax). 1. Create an authority Moyen Âge 2. In a record, use the auth-finder-search plugin (6XX) fields 3. Enter "Moyen Âg" and see the  is missing in the suggested authority : "Moyen ge" See screenshot -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 --- Comment #7 from mathieu saby <mathieu.saby@univ-rennes2.fr> --- Created attachment 17399 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=17399&action=edit screenshot : Missing  in search-authority plugin -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 Kyle M Hall <kyle.m.hall@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |kyle.m.hall@gmail.com --- Comment #8 from Kyle M Hall <kyle.m.hall@gmail.com> --- Tested, works as advertised. When I try to use git bz to attach I get an error: Traceback (most recent call last): File "/usr/local/bin/git-bz", line 2218, in <module> do_attach(*args) File "/usr/local/bin/git-bz", line 1670, in do_attach attach_commits(bug, commits, edit_comments=global_options.edit) File "/usr/local/bin/git-bz", line 1608, in attach_commits description, body, obsoletes, statuses = edit_attachment_comment(bug, commit.subject, body) File "/usr/local/bin/git-bz", line 1571, in edit_attachment_comment lines = edit_template(template.getvalue()) File "/usr/local/bin/git-bz", line 760, in edit_template f.write(template) UnicodeEncodeError: 'ascii' codec can't encode character u'\xc2' in position 61: ordinal not in range(128) So consider this signed off, unless there is some underlying issue that needs to be resolved. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 mathieu saby <mathieu.saby@univ-rennes2.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff --- Comment #9 from mathieu saby <mathieu.saby@univ-rennes2.fr> --- I sign it off too. After applying the patch, - NSB/NSE are still removed by nsb_clean - tools/exports.pl --clean is no more suppressing  - authority search plugins is no more suppressing  But I am not sure the removal of C2 char before NSB or NSE is usefull, as it is not a control character in UNIMARC standard... M. Saby -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 mathieu saby <mathieu.saby@univ-rennes2.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #16465|0 |1 is obsolete| | --- Comment #10 from mathieu saby <mathieu.saby@univ-rennes2.fr> --- Created attachment 17407 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=17407&action=edit [PATCH][SIGNED OFF] Bug 9859:fix nsb_clean side effect -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 mathieu saby <mathieu.saby@univ-rennes2.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 mathieu saby <mathieu.saby@univ-rennes2.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #11 from mathieu saby <mathieu.saby@univ-rennes2.fr> --- Sorry, I have just sign off, but I see the code is wrong : + s/($C2){0,1}($NSE|$NSE2)//g ; should be + s/($C2){0,1}($NSB2|$NSE2)//g ; B is for begin ; E for end... Stéphane, could you re-write it, and addin the commit the description of what your patch is doing, and what problem it solved ? (You can use the comments I left in the commit ;-) ) Mathieu -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 mathieu saby <mathieu.saby@univ-rennes2.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |matthias.meusburger@biblibr | |e.com --- Comment #12 from mathieu saby <mathieu.saby@univ-rennes2.fr> --- Regarding C2 char : There is a comment in Charset.pm : + my $C2 = '\xC2' ; # What is this char ? It is sometimes left by the regexp after removing NSB / NSE I think it was added by Matthias Meusburger, when he created the function nsb_clean in Bug 7400 (commit 1e7437bbae653840136fd1c2faed80a83aa08d2b). Maybe Matthias could tell us in which precise cases he found this C2 char? I place him in copy of this bug. M. Saby -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 stephane.delaune@biblibre.com changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #13 from stephane.delaune@biblibre.com --- (In reply to mathieu saby from comment #11)
Sorry, I have just sign off, but I see the code is wrong :
+ s/($C2){0,1}($NSE|$NSE2)//g ; should be + s/($C2){0,1}($NSB2|$NSE2)//g ;
B is for begin ; E for end...
Stéphane, could you re-write it, and addin the commit the description of what your patch is doing, and what problem it solved ? (You can use the comments I left in the commit ;-) )
Mathieu
there is two lines in patch : + s/($C2){0,1}($NSB|$NSB2)//g ; to proceed B for Begin and + s/($C2){0,1}($NSE|$NSE2)//g ; to proceed E for End so it don't need to be rewrite for this -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 --- Comment #14 from mathieu saby <mathsabypro@gmail.com> --- Hello That's pretty old, I don't remember this bug at all, and I won't test it again ;-) Do you mean in your previous comment that for you the patch is OK? If so, let's wait for QA. Mathiue -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 --- Comment #15 from stephane.delaune@biblibre.com --- (In reply to mathieu saby from comment #14)
Hello That's pretty old, I don't remember this bug at all, and I won't test it again ;-) Do you mean in your previous comment that for you the patch is OK?
If so, let's wait for QA.
Mathiue
Yes the patch is ok -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #16469|0 |1 is obsolete| | CC| |katrin.fischer@bsz-bw.de -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #17407|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 --- Comment #16 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 32415 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=32415&action=edit 0001-PASSED-QA-Bug-9859-fix-nsb_clean-side-effect.patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 --- Comment #17 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 32416 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=32416&action=edit 0002-Bug-9859-Follow-up-Adding-a-simple-unit-test-for-nsb.patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 --- Comment #18 from stephane.delaune@biblibre.com --- Thanks Katrin -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9859 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |tomascohen@gmail.com --- Comment #19 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patch pushed to master. Thanks Stephane! -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org