search

DevInsideYou / SpotifyWeb

Displays the current track in the status bar of Sublime Text.
MIT License
12 stars 3 forks source link

Adding podcast support, showing podcast name and episode show title, … #4

Closed HenriqueBrun closed 4 years ago

HenriqueBrun commented 4 years ago

…adding artist name or names or the option to show only "Various Artists" when there are more than one artist

HenriqueBrun commented 4 years ago

Adding podcast support, showing podcast name and episode show title and adding artist name, names or the option to show only "Various Artists" when there are more than one artist

agilesteel commented 4 years ago

Hey, thanks for contributing! Listen I wish I could merge it in, but there is an issue (not related to your change). When I wrote the plugin back in the day I made a mistake of caching the auth token in a file inside of the plugin folder. This means that every time I release a new version packagecontrol will overwrite the entire plugin and thus delete the token and people's browser will pop up to reauthenticate. Once I noticed this annoyance I told myself that "the next" version I release will at the very least move the location of the cache file to the home folder for example. This way it will be the last time this annoys people. But I never worked on the plugin ever since and I don't have much time these days. So I'm sorry to tell you this, but unless you spend a bit of time and do this for me (change the destination of the cache, IIRC fix the tests and test it manually on at least windows and some version of linux) I won't be able to accept your pull request. It shouldn't take too long, but I really don't have the time now.

If you do this for me I will add you as a contributor to this project! If you want of course. Btw for the future: I think it would be a good user experience if users were able to specify a pattern for rendering the info. Sth like "%artist - $title". This way everyone can configure it to look like they want.

Sorry again.

HenriqueBrun commented 4 years ago

Thanks for replying, no need to be sorry, I'll do those changes when I find the time, probably this weekend!

Quarantine is giving me lots of free time after work.

agilesteel commented 4 years ago

Awesome! Thanks! @ Quarantine well at least something good came out of it...

HenriqueBrun commented 4 years ago

Hi again Vladyslav,

Fixed the cache problem, took the liberty to change its name felling that having a file named only '.cache-token.' outside the package folder could be mistaken with something else by someone.

Added the option to choose what comes first: track/episode name or artist/show name, not as elaborate as you suggested but could be something to be changed on the future.

Both run the UnitTest on native Ubuntu 18.04 and native Windows 10 as tested it myself.

Contact me if you have any more ideas, if some come to me I`ll pitch it to you.

I'll will be happy if you want to add me as contributor.

Thank you for your initial effort, it was just what I searched for in Package Manager.

My regards,

agilesteel commented 4 years ago

Hey, I'll merge it but I'll change a couple of things. Nothing major, I'll just rename and rearrange a couple of things and I'll use ", " to separate artists instead of " - ". Thanks for your contribution! I'll make you a contributor but please let's keep only merging into master via PRs. Cool?

agilesteel commented 4 years ago

https://github.com/DevInsideYou/SpotifyWeb/releases/tag/v0.1.0

IIRC it will be published automatically soon. Hit me up on discord if you wanna hang out. https://discord.devinsideyou.com

Thanks again!

HenriqueBrun commented 4 years ago

Cool! though that the comma for hyphen change was well suited!