brianstrauch / spotify-cli

Spotify for the terminal
Apache License 2.0
317 stars 16 forks source link

spotify play --playlist #27

Closed brianstrauch closed 3 years ago

Threpio commented 3 years ago

Is anyone working on the playing of a playlist?

Also curious as to see if there is the possibility of implementing the ability to see the 'Liked songs' playlist (I use that for my spotify usage.

brianstrauch commented 3 years ago

Created an issue for playing the "Liked Songs" playlist! 😄 Although, this issue has to be completed first. Let me know if you're interested in working on it and I'll assign it to you. @Threpio

Threpio commented 3 years ago

Was having a look at the codebase whilst I was curious - Not 100% sure I know how to apply it straight up. The generic Spotify libraries don't seem to support referencing a playlist as a command/variable. If I think I can take it on I will ping and take it - For the meanwhile it might be worth leaving it to someone a bit more experienced.

Threpio commented 3 years ago

I didn't realise that you were referencing your own Spotify go api library. Do we need to create the 'play playlist' functionality there and then update the reference/commands here? Sorry for the time

brianstrauch commented 3 years ago

@Threpio No worries, the only change you'd have to implement in the library is searching by playlist/album (which should be relatively simple; just a few lines of code). It currently hardcodes "track" in the search parameters: https://github.com/brianstrauch/spotify/blob/5e406af4be1b0cc7d6fb0cdec0288bb09013d77c/search.go#L13

Threpio commented 3 years ago
Screenshot 2021-07-16 at 17 44 11

A first stab at the changes to the spotify library - This could then be called in here. I think I was just going to reference the 'GetPlaylist' Function that had already been created?

EDIT: I have realised that this only returns playlists of the user - I assume other playlists being played will be needed aswell.

brianstrauch commented 3 years ago

@Threpio Actually... I think we can just use the existing api.Play() function! That function takes a variable number of uris, so you could probably just pass the URI of a playlist into the function and it would play that playlist. We would just have to make a new api.SearchForPlaylist() function to return a URI for a given playlist name.

Threpio commented 3 years ago

the GetPlaylist function returns a Playlist object - Which I believe contains the URI?

playlist.Meta.URI

brianstrauch commented 3 years ago

@Threpio Hmm... I was originally thinking we could give people the option to play any playlist, but being able to play only your own playlists is probably better for UX. In that case, we should call the api.GetPlaylists() function and do a case-insensitive search for the playlist name the user gave us. If we find it, we can grab the URI value and pass that to the Play() function.

So, the good news is that we won't have to implement anything new in the SDK, just the CLI. 😄

brianstrauch commented 3 years ago

Although the --album flag will need to implement a new Search() function. I'll split that off into a new issue

Threpio commented 3 years ago

--Album can be for a future PR.

I won't be able to do this this evening but am happy to look at it this weekend :)

Edit: For clarity should I not be calling the GetPlaylist (singular) function so that we can have the Params as part of this?

Do we want this as a flag on play or Playlists?

Threpio commented 3 years ago
Screenshot 2021-07-17 at 10 46 22

I believe that a small change will be needed to the spotify library and it can't just be done here. The api.Play() function can take uri's of songs but a playlist or album requires a 'context_uri' field that we do not currently send?

brianstrauch commented 3 years ago

@Threpio Commented on your PR, but we can add context_uri to the existing Play() function.