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

Add ability to exclude backends during search #2001

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

djmattyg007
Copy link
Contributor

@djmattyg007 djmattyg007 commented Jul 30, 2021

This lets you exclude just a single search results provider, rather than
having to manually select all providers you do want search results from.
This will be useful in situations where it isn't possible for human
users to select which backends they want results from, such as through
the MPD frontend.

I also took the chance to rename the function 'get_backends_to_uris' to
'get_backends_for_uris', because it's a more accurate representation of
what it does.

@djmattyg007
Copy link
Contributor Author

My primary motivation for this functionality is to exclude search results from the Youtube extension when using an MPD client. I'll be submitting a follow-up patch to the MPD extension once this is merged.

@djmattyg007 djmattyg007 force-pushed the ignore_backends_during_search branch from 14a80ca to 7a9e7ad Compare July 30, 2021 23:25
@kingosticks
Copy link
Member

Just looking at the API it's a bit weird that there is an argument for including stuff called uris which takes uris and your new exclude_backends arg for excluding stuff that wants just schemes.

Also, out of interest, why would the MPD backend always want to exclude youtube results?

@djmattyg007
Copy link
Contributor Author

Just looking at the API it's a bit weird that there is an argument for including stuff called uris which takes uris and your new exclude_backends arg for excluding stuff that wants just schemes.

I could make it take full URIs if you want, but it would just add unnecessary work to the process. The logic only requires URI schemes.

Also, out of interest, why would the MPD backend always want to exclude youtube results?

The idea is that it would be based on configuration. In my experience I don't generally want search results from Youtube most of the time, but the MPD interface doesn't provide for the ability to select which backends I want search results from. Therefore, it would need to be handled server-side with configuration.

@kingosticks
Copy link
Member

I could make it take full URIs if you want, but it would just add unnecessary work to the process. The logic only requires URI schemes.

It's just a weird API to have both uris and schemes for these almost complementary options. It feels like they should both work the same and not be very subtly different:

  • search(query, ['spotify:'], False, None)
  • search(query, None, False, ['spotify'])

I can just see myself trying to do exclude_backends=['foo:'] and wondering why it doesn't work.

The idea is that it would be based on configuration. In my experience I don't generally want search results from Youtube most of the time, but the MPD interface doesn't provide for the ability to select which backends I want search results from. Therefore, it would need to be handled server-side with configuration.

Wouldn't it be preferable to add a disable searching option to Mopidy-Youtube itself? This would work regardless of the frontend being used.

@djmattyg007
Copy link
Contributor Author

I can just see myself trying to do exclude_backends=['foo:'] and wondering why it doesn't work.

Sure, I'll add support for full URIs.

Wouldn't it be preferable to add a disable searching option to Mopidy-Youtube itself? This would work regardless of the frontend being used.

I don't want to fully disable searching in the Youtube extension. The Mopidy frontends I use regularly (musicbox and iris) support choosing which backend(s) to retrieve search results from, and sometimes I do want search results from Youtube. However most of the time I only want results from non-youtube.

On the other hand, the MPD protocol has no concept of mopidy backends, so there's no way for the end user to filter out Youtube search results there.

@djmattyg007 djmattyg007 force-pushed the ignore_backends_during_search branch 2 times, most recently from 39f0661 to a6411ed Compare July 31, 2021 23:02
@kingosticks
Copy link
Member

kingosticks commented Aug 1, 2021

On the other hand, the MPD protocol has no concept of mopidy backends, so there's no way for the end user to filter out Youtube search results there.

We could support some more MPD filters, this one, combined with the negation operator (!) should allow us to exclude backends:

(base 'VALUE'): restrict the search to songs in the given directory (relative to the music directory).

But the user would have to do this all manually from their client. I think I would actually use this myself, I usually want to restrict searches to Spotify and being able to do that via MPD would be nice.

@djmattyg007
Copy link
Contributor Author

For my setup at home, this will need to work from clients like MALP and be usable by non-technical people who have no idea what MPD is. Manually crafting search queries isn't an option for me.

Regardless, this functionality would need to be supported in the Mopidy core.

@kingosticks
Copy link
Member

kingosticks commented Aug 1, 2021

True, it is too technical for most.

Going back to your use case, if your new config setting in MPD is set to exclude youtube then Mopidy-MPD can call search() with the uris argument set to everything except youtube. I.e isn't this already supported by our API without anyone needing to do anything manually?

@djmattyg007
Copy link
Contributor Author

I suppose so, but that feels like a lot more work than just passing a list of URI schemes directly from the configuration.

This lets you exclude just a single search results provider, rather than
having to manually select all providers you do want search results from.
This will be useful in situations where it isn't possible for human
users to select which backends they want results from, such as through
the MPD frontend.

I also took the chance to rename the function 'get_backends_to_uris' to
'get_backends_for_uris', because it's a more accurate representation of
what it does.
@djmattyg007 djmattyg007 force-pushed the ignore_backends_during_search branch from a6411ed to ed39c37 Compare July 9, 2022 08:17
@jodal jodal deleted the branch main March 1, 2024 23:15
@jodal jodal closed this Mar 1, 2024
@jodal jodal reopened this Mar 1, 2024
@jodal jodal changed the base branch from develop to main March 1, 2024 23:20
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.

3 participants