Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

spotify play --playlist #27

Closed
brianstrauch opened this issue Jul 2, 2021 · 13 comments
Closed

spotify play --playlist #27

brianstrauch opened this issue Jul 2, 2021 · 13 comments
Assignees

Comments

@brianstrauch
Copy link
Owner

No description provided.

@Threpio
Copy link
Contributor

Threpio commented Jul 16, 2021

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
Copy link
Owner Author

brianstrauch commented Jul 16, 2021

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
Copy link
Contributor

Threpio commented Jul 16, 2021

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
Copy link
Contributor

Threpio commented Jul 16, 2021

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
Copy link
Owner Author

brianstrauch commented Jul 16, 2021

@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
Copy link
Contributor

Threpio commented Jul 16, 2021

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
Copy link
Owner Author

brianstrauch commented Jul 16, 2021

@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
Copy link
Contributor

Threpio commented Jul 16, 2021

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

playlist.Meta.URI

@brianstrauch
Copy link
Owner Author

@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
Copy link
Owner Author

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

@brianstrauch brianstrauch changed the title spotify play/queue --album/playlist spotify play --playlist Jul 16, 2021
@Threpio
Copy link
Contributor

Threpio commented Jul 16, 2021

--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
Copy link
Contributor

Threpio commented Jul 17, 2021

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
Copy link
Owner Author

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

Threpio added a commit to Threpio/spotify-cli that referenced this issue Jul 26, 2021
Threpio added a commit to Threpio/spotify-cli that referenced this issue Jul 26, 2021
Threpio added a commit to Threpio/spotify-cli that referenced this issue Jul 26, 2021
brianstrauch added a commit that referenced this issue Aug 1, 2021
* Tests broken but playlist functionality added

* Fixed tests but need to make new tests for 'play-playlist'

* Update internal/play/play_test.go

Co-authored-by: Brian Strauch <bstrauch24@gmail.com>

* Update internal/p/p_test.go

Co-authored-by: Brian Strauch <bstrauch24@gmail.com>

* Syntax updates to mock_api.go

Co-authored-by: Brian Strauch <bstrauch24@gmail.com>

* Added switch case to play functions and adapted to queryType - Requires PR on spotify

* Rename errors as per suggestion

Co-authored-by: Brian Strauch <bstrauch24@gmail.com>

* Fixing Tests

Co-authored-by: Brian Strauch <bstrauch24@gmail.com>

* Fixing Tests

Co-authored-by: Brian Strauch <bstrauch24@gmail.com>

* Fixing Tests

Co-authored-by: Brian Strauch <bstrauch24@gmail.com>

* Syntax cleanup

Co-authored-by: Brian Strauch <bstrauch24@gmail.com>

* Syntax cleanup

Co-authored-by: Brian Strauch <bstrauch24@gmail.com>

* Fixing Tests

Co-authored-by: Brian Strauch <bstrauch24@gmail.com>

* Fixing command flags as per suggestion

Co-authored-by: Brian Strauch <bstrauch24@gmail.com>

* Update internal/p/p_test.go

Co-authored-by: Brian Strauch <bstrauch24@gmail.com>

* Syntax Updates

Co-authored-by: Brian Strauch <bstrauch24@gmail.com>

* Update internal/errors.go

Co-authored-by: Brian Strauch <bstrauch24@gmail.com>

* Updated p to better reflect play

* Revert "Merge branch 'playlist-#27' of https://github.com/Threpio/spotify-cli into playlist-#27"

This reverts commit 6fdf6dd, reversing
changes made to bdabdb5.

* updated to 0.7.0 -> Failed with some merging so git is messy

* Sorted out errors again

* More sorting out of play functions

* Running go fmt

* Forgot contextQuery Logic for p() function

Co-authored-by: Brian Strauch <bstrauch24@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants