Fix playlist not found for disambiguated MPD playlists

[kingosticks]

As part of the fix for #114 the following MPD commands don't work for playlists with disambiguated names.

• listplaylist
• listplaylistinfo
• load

This pull request builds on the work in #396 with a generic MPD playlist lookup helper function and some additional tests.

[adamcik]

Just checking if self._playlist_uri_from_name: is preferred from a python style point of view.

[kingosticks]

Yep, good point.

[adamcik]

I have some more feedback, I just need to think some things over a bit so I don't send you off in the wrong direction with bad advice.

[kingosticks]

This was not meant to make it into the changes, I shall remove this.

[adamcik]

So path I was heading down in my head was if we should never rest the naming's. Instead simply having a lazy mechanism where we check mapping[name] == uri and if there is a mismatch we alter the name until there isn't one and add it. Potential downside if done wrong is of course a name = 'foo [1][1]' type situation.

Otherwise the _foo probably doesn't need to be on the class, just in __init__ is fine. Secondly should there we a helper method for accessing the other case as well for symmetry in the API?

Beyond those questions I think this is more or less ready.

[kingosticks]

Thanks for taking a look, I know you're busy with the other changes. But I'm not sure I quite follow you, what do you mean by "never rest the namings"? Never store the mapping structures? I'm not sure how that could work. Unless I've misunderstood, the scheme you describe sounds exactly like what we've already got in create_unique_name().

I don't know why playlist_name_from_uri/_playlist_uri_from_name (I assume these are your _foo) are not just defined in __init__; are the other class members only there for documentation reasons? Does it actually make more sense for the mapping structures to persist as class variables? Or maybe that's dumb.

I did consider making the opposite helper function, I'm not sure why I didn't bother, might as well do it (and use it once!).

[jodal]

No semicolon, please :-)

[jodal]

I'd say, ignore @adamcik's inconsitent ramblings for now and let's keep this pull request to fixing the broken commands instead of redoing #396. That can wait for later if @adamcik can explain his thinking. :-)

I've added two minor comments, and agree that:

• a lookup_playlist_name_from_uri() method would be nice for symmetry, and then make the other instance variable "private" as well with the _ prefix, since everything outside the MpdContext class now use the two methods.
• Yes, the other class variables are there just for the documentation. The two relevant variables here is fine with just being initialized in __init__.

If you fix these four things, I'll merge. If you don't have the time, just say so, and I'll fix it myself. I want this in for 0.14. :-)

[kingosticks]

Sure, I'll get on it now.

[adamcik]

Just to clarify, not something you should act on for this PR: I actually meant «never reset the namings», that's what I get for not reading over before firing this off when tired :-) So basically more or less as is, just get rid of the .clear() so that we only ever add new mapping, never recompute them. Hope this makes a bit more sense.

[jodal]

Thanks :-)

[kingosticks]

Oh I see, that does make a lot more sense! Anyway, hopefully this is good enough for now.