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

Possible 1.x core improvements #1137

Closed
adamcik opened this Issue Apr 18, 2015 · 6 comments

Comments

2 participants
@adamcik
Member

adamcik commented Apr 18, 2015

Notes and ideas from working on #1136 - 1 and 3 I'm fairly sure we want to fix, rest are nice to haves that we can probably ignore.

  1. Support playback.play(tlid=...) - should simplify some MPD code paths if we can let the caller know if the the tlid exists via a return value.
  2. playback.get_current_tlid() mostly for completeness, as I think I found that MPD seekid was the only place we "need" it.
  3. tracklist.move() that supports start=None or end=None for open slices - would remove a bunch of MPD code, would also check if this applies to any of the other similar methods.
  4. tracklist.get_tlid(index) this would allow us to not do slice(pos, pos + 1) in play(index) for the MPD code, alternative would be play(index) but I'm not sold on that.
  5. tracklist.get_tl_track(tlid) not 100% sure about this, but I seemed to run into places where I needed this, but if "everything" takes tlid this is a non-issue.
@tkem

This comment has been minimized.

Member

tkem commented May 1, 2015

+1 for get_current_tlid(), since that's quite useful for clients that want to highlight the current track in a tracklist view, but don't want to re-fetch full track information. BTW, I always wondered why get_current_track is in playback, while next, previous and eot are in tracklist. Seems a little asymmetric to me, but I guess you can argue both ways.

@adamcik

This comment has been minimized.

Member

adamcik commented May 1, 2015

Reasoning was mostly with who "owns" knowledge about what and minimizing crossing between parts. Which makes testing much nicer. And was perfectly fine back when only had MPD as the frontend. But a lot of my changes recently have gone a bit across these borders for the sake of convenience and giving people a more sane API.

Note that you can also just do index() without a track or tlid to get the currently playing track. But a get_current_tlid() would be more robust against any changes in the tracklist.

@tkem

This comment has been minimized.

Member

tkem commented May 1, 2015

Hey, didn't know about index acting this way without parameters; AFAICS this isn't documented anywhere, so I'm wondering if this is "officially" supported. However, this isn't really an issue, but a method that only returns the tlid for the current track would really be helpful.

@adamcik

This comment has been minimized.

Member

adamcik commented May 1, 2015

This will be new in 1.1, and you are correct, it didn't work that way before.

@tkem

This comment has been minimized.

Member

tkem commented May 4, 2015

Ah, now I see it in the develop docs. Nice!

@adamcik adamcik added this to the v1.1 - Robust core milestone May 5, 2015

@adamcik

This comment has been minimized.

Member

adamcik commented May 12, 2015

I think most the important ones are in. Going ahead and closing this and we can always revisit as we evolve the API.

@adamcik adamcik closed this May 12, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment