From 705fa2b52cf43eb9a65fc3203934d295a2e6a291 Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Mon, 5 Aug 2019 18:12:38 +0200 Subject: [PATCH 1/4] PICARD-1559: Create directories when moving files Use the renaming script for the directory part if only moving files without renaming file is set. Keep the filename in this case. --- picard/file.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/picard/file.py b/picard/file.py index 0b5033dbc..fb7669919 100644 --- a/picard/file.py +++ b/picard/file.py @@ -375,6 +375,7 @@ class File(QtCore.QObject, Item): def _format_filename(self, new_dirname, new_filename, metadata, settings): # TODO: tests !! + old_filename = new_filename new_filename, ext = self._fixed_splitext(new_filename) ext = ext.lower() new_filename = new_filename + ext @@ -385,6 +386,8 @@ class File(QtCore.QObject, Item): new_filename = self._script_to_filename(naming_format, metadata, settings) # NOTE: the _script_to_filename strips the extension away new_filename = new_filename + ext + if not settings['rename_files']: + new_filename = os.path.join(os.path.dirname(new_filename), old_filename) if not settings['move_files']: new_filename = os.path.basename(new_filename) win_compat = IS_WIN or settings['windows_compatibility'] @@ -416,7 +419,7 @@ class File(QtCore.QObject, Item): new_dirname = os.path.dirname(filename) new_filename = os.path.basename(filename) - if settings["rename_files"]: + if settings["rename_files"] or settings["move_files"]: new_filename = self._format_filename(new_dirname, new_filename, metadata, settings) new_path = os.path.join(new_dirname, new_filename) From 9d67a45bde6cc6845518dd5480c4298185941266 Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Mon, 5 Aug 2019 19:32:47 +0200 Subject: [PATCH 2/4] Added tests for File.make_filename --- picard/file.py | 5 +-- picard/ui/options/renaming.py | 2 +- test/test_file.py | 76 +++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 4 deletions(-) diff --git a/picard/file.py b/picard/file.py index fb7669919..d94205c5d 100644 --- a/picard/file.py +++ b/picard/file.py @@ -374,7 +374,6 @@ class File(QtCore.QObject, Item): return new_filename, ext def _format_filename(self, new_dirname, new_filename, metadata, settings): - # TODO: tests !! old_filename = new_filename new_filename, ext = self._fixed_splitext(new_filename) ext = ext.lower() @@ -407,7 +406,7 @@ class File(QtCore.QObject, Item): new_filename = unicodedata.normalize("NFD", new_filename) return new_filename - def _make_filename(self, filename, metadata, settings=None): + def make_filename(self, filename, metadata, settings=None): """Constructs file name based on metadata and file naming formats.""" if settings is None: settings = config.setting @@ -431,7 +430,7 @@ class File(QtCore.QObject, Item): def _rename(self, old_filename, metadata): new_filename, ext = os.path.splitext( - self._make_filename(old_filename, metadata)) + self.make_filename(old_filename, metadata)) if old_filename == new_filename + ext: return old_filename diff --git a/picard/ui/options/renaming.py b/picard/ui/options/renaming.py index 784e324d9..c1e260a05 100644 --- a/picard/ui/options/renaming.py +++ b/picard/ui/options/renaming.py @@ -150,7 +150,7 @@ class RenamingOptionsPage(OptionsPage): if s_enabled and s_text: parser = ScriptParser() parser.eval(s_text, file.metadata) - filename = file._make_filename(file.filename, file.metadata, settings) + filename = file.make_filename(file.filename, file.metadata, settings) if not settings["move_files"]: return os.path.basename(filename) return filename diff --git a/test/test_file.py b/test/test_file.py index bfd4d1457..981f93cef 100644 --- a/test/test_file.py +++ b/test/test_file.py @@ -4,8 +4,11 @@ from tempfile import mkdtemp from test.picardtestcase import PicardTestCase +from picard import config +from picard.const import DEFAULT_FILE_NAMING_FORMAT from picard.const.sys import IS_MACOS from picard.file import File +from picard.metadata import Metadata def is_macos_10_14(): @@ -118,3 +121,76 @@ class TestPreserveTimes(PicardTestCase): with self.assertRaises(self.file.PreserveTimesUtimeError): result = self.file._preserve_times(self.file.filename, save) + + +class FileNamingTest(PicardTestCase): + + def setUp(self): + super().setUp() + self.file = File('/somepath/somefile.mp3') + config.setting = { + 'ascii_filenames': False, + 'clear_existing_tags': False, + 'enabled_plugins': [], + 'file_naming_format': '%album%/%title%', + 'move_files_to': '/media/music', + 'move_files': False, + 'rename_files': False, + 'windows_compatibility': True, + } + self.metadata = Metadata({ + 'album': 'somealbum', + 'title': 'sometitle', + }) + + def test_make_filename_no_move_and_rename(self): + filename = self.file.make_filename(self.file.filename, self.metadata) + self.assertEqual(self.file.filename, filename) + + def test_make_filename_rename_only(self): + config.setting['rename_files'] = True + filename = self.file.make_filename(self.file.filename, self.metadata) + self.assertEqual('/somepath/sometitle.mp3', filename) + + def test_make_filename_move_only(self): + config.setting['move_files'] = True + filename = self.file.make_filename(self.file.filename, self.metadata) + self.assertEqual('/media/music/somealbum/somefile.mp3', filename) + + def test_make_filename_move_and_rename(self): + config.setting['rename_files'] = True + config.setting['move_files'] = True + filename = self.file.make_filename(self.file.filename, self.metadata) + self.assertEqual('/media/music/somealbum/sometitle.mp3', filename) + + def test_make_filename_move_relative_path(self): + config.setting['move_files'] = True + config.setting['move_files_to'] = 'subdir' + filename = self.file.make_filename(self.file.filename, self.metadata) + self.assertEqual('/somepath/subdir/somealbum/somefile.mp3', filename) + + def test_make_filename_replace_trailing_dots(self): + config.setting['rename_files'] = True + config.setting['move_files'] = True + config.setting['windows_compatibility'] = True + metadata = Metadata({ + 'album': 'somealbum.', + 'title': 'sometitle', + }) + filename = self.file.make_filename(self.file.filename, metadata) + self.assertEqual('/media/music/somealbum_/sometitle.mp3', filename) + + config.setting['windows_compatibility'] = False + filename = self.file.make_filename(self.file.filename, metadata) + self.assertEqual('/media/music/somealbum./sometitle.mp3', filename) + + def test_make_filename_replace_leading_dots(self): + config.setting['rename_files'] = True + config.setting['move_files'] = True + config.setting['windows_compatibility'] = True + metadata = Metadata({ + 'album': '.somealbum', + 'title': '.sometitle', + }) + filename = self.file.make_filename(self.file.filename, metadata) + self.assertEqual('/media/music/_somealbum/_sometitle.mp3', filename) From e1b8ba37c45e142f28cd0c5fb16adc90bc0f3245 Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Mon, 5 Aug 2019 19:39:41 +0200 Subject: [PATCH 3/4] PICARD-1559: Refactor renaming options UI The renaming script and compatibility settings are now independent of rename_files being enabled or not. --- picard/ui/ui_options_renaming.py | 39 ++--- ui/options_renaming.ui | 239 ++++++++++++++----------------- 2 files changed, 125 insertions(+), 153 deletions(-) diff --git a/picard/ui/ui_options_renaming.py b/picard/ui/ui_options_renaming.py index a3c2b5852..041f943e6 100644 --- a/picard/ui/ui_options_renaming.py +++ b/picard/ui/ui_options_renaming.py @@ -3,8 +3,10 @@ # Automatically generated - don't edit. # Use `python setup.py build_ui` to update it. + from PyQt5 import QtCore, QtGui, QtWidgets + class Ui_RenamingOptionsPage(object): def setupUi(self, RenamingOptionsPage): RenamingOptionsPage.setObjectName("RenamingOptionsPage") @@ -52,24 +54,16 @@ class Ui_RenamingOptionsPage(object): self.delete_empty_dirs.setObjectName("delete_empty_dirs") self.verticalLayout_4.addWidget(self.delete_empty_dirs) self.verticalLayout_5.addWidget(self.move_files) - self.rename_files = QtWidgets.QGroupBox(RenamingOptionsPage) - sizePolicy = QtWidgets.QSizePolicy(QtWidgets.QSizePolicy.Expanding, QtWidgets.QSizePolicy.Preferred) - sizePolicy.setHorizontalStretch(0) - sizePolicy.setVerticalStretch(0) - sizePolicy.setHeightForWidth(self.rename_files.sizePolicy().hasHeightForWidth()) - self.rename_files.setSizePolicy(sizePolicy) - self.rename_files.setCheckable(True) - self.rename_files.setChecked(False) + self.rename_files = QtWidgets.QCheckBox(RenamingOptionsPage) self.rename_files.setObjectName("rename_files") - self.verticalLayout_3 = QtWidgets.QVBoxLayout(self.rename_files) - self.verticalLayout_3.setObjectName("verticalLayout_3") - self.ascii_filenames = QtWidgets.QCheckBox(self.rename_files) + self.verticalLayout_5.addWidget(self.rename_files) + self.ascii_filenames = QtWidgets.QCheckBox(RenamingOptionsPage) self.ascii_filenames.setObjectName("ascii_filenames") - self.verticalLayout_3.addWidget(self.ascii_filenames) - self.windows_compatibility = QtWidgets.QCheckBox(self.rename_files) + self.verticalLayout_5.addWidget(self.ascii_filenames) + self.windows_compatibility = QtWidgets.QCheckBox(RenamingOptionsPage) self.windows_compatibility.setObjectName("windows_compatibility") - self.verticalLayout_3.addWidget(self.windows_compatibility) - self.file_naming_format_group = QtWidgets.QGroupBox(self.rename_files) + self.verticalLayout_5.addWidget(self.windows_compatibility) + self.file_naming_format_group = QtWidgets.QGroupBox(RenamingOptionsPage) self.file_naming_format_group.setObjectName("file_naming_format_group") self.verticalLayout_2 = QtWidgets.QVBoxLayout(self.file_naming_format_group) self.verticalLayout_2.setObjectName("verticalLayout_2") @@ -110,15 +104,14 @@ class Ui_RenamingOptionsPage(object): self.file_naming_format_default.setObjectName("file_naming_format_default") self.horizontalLayout.addWidget(self.file_naming_format_default) self.verticalLayout_2.addLayout(self.horizontalLayout) - self.verticalLayout_3.addWidget(self.file_naming_format_group) - self.file_naming_format_documentation = QtWidgets.QLabel(self.rename_files) + self.verticalLayout_5.addWidget(self.file_naming_format_group) + self.file_naming_format_documentation = QtWidgets.QLabel(RenamingOptionsPage) self.file_naming_format_documentation.setText("") self.file_naming_format_documentation.setTextFormat(QtCore.Qt.RichText) self.file_naming_format_documentation.setWordWrap(True) self.file_naming_format_documentation.setOpenExternalLinks(True) self.file_naming_format_documentation.setObjectName("file_naming_format_documentation") - self.verticalLayout_3.addWidget(self.file_naming_format_documentation) - self.verticalLayout_5.addWidget(self.rename_files) + self.verticalLayout_5.addWidget(self.file_naming_format_documentation) self.groupBox = QtWidgets.QGroupBox(RenamingOptionsPage) self.groupBox.setObjectName("groupBox") self.verticalLayout = QtWidgets.QVBoxLayout(self.groupBox) @@ -149,10 +142,7 @@ class Ui_RenamingOptionsPage(object): RenamingOptionsPage.setTabOrder(self.move_files_to_browse, self.move_additional_files) RenamingOptionsPage.setTabOrder(self.move_additional_files, self.move_additional_files_pattern) RenamingOptionsPage.setTabOrder(self.move_additional_files_pattern, self.delete_empty_dirs) - RenamingOptionsPage.setTabOrder(self.delete_empty_dirs, self.rename_files) - RenamingOptionsPage.setTabOrder(self.rename_files, self.ascii_filenames) - RenamingOptionsPage.setTabOrder(self.ascii_filenames, self.windows_compatibility) - RenamingOptionsPage.setTabOrder(self.windows_compatibility, self.file_naming_format) + RenamingOptionsPage.setTabOrder(self.delete_empty_dirs, self.file_naming_format) RenamingOptionsPage.setTabOrder(self.file_naming_format, self.file_naming_format_default) def retranslateUi(self, RenamingOptionsPage): @@ -162,7 +152,7 @@ class Ui_RenamingOptionsPage(object): self.move_files_to_browse.setText(_("Browse...")) self.move_additional_files.setText(_("Move additional files (case insensitive):")) self.delete_empty_dirs.setText(_("Delete empty directories")) - self.rename_files.setTitle(_("Rename files when saving")) + self.rename_files.setText(_("Rename files when saving")) self.ascii_filenames.setText(_("Replace non-ASCII characters")) self.windows_compatibility.setText(_("Windows compatibility")) self.file_naming_format_group.setTitle(_("Name files like this")) @@ -173,4 +163,3 @@ class Ui_RenamingOptionsPage(object): "


