Feature/extra tags #559
Feature/extra tags #559
Conversation
|
I created #507 in order to avoid having to extend the model infinitelly for such cases, thought it is quite uncomplete and needs maturing, if we add all the fields we need to map into the models, we will end up with lots of parameters undefined for a great buch of backends. |
|
@txomon I think I get the idea of #507, but what I have implemented with this, and also albumartist, is just to cover the basics supported by MPD: http://mpd.wikia.com/wiki/MusicPlayerDaemonCommands#Scope_specifiers I think the list MPD supports is here: http://git.musicpd.org/cgit/master/mpd.git/tree/src/tag.h?h=v0.17.x If this goes in, I plan to get comment done - and them I think I am happy. Should I understand the metadata support to be the best way forward to querying for tags not common to all the backends? I definitely see the idea in that. |
|
Yeah, my idea was mainly to release the models from the most possible fields, so that we could have very tiny objects, and that each frontend could select what did he actually required. I say this because for example, on the mpd frontend, for the playlistinfo command, you just can say 3 or 4 things, not really anything about albumartist, etc. On the other side, on the http frontend, you might require all of the fields. In this way, we could also put per-backend metadata, so that if a frontend is developed for a backend, it could also get the extra information on an standard way. |
|
That makes a lot of sense....darn! :) |
|
I'm interested in merging this now and maybe extract some of it from the models again when we have a better generic metadata system. @ZenithDK Can you merge in |
|
@jodal - yes, no worries. Oh, will you handle these new tags in |
|
I don't think there's anything to change in |
|
I just saw that you mapped |
|
Yes, I did. Spotify does have a concept about albums and the artist of the album. Though, Spotify does not have a concept about genre, composer, or performer, so I don't think there's anything to do in |
|
I've made it possible to use the |
|
Very unfortunate that all the tests can pass, even though I had some missing regexps :-S |
|
When this is merged, we should update the documentation similar to what @kingosticks mentions in #543 |
| result_tracks += [_artist_as_track(a) for a in _get_artists(results)] | ||
| if 'album' not in query: | ||
| if 'album' not in query and 'genre' not in query: |
There was a problem hiding this comment.
Why don't you want albums in the result when genre is in the search query?
There was a problem hiding this comment.
Well, I guess I don't need to filter really - but my thinking was along the lines of albums not having genres?
The genre relates to the track, right?
So in reality we will never get a hit on an album?
| @@ -21,7 +21,6 @@ def test_currentsong(self): | |||
| self.assertInResponse('Title: ') | |||
| self.assertInResponse('Album: ') | |||
| self.assertInResponse('Track: 0') | |||
| self.assertInResponse('Date: ') | |||
There was a problem hiding this comment.
Maybe keep it, but change to assertNotInResponse?
|
+1, with comments |
|
There's only one thing left for the tags now - that's to see if anyone has some multi-genre tracks. I would like to do that work in a separate PR though, if okay. |
Add support for the genre, composer, and performer tags into mopidy.
Added tests as well.