search

emericg / OpenSubtitlesDownload

Automatically find and download the right subtitles for your favorite videos!
https://emeric.io/OpenSubtitlesDownload
GNU General Public License v3.0
595 stars 66 forks source link

Paths with spaces issue fix on Windows #52

Closed darkdread closed 4 years ago

darkdread commented 4 years ago

Fixed paths with spaces not working when spawning instances due to split() func. Added drag & drop batch file to download english subtitle for Windows. Added gitignore for vscode files.

emericg commented 4 years ago

Hi, So I've committed these changes, thanks!

I'm not entirely sure what the first commit fixes, but the new code is better than the old one anyway so it's good. The drag & drop .bat file is great too. I usually don't want OS/editor specific stuff like that (that's also why I have no interest in adding a file to ignore the vs studio file that you have on your computer) in this repo, but it does a similar job than the linux desktop files, so I've put it next to them. I will add some comments about these files in the wiki soon (probably).

I've edited the pull request, which actually committed changes on your repository? I've never done that before that was a bit weird, sorry. I would have liked two pull requests to discuss the features separately, and in the futur be also careful with including paths or settings from your own computer ^^ I could have smash all of the commits into one (to remove traces of adding/removing settings) but it would have also squash the two features into one, and I was also afraid it would attribute the commit to me instead of you so... Never mind for this time :)

darkdread commented 4 years ago

Hi, So I've committed these changes, thanks!

I'm not entirely sure what the first commit fixes, but the new code is better than the old one anyway so it's good. The drag & drop .bat file is great too. I usually don't want OS/editor specific stuff like that (that's also why I have no interest in adding a file to ignore the vs studio file that you have on your computer) in this repo, but it does a similar job than the linux desktop files, so I've put it next to them. I will add some comments about these files in the wiki soon (probably).

I've edited the pull request, which actually committed changes on your repository? I've never done that before that was a bit weird, sorry. I would have liked two pull requests to discuss the features separately, and in the futur be also careful with including paths or settings from your own computer ^^ I could have smash all of the commits into one (to remove traces of adding/removing settings) but it would have also squash the two features into one, and I was also afraid it would attribute the commit to me instead of you so... Never mind for this time :)

Hi repo owner,

The first commit fixes spawning of child instances with dir containing spaces such as: C:\My Computer\file.mp4 Because the split() function splits away our space in the path, we will not be able to capture the dir correctly. The command_splitted variable would be as follows: ['E:\\Python3.7\\python.exe', 'C:\\My', 'Computer\\OpenSubtitlesDownload.py', '-g', 'cli', '-s', 'hash_then_filename', '-t', 'auto'] As you can see, when the child instance is querying for the OpenSubtitlesDownload.py python file, we will encounter error because the second element is supposed to be the path for OpenSubtitlesDownload.py.

I hope this answers your question. and sorry for the extra commits; this is my first pull request 😹