Fixes #44 and minor changes #46
Conversation
| download_directory = args.output + '/' + playlist | ||
| # Check whether directory has a trailing slash or not | ||
| if len(download_directory) >= 0 and download_directory[-1] != '/': | ||
| download_directory += '/' |
There was a problem hiding this comment.
@SathyaBhat why were you checking if the path had a trailing slash? it's not really needed. Needed on windows?
There was a problem hiding this comment.
@iambibhas i'm not sure. that came as a contrib, didn't pay much attention
There was a problem hiding this comment.
@iambibhas well now I know why that piece of code is required :)
| @@ -29,22 +30,24 @@ def spotify_dl(): | |||
| parser.add_argument('-v', '--version', action='store_true', | |||
| help='Shows current version of the program') | |||
| parser.add_argument('-o', '--output', type=str, action='store', | |||
| nargs='*', help='Specify download directory.') | |||
There was a problem hiding this comment.
nargs='*' sends that arguments as a list. I'm assuming that there can only be one output directory at a time for a playlist. so we just need one value. returning list just makes thing complicated. This fixes #44.
| parser.add_argument('-u', '--user_id', action='store', | ||
| help='Specify the playlist owner\'s userid when it' | ||
| ' is different than your spotify userid') | ||
| parser.add_argument('-i', '--uri', type=str, action='store', | ||
| nargs='*', help='Given a URI, download it.') | ||
| parser.add_argument('-f', '--format_str', type=str, action='store', | ||
| nargs='*', help='Specify youtube-dl format string.', | ||
| default=['bestaudio/best']) |
There was a problem hiding this comment.
Same here. Only one audio quality at a time, right?
| uri = ":".join(url) | ||
| uri = "spotify:" + uri | ||
| args.uri = [] | ||
| args.uri.append(uri) |
There was a problem hiding this comment.
why do you need it to be a list?
| @@ -16,6 +16,7 @@ | |||
| setup( | |||
| name='spotify_dl', | |||
| version=version, | |||
| python_requires='>=3', | |||
There was a problem hiding this comment.
This only tells pip>=9.0 that the package is for python3 only. For older pip versions, it'll still get installed and fail. Do you want the package to support python2?
There was a problem hiding this comment.
the package still works on py2 I believe, but yeah better to make it py3+
There was a problem hiding this comment.
Can make it support both. there are still too many people using python2 as default.
There was a problem hiding this comment.
@iambibhas sorry for the late reply. I do mention Py3 is required, so feel better to enforce that
No description provided.