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

Use comma as "or" operator for queries #1423

Merged
merged 4 commits into from Apr 17, 2015
Merged

Use comma as "or" operator for queries #1423

merged 4 commits into from Apr 17, 2015

Conversation

tomjaspers
Copy link
Collaborator

Scan the query parts for commas (u',') and treat these as (sub) AndQuerys
that need to be wrapped in an OrQuery (if there are more than one).

Supports foo, bar and foo , bar.

See #976

Scan the query parts for commas (u',') and treat these as sub-AndQueries
that need to be wrapped in an OrQuery (if there are more than 1 of these)
@brunal
Copy link
Collaborator

brunal commented Apr 16, 2015

That means that any query holding a comma should now be escaped... I'm not sure how I feel about that.

@sampsyo sampsyo added this to the 1.3.12 milestone Apr 16, 2015
@sampsyo
Copy link
Member

sampsyo commented Apr 16, 2015

Awesome! This looks great to me. As that thread in #976 suggested, a comma seems like the right thing to me. It's not quite true, @brunal, that every comma will need to be escaped—commas are only ORs when they appear by themselves or at the end of a term. So foo, bar is an OR query but foo,bar is not. In the shell, quotes even behave as expected, so:

$ beet ls "foo, bar"

does not invoke an OR query.

I'm inclined to merge this now. We could restrict this to only work with separated commas (foo , bar), but I'm inclined to say that this is less surprising as-is.

@tomjaspers
Copy link
Collaborator Author

@SampSo My original implementation only worked with separated commas, but I found myself constantly typing foo, bar during testing, so for ease of use I'd definitely try and keep it as-is (if possible).

@sampsyo sampsyo merged commit 0a4e830 into beetbox:master Apr 17, 2015
sampsyo added a commit that referenced this pull request Apr 17, 2015
Use comma as "or" operator for queries
sampsyo added a commit that referenced this pull request Apr 17, 2015
sampsyo added a commit that referenced this pull request Apr 17, 2015
@sampsyo
Copy link
Member

sampsyo commented Apr 17, 2015

OK, merged! We can at least see how people like this design. Thanks again! This will be awesome to have. ✨

@tomjaspers tomjaspers deleted the or-query-operator branch April 17, 2015 06:30
@raffomania
Copy link

Awesome! Thanks a lot, @tomjaspers!

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

Successfully merging this pull request may close these issues.

None yet

4 participants