dbr / tvnamer

Automatic TV episode file renamer, uses data from thetvdb.com via tvdb_api
https://pypi.python.org/pypi/tvnamer/
The Unlicense
912 stars 115 forks source link

Subtitles with language codes & multi-episode files #68

Closed lahwaacz closed 11 years ago

lahwaacz commented 11 years ago

Move subtitles along with video files (#66)

Allows to keep language codes in subtitle name intact - for example 'original-name.eng.srt' is renamed to 'new-name.eng.srt'.

Better handling of multi-episode files

There are some episodes on thetvdb.com that are part of multi-episode but don't have number in the name (for example Star Trek DS9, 1x01 'Emissary' and 1x02 'Emissary (2)'). Using previous code, the episode name would be 'Emissary, Emissary (2)'. After the change the name is correctly 'Emissary (1-2)'.

dbr commented 11 years ago

The change to handle multi-episode files looks good.

Would you mind adding two test cases for this? Just copy an existing test from tests/test_configfunctional.py - one test with the default multiep_format, and the second with a slightly tweaked format setting.

The subtitle handling I'm not convinced about.. I'd rather not explicitly support renaming subtitles - I prefer that they be renamed like any other file..

Instead, to allow for your use-case of preserving an .eng.srt as the extension, perhaps we could add a extension_pattern option, which could be used like this:

"extension_pattern": "(\.(eng|cze))?(\.[a-zA-Z0-9]+$)",

..and instead of os.path.splitext we use this option to split the extension?

lahwaacz commented 11 years ago

Added the test for multi-episode filenames, I hope I got it right...

Regarding the subtitle handling, I admit using extension_pattern is much simpler. IMO the problem is integrating this method into current code, because os.path.splitext is used about 8 times in utils.py, so simply replacing it with regex matching would be very error-prone. And it could be conflicting with regular expressions in output_filename_replacements (haven't looked at it though). I'm willing to rewrite this, but I'm afraid it will be quite big change...

P.s.: I'm also wondering why there are newPath and newName methods in class Renamer in utils.py. Functionality of newName is already contained in newPath.

lahwaacz commented 11 years ago

I've rewritten the code to use extension_pattern for splitting extension. Test included. All other tests passed with default config, so it should be OK. However, using extension_pattern and valid_extensions together might be confusing. Currently the code for checking valid_extensions still uses os.path.splitext (for backward compatibility), and extensions in valid_extensions array do not include dots. I'd also like to preserve the helper functions for copying and renaming files in order to preserve access and modification time.

dbr commented 11 years ago

At long last, merged (well, squashed and merged as 0388b4861d8fde558172fb3723a9bf0f649012dc)

There was one minor change required so the tests passed on OS X (Star Trek: etc became Star Trek_ etc), but that was easily fixed

Thank you!