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

Fix download single track JS error #7

Merged
merged 1 commit into from Oct 5, 2017

Conversation

Projects
None yet
2 participants
@lnfnunes
Contributor

lnfnunes commented Oct 5, 2017

This should fix issue #6

As a plus, I've noticed that single file download wasn't writing to .downloaded manifest file, so it always re-download even if the file was already downloaded. I've fixed that too.

Evidences:

  • No JS error
    image
  • No re-download
    image
@danguilherme

This comment has been minimized.

Show comment
Hide comment
@danguilherme

danguilherme Oct 5, 2017

Owner

Great, thanks! I don't have access to a computer right now, tomorrow I'll test it and then we can merge.

Also, I noticed you're downloading tracks as audio. There were an issue with files not playing in certain apps (issue #1). Hopefully it's fixed, but I could not test in a wide range of apps. Feel free to report any issue you may have with them!

Owner

danguilherme commented Oct 5, 2017

Great, thanks! I don't have access to a computer right now, tomorrow I'll test it and then we can merge.

Also, I noticed you're downloading tracks as audio. There were an issue with files not playing in certain apps (issue #1). Hopefully it's fixed, but I could not test in a wide range of apps. Feel free to report any issue you may have with them!

@lnfnunes

This comment has been minimized.

Show comment
Hide comment
@lnfnunes

lnfnunes Oct 5, 2017

Contributor

Ok! But ATM I've no problem with downloaded audios...
Actually in this fix case, I've used audio files to be faster as video. My focus was on download logic only 😛

Contributor

lnfnunes commented Oct 5, 2017

Ok! But ATM I've no problem with downloaded audios...
Actually in this fix case, I've used audio files to be faster as video. My focus was on download logic only 😛

@danguilherme

This comment has been minimized.

Show comment
Hide comment
@danguilherme

danguilherme Oct 5, 2017

Owner

It works perfectly!

Will need just a little change.
As the metadata is being saved on every track (on downloadTrack), we don't need it to be saved again when downloading playlists (downloadPlaylist uses downloadTrack). You'd just need to remove the saveMetadata call from it: https://github.com/lnfnunes/spotivy/blob/d1d64c6f55283a89bdbd95fee753fc31587737d0/index.js#L226.

Owner

danguilherme commented Oct 5, 2017

It works perfectly!

Will need just a little change.
As the metadata is being saved on every track (on downloadTrack), we don't need it to be saved again when downloading playlists (downloadPlaylist uses downloadTrack). You'd just need to remove the saveMetadata call from it: https://github.com/lnfnunes/spotivy/blob/d1d64c6f55283a89bdbd95fee753fc31587737d0/index.js#L226.

@lnfnunes

This comment has been minimized.

Show comment
Hide comment
@lnfnunes

lnfnunes Oct 5, 2017

Contributor

done!

Contributor

lnfnunes commented Oct 5, 2017

done!

@danguilherme danguilherme merged commit 9540f29 into danguilherme:master Oct 5, 2017

@danguilherme

This comment has been minimized.

Show comment
Hide comment
@danguilherme

danguilherme Oct 5, 2017

Owner

Thank you!

Owner

danguilherme commented Oct 5, 2017

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment