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

Issue for multiple downloads when opt_search_overwrite == 'off' #35

Closed rhcpfan closed 6 years ago

rhcpfan commented 6 years ago

In the following code block, for every call to videoPathList.remove(videoPathDispatch), an item in videoPathList will be skipped (removing elements while iterating).

# Check if the subtitles exists videoPathList
if opt_search_overwrite == 'off':
    for videoPathDispatch in videoPathList:
        if checkSubtitlesExists(videoPathDispatch) == True:
            videoPathList.remove(videoPathDispatch)

A simple solution would be to use a second array to hold the paths to remove later:

paths_to_remove = []

if opt_search_overwrite == 'off':
    for videoPathDispatch in videoPathList:
        if checkSubtitlesExists(videoPathDispatch) == True:
            paths_to_remove.append(videoPathDispatch)

for path_to_remove in paths_to_remove:
    videoPathList.remove(path_to_remove)

Hope it helps, keep up the good work!

rhcpfan commented 6 years ago

Or even better, use list comprehension

if opt_search_overwrite == 'off':
    videoPathList = [path for path in videoPathList if not checkSubtitlesExists(path)]
emericg commented 6 years ago

Thanks man, it works great and it's also cleaner than before. We hacked so many features into this script that it's hard to test them all...

Next time send a pull request directly ;)