")) self.file_naming_format_default.setText(_("Default")) self.groupBox.setTitle(_("Examples")) - diff --git a/ui/options_renaming.ui b/ui/options_renaming.ui index bfb11b7c0..1afef9e4c 100644 --- a/ui/options_renaming.ui +++ b/ui/options_renaming.ui @@ -19,7 +19,7 @@ 0 - + @@ -97,150 +97,136 @@ - - - - 0 - 0 - - - + + Rename files when saving - - true + + + + + + Replace non-ASCII characters - - false + + + + + + Windows compatibility - + + + + + + Name files like this + + - - - Replace non-ASCII characters + + + false - - - - - - Windows compatibility + + + 0 + 0 + - - - - - - Name files like this + + + 0 + 0 + - - - - - false - - - - 0 - 0 - - - - - 0 - 0 - - - - - Monospace - - - - IBeamCursor - - - false - - - QTextEdit::NoWrap - - - <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0//EN" "http://www.w3.org/TR/REC-html40/strict.dtd"> + + + Monospace + + + + IBeamCursor + + + false + + + QTextEdit::NoWrap + + + <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0//EN" "http://www.w3.org/TR/REC-html40/strict.dtd"> <html><head><meta name="qrichtext" content="1" /><style type="text/css"> p, li { white-space: pre-wrap; } </style></head><body style=" font-family:'Monospace'; font-size:10pt; font-weight:400; font-style:normal;"> <p style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px;"><br /></p></body></html> - - - 20 - - - false - - - Qt::TextEditorInteraction - - - - - - - 2 - - - - - - - - Qt::AlignCenter - - - - - - - - 0 - 0 - - - - - 0 - 0 - - - - Default - - - - - - + + + 20 + + + false + + + Qt::TextEditorInteraction + - - - + + + 2 - - Qt::RichText - - - true - - - true - - + + + + + + + Qt::AlignCenter + + + + + + + + 0 + 0 + + + + + 0 + 0 + + + + Default + + + + + + + + + + + Qt::RichText + + + true + + + true + + + @@ -300,9 +286,6 @@ p, li { white-space: pre-wrap; } move_additional_files move_additional_files_pattern delete_empty_dirs - rename_files - ascii_filenames - windows_compatibility file_naming_format file_naming_format_default From 157d3cfc97342105b3963e96c93aca120ec6a6b0 Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Wed, 7 Aug 2019 08:47:22 +0200 Subject: [PATCH 4/4] Fixed file naming tests on Windows --- test/test_file.py | 45 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 11 deletions(-) diff --git a/test/test_file.py b/test/test_file.py index 981f93cef..7c9123fed 100644 --- a/test/test_file.py +++ b/test/test_file.py @@ -1,12 +1,15 @@ import os import shutil from tempfile import mkdtemp +import unittest from test.picardtestcase import PicardTestCase from picard import config -from picard.const import DEFAULT_FILE_NAMING_FORMAT -from picard.const.sys import IS_MACOS +from picard.const.sys import ( + IS_MACOS, + IS_WIN, +) from picard.file import File from picard.metadata import Metadata @@ -120,7 +123,7 @@ class TestPreserveTimes(PicardTestCase): os.remove(self.file.filename) with self.assertRaises(self.file.PreserveTimesUtimeError): - result = self.file._preserve_times(self.file.filename, save) + self.file._preserve_times(self.file.filename, save) class FileNamingTest(PicardTestCase): @@ -145,29 +148,35 @@ class FileNamingTest(PicardTestCase): def test_make_filename_no_move_and_rename(self): filename = self.file.make_filename(self.file.filename, self.metadata) - self.assertEqual(self.file.filename, filename) + self.assertEqual(os.path.realpath(self.file.filename), filename) def test_make_filename_rename_only(self): config.setting['rename_files'] = True filename = self.file.make_filename(self.file.filename, self.metadata) - self.assertEqual('/somepath/sometitle.mp3', filename) + self.assertEqual(os.path.realpath('/somepath/sometitle.mp3'), filename) def test_make_filename_move_only(self): config.setting['move_files'] = True filename = self.file.make_filename(self.file.filename, self.metadata) - self.assertEqual('/media/music/somealbum/somefile.mp3', filename) + self.assertEqual( + os.path.realpath('/media/music/somealbum/somefile.mp3'), + filename) def test_make_filename_move_and_rename(self): config.setting['rename_files'] = True config.setting['move_files'] = True filename = self.file.make_filename(self.file.filename, self.metadata) - self.assertEqual('/media/music/somealbum/sometitle.mp3', filename) + self.assertEqual( + os.path.realpath('/media/music/somealbum/sometitle.mp3'), + filename) def test_make_filename_move_relative_path(self): config.setting['move_files'] = True config.setting['move_files_to'] = 'subdir' filename = self.file.make_filename(self.file.filename, self.metadata) - self.assertEqual('/somepath/subdir/somealbum/somefile.mp3', filename) + self.assertEqual( + os.path.realpath('/somepath/subdir/somealbum/somefile.mp3'), + filename) def test_make_filename_replace_trailing_dots(self): config.setting['rename_files'] = True @@ -178,11 +187,23 @@ class FileNamingTest(PicardTestCase): 'title': 'sometitle', }) filename = self.file.make_filename(self.file.filename, metadata) - self.assertEqual('/media/music/somealbum_/sometitle.mp3', filename) + self.assertEqual( + os.path.realpath('/media/music/somealbum_/sometitle.mp3'), + filename) + @unittest.skipUnless(not IS_WIN, "non-windows test") + def test_make_filename_keep_trailing_dots(self): + config.setting['rename_files'] = True + config.setting['move_files'] = True config.setting['windows_compatibility'] = False + metadata = Metadata({ + 'album': 'somealbum.', + 'title': 'sometitle', + }) filename = self.file.make_filename(self.file.filename, metadata) - self.assertEqual('/media/music/somealbum./sometitle.mp3', filename) + self.assertEqual( + os.path.realpath('/media/music/somealbum./sometitle.mp3'), + filename) def test_make_filename_replace_leading_dots(self): config.setting['rename_files'] = True @@ -193,4 +214,6 @@ class FileNamingTest(PicardTestCase): 'title': '.sometitle', }) filename = self.file.make_filename(self.file.filename, metadata) - self.assertEqual('/media/music/_somealbum/_sometitle.mp3', filename) + self.assertEqual( + os.path.realpath('/media/music/_somealbum/_sometitle.mp3'), + filename)