diff --git a/picard/file.py b/picard/file.py index fc4cd85e6..06fa73390 100644 --- a/picard/file.py +++ b/picard/file.py @@ -353,8 +353,7 @@ class File(QtCore.QObject, Item): if config.setting["rename_files"] or config.setting["move_files"]: new_filename = self._rename(old_filename, metadata, config.setting) # Move extra files (images, playlists, etc.) - if config.setting["move_files"] and config.setting["move_additional_files"]: - self._move_additional_files(old_filename, new_filename) + self._move_additional_files(old_filename, new_filename, config) # Delete empty directories if config.setting["delete_empty_dirs"]: dirname = os.path.dirname(old_filename) @@ -546,14 +545,22 @@ class File(QtCore.QObject, Item): for image in images: image.save(dirname, metadata, counters) - def _move_additional_files(self, old_filename, new_filename): + def _move_additional_files(self, old_filename, new_filename, config): """Move extra files, like images, playlists...""" + if not config.setting["move_files"] or not config.setting["move_additional_files"]: + return new_path = os.path.dirname(new_filename) old_path = os.path.dirname(old_filename) if new_path == old_path: # skip, same directory, nothing to move return - config = get_config() + pattern_regexes = self._compile_move_additional_files_pattern(config) + if not pattern_regexes: + return + moves = self._get_additional_files_moves(old_path, new_path, pattern_regexes) + self._apply_additional_files_moves(moves) + + def _compile_move_additional_files_pattern(self, config): patterns = config.setting["move_additional_files_pattern"] pattern_regexes = set() for pattern in patterns.split(): @@ -563,24 +570,26 @@ class File(QtCore.QObject, Item): pattern_regex = re.compile(fnmatch.translate(pattern), re.IGNORECASE) match_hidden = pattern.startswith('.') pattern_regexes.add((pattern_regex, match_hidden)) - if not pattern_regexes: - return + return pattern_regexes + + def _get_additional_files_moves(self, old_path, new_path, patterns): moves = set() try: - # TODO: use with statement with python 3.6+ - for entry in os.scandir(old_path): - is_hidden = entry.name.startswith('.') - for pattern_regex, match_hidden in pattern_regexes: - if is_hidden and not match_hidden: - continue - if pattern_regex.match(entry.name): - new_file_path = os.path.join(new_path, entry.name) - moves.add((entry.path, new_file_path)) - break # we are done with this file + with os.scandir(old_path) as scan: + for entry in scan: + is_hidden = entry.name.startswith('.') + for pattern_regex, match_hidden in patterns: + if is_hidden and not match_hidden: + continue + if pattern_regex.match(entry.name): + new_file_path = os.path.join(new_path, entry.name) + moves.add((entry.path, new_file_path)) + break # we are done with this file except OSError as why: log.error("Failed to scan %r: %s", old_path, why) - return + return moves + def _apply_additional_files_moves(self, moves): for old_file_path, new_file_path in moves: # FIXME we shouldn't do this from a thread! if self.tagger.files.get(decode_filename(old_file_path)): diff --git a/picard/formats/apev2.py b/picard/formats/apev2.py index 48e9b4ab9..746b07b51 100644 --- a/picard/formats/apev2.py +++ b/picard/formats/apev2.py @@ -49,6 +49,11 @@ from picard.util import ( encode_filename, sanitize_date, ) +from picard.util.filenaming import ( + get_available_filename, + move_ensure_casing, + replace_extension, +) from .mutagenext import ( aac, @@ -311,14 +316,20 @@ class WavPackFile(APEv2File): NAME = "WavPack" _File = mutagen.wavpack.WavPack - def _save_and_rename(self, old_filename, metadata): + def _move_or_rename_wvc(self, old_filename, new_filename): + wvc_filename = replace_extension(old_filename, ".wvc") + if not isfile(wvc_filename): + return + wvc_new_filename = replace_extension(new_filename, ".wvc") + wvc_new_filename = get_available_filename(wvc_new_filename, wvc_filename) + log.debug('Moving Wavepack correction file %r => %r', wvc_filename, wvc_new_filename) + move_ensure_casing(wvc_filename, wvc_new_filename) + + def _move_additional_files(self, old_filename, new_filename, config): """Includes an additional check for WavPack correction files""" - wvc_filename = old_filename.replace(".wv", ".wvc") - if isfile(wvc_filename): - config = get_config() - if config.setting["rename_files"] or config.setting["move_files"]: - self._rename(wvc_filename, metadata, config.setting) - return File._save_and_rename(self, old_filename, metadata) + if config.setting["rename_files"] or config.setting["move_files"]: + self._move_or_rename_wvc(old_filename, new_filename) + return super()._move_additional_files(old_filename, new_filename, config) class OptimFROGFile(APEv2File): diff --git a/picard/util/filenaming.py b/picard/util/filenaming.py index 6ae70a44a..a073e02c5 100644 --- a/picard/util/filenaming.py +++ b/picard/util/filenaming.py @@ -525,3 +525,18 @@ def get_available_filename(new_path, old_path=None): new_path = "%s (%d)%s" % (tmp_filename, i, ext) i += 1 return new_path + + +def replace_extension(filename, new_ext): + """Replaces the extension in filename with new_ext. + + If the file has no extension the extension is added. + + Args: + filename: A file name + new_ext: New file extension + + Returns: filename with replaced file extension + """ + name, ext = os.path.splitext(filename) + return name + '.' + new_ext.lstrip('.') diff --git a/test/formats/test_apev2.py b/test/formats/test_apev2.py index a8b12028e..c7ce2dd95 100644 --- a/test/formats/test_apev2.py +++ b/test/formats/test_apev2.py @@ -184,8 +184,8 @@ class WavPackTest(CommonApeTests.ApeTestCase): } unexpected_info = ['~video'] - @skipUnlessTestfile - def test_save_wavpack_correction_file(self): + def setUp(self): + super().setUp() config.setting['rename_files'] = True config.setting['move_files'] = False config.setting['ascii_filenames'] = False @@ -196,18 +196,40 @@ class WavPackTest(CommonApeTests.ApeTestCase): config.setting['save_images_to_files'] = False config.setting['file_renaming_scripts'] = {'test_id': {'script': '%title%'}} config.setting['selected_file_naming_script_id'] = 'test_id' + + def _save_with_wavpack_correction_file(self, source_file_wvc): # Create dummy WavPack correction file - source_file_wvc = self.filename + 'c' open(source_file_wvc, 'a').close() # Open file and rename it f = open_(self.filename) - metadata = Metadata({'title': 'renamed_' + os.path.basename(self.filename)}) + f._copy_loaded_metadata(f._load(self.filename)) + f.metadata['title'] = 'renamed_' + os.path.basename(self.filename) self.assertTrue(os.path.isfile(self.filename)) - target_file_wv = f._save_and_rename(self.filename, metadata) + target_file_wv = f._save_and_rename(self.filename, f.metadata) target_file_wvc = target_file_wv + 'c' # Register cleanups self.addCleanup(os.unlink, target_file_wv) self.addCleanup(os.unlink, target_file_wvc) + return (target_file_wv, target_file_wvc) + + @skipUnlessTestfile + def test_save_wavpack_correction_file(self): + source_file_wvc = self.filename + 'c' + (target_file_wv, target_file_wvc) = self._save_with_wavpack_correction_file(source_file_wvc) + # Check both the WavPack file and the correction file got moved + self.assertFalse(os.path.isfile(self.filename)) + self.assertFalse(os.path.isfile(source_file_wvc)) + self.assertTrue(os.path.isfile(target_file_wv)) + self.assertTrue(os.path.isfile(target_file_wvc)) + + @skipUnlessTestfile + def test_save_wavpack_correction_file_with_move_additional_files(self): + config.setting['move_files'] = True + config.setting['move_files_to'] = self.mktmpdir() + config.setting['move_additional_files'] = True + config.setting['move_additional_files_pattern'] = '*.wvc' + source_file_wvc = self.filename + 'c' + (target_file_wv, target_file_wvc) = self._save_with_wavpack_correction_file(source_file_wvc) # Check both the WavPack file and the correction file got moved self.assertFalse(os.path.isfile(self.filename)) self.assertFalse(os.path.isfile(source_file_wvc)) diff --git a/test/test_filesystem.py b/test/test_filesystem.py index 8c966a425..0bec4f8db 100644 --- a/test/test_filesystem.py +++ b/test/test_filesystem.py @@ -4,7 +4,7 @@ # # Copyright (C) 2018 Antonio Larrosa # Copyright (C) 2018 Wieland Hoffmann -# Copyright (C) 2018-2019 Philipp Wolfer +# Copyright (C) 2018-2019, 2021 Philipp Wolfer # Copyright (C) 2018-2020 Laurent Monin # # This program is free software; you can redistribute it and/or @@ -99,44 +99,51 @@ class TestFileSystem(PicardTestCase): def _move_additional_files(self, files): f = picard.formats.open_(files['old_mp3']) - f._move_additional_files(files['old_mp3'], files['new_mp3']) + f._move_additional_files(files['old_mp3'], files['new_mp3'], config) + def _assert_additional_files_moved(self, files): + self._move_additional_files(files) self._assertFile(files['new_img']) self._assertNoFile(files['old_img']) + def _assert_additional_files_not_moved(self, files): + self._move_additional_files(files) + self._assertNoFile(files['new_img']) + self._assertFile(files['old_img']) + def test_move_additional_files_source_unicode(self): files = self._prepare_files(src_rel_path='música') - - self._move_additional_files(files) + self._assert_additional_files_moved(files) def test_move_additional_files_target_unicode(self): files = self._prepare_files(tgt_rel_path='música') - - self._move_additional_files(files) + self._assert_additional_files_moved(files) def test_move_additional_files_duplicate_patterns(self): files = self._prepare_files() - config.setting['move_additional_files_pattern'] = 'cover.jpg *.jpg' - - self._move_additional_files(files) + self._assert_additional_files_moved(files) def test_move_additional_files_hidden_nopattern(self): files = self._prepare_files() - config.setting['move_additional_files_pattern'] = '*.jpg' - - self._move_additional_files(files) - + self._assert_additional_files_moved(files) self._assertNoFile(files['new_hidden_img']) self._assertFile(files['old_hidden_img']) def test_move_additional_files_hidden_pattern(self): files = self._prepare_files() - config.setting['move_additional_files_pattern'] = '*.jpg .*.jpg' - - self._move_additional_files(files) - + self._assert_additional_files_moved(files) self._assertFile(files['new_hidden_img']) self._assertNoFile(files['old_hidden_img']) + + def test_move_additional_files_disabled(self): + config.setting['move_additional_files'] = False + files = self._prepare_files(src_rel_path='música') + self._assert_additional_files_not_moved(files) + + def test_move_files_disabled(self): + config.setting['move_files'] = False + files = self._prepare_files(src_rel_path='música') + self._assert_additional_files_not_moved(files) diff --git a/test/test_util_filenaming.py b/test/test_util_filenaming.py index aad6f64ab..050bc8fd8 100644 --- a/test/test_util_filenaming.py +++ b/test/test_util_filenaming.py @@ -44,6 +44,7 @@ from picard.util.filenaming import ( make_save_path, make_short_filename, move_ensure_casing, + replace_extension, samefile_different_casing, ) @@ -245,3 +246,11 @@ class GetAvailableFilenameTest(PicardTestCase): oldname = self._add_number(filename, expected_number) new_filename = get_available_filename(filename, oldname) self.assertEqual(self._add_number(filename, expected_number), new_filename) + + +class ReplaceExtensionTest(PicardTestCase): + + def test_replace(self): + self.assertEqual('foo/bar.wvc', replace_extension('foo/bar.wv', '.wvc')) + self.assertEqual('foo/bar.wvc', replace_extension('foo/bar.wv', 'wvc')) + self.assertEqual('foo/bar.wvc', replace_extension('foo/bar', 'wvc'))