From acaf5ed51071ef01866ebcee76904950813f003d Mon Sep 17 00:00:00 2001 From: Timmy Welch Date: Sat, 17 Sep 2022 01:28:26 -0700 Subject: [PATCH] Fix issues with renaming Stop a crash when renaming Properly handle replacements on linux/macos --- comictaggerlib/cli.py | 5 +++- comictaggerlib/filerenamer.py | 39 ++++++++++++++++++++++--------- comictaggerlib/renamewindow.py | 42 ++++++++++++++++++++-------------- testing/filenames.py | 7 ++++++ tests/comicarchive_test.py | 12 ++++++++++ 5 files changed, 76 insertions(+), 29 deletions(-) diff --git a/comictaggerlib/cli.py b/comictaggerlib/cli.py index ece4288..b76e9fe 100644 --- a/comictaggerlib/cli.py +++ b/comictaggerlib/cli.py @@ -525,7 +525,10 @@ def process_file_cli( suffix = "" if not opts.dryrun: # rename the file - ca.rename(utils.unique_file(full_path)) + try: + ca.rename(utils.unique_file(full_path)) + except OSError: + logger.exception("Failed to rename comic archive: %s", ca.path) else: suffix = " (dry-run, no change)" diff --git a/comictaggerlib/filerenamer.py b/comictaggerlib/filerenamer.py index 6812555..561fc48 100644 --- a/comictaggerlib/filerenamer.py +++ b/comictaggerlib/filerenamer.py @@ -31,13 +31,28 @@ from comicapi.issuestring import IssueString logger = logging.getLogger(__name__) +class Replacement(NamedTuple): + find: str + replce: str + strict_only: bool + + class Replacements(NamedTuple): - literal_text: list[tuple[str, str]] - format_value: list[tuple[str, str]] + literal_text: list[Replacement] + format_value: list[Replacement] REPLACEMENTS = Replacements( - literal_text=[(": ", " - "), (":", "-")], format_value=[(": ", " - "), (":", "-"), ("/", "-"), ("\\", "-")] + literal_text=[ + Replacement(": ", " - ", True), + Replacement(":", "-", True), + ], + format_value=[ + Replacement(": ", " - ", True), + Replacement(":", "-", True), + Replacement("/", "-", False), + Replacement("\\", "-", True), + ], ) @@ -64,11 +79,15 @@ class MetadataFormatter(string.Formatter): return "" return cast(str, super().format_field(value, format_spec)) - def handle_replacements(self, string: str, replacements: list[tuple[str, str]]) -> str: - for f, r in replacements: - string = string.replace(f, r) + def handle_replacements(self, string: str, replacements: list[Replacement]) -> str: + for find, replace, strict_only in replacements: + if self.is_strict() or not strict_only: + string = string.replace(find, replace) return string + def is_strict(self) -> bool: + return self.platform in [Platform.UNIVERSAL, Platform.WINDOWS] + def _vformat( self, format_string: str, @@ -89,8 +108,7 @@ class MetadataFormatter(string.Formatter): if lstrip: literal_text = literal_text.lstrip("-_)}]#") if self.smart_cleanup: - if self.platform in [Platform.UNIVERSAL, Platform.WINDOWS]: - literal_text = self.handle_replacements(literal_text, self.replacements.literal_text) + literal_text = self.handle_replacements(literal_text, self.replacements.literal_text) lspace = literal_text[0].isspace() if literal_text else False rspace = literal_text[-1].isspace() if literal_text else False literal_text = " ".join(literal_text.split()) @@ -136,9 +154,8 @@ class MetadataFormatter(string.Formatter): result[-1], _, _ = result[-1].rstrip().rpartition(" ") result[-1] = result[-1].rstrip("-_({[#") if self.smart_cleanup: - if self.platform in [Platform.UNIVERSAL, Platform.WINDOWS]: - # colons and slashes get special treatment - fmt_obj = self.handle_replacements(fmt_obj, self.replacements.format_value) + # colons and slashes get special treatment + fmt_obj = self.handle_replacements(fmt_obj, self.replacements.format_value) fmt_obj = " ".join(fmt_obj.split()) fmt_obj = str(sanitize_filename(fmt_obj, platform=self.platform)) result.append(fmt_obj) diff --git a/comictaggerlib/renamewindow.py b/comictaggerlib/renamewindow.py index 2fee991..6380785 100644 --- a/comictaggerlib/renamewindow.py +++ b/comictaggerlib/renamewindow.py @@ -175,29 +175,37 @@ class RenameWindow(QtWidgets.QDialog): center_window_on_parent(prog_dialog) QtCore.QCoreApplication.processEvents() - for idx, comic in enumerate(zip(self.comic_archive_list, self.rename_list)): + try: + for idx, comic in enumerate(zip(self.comic_archive_list, self.rename_list)): - QtCore.QCoreApplication.processEvents() - if prog_dialog.wasCanceled(): - break - idx += 1 - prog_dialog.setValue(idx) - prog_dialog.setLabelText(comic[1]) - center_window_on_parent(prog_dialog) - QtCore.QCoreApplication.processEvents() + QtCore.QCoreApplication.processEvents() + if prog_dialog.wasCanceled(): + break + idx += 1 + prog_dialog.setValue(idx) + prog_dialog.setLabelText(comic[1]) + center_window_on_parent(prog_dialog) + QtCore.QCoreApplication.processEvents() - folder = get_rename_dir(comic[0], self.settings.rename_dir if self.settings.rename_move_dir else None) + folder = get_rename_dir(comic[0], self.settings.rename_dir if self.settings.rename_move_dir else None) - full_path = folder / comic[1] + full_path = folder / comic[1] - if full_path == comic[0].path: - logger.info("%s: Filename is already good!", comic[1]) - continue + if full_path == comic[0].path: + logger.info("%s: Filename is already good!", comic[1]) + continue - if not comic[0].is_writable(check_rar_status=False): - continue + if not comic[0].is_writable(check_rar_status=False): + continue - comic[0].rename(utils.unique_file(full_path)) + comic[0].rename(utils.unique_file(full_path)) + except Exception as e: + logger.exception("Failed to rename comic archive: %s", comic[0].path) + QtWidgets.QMessageBox.critical( + self, + "There was an issue when renaming!", + f"Renaming failed!

{type(e).__name__}: {e}

", + ) prog_dialog.hide() QtCore.QCoreApplication.processEvents() diff --git a/testing/filenames.py b/testing/filenames.py index ffcb467..79fdb23 100644 --- a/testing/filenames.py +++ b/testing/filenames.py @@ -792,6 +792,13 @@ rnames = [ "Anda's Game https---comicvine.gamespot.com-cory-doctorows-futuristic-tales-of-the-here-and-no-4000-140529-.cbz", does_not_raise(), ), + ( + "{title} {web_link}", # Ensure slashes are replaced in metadata on linux/macos + False, + "Linux", + "Anda's Game https:--comicvine.gamespot.com-cory-doctorows-futuristic-tales-of-the-here-and-no-4000-140529-.cbz", + does_not_raise(), + ), ( "{series}:{title} #{issue} ({year})", # on windows the ':' is replaced False, diff --git a/tests/comicarchive_test.py b/tests/comicarchive_test.py index 0f1aadf..0cd8cee 100644 --- a/tests/comicarchive_test.py +++ b/tests/comicarchive_test.py @@ -98,3 +98,15 @@ def test_rename(tmp_comic, tmp_path): assert not old_path.exists() assert tmp_comic.path.exists() assert tmp_comic.path != old_path + + +def test_rename_ro_dest(tmp_comic, tmp_path): + old_path = tmp_comic.path + dest = tmp_path / "tmp" + dest.mkdir(mode=0o000) + with pytest.raises(OSError): + tmp_comic.rename(dest / "test.cbz") + dest.chmod(mode=0o777) + assert old_path.exists() + assert tmp_comic.path.exists() + assert tmp_comic.path == old_path