search

chiply / spot4e

Emacs client for Spotify search and playback using helm
MIT License
45 stars 4 forks source link

Fixes to make spot4e terminal friendly #7

Closed christoofar closed 5 years ago

christoofar commented 6 years ago

1) URLEncode is really needed for the auth url because it makes it harder to mark the URL to copy over to a "real" browser (suppose the user has eww as the default? What then? Spotify don't like that) 2) Copy the auth url into the kill ring (so you can bin it out or launch a GUI session on the same daemon to get it out and complete the auth)

chiply commented 5 years ago

Hey - sorry for the late response. Thanks so much for the improvements. I just have to review and resolve conflicts before I merge. Should be done before the end of the week :)

chiply commented 5 years ago

Hey - question about change 2: copying the auth-url to the kill ring. Could you clarify the purpose of doing this? This url doesn't contain that code you get prompted for in the minibuffer after running spot4e-authorize. That url comes after you complete authorization in your browser. Just making sure you're getting what you need out of that change.

christoofar commented 5 years ago

I think this comes up when you're running in a terminal (emacs -nw or emacsclient -nw -c)

chiply commented 5 years ago

I can authorize from the terminal without issues. Sorry - I just want to make sure I can merge your pull request with understanding exactly what everything is for.

From what I understand, you want the auth url in the kill ring as so you can paste it into an eww or w3m search or something like that? If that's the case I'm hesitant to add that feature you can get that behavior by customizing browse-url-browser-function. spot4e calls browse-url which uses the default function, but you can set it to 'eww-browse' or 'w3m-browse' (or wtv the browser functions for those browsers might be). With that in mind, do you still want access to the url from the kill ring? Keep in mind all the url does is take you to the authorization page...

Best

chiply commented 5 years ago

Hey Chris, I merged this pull request, but deleted the kill-new line. Maybe we can revisit that in the future. We can discuss more and if its necessary, you can submit another pull request.

I don't know why you aren't showing up as a contributor or why this pull request isn't showing up properly (I'm still getting used to github). But your commits are in the history if you look. You have also inadvertently resolve one of my issues with the url-encoding change, so thank you!