Skip to content
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

Configurable default query terms operator (AND or OR) #36

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

phurni
Copy link
Contributor

@phurni phurni commented Sep 14, 2012

Refactor of SearchIndex and added new feature to let the user chose the default operator between query terms to AND (legacy) or OR (new).

Explicit AND can also be expressed when the default operator is OR (best of both worlds).

Upgrading to this new version will not break existing code (tests are green).

@dougal
Copy link
Owner

dougal commented Dec 20, 2012

Hi Pascal.

Some cool stuff in here, I especially like the Query class.

I won't be adding it to the next release, but I will be looking at this more thoroughly for an upcoming release.

Thanks for your contribution.

D.

@phurni
Copy link
Contributor Author

phurni commented Dec 20, 2012

You're welcome.

Take a look at the relevant factor, because I really think this is a bug (relevant factor being reversed)

Best Regards,

@dougal
Copy link
Owner

dougal commented Dec 20, 2012

This commit? de0a0b2

@phurni
Copy link
Contributor Author

phurni commented Dec 20, 2012

I put that in its own branch, this commit: phurni@369f751

Note thought that I didn't pushed the patch upstream here, I'll do that in the following days.

@dougal
Copy link
Owner

dougal commented Dec 20, 2012

Ooh, that is a doozy of a bug! Definitely worthy of a new release ASAP.

I'll get on that tomorrow.

Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants