--list flag accepts only text files using mimetypes #414
Conversation
spotdl/handle.py
Outdated
| @@ -263,6 +264,10 @@ def get_arguments(raw_args=None, to_group=True, to_merge=True): | |||
| if parsed.config is not None and to_merge: | |||
| parsed = override_config(parsed.config, parser) | |||
|
|
|||
| file = parsed.list | |||
There was a problem hiding this comment.
file function is removed from Python 3 but let's still try to avoid to it. Text editors and GitHub still highlight file as something special and it kinda confuses the eyes. I think we could completely avoid setting this variable here and simply go with parsed.list for the checks below.
(Accessing an attribute on an object may be slower than accessing a local variable but the effect should be negligible, we should probably not worry about it)
spotdl/handle.py
Outdated
| @@ -263,6 +264,10 @@ def get_arguments(raw_args=None, to_group=True, to_merge=True): | |||
| if parsed.config is not None and to_merge: | |||
| parsed = override_config(parsed.config, parser) | |||
|
|
|||
| file = parsed.list | |||
| if mimetypes.MimeTypes().guess_type(file)[0] != "text/plain": | |||
There was a problem hiding this comment.
This looks okay but to be consistent with rest of the codebase; let's use if not .. == .. rather than if .. != ...
spotdl/handle.py
Outdated
| @@ -263,6 +264,10 @@ def get_arguments(raw_args=None, to_group=True, to_merge=True): | |||
| if parsed.config is not None and to_merge: | |||
| parsed = override_config(parsed.config, parser) | |||
|
|
|||
| file = parsed.list | |||
| if mimetypes.MimeTypes().guess_type(file)[0] != "text/plain": | |||
| parser.error("{0} is not of a valid file format to be used with --list".format(file)) | |||
There was a problem hiding this comment.
We could make the error message here a bit constructive. Something like mentioning that the file needs to be a plain text file for it to be accepted.
|
Looks good! I'll take a look what's up with tests and then we could merge this in. |
I ended up implementing the mime-type checking using python's built-in
mimetypes. It gets the job done but isn't as accurate as the python-magic library originally proposed.The reason I substituted away was because
python-magicwould add another dependency. Although if someone insists I can switch topython-magicinstead and add python-magic-bin tosetup.py.EDIT: Solution to #354