https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15541 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|Academy | --- Comment #16 from David Cook <dcook@prosentient.com.au> --- (In reply to Marcel de Rooy from comment #11)
[1] Although your changes in SimpleSearch are very small, it would probably help if you would add a test case in t/db_dependent/Search.t. (It seems to fail some tests currently, separate from your patch.)
When I run "prove t/db_dependent/Search.t", it says that all tests are successful, although there are lots of warnings generated. I'll review the test code to see if I can add a test...
[2] The source_normalizer or norms just lead me to two FIXMEs: # FIXME normalize, substr # FIXME - default normalizer
These FIXME messages pre-date my patch.
You introduce none and raw in the regex while this field actually does nothing (and should not have been there in this stage).
I don't understand this sentence.
Your patch starts using this field, while the only thing you want is not running _normalize for (mainly?) URLs.
URLs are certainly the main thing I want to not normalize, but honestly nothing should be normalized during the matching, as Zebra already handles normalization for search queries.
If we do not really solve the problem of this unused field, I think we should not touch it. BTW What would be the difference between raw and none? The regex suggests more than what we offer.
There is no difference between raw and none. They're just synonyms. I'm open to removing _normalize() from the matching completely. Honestly, I figured that the community would be against it, so I created a way of opting out of the default _normalize(). That way default system behaviour doesn't change, while some people will know how to deactivate it.
[3] I have some doubts about this new condition: if ($QParser && $matchpoint->{'index'} !~ m/\w,\w/) { Somehow you managed to bump in another unfinished area here too :) I would rather leave the condition as it was. In the QParser branch you could choose to ignore/remove the second specifier word with a similar regex.
It's not really a new condition. If you look at C4::Search::SimpleSearch, you'll see that it's already there: $QParser = C4::Context->queryparser if (C4::Context->preference('UseQueryParser') && ! ($query =~ m/\w,\w|\w=\w/)); Also, you can't remove the second specifier word, because it's essential to the query. You also can't ignore it, because the QParser can't handle it. That's why I added the same condition that's found in C4::Search::SimpleSearch. You need to catch this condition before going into the QParser code block. -- Can the QA team indicate how to proceed? I'll look at adding a unit test. However, the QParser condition or one similar to it needs to stay. As for _normalize(), I'll remove it completely, if that's a change which will be accepted by the QA team and the RM. -- You are receiving this mail because: You are watching all bug changes.