forked from bikalims/bika.lims
-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added wildcard to bika listing search #8
Open
mikejmets
wants to merge
1
commit into
rockfruit:client/uw-wip
Choose a base branch
from
mikejmets:LIMS-2240-Improved-Method-Search
base: client/uw-wip
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about
I'm probably missing something, but
*
doesn't seem to be the right one to match at start of word:Unless you mean to use
MatchGlob
. It looks likeMatchRegexp
wants a re object as parameter, but didn't dig to see if it accepts strings too. I'm guessing it does ..There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for reviewing this Jean. MatchRegexp seems to handle * correctly and when it makes no difference to the result if I use MatchGlob. BTW, this works well for text but not so well for numbers. This client has strings like "1,1,1-Methyl Hydride" but advanced query is not a friend of numbers. Any ideas?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm that sounds surprising. A regexp like
term*
would match any string that containster
with or withoutm
, anywhere in the string (forre.search()
) or at the start (forre.match()
). That doesn't sound like what was intended.A glob search for
term*
would match strings starting withterm
. (The regexp would also match this, which looks correct, but it would also matchterq
, which is wrong).I had a quick look at
AdvancedQuery
but it doesn't importre
orglob
. I don't know where the regexp evaluation actually happens.Regarding numbers, that's on the level of the index implementation, below
AdvancedQuery
. Text indexes normally filter out things like numbers and stopwords. To nicely match chemical names, you'll need a custom index. This is probably much better done usingcollective.solr
.I see Bika doesn't use
ManagableIndex
..