Skip to content
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

Support searching by Spotify share URLs and URIs analogously to the desktop client #623

Merged
merged 17 commits into from
Oct 22, 2020

Conversation

Utagai
Copy link
Contributor

@Utagai Utagai commented Oct 16, 2020

This closes #41.

To be more precise, this actually adds significantly more functionality than what is requested in #41. In particular, this PR adds support for opening:

  • Tracks (will open its containing album and move the selection to the searched track)
  • Albums
  • Artists
  • Playlists
  • Shows/Podcasts

Via either their Spotify share URL, e.g.:

https://open.spotify.com/album/6QdCohkHKNTVoaSx1ZzitH?si=vJQhe4H5RwiGcDf4xr9DuQ

Or their Spotify URI:

spotify:album:6QdCohkHKNTVoaSx1ZzitH

On the official desktop client, searching with either of these leads you to the Metallica album, Metallica.

If the given input string fails to match either URI format, it will default to the original raw phrase search.

Here are some representative screenshots:
image
Press enter...
image
Note The highlighting/focusing of the track is automatically done.

There were some decisions I made in this PR that may be worth discussing. For those, I've let a comment.

Copy link
Contributor Author

@Utagai Utagai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments that I think are worth calling out on this patch.

src/handlers/input.rs Show resolved Hide resolved
src/network.rs Show resolved Hide resolved
src/network.rs Show resolved Hide resolved
@Utagai Utagai changed the title Support searching by Spotify open URLs and URIs analogously to the desktop client Support searching by Spotify share URLs and URIs analogously to the desktop client Oct 16, 2020
@Utagai Utagai mentioned this pull request Oct 19, 2020
Copy link
Owner

@Rigellute Rigellute left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏 good work @Utagai. This will be a very welcome enhancement.

I also appreciate the clarity of context you've provided with comments in code/github.

src/handlers/input.rs Show resolved Hide resolved
src/handlers/input.rs Show resolved Hide resolved
src/network.rs Show resolved Hide resolved
src/network.rs Show resolved Hide resolved
@Rigellute
Copy link
Owner

If you can please rebase, I'll merge!

@Utagai
Copy link
Contributor Author

Utagai commented Oct 21, 2020

@Rigellute Done.

@Rigellute Rigellute merged commit ed0bf03 into Rigellute:master Oct 22, 2020
@Rigellute
Copy link
Owner

Thanks again @Utagai

lanej pushed a commit to lanej/spotify-tui that referenced this pull request Jul 13, 2021
…esktop client (Rigellute#623)

* Generalize input processing to both URLs and URIs

Unfortunately, I would have preferred the small refactor of adding
process_input() to be in a separate commit, but I quite genuinely just
forgot to commit my work, so these guys are mushed together here.

* Add the boilerplate code for supporting tracks, playlists and podcasts

* Introduce a spotify_resource_id() closure to reduce some duplication

* Support searching playlists by URL/Spotify-URI

* Support searching shows by URL/Spotify-URI

* Support searching individual tracks by URL/Spotify-URI

* Add initial parsing tests and required refactors

* Add full suite of happy path test cases

* Verify that we fail to match on invalid strings

* Remove debug prints

* Correct seek to track index on track search

Albeit, not very cleanly. Want to try getting it nicer before I put this
up...

* Handle query parameters in URIs

* Always push a GetPlaylist to the navigation stack to avoid UI inconsistency

* Clear the playlist selection no matter what kind of search we do

* Remove debug prints

* Remove redundant clone

* Prefer unwrap_or_else()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Open playlists by url?
2 participants