tp7 / Sushi

Automatic subtitle shifter based on audio
MIT License
639 stars 45 forks source link

Add "--temp-dir" arg #22

Closed shinchiro closed 9 years ago

shinchiro commented 9 years ago

Specify temporary folder to use when demuxing. Also print default value for some options

tp7 commented 9 years ago

Is there any reason why temp directory shouldn't work without --no-cleanup? Sure it makes little sense but simplifies the logic.

shinchiro commented 9 years ago

You're right..

tp7 commented 9 years ago

Do note that right now temp dir is not deleted even when cleanup is true and it's left empty after processing. I think this behavior is okay but it is debatable.

To make it work properly one would need to remember which directories got created and later remove them only if they're. Some sample code below.

def create_directory_if_not_exists(path):
    abspath = os.path.abspath(path)
    head, tail = abspath, None
    while True:
        try:
            os.mkdir(head)
        except OSError as e:
            if e.errno == errno.EEXIST:
                return head if head != abspath else None, abspath
            elif e.errno == errno.ENOENT:
                head, tail = os.path.split(head)
            else:
                raise
        else:
            return head, abspath
class CreateDirectoryIfNotExistsTestCase(unittest.TestCase):
    def setUp(self):
        self.temp_dir = tempfile.mkdtemp()

    def tearDown(self):
        shutil.rmtree(self.temp_dir)

    def test_existing_directory(self):
        created, full_path = create_directory_if_not_exists(self.temp_dir)

        self.assertIsNone(created)
        self.assertEqual(self.temp_dir, full_path)

    def test_one_level_deep(self):
        path = self.temp_dir + "/inside"
        created, full_path = create_directory_if_not_exists(path)

        self.assertEqual(path, created)
        self.assertEqual(path, full_path)

    def test_three_levels_deep(self):
        one_level = self.temp_dir + "/one"
        three_levels = one_level + "/two/three"
        created, full_path = create_directory_if_not_exists(three_levels)

        self.assertEqual(one_level, created)
        self.assertEqual(three_levels, full_path)

    def test_relative(self):
        path = self.temp_dir + "/inside/deeper/../once_more"
        created, full_path = create_directory_if_not_exists(path)

        self.assertEqual(self.temp_dir + "/inside", created)
        self.assertEqual(self.temp_dir + "/inside/once_more", full_path)

This is quite a lot of code for a minor feature so let's leave it as is for now.