Style and typo fixes

Several custom conversions (the s in {title!s}) have been created
u - str.upper()
l - str.casefold()
S - str.swapcase()
t - str.title()
c - str.Capitalize()

A new syntax has been added '{title+str}' and '{title-str}':
The + indicates an alternate value.
The - indicates a default value.

If the title of a comic is not set then
'{title-str}' will output 'str'
and
'{title+str} will output ''

If the title of a comic is 'hello' then
'{title+str}' will output 'str'
and
'{title-str}' will output 'hello'

.github/workflows/build.yaml

@@ -80,11 +80,25 @@ jobs:
         run: |
           choco install -y zip
         if: runner.os == 'Windows'
+      - name: Install macos dependencies
+        run: |
+          brew install icu4c pkg-config
+          export PKG_CONFIG_PATH="/usr/local/opt/icu4c/lib/pkgconfig";
+          export PATH="/usr/local/opt/icu4c/bin:/usr/local/opt/icu4c/sbin:$PATH"
+          python -m pip install --no-binary=pyicu pyicu
+        if: runner.os == 'macOS'
+      - name: Install linux dependencies
+        run: |
+          sudo apt-get install pkg-config libicu-dev
+          export PKG_CONFIG_PATH="/usr/local/opt/icu4c/lib/pkgconfig";
+          export PATH="/usr/local/opt/icu4c/bin:/usr/local/opt/icu4c/sbin:$PATH"
+          python -m pip install --no-binary=pyicu pyicu
+        if: runner.os == 'Linux'
 
       - name: Build and install PyPi packages
         run: |
           make clean pydist
-          python -m pip install "dist/$(python setup.py --fullname)-py3-none-any.whl[GUI,CBR]"
+          python -m pip install "dist/$(python setup.py --fullname)-py3-none-any.whl[all]"
 
       - name: build
         run: |

.github/workflows/package.yaml

@@ -41,12 +41,25 @@ jobs:
         run: |
           choco install -y zip
         if: runner.os == 'Windows'
+      - name: Install macos dependencies
+        run: |
+          brew install icu4c pkg-config
+          export PKG_CONFIG_PATH="/usr/local/opt/icu4c/lib/pkgconfig";
+          export PATH="/usr/local/opt/icu4c/bin:/usr/local/opt/icu4c/sbin:$PATH"
+          python -m pip install --no-binary=pyicu pyicu
+        if: runner.os == 'macOS'
+      - name: Install linux dependencies
+        run: |
+          sudo apt-get install pkg-config libicu-dev
+          export PKG_CONFIG_PATH="/usr/local/opt/icu4c/lib/pkgconfig";
+          export PATH="/usr/local/opt/icu4c/bin:/usr/local/opt/icu4c/sbin:$PATH"
+          python -m pip install --no-binary=pyicu pyicu
+        if: runner.os == 'Linux'
 
       - name: Build, Install and Test PyPi packages
         run: |
           make clean pydist
-          python -m pip install "dist/$(python setup.py --fullname)-py3-none-any.whl[GUI,CBR]"
-          echo "CT_FULL_NAME=$(python setup.py --fullname)" >> $GITHUB_ENV
+          python -m pip install "dist/$(python setup.py --fullname)-py3-none-any.whl[all]"
           python -m flake8
           python -m pytest
 
@@ -61,12 +74,19 @@ jobs:
         run: |
           make dist
 
+      - name: Get release name
+        if: startsWith(github.ref, 'refs/tags/')
+        shell: bash
+        run: |
+          echo "release_name=$(git tag -l --format "%(refname:strip=2): %(contents:lines=1)" ${{ github.ref_name }})" >> $GITHUB_ENV
+
       - name: Release
         uses: softprops/action-gh-release@v1
         if: startsWith(github.ref, 'refs/tags/')
         with:
+          name: env.release_name
           prerelease: "${{ contains(github.ref, '-') }}" # alpha-releases should be 1.3.0-alpha.x full releases should be 1.3.0
           draft: false
           files: |
             dist/!(*Linux).zip
-            dist/*.whl
+            dist/*${{ fromJSON('["never", ""]')[runner.os == 'Linux'] }}.whl

.pre-commit-config.yaml

@@ -1,7 +1,7 @@
 exclude: ^scripts
 repos:
 -   repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.2.0
+    rev: v4.3.0
     hooks:
     -   id: trailing-whitespace
     -   id: end-of-file-fixer
@@ -10,7 +10,7 @@ repos:
     -   id: name-tests-test
     -   id: requirements-txt-fixer
 -   repo: https://github.com/asottile/setup-cfg-fmt
-    rev: v1.20.1
+    rev: v2.0.0
     hooks:
     -   id: setup-cfg-fmt
 -   repo: https://github.com/PyCQA/isort
@@ -19,12 +19,12 @@ repos:
     -   id: isort
         args: [--af,--add-import, 'from __future__ import annotations']
 -   repo: https://github.com/asottile/pyupgrade
-    rev: v2.32.1
+    rev: v2.37.3
     hooks:
     -   id: pyupgrade
         args: [--py39-plus]
 -   repo: https://github.com/psf/black
-    rev: 22.3.0
+    rev: 22.6.0
     hooks:
     -   id: black
 -   repo: https://github.com/PyCQA/autoflake
@@ -33,12 +33,12 @@ repos:
     -   id: autoflake
         args: [-i]
 -   repo: https://github.com/PyCQA/flake8
-    rev: 4.0.1
+    rev: 5.0.4
     hooks:
     -   id: flake8
-        additional_dependencies: [flake8-encodings, flake8-warnings, flake8-builtins, flake8-eradicate, flake8-length, flake8-print]
+        additional_dependencies: [flake8-encodings, flake8-warnings, flake8-builtins, flake8-length, flake8-print]
 -   repo: https://github.com/pre-commit/mirrors-mypy
-    rev: v0.960
+    rev: v0.971
     hooks:
     -   id: mypy
         additional_dependencies: [types-setuptools, types-requests]

README.md

@@ -2,6 +2,7 @@
 [![GitHub release (latest by date)](https://img.shields.io/github/downloads/comictagger/comictagger/latest/total)](https://github.com/comictagger/comictagger/releases/latest)
 [![PyPI](https://img.shields.io/pypi/v/comictagger)](https://pypi.org/project/comictagger/)
 [![PyPI - Downloads](https://img.shields.io/pypi/dm/comictagger)](https://pypistats.org/packages/comictagger)
+[![Chocolatey package](https://img.shields.io/chocolatey/dt/comictagger?color=blue&label=chocolatey)](https://community.chocolatey.org/packages/comictagger)
 [![PyPI - License](https://img.shields.io/pypi/l/comictagger)](https://opensource.org/licenses/Apache-2.0)
 
 [![GitHub Discussions](https://img.shields.io/github/discussions/comictagger/comictagger)](https://github.com/comictagger/comictagger/discussions)
@@ -48,6 +49,12 @@ A pip package is provided, you can install it with:
 
 There are two optional dependencies GUI and CBR. You can install the optional dependencies by specifying one or more of `GUI`,`CBR` or `all` in braces e.g. `comictagger[CBR,GUI]`
 
+### Chocolatey installation (Windows only)
+
+A [Chocolatey package](https://community.chocolatey.org/packages/comictagger), maintained by @Xav83, is provided, you can install it with:
+```powershell
+choco install comictagger
+```
 ### From source
 
  1. Ensure you have python 3.9 installed

comicapi/comet.py

@@ -88,9 +88,9 @@ class CoMet:
             assign("readingDirection", "rtl")
 
         if md.year is not None:
-            date_str = str(md.year).zfill(4)
+            date_str = f"{md.year:04}"
             if md.month is not None:
-                date_str += "-" + str(md.month).zfill(2)
+                date_str += f"-{md.month:02}"
             assign("date", date_str)
 
         assign("coverImage", md.cover_image)

comicapi/comicarchive.py

@@ -114,7 +114,7 @@ class SevenZipArchiver(UnknownArchiver):
         return False
 
     def read_file(self, archive_file: str) -> bytes:
-        data = bytes()
+        data = b""
         try:
             with py7zr.SevenZipFile(self.path, "r") as zf:
                 data = zf.read(archive_file)[archive_file].read()
@@ -148,7 +148,7 @@ class SevenZipArchiver(UnknownArchiver):
     def get_filename_list(self) -> list[str]:
         try:
             with py7zr.SevenZipFile(self.path, "r") as zf:
-                namelist: list[str] = zf.getnames()
+                namelist: list[str] = [file.filename for file in zf.list() if not file.is_directory]
 
             return namelist
         except (py7zr.Bad7zFile, OSError) as e:
@@ -248,7 +248,7 @@ class ZipArchiver(UnknownArchiver):
     def get_filename_list(self) -> list[str]:
         try:
             with zipfile.ZipFile(self.path, mode="r") as zf:
-                namelist = zf.namelist()
+                namelist = [file.filename for file in zf.infolist() if not file.is_dir()]
             return namelist
         except (zipfile.BadZipfile, OSError) as e:
             logger.error("Error listing files in zip archive [%s]: %s", e, self.path)
@@ -379,14 +379,15 @@ class RarArchiver(UnknownArchiver):
 
     def get_comment(self) -> str:
         rarc = self.get_rar_obj()
-        return str(rarc.comment) if rarc else ""
+        return rarc.comment.decode("utf-8") if rarc else ""
 
     def set_comment(self, comment: str) -> bool:
         if rar_support and self.rar_exe_path:
             try:
                 # write comment to temp file
-                with tempfile.NamedTemporaryFile() as tmp_file:
-                    tmp_file.write(comment.encode("utf-8"))
+                with tempfile.TemporaryDirectory() as tmp_dir:
+                    tmp_file = pathlib.Path(tmp_dir) / "rar_comment.txt"
+                    tmp_file.write_text(comment, encoding="utf-8")
 
                     working_dir = os.path.dirname(os.path.abspath(self.path))
 
@@ -396,7 +397,7 @@ class RarArchiver(UnknownArchiver):
                         "c",
                         f"-w{working_dir}",
                         "-c-",
-                        f"-z{tmp_file.name}",
+                        f"-z{tmp_file}",
                         str(self.path),
                     ]
                     subprocess.run(
@@ -422,7 +423,7 @@ class RarArchiver(UnknownArchiver):
 
         rarc = self.get_rar_obj()
         if rarc is None:
-            return bytes()
+            return b""
 
         tries = 0
         while tries < 7:
@@ -665,7 +666,7 @@ class FolderArchiver(UnknownArchiver):
 
 
 class ComicArchive:
-    logo_data = bytes()
+    logo_data = b""
 
     class ArchiveType:
         SevenZip, Zip, Rar, Folder, Pdf, Unknown = list(range(6))
@@ -683,7 +684,7 @@ class ComicArchive:
         self._has_cbi: bool | None = None
         self._has_cix: bool | None = None
         self._has_comet: bool | None = None
-        self.path = pathlib.Path(path)
+        self.path = pathlib.Path(path).absolute()
         self.page_count: int | None = None
         self.page_list: list[str] = []
 
@@ -750,7 +751,7 @@ class ComicArchive:
         if new_path == self.path:
             return
         os.makedirs(new_path.parent, 0o777, True)
-        shutil.move(path, new_path)
+        shutil.move(self.path, new_path)
         self.path = new_path
         self.archiver.path = pathlib.Path(path)
 
@@ -853,13 +854,13 @@ class ComicArchive:
         return retcode
 
     def get_page(self, index: int) -> bytes:
-        image_data = bytes()
+        image_data = b""
 
         filename = self.get_page_name(index)
 
         if filename:
             try:
-                image_data = self.archiver.read_file(filename) or bytes()
+                image_data = self.archiver.read_file(filename) or b""
             except Exception:
                 logger.error("Error reading in page %d. Substituting logo page.", index)
                 image_data = ComicArchive.logo_data
@@ -934,7 +935,7 @@ class ComicArchive:
             # seems like some archive creators are on Windows, and don't know about case-sensitivity!
             if sort_list:
 
-                files = cast(list[str], natsort.natsorted(files, alg=natsort.ns.IC | natsort.ns.I | natsort.ns.U))
+                files = cast(list[str], natsort.os_sorted(files))
 
             # make a sub-list of image files
             self.page_list = []
@@ -1033,7 +1034,7 @@ class ComicArchive:
             raw_cix = self.archiver.read_file(self.ci_xml_filename) or b""
         except Exception as e:
             logger.error("Error reading in raw CIX! for %s: %s", self.path, e)
-            raw_cix = bytes()
+            raw_cix = b""
         return raw_cix
 
     def write_cix(self, metadata: GenericMetadata) -> bool:

comicapi/comicbookinfo.py

@@ -97,7 +97,14 @@ class ComicBookInfo:
         metadata.country = utils.xlate(cbi["country"])
         metadata.critical_rating = utils.xlate(cbi["rating"], True)
 
-        metadata.credits = cbi["credits"]
+        metadata.credits = [
+            Credits(
+                person=x["person"] if "person" in x else "",
+                role=x["role"] if "role" in x else "",
+                primary=x["primary"] if "primary" in x else False,
+            )
+            for x in cbi["credits"]
+        ]
         metadata.tags = set(cbi["tags"]) if cbi["tags"] is not None else set()
 
         # make sure credits and tags are at least empty lists and not None
@@ -105,8 +112,8 @@ class ComicBookInfo:
             metadata.credits = []
 
         # need the language string to be ISO
-        if metadata.language is not None:
-            metadata.language = utils.get_language(metadata.language)
+        if metadata.language:
+            metadata.language = utils.get_language_iso(metadata.language)
 
         metadata.is_empty = False
 

comicapi/filenameparser.py

@@ -24,17 +24,21 @@ import logging
 import os
 import re
 from operator import itemgetter
-from typing import Callable, Match, TypedDict
+from re import Match
+from typing import Callable, TypedDict
 from urllib.parse import unquote
 
 from text2digits import text2digits
 
 from comicapi import filenamelexer, issuestring
 
+logger = logging.getLogger(__name__)
+
 t2d = text2digits.Text2Digits(add_ordinal_ending=False)
 t2do = text2digits.Text2Digits(add_ordinal_ending=True)
 
-logger = logging.getLogger(__name__)
+placeholders_no_dashes = [re.compile(r"[-_]"), re.compile(r"  +")]
+placeholders_allow_dashes = [re.compile(r"[_]"), re.compile(r"  +")]
 
 
 class FileNameParser:
@@ -53,9 +57,9 @@ class FileNameParser:
 
     def fix_spaces(self, string: str, remove_dashes: bool = True) -> str:
         if remove_dashes:
-            placeholders = [r"[-_]", r"  +"]
+            placeholders = placeholders_no_dashes
         else:
-            placeholders = [r"[_]", r"  +"]
+            placeholders = placeholders_allow_dashes
         for ph in placeholders:
             string = re.sub(ph, self.repl, string)
         return string
@@ -67,17 +71,10 @@ class FileNameParser:
 
         # replace any name separators with spaces
         tmpstr = self.fix_spaces(filename)
-        found = False
 
-        match = re.search(r"(?<=\sof\s)\d+(?=\s)", tmpstr, re.IGNORECASE)
+        match = re.search(r"(?:\s\(?of\s)(\d+)(?: |\))", tmpstr, re.IGNORECASE)
         if match:
-            count = match.group()
-            found = True
-
-        if not found:
-            match = re.search(r"(?<=\(of\s)\d+(?=\))", tmpstr, re.IGNORECASE)
-            if match:
-                count = match.group()
+            count = match.group(1)
 
         return count.lstrip("0")
 
@@ -180,10 +177,12 @@ class FileNameParser:
         if "--" in filename:
             # the pattern seems to be that anything to left of the first "--" is the series name followed by issue
             filename = re.sub(r"--.*", self.repl, filename)
+            # never happens
 
         elif "__" in filename:
             # the pattern seems to be that anything to left of the first "__" is the series name followed by issue
             filename = re.sub(r"__.*", self.repl, filename)
+            # never happens
 
         filename = filename.replace("+", " ")
         tmpstr = self.fix_spaces(filename, remove_dashes=False)
@@ -425,6 +424,7 @@ def parse(p: Parser) -> Callable[[Parser], Callable | None] | None:
         likely_year = False
         if p.firstItem and p.first_is_alt:
             p.alt = True
+            p.firstItem = False
             return parse_issue_number
 
         # The issue number should hopefully not be in parentheses
@@ -440,9 +440,6 @@ def parse(p: Parser) -> Callable[[Parser], Callable | None] | None:
                             # Series has already been started/parsed,
                             # filters out leading alternate numbers leading alternate number
                             if len(p.series_parts) > 0:
-                                # Unset first item
-                                if p.firstItem:
-                                    p.firstItem = False
                                 return parse_issue_number
             else:
                 p.operator_rejected.append(item)
@@ -506,9 +503,6 @@ def parse(p: Parser) -> Callable[[Parser], Callable | None] | None:
             p.series_parts.append(item)
             p.used_items.append(item)
 
-            # Unset first item
-            if p.firstItem:
-                p.firstItem = False
             p.get()
             return parse_series
 
@@ -590,6 +584,8 @@ def parse(p: Parser) -> Callable[[Parser], Callable | None] | None:
         if series_append:
             p.series_parts.append(item)
             p.used_items.append(item)
+            if p.firstItem:
+                p.firstItem = False
             return parse_series
 
     # We found text, it's probably the title or series
@@ -602,6 +598,8 @@ def parse(p: Parser) -> Callable[[Parser], Callable | None] | None:
 
     # Usually the word 'of' eg 1 (of 6)
     elif item.typ == filenamelexer.ItemType.InfoSpecifier:
+        if p.firstItem:
+            p.firstItem = False
         return parse_info_specifier
 
     # Operator is a symbol that acts as some sort of separator eg - : ;
@@ -622,9 +620,13 @@ def parse(p: Parser) -> Callable[[Parser], Callable | None] | None:
                 p.irrelevant.extend([item, p.input[p.pos], p.get()])
             else:
                 p.backup()
+                if p.firstItem:
+                    p.firstItem = False
                 return parse_series
         # This is text that just happens to also be a month/day
         else:
+            if p.firstItem:
+                p.firstItem = False
             return parse_series
 
     # Specifically '__' or '--', no further title/series parsing is done to keep compatibility with wiki
@@ -853,10 +855,11 @@ def resolve_year(p: Parser) -> None:
             p.used_items.append(vol)
 
             # Remove volume from series and title
-            if selected_year in p.series_parts:
-                p.series_parts.remove(selected_year)
-            if selected_year in p.title_parts:
-                p.title_parts.remove(selected_year)
+            # note: this never happens
+            if vol in p.series_parts:
+                p.series_parts.remove(vol)
+            if vol in p.title_parts:
+                p.title_parts.remove(vol)
 
         # Remove year from series and title
         if selected_year in p.series_parts:

comicapi/genericmetadata.py

@@ -93,7 +93,7 @@ class GenericMetadata:
     comments: str | None = None  # use same way as Summary in CIX
 
     volume_count: int | None = None
-    critical_rating: float | None = None  # rating in cbl; CommunityRating in CIX
+    critical_rating: float | None = None  # rating in CBL; CommunityRating in CIX
     country: str | None = None
 
     alternate_series: str | None = None
@@ -270,8 +270,10 @@ class GenericMetadata:
     def get_primary_credit(self, role: str) -> str:
         primary = ""
         for credit in self.credits:
+            if "role" not in credit or "person" not in credit:
+                continue
             if (primary == "" and credit["role"].casefold() == role.casefold()) or (
-                credit["role"].casefold() == role.casefold() and credit["primary"]
+                credit["role"].casefold() == role.casefold() and "primary" in credit and credit["primary"]
             ):
                 primary = credit["person"]
         return primary

comicapi/utils.py

@@ -19,11 +19,11 @@ import json
 import logging
 import os
 import pathlib
-import re
 import unicodedata
 from collections import defaultdict
+from collections.abc import Mapping
 from shutil import which  # noqa: F401
-from typing import Any, Mapping
+from typing import Any
 
 import pycountry
 import thefuzz.fuzz
@@ -57,22 +57,20 @@ def get_recursive_filelist(pathlist: list[str]) -> list[str]:
 
         if os.path.isdir(p):
             filelist.extend(x for x in glob.glob(f"{p}{os.sep}/**", recursive=True) if not os.path.isdir(x))
-        else:
-            filelist.append(p)
+        elif str(p) not in filelist:
+            filelist.append(str(p))
 
     return filelist
 
 
 def add_to_path(dirname: str) -> None:
-    if dirname is not None and dirname != "":
+    if dirname:
+        dirname = os.path.abspath(dirname)
+        paths = [os.path.normpath(x) for x in os.environ["PATH"].split(os.pathsep)]
 
-        # verify that path doesn't already contain the given dirname
-        tmpdirname = re.escape(dirname)
-        pattern = r"(^|{sep}){dir}({sep}|$)".format(dir=tmpdirname, sep=os.pathsep)
-
-        match = re.search(pattern, os.environ["PATH"])
-        if not match:
-            os.environ["PATH"] = dirname + os.pathsep + os.environ["PATH"]
+        if dirname not in paths:
+            paths.insert(0, dirname)
+            os.environ["PATH"] = os.pathsep.join(paths)
 
 
 def xlate(data: Any, is_int: bool = False, is_float: bool = False) -> Any:
@@ -123,13 +121,9 @@ def remove_articles(text: str) -> str:
         "the",
         "the",
         "with",
-        "ms",
-        "mrs",
-        "mr",
-        "dr",
     ]
     new_text = ""
-    for word in text.split(" "):
+    for word in text.split():
         if word not in articles:
             new_text += word + " "
 
@@ -141,19 +135,16 @@ def remove_articles(text: str) -> str:
 def sanitize_title(text: str, basic: bool = False) -> str:
     # normalize unicode and convert to ascii. Does not work for everything eg ½ to 1⁄2 not 1/2
     text = unicodedata.normalize("NFKD", text).casefold()
-    if basic:
-        # comicvine keeps apostrophes a part of the word
-        text = text.replace("'", "")
-        text = text.replace('"', "")
-    else:
+    # comicvine keeps apostrophes a part of the word
+    text = text.replace("'", "")
+    text = text.replace('"', "")
+    if not basic:
         # comicvine ignores punctuation and accents
         # remove all characters that are not a letter, separator (space) or number
         # replace any "dash punctuation" with a space
         # makes sure that batman-superman and self-proclaimed stay separate words
         text = "".join(
-            c if not unicodedata.category(c) in ("Pd",) else " "
-            for c in text
-            if unicodedata.category(c)[0] in "LZN" or unicodedata.category(c) in ("Pd",)
+            c if unicodedata.category(c)[0] not in "P" else " " for c in text if unicodedata.category(c)[0] in "LZNP"
         )
         # remove extra space and articles and all lower case
         text = remove_articles(text).strip()
@@ -161,10 +152,10 @@ def sanitize_title(text: str, basic: bool = False) -> str:
     return text
 
 
-def titles_match(search_title: str, record_title: str, threshold: int = 90) -> int:
+def titles_match(search_title: str, record_title: str, threshold: int = 90) -> bool:
     sanitized_search = sanitize_title(search_title)
     sanitized_record = sanitize_title(record_title)
-    ratio = thefuzz.fuzz.ratio(sanitized_search, sanitized_record)
+    ratio: int = thefuzz.fuzz.ratio(sanitized_search, sanitized_record)
     logger.debug(
         "search title: %s ; record title: %s ; ratio: %d ; match threshold: %d",
         search_title,
@@ -176,12 +167,12 @@ def titles_match(search_title: str, record_title: str, threshold: int = 90) -> i
 
 
 def unique_file(file_name: pathlib.Path) -> pathlib.Path:
-    name = file_name.name
+    name = file_name.stem
     counter = 1
     while True:
         if not file_name.exists():
             return file_name
-        file_name = file_name.with_name(name + " (" + str(counter) + ")")
+        file_name = file_name.with_stem(name + " (" + str(counter) + ")")
         counter += 1
 
 
@@ -202,23 +193,19 @@ def get_language_from_iso(iso: str | None) -> str | None:
     return languages[iso]
 
 
-def get_language(string: str | None) -> str | None:
+def get_language_iso(string: str | None) -> str | None:
     if string is None:
         return None
+    lang = string.casefold()
 
-    lang = get_language_from_iso(string)
-
-    if lang is None:
-        try:
-            return str(pycountry.languages.lookup(string).name)
-        except LookupError:
-            return None
+    try:
+        return getattr(pycountry.languages.lookup(string), "alpha_2", None)
+    except LookupError:
+        pass
     return lang
 
 
 def get_publisher(publisher: str) -> tuple[str, str]:
-    if publisher is None:
-        return ("", "")
     imprint = ""
 
     for pub in publishers.values():

comictaggerlib/cli.py

@@ -165,13 +165,13 @@ def post_process_matches(
 
 
 def cli_mode(opts: argparse.Namespace, settings: ComicTaggerSettings) -> None:
-    if len(opts.files) < 1:
+    if len(opts.file_list) < 1:
         logger.error("You must specify at least one filename.  Use the -h option for more info")
         return
 
     match_results = OnlineMatchResults()
 
-    for f in opts.files:
+    for f in opts.file_list:
         process_file_cli(f, opts, settings, match_results)
         sys.stdout.flush()
 
@@ -212,7 +212,7 @@ def create_local_metadata(opts: argparse.Namespace, ca: ComicArchive, settings:
 def process_file_cli(
     filename: str, opts: argparse.Namespace, settings: ComicTaggerSettings, match_results: OnlineMatchResults
 ) -> None:
-    batch_mode = len(opts.files) > 1
+    batch_mode = len(opts.file_list) > 1
 
     ca = ComicArchive(filename, settings.rar_exe_path, ComicTaggerSettings.get_graphic("nocover.png"))
 
@@ -472,7 +472,7 @@ def process_file_cli(
             match_results.good_matches.append(str(ca.path.absolute()))
 
     elif opts.rename:
-
+        original_path = ca.path
         msg_hdr = ""
         if batch_mode:
             msg_hdr = f"{ca.path}: "
@@ -500,15 +500,19 @@ def process_file_cli(
 
         try:
             new_name = renamer.determine_name(ext=new_ext)
-        except Exception:
+        except ValueError:
             logger.exception(
                 msg_hdr + "Invalid format string!\n"
                 "Your rename template is invalid!\n\n"
+                "%s\n\n"
                 "Please consult the template help in the settings "
                 "and the documentation on the format at "
-                "https://docs.python.org/3/library/string.html#format-string-syntax"
+                "https://docs.python.org/3/library/string.html#format-string-syntax",
+                settings.rename_template,
             )
             return
+        except Exception:
+            logger.exception("Formatter failure: %s metadata: %s", settings.rename_template, renamer.metadata)
 
         folder = get_rename_dir(ca, settings.rename_dir if settings.rename_move_dir else None)
 
@@ -521,11 +525,14 @@ 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)"
 
-        print(f"renamed '{os.path.basename(ca.path)}' -> '{new_name}' {suffix}")
+        print(f"renamed '{original_path.name}' -> '{new_name}' {suffix}")
 
     elif opts.export_to_zip:
         msg_hdr = ""

comictaggerlib/comicvinetalker.py

@@ -21,6 +21,7 @@ import re
 import time
 from datetime import datetime
 from typing import Any, Callable, cast
+from urllib.parse import urlencode, urljoin, urlsplit
 
 import requests
 from bs4 import BeautifulSoup
@@ -104,7 +105,13 @@ class ComicVineTalker:
         self.issue_id: int | None = None
 
         self.api_key = ComicVineTalker.api_key or default_api_key
-        self.api_base_url = ComicVineTalker.api_base_url or default_url
+        tmp_url = urlsplit(ComicVineTalker.api_base_url or default_url)
+
+        # joinurl only works properly if there is a trailing slash
+        if tmp_url.path and tmp_url.path[-1] != "/":
+            tmp_url = tmp_url._replace(path=tmp_url.path + "/")
+
+        self.api_base_url = tmp_url.geturl()
 
         self.log_func: Callable[[str], None] | None = None
 
@@ -127,10 +134,16 @@ class ComicVineTalker:
         if not url:
             url = self.api_base_url
         try:
-            test_url = url + "/issue/1/?api_key=" + key + "&format=json&field_list=name"
+            test_url = urljoin(url, "issue/1/")
 
             cv_response: resulttypes.CVResult = requests.get(
-                test_url, headers={"user-agent": "comictagger/" + ctversion.version}
+                test_url,
+                headers={"user-agent": "comictagger/" + ctversion.version},
+                params={
+                    "api_key": key,
+                    "format": "json",
+                    "field_list": "name",
+                },
             ).json()
 
             # Bogus request, but if the key is wrong, you get error 100: "Invalid API Key"
@@ -171,8 +184,8 @@ class ComicVineTalker:
 
     def get_url_content(self, url: str, params: dict[str, Any]) -> Any:
         # connect to server:
-        #  if there is a 500 error, try a few more times before giving up
-        #  any other error, just bail
+        # if there is a 500 error, try a few more times before giving up
+        # any other error, just bail
         for tries in range(3):
             try:
                 resp = requests.get(url, params=params, headers={"user-agent": "comictagger/" + ctversion.version})
@@ -186,10 +199,15 @@ class ComicVineTalker:
                     break
 
             except requests.exceptions.RequestException as e:
-                self.write_log(str(e) + "\n")
+                self.write_log(f"{e}\n")
                 raise ComicVineTalkerException(ComicVineTalkerException.Network, "Network Error!") from e
+            except json.JSONDecodeError as e:
+                self.write_log(f"{e}\n")
+                raise ComicVineTalkerException(ComicVineTalkerException.Unknown, "ComicVine did not provide json")
 
-        raise ComicVineTalkerException(ComicVineTalkerException.Unknown, "Error on Comic Vine server")
+        raise ComicVineTalkerException(
+            ComicVineTalkerException.Unknown, f"Error on Comic Vine server: {resp.status_code}"
+        )
 
     def search_for_series(
         self,
@@ -222,7 +240,7 @@ class ComicVineTalker:
             "limit": 100,
         }
 
-        cv_response = self.get_cv_content(self.api_base_url + "/search", params)
+        cv_response = self.get_cv_content(urljoin(self.api_base_url, "search"), params)
 
         search_results: list[resulttypes.CVVolumeResults] = []
 
@@ -268,7 +286,7 @@ class ComicVineTalker:
             page += 1
 
             params["page"] = page
-            cv_response = self.get_cv_content(self.api_base_url + "/search", params)
+            cv_response = self.get_cv_content(urljoin(self.api_base_url, "search"), params)
 
             search_results.extend(cast(list[resulttypes.CVVolumeResults], cv_response["results"]))
             current_result_count += cv_response["number_of_page_results"]
@@ -291,7 +309,7 @@ class ComicVineTalker:
         if cached_volume_result is not None:
             return cached_volume_result
 
-        volume_url = self.api_base_url + "/volume/" + CVTypeID.Volume + "-" + str(series_id)
+        volume_url = urljoin(self.api_base_url, f"volume/{CVTypeID.Volume}-{series_id}")
 
         params = {
             "api_key": self.api_key,
@@ -317,12 +335,12 @@ class ComicVineTalker:
 
         params = {
             "api_key": self.api_key,
-            "filter": "volume:" + str(series_id),
+            "filter": f"volume:{series_id}",
             "format": "json",
             "field_list": "id,volume,issue_number,name,image,cover_date,site_detail_url,description,aliases",
             "offset": 0,
         }
-        cv_response = self.get_cv_content(self.api_base_url + "/issues/", params)
+        cv_response = self.get_cv_content(urljoin(self.api_base_url, "issues/"), params)
 
         current_result_count = cv_response["number_of_page_results"]
         total_result_count = cv_response["number_of_total_results"]
@@ -337,7 +355,7 @@ class ComicVineTalker:
             offset += cv_response["number_of_page_results"]
 
             params["offset"] = offset
-            cv_response = self.get_cv_content(self.api_base_url + "/issues/", params)
+            cv_response = self.get_cv_content(urljoin(self.api_base_url, "issues/"), params)
 
             volume_issues_result.extend(cast(list[resulttypes.CVIssuesResults], cv_response["results"]))
             current_result_count += cv_response["number_of_page_results"]
@@ -367,7 +385,7 @@ class ComicVineTalker:
             "filter": flt,
         }
 
-        cv_response = self.get_cv_content(self.api_base_url + "/issues/", params)
+        cv_response = self.get_cv_content(urljoin(self.api_base_url, "issues/"), params)
 
         current_result_count = cv_response["number_of_page_results"]
         total_result_count = cv_response["number_of_total_results"]
@@ -382,7 +400,7 @@ class ComicVineTalker:
             offset += cv_response["number_of_page_results"]
 
             params["offset"] = offset
-            cv_response = self.get_cv_content(self.api_base_url + "/issues/", params)
+            cv_response = self.get_cv_content(urljoin(self.api_base_url, "issues/"), params)
 
             filtered_issues_result.extend(cast(list[resulttypes.CVIssuesResults], cv_response["results"]))
             current_result_count += cv_response["number_of_page_results"]
@@ -407,7 +425,7 @@ class ComicVineTalker:
                 break
 
         if f_record is not None:
-            issue_url = self.api_base_url + "/issue/" + CVTypeID.Issue + "-" + str(f_record["id"])
+            issue_url = urljoin(self.api_base_url, f"issue/{CVTypeID.Issue}-{f_record['id']}")
             params = {"api_key": self.api_key, "format": "json"}
             cv_response = self.get_cv_content(issue_url, params)
             issue_results = cast(resulttypes.CVIssueDetailResults, cv_response["results"])
@@ -420,7 +438,7 @@ class ComicVineTalker:
 
     def fetch_issue_data_by_issue_id(self, issue_id: int, settings: ComicTaggerSettings) -> GenericMetadata:
 
-        issue_url = self.api_base_url + "/issue/" + CVTypeID.Issue + "-" + str(issue_id)
+        issue_url = urljoin(self.api_base_url, f"issue/{CVTypeID.Issue}-{issue_id}")
         params = {"api_key": self.api_key, "format": "json"}
         cv_response = self.get_cv_content(issue_url, params)
 
@@ -609,7 +627,8 @@ class ComicVineTalker:
         if cached_details["image_url"] is not None:
             return cached_details
 
-        issue_url = self.api_base_url + "/issue/" + CVTypeID.Issue + "-" + str(issue_id)
+        issue_url = urljoin(self.api_base_url, f"issue/{CVTypeID.Issue}-{issue_id}")
+        logger.error("%s, %s", self.api_base_url, issue_url)
 
         params = {"api_key": self.api_key, "format": "json", "field_list": "image,cover_date,site_detail_url"}
 
@@ -708,19 +727,20 @@ class ComicVineTalker:
             ComicVineTalker.url_fetch_complete(details["image_url"], details["thumb_image_url"])
             return
 
-        issue_url = (
-            self.api_base_url
-            + "/issue/"
-            + CVTypeID.Issue
-            + "-"
-            + str(issue_id)
-            + "/?api_key="
-            + self.api_key
-            + "&format=json&field_list=image,cover_date,site_detail_url"
+        issue_url = urlsplit(self.api_base_url)
+        issue_url = issue_url._replace(
+            query=urlencode(
+                {
+                    "api_key": self.api_key,
+                    "format": "json",
+                    "field_list": "image,cover_date,site_detail_url",
+                }
+            ),
+            path=f"issue/{CVTypeID.Issue}-{issue_id}",
         )
 
         self.nam.finished.connect(self.async_fetch_issue_cover_url_complete)
-        self.nam.get(QtNetwork.QNetworkRequest(QtCore.QUrl(issue_url)))
+        self.nam.get(QtNetwork.QNetworkRequest(QtCore.QUrl(issue_url.geturl())))
 
     def async_fetch_issue_cover_url_complete(self, reply: QtNetwork.QNetworkReply) -> None:
         # read in the response

comictaggerlib/coverimagewidget.py

@@ -113,7 +113,7 @@ class CoverImageWidget(QtWidgets.QWidget):
         self.page_loader = None
         self.imageIndex = -1
         self.imageCount = 1
-        self.imageData = bytes()
+        self.imageData = b""
 
         self.btnLeft.setIcon(QtGui.QIcon(ComicTaggerSettings.get_graphic("left.png")))
         self.btnRight.setIcon(QtGui.QIcon(ComicTaggerSettings.get_graphic("right.png")))
@@ -136,7 +136,7 @@ class CoverImageWidget(QtWidgets.QWidget):
         self.page_loader = None
         self.imageIndex = -1
         self.imageCount = 1
-        self.imageData = bytes()
+        self.imageData = b""
 
     def clear(self) -> None:
         self.reset_widget()

comictaggerlib/filerenamer.py

@@ -20,10 +20,9 @@ import logging
 import os
 import pathlib
 import string
-import sys
-from typing import Any, cast
+from typing import Any, NamedTuple, cast
 
-from pathvalidate import sanitize_filename
+from pathvalidate import Platform, normalize_platform, sanitize_filename
 
 from comicapi.comicarchive import ComicArchive
 from comicapi.genericmetadata import GenericMetadata
@@ -32,6 +31,31 @@ 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[Replacement]
+    format_value: list[Replacement]
+
+
+REPLACEMENTS = Replacements(
+    literal_text=[
+        Replacement(": ", " - ", True),
+        Replacement(":", "-", True),
+    ],
+    format_value=[
+        Replacement(": ", " - ", True),
+        Replacement(":", "-", True),
+        Replacement("/", "-", False),
+        Replacement("\\", "-", True),
+    ],
+)
+
+
 def get_rename_dir(ca: ComicArchive, rename_dir: str | pathlib.Path | None) -> pathlib.Path:
     folder = ca.path.parent.absolute()
     if rename_dir is not None:
@@ -42,16 +66,55 @@ def get_rename_dir(ca: ComicArchive, rename_dir: str | pathlib.Path | None) -> p
 
 
 class MetadataFormatter(string.Formatter):
-    def __init__(self, smart_cleanup: bool = False, platform: str = "auto") -> None:
+    def __init__(
+        self, smart_cleanup: bool = False, platform: str = "auto", replacements: Replacements = REPLACEMENTS
+    ) -> None:
         super().__init__()
         self.smart_cleanup = smart_cleanup
-        self.platform = platform
+        self.platform = normalize_platform(platform)
+        self.replacements = replacements
 
     def format_field(self, value: Any, format_spec: str) -> str:
         if value is None or value == "":
             return ""
         return cast(str, super().format_field(value, format_spec))
 
+    def convert_field(self, value: Any, conversion: str) -> str:
+        if conversion == "u":
+            return str(value).upper()
+        if conversion == "l":
+            return str(value).casefold()
+        if conversion == "c":
+            return str(value).capitalize()
+        if conversion == "S":
+            return str(value).swapcase()
+        if conversion == "t":
+            return str(value).title()
+        return cast(str, super().convert_field(value, conversion))
+
+    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 none_replacement(self, value: Any, replacement: str, r: str) -> Any:
+        if r == "-" and value is None or value == "":
+            return replacement
+        if r == "+" and value is not None:
+            return replacement
+        return value
+
+    def split_replacement(self, field_name: str) -> tuple[str, str, str]:
+        if "-" in field_name:
+            return field_name.rpartition("-")
+        if "+" in field_name:
+            return field_name.rpartition("+")
+        return field_name, "", ""
+
+    def is_strict(self) -> bool:
+        return self.platform in [Platform.UNIVERSAL, Platform.WINDOWS]
+
     def _vformat(
         self,
         format_string: str,
@@ -72,6 +135,7 @@ class MetadataFormatter(string.Formatter):
                 if lstrip:
                     literal_text = literal_text.lstrip("-_)}]#")
                 if self.smart_cleanup:
+                    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())
@@ -87,6 +151,7 @@ class MetadataFormatter(string.Formatter):
             lstrip = False
             # if there's a field, output it
             if field_name is not None and field_name != "":
+                field_name, r, replacement = self.split_replacement(field_name)
                 field_name = field_name.casefold()
                 # this is some markup, find the object and do the formatting
 
@@ -99,6 +164,8 @@ class MetadataFormatter(string.Formatter):
                 obj, arg_used = self.get_field(field_name, args, kwargs)
                 used_args.add(arg_used)
 
+                obj = self.none_replacement(obj, replacement, r)
+
                 # do any conversion on the resulting object
                 obj = self.convert_field(obj, conversion)  # type: ignore
 
@@ -109,19 +176,28 @@ class MetadataFormatter(string.Formatter):
 
                 # format the object and append to the result
                 fmt_obj = self.format_field(obj, format_spec)
-                if fmt_obj == "" and len(result) > 0 and self.smart_cleanup and literal_text:
-                    lstrip = True
+                if fmt_obj == "" and result and self.smart_cleanup and literal_text:
+                    if self.str_contains(result[-1], "({["):
+                        lstrip = True
                     if result:
                         if " " in result[-1]:
-                            result[-1], _, _ = result[-1].rpartition(" ")
+                            result[-1], _, _ = result[-1].rstrip().rpartition(" ")
                         result[-1] = result[-1].rstrip("-_({[#")
                 if self.smart_cleanup:
+                    # 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)
 
         return "".join(result), False
 
+    def str_contains(self, chars: str, string: str) -> bool:
+        for char in chars:
+            if char in string:
+                return True
+        return False
+
 
 class FileRenamer:
     def __init__(self, metadata: GenericMetadata | None, platform: str = "auto") -> None:
@@ -172,13 +248,6 @@ class FileRenamer:
 
         new_basename = ""
         for component in pathlib.PureWindowsPath(template).parts:
-            if (
-                self.platform.casefold() in ["universal", "windows"] or sys.platform.casefold() in ["windows"]
-            ) and self.smart_cleanup:
-                # colons get special treatment
-                component = component.replace(": ", " - ")
-                component = component.replace(":", "-")
-
             new_basename = str(
                 sanitize_filename(fmt.vformat(component, args=[], kwargs=Default(md_dict)), platform=self.platform)
             ).strip()

comictaggerlib/imagefetcher.py

@@ -97,14 +97,14 @@ class ImageFetcher:
             # if we found it, just emit the signal asap
             if image_data:
                 ImageFetcher.image_fetch_complete(QtCore.QByteArray(image_data))
-                return bytes()
+                return b""
 
             # didn't find it.  look online
             self.nam.finished.connect(self.finish_request)
             self.nam.get(QtNetwork.QNetworkRequest(QtCore.QUrl(url)))
 
             # we'll get called back when done...
-        return bytes()
+        return b""
 
     def finish_request(self, reply: QtNetwork.QNetworkReply) -> None:
         # read in the image data
@@ -159,10 +159,10 @@ class ImageFetcher:
             row = cur.fetchone()
 
             if row is None:
-                return bytes()
+                return b""
 
             filename = row[0]
-            image_data = bytes()
+            image_data = b""
 
             try:
                 with open(filename, "rb") as f:

comictaggerlib/issueidentifier.py

@@ -157,7 +157,7 @@ class IssueIdentifier:
             cropped_im = im.crop((int(w / 2), 0, w, h))
         except Exception:
             logger.exception("cropCover() error")
-            return bytes()
+            return b""
 
         output = io.BytesIO()
         cropped_im.save(output, format="PNG")
@@ -524,7 +524,7 @@ class IssueIdentifier:
             self.log_msg("")
 
         if len(self.match_list) == 0:
-            self.log_msg(":-(no matches!")
+            self.log_msg(":-( no matches!")
             self.search_result = self.result_no_matches
             return self.match_list
 

comictaggerlib/main.py

@@ -159,7 +159,7 @@ def ctmain() -> None:
 
     logger.debug("Installed Packages")
     for pkg in sorted(importlib_metadata.distributions(), key=lambda x: x.name):
-        logger.debug("%s\t%s", pkg.name, pkg.version)
+        logger.debug("%s\t%s", pkg.metadata["Name"], pkg.metadata["Version"])
 
     utils.load_publishers()
     update_publishers()

comictaggerlib/options.py

@@ -405,6 +405,8 @@ def parse_cmd_line() -> argparse.Namespace:
         opts.copy = opts.copy[0]
 
     if opts.recursive:
-        opts.file_list = utils.get_recursive_filelist(opts.file_list)
+        opts.file_list = utils.get_recursive_filelist(opts.files)
+    else:
+        opts.file_list = opts.files
 
     return opts

comictaggerlib/pagelisteditor.py

@@ -340,9 +340,9 @@ class PageListEditor(QtWidgets.QWidget):
             else:
                 text += " (Error: " + page_dict["Type"] + ")"
         if "DoublePage" in page_dict:
-            text += " " + "\U00002461"
+            text += " ②"
         if "Bookmark" in page_dict:
-            text += " " + "\U0001F516"
+            text += " 🔖"
         return text
 
     def get_page_list(self) -> list[ImageMetadata]:

comictaggerlib/renamewindow.py

@@ -97,7 +97,8 @@ class RenameWindow(QtWidgets.QDialog):
             new_ext = self.config_renamer(ca)
             try:
                 new_name = self.renamer.determine_name(new_ext)
-            except Exception as e:
+            except ValueError as e:
+                logger.exception("Invalid format string: %s", self.settings.rename_template)
                 QtWidgets.QMessageBox.critical(
                     self,
                     "Invalid format string!",
@@ -109,6 +110,19 @@ class RenameWindow(QtWidgets.QDialog):
                     "https://docs.python.org/3/library/string.html#format-string-syntax</a>",
                 )
                 return
+            except Exception as e:
+                logger.exception(
+                    "Formatter failure: %s metadata: %s", self.settings.rename_template, self.renamer.metadata
+                )
+                QtWidgets.QMessageBox.critical(
+                    self,
+                    "The formatter had an issue!",
+                    "The formatter has experienced an unexpected error!"
+                    f"<br/><br/>{type(e).__name__}: {e}<br/><br/>"
+                    "Please open an issue at "
+                    "<a href='https://github.com/comictagger/comictagger'>"
+                    "https://github.com/comictagger/comictagger</a>",
+                )
 
             row = self.twList.rowCount()
             self.twList.insertRow(row)
@@ -161,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!<br/><br/>{type(e).__name__}: {e}<br/><br/>",
+            )
 
         prog_dialog.hide()
         QtCore.QCoreApplication.processEvents()

comictaggerlib/settings.py

@@ -22,7 +22,8 @@ import pathlib
 import platform
 import sys
 import uuid
-from typing import Iterator, TextIO, no_type_check
+from collections.abc import Iterator
+from typing import TextIO, no_type_check
 
 from comicapi import utils
 

comictaggerlib/settingswindow.py

@@ -269,17 +269,32 @@ class SettingsWindow(QtWidgets.QDialog):
     def accept(self) -> None:
         self.rename_test()
         if self.rename_error is not None:
-            QtWidgets.QMessageBox.critical(
-                self,
-                "Invalid format string!",
-                "Your rename template is invalid!"
-                f"<br/><br/>{self.rename_error}<br/><br/>"
-                "Please consult the template help in the "
-                "settings and the documentation on the format at "
-                "<a href='https://docs.python.org/3/library/string.html#format-string-syntax'>"
-                "https://docs.python.org/3/library/string.html#format-string-syntax</a>",
-            )
-            return
+            if isinstance(self.rename_error, ValueError):
+                logger.exception("Invalid format string: %s", self.settings.rename_template)
+                QtWidgets.QMessageBox.critical(
+                    self,
+                    "Invalid format string!",
+                    "Your rename template is invalid!"
+                    f"<br/><br/>{self.rename_error}<br/><br/>"
+                    "Please consult the template help in the "
+                    "settings and the documentation on the format at "
+                    "<a href='https://docs.python.org/3/library/string.html#format-string-syntax'>"
+                    "https://docs.python.org/3/library/string.html#format-string-syntax</a>",
+                )
+                return
+            else:
+                logger.exception(
+                    "Formatter failure: %s metadata: %s", self.settings.rename_template, self.renamer.metadata
+                )
+                QtWidgets.QMessageBox.critical(
+                    self,
+                    "The formatter had an issue!",
+                    "The formatter has experienced an unexpected error!"
+                    f"<br/><br/>{type(self.rename_error).__name__}: {self.rename_error}<br/><br/>"
+                    "Please open an issue at "
+                    "<a href='https://github.com/comictagger/comictagger'>"
+                    "https://github.com/comictagger/comictagger</a>",
+                )
 
         # Copy values from form to settings and save
         self.settings.rar_exe_path = str(self.leRarExePath.text())

comictaggerlib/taggerwindow.py

@@ -26,7 +26,8 @@ import pprint
 import re
 import sys
 import webbrowser
-from typing import Any, Callable, Iterable, cast
+from collections.abc import Iterable
+from typing import Any, Callable, cast
 from urllib.parse import urlparse
 
 import natsort
@@ -1711,7 +1712,7 @@ Have fun!
             )
             if dlg.ignore_leading_digits_in_filename and md.series is not None:
                 # remove all leading numbers
-                md.series = re.sub(r"([\d.]*)(.*)", "\\2", md.series)
+                md.series = re.sub(r"([\d.]*)(.*)", r"\2", md.series)
 
         # use the dialog specified search string
         if dlg.search_string:
@@ -1854,7 +1855,7 @@ Have fun!
                 logger.error("Failed to load metadata for %s: %s", ca.path, e)
             image_data = ca.get_page(cover_idx)
             self.atprogdialog.set_archive_image(image_data)
-            self.atprogdialog.set_test_image(bytes())
+            self.atprogdialog.set_test_image(b"")
 
             QtCore.QCoreApplication.processEvents()
             if self.atprogdialog.isdone:

requirements-speedup.txt

@@ -0,0 +1 @@
+thefuzz[speedup]>=0.19.0

requirements.txt

@@ -1,12 +1,13 @@
-beautifulsoup4 >= 4.1
-importlib_metadata
+beautifulsoup4>=4.1
+importlib_metadata>=3.3.0
 natsort>=8.1.0
 pathvalidate
-pillow>=4.3.0
+pillow>=9.1.0
 py7zr
 pycountry
+pyicu; sys_platform == 'linux' or sys_platform == 'darwin'
 requests==2.*
 text2digits
-thefuzz[speedup]>=0.19.0
+thefuzz>=0.19.0
 typing_extensions
 wordninja

testing/comicdata.py

@@ -99,14 +99,22 @@ metadata_keys = [
 ]
 
 credits = [
-    ("writer", "Dara Naraghi"),
-    ("writeR", "Dara Naraghi"),
+    (comicapi.genericmetadata.md_test, "writer", "Dara Naraghi"),
+    (comicapi.genericmetadata.md_test, "writeR", "Dara Naraghi"),
+    (
+        comicapi.genericmetadata.md_test.replace(
+            credits=[{"person": "Dara Naraghi", "role": "writer"}, {"person": "Dara Naraghi", "role": "writer"}]
+        ),
+        "writeR",
+        "Dara Naraghi",
+    ),
 ]
 
 imprints = [
     ("marvel", ("", "Marvel")),
     ("marvel comics", ("", "Marvel")),
     ("aircel", ("Aircel Comics", "Marvel")),
+    ("nothing", ("", "nothing")),
 ]
 
 additional_imprints = [

testing/filenames.py

@@ -733,9 +733,93 @@ fnames = [
         },
         True,
     ),
+    (
+        "Cory Doctorow's Futuristic Tales of the Here and Now: Anda's Game #001 (2007).cbz",
+        "full-date, issue in parenthesis",
+        {
+            "issue": "1",
+            "series": "Cory Doctorow's Futuristic Tales of the Here and Now",
+            "title": "Anda's Game",
+            "volume": "",
+            "year": "2007",
+            "remainder": "",
+            "issue_count": "",
+        },
+        True,
+    ),
 ]
 
 rnames = [
+    (
+        "{series!c} {price} {year}",  # Capitalize
+        False,
+        "universal",
+        "Cory doctorow's futuristic tales of the here and now 2007.cbz",
+        does_not_raise(),
+    ),
+    (
+        "{series!t} {price} {year}",  # Title Case
+        False,
+        "universal",
+        "Cory Doctorow'S Futuristic Tales Of The Here And Now 2007.cbz",
+        does_not_raise(),
+    ),
+    (
+        "{series!S} {price} {year}",  # Swap Case
+        False,
+        "universal",
+        "cORY dOCTOROW'S fUTURISTIC tALES OF THE hERE AND nOW 2007.cbz",
+        does_not_raise(),
+    ),
+    (
+        "{title!l} {price} {year}",  # Lowercase
+        False,
+        "universal",
+        "anda's game 2007.cbz",
+        does_not_raise(),
+    ),
+    (
+        "{title!u} {price} {year}",  # Upper Case
+        False,
+        "universal",
+        "ANDA'S GAME 2007.cbz",
+        does_not_raise(),
+    ),
+    (
+        "{title} {price} {year+}",  # Empty alternate value
+        False,
+        "universal",
+        "Anda's Game.cbz",
+        does_not_raise(),
+    ),
+    (
+        "{title} {price} {year+year!u}",  # Alternate value Upper Case
+        False,
+        "universal",
+        "Anda's Game YEAR.cbz",
+        does_not_raise(),
+    ),
+    (
+        "{title} {price} {year+year}",  # Alternate Value
+        False,
+        "universal",
+        "Anda's Game year.cbz",
+        does_not_raise(),
+    ),
+    (
+        "{title} {price-0} {year}",  # Default value
+        False,
+        "universal",
+        "Anda's Game 0 2007.cbz",
+        does_not_raise(),
+    ),
+    (
+        "{title} {price+0} {year}",  # Alternate Value
+        False,
+        "universal",
+        "Anda's Game 2007.cbz",
+        does_not_raise(),
+    ),
     (
         "{series} #{issue} - {title} ({year}) ({price})",  # price should be none
         False,
@@ -743,6 +827,13 @@ rnames = [
         "Cory Doctorow's Futuristic Tales of the Here and Now #001 - Anda's Game (2007).cbz",
         does_not_raise(),
     ),
+    (
+        "{series} #{issue} - {title} {volume:02} ({year})",  # Ensure format specifier works
+        False,
+        "universal",
+        "Cory Doctorow's Futuristic Tales of the Here and Now #001 - Anda's Game 01 (2007).cbz",
+        does_not_raise(),
+    ),
     (
         "{series} #{issue} - {title} ({year})({price})",  # price should be none, test no  space between ')('
         False,
@@ -764,6 +855,27 @@ rnames = [
         "Cory Doctorow's Futuristic Tales of the Here and Now #001 - Anda's Game (2007).cbz",
         does_not_raise(),
     ),
+    (
+        "{title} {web_link}",  # Ensure colon is replaced in metadata
+        False,
+        "universal",
+        "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,
+        "universal",
+        "Cory Doctorow's Futuristic Tales of the Here and Now-Anda's Game #001 (2007).cbz",
+        does_not_raise(),
+    ),
     (
         "{series}: {title} #{issue} ({year})",  # on windows the ':' is replaced
         False,
@@ -795,7 +907,7 @@ rnames = [
     (
         r"{publisher}\  {series} #{issue} - {title} ({year})",  # backslashes separate directories
         False,
-        "universal",
+        "Linux",
         "Cory Doctorow's Futuristic Tales of the Here and Now #001 - Anda's Game (2007).cbz",
         does_not_raise(),
     ),
@@ -807,10 +919,10 @@ rnames = [
         does_not_raise(),
     ),
     (
-        "{series} #  {issue} - {locations} ({year})",
+        "{series} #{issue} - {locations} ({year})",
         False,
         "universal",
-        "Cory Doctorow's Futuristic Tales of the Here and Now # 001 - lonely cottage (2007).cbz",
+        "Cory Doctorow's Futuristic Tales of the Here and Now #001 - lonely cottage (2007).cbz",
         does_not_raise(),
     ),
     (
@@ -848,6 +960,20 @@ rnames = [
         "Cory Doctorow's Futuristic Tales of the Here and Now - Anda's Game {test} #001 (2007).cbz",
         does_not_raise(),
     ),
+    (
+        "{series} - {title} #{issue} ({year} {price})",  # Test null value in parenthesis with a non-null value
+        False,
+        "universal",
+        "Cory Doctorow's Futuristic Tales of the Here and Now - Anda's Game #001 (2007).cbz",
+        does_not_raise(),
+    ),
+    (
+        "{series} - {title} #{issue} (of {price})",  # null value with literal text in parenthesis
+        False,
+        "universal",
+        "Cory Doctorow's Futuristic Tales of the Here and Now - Anda's Game #001.cbz",
+        does_not_raise(),
+    ),
     (
         "{series} - {title} {1} #{issue} ({year})",  # Test numeric key
         False,

tests/comicarchive_test.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import platform
 import shutil
 
 import pytest
@@ -15,6 +16,8 @@ def test_getPageNameList():
     pageNameList = c.get_page_name_list()
 
     assert pageNameList == [
+        "!cover.jpg",
+        "00.jpg",
         "page0.jpg",
         "Page1.jpeg",
         "Page2.png",
@@ -44,6 +47,58 @@ def test_save_cix(tmp_comic):
     md = tmp_comic.read_cix()
 
 
+def test_save_cbi(tmp_comic):
+    md = tmp_comic.read_cix()
+    md.set_default_page_list(tmp_comic.get_number_of_pages())
+
+    assert tmp_comic.write_cbi(md)
+
+    md = tmp_comic.read_cbi()
+
+
+@pytest.mark.xfail(not (comicapi.comicarchive.rar_support and shutil.which("rar")), reason="rar support")
+def test_save_cix_rar(tmp_path):
+    cbr_path = datadir / "fake_cbr.cbr"
+    shutil.copy(cbr_path, tmp_path)
+
+    tmp_comic = comicapi.comicarchive.ComicArchive(tmp_path / cbr_path.name)
+    assert tmp_comic.write_cix(comicapi.genericmetadata.md_test)
+
+    md = tmp_comic.read_cix()
+    assert md.replace(pages=[]) == comicapi.genericmetadata.md_test.replace(pages=[])
+
+
+@pytest.mark.xfail(not (comicapi.comicarchive.rar_support and shutil.which("rar")), reason="rar support")
+def test_save_cbi_rar(tmp_path):
+    cbr_path = datadir / "fake_cbr.cbr"
+    shutil.copy(cbr_path, tmp_path)
+
+    tmp_comic = comicapi.comicarchive.ComicArchive(tmp_path / cbr_path.name)
+    assert tmp_comic.write_cbi(comicapi.genericmetadata.md_test)
+
+    md = tmp_comic.read_cbi()
+    assert md.replace(pages=[]) == comicapi.genericmetadata.md_test.replace(
+        pages=[],
+        day=None,
+        alternate_series=None,
+        alternate_number=None,
+        alternate_count=None,
+        imprint=None,
+        notes=None,
+        web_link=None,
+        format=None,
+        manga=None,
+        page_count=None,
+        maturity_rating=None,
+        story_arc=None,
+        series_group=None,
+        scan_info=None,
+        characters=None,
+        teams=None,
+        locations=None,
+    )
+
+
 def test_page_type_save(tmp_comic):
     md = tmp_comic.read_cix()
     t = md.pages[0]
@@ -88,3 +143,25 @@ def test_copy_from_archive(archiver, tmp_path, cbz):
 
     md = comic_archive.read_cix()
     assert md == comicapi.genericmetadata.md_test
+
+
+def test_rename(tmp_comic, tmp_path):
+    old_path = tmp_comic.path
+    tmp_comic.rename(tmp_path / "test.cbz")
+    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):
+        if platform.system() == "Windows":
+            raise OSError("Windows sucks")
+        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

tests/conftest.py

@@ -5,7 +5,8 @@ import datetime
 import io
 import shutil
 import unittest.mock
-from typing import Any, Generator
+from collections.abc import Generator
+from typing import Any
 
 import pytest
 import requests

tests/genericmetadata_test.py

@@ -37,6 +37,6 @@ def test_add_credit_primary():
     assert md.credits == [comicapi.genericmetadata.CreditMetadata(person="test", role="writer", primary=True)]
 
 
-@pytest.mark.parametrize("role, expected", credits)
+@pytest.mark.parametrize("md, role, expected", credits)
 def test_get_primary_credit(md, role, expected):
     assert md.get_primary_credit(role) == expected

tests/metadata_test.py

@@ -18,7 +18,6 @@ def test_cbi():
     md = CBI.metadata_from_string(string)
     md_test = comicapi.genericmetadata.md_test.replace(
         day=None,
-        language="English",
         page_count=None,
         maturity_rating=None,
         story_arc=None,

tests/utils_test.py

@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+import os
+
 import pytest
 
 import comicapi.utils
@@ -22,13 +24,16 @@ def test_recursive_list_with_file(tmp_path) -> None:
     temp_txt = tmp_path / "info.txt"
     temp_txt.write_text("this is here")
 
-    expected_result = {str(foo_png), str(temp_cbr), str(temp_file), str(temp_txt)}
-    result = set(comicapi.utils.get_recursive_filelist([tmp_path]))
+    temp_txt2 = tmp_path / "info2.txt"
+    temp_txt2.write_text("this is here")
+
+    expected_result = {str(foo_png), str(temp_cbr), str(temp_file), str(temp_txt), str(temp_txt2)}
+    result = set(comicapi.utils.get_recursive_filelist([str(temp_txt2), tmp_path]))
 
     assert result == expected_result
 
 
-values = [
+xlate_values = [
     ({"data": "", "is_int": False, "is_float": False}, None),
     ({"data": None, "is_int": False, "is_float": False}, None),
     ({"data": None, "is_int": True, "is_float": False}, None),
@@ -52,6 +57,71 @@ values = [
 ]
 
 
-@pytest.mark.parametrize("value, result", values)
+@pytest.mark.parametrize("value, result", xlate_values)
 def test_xlate(value, result):
     assert comicapi.utils.xlate(**value) == result
+
+
+language_values = [
+    ("english", "en"),
+    ("ENGLISH", "en"),
+    ("EnglisH", "en"),
+    ("", ""),
+    ("aaa", None),  # does not have a 2-letter code
+    (None, None),
+]
+
+
+@pytest.mark.parametrize("value, result", language_values)
+def test_get_language_iso(value, result):
+    assert result == comicapi.utils.get_language_iso(value)
+
+
+def test_unique_file(tmp_path):
+    file = tmp_path / "test.cbz"
+    assert file == comicapi.utils.unique_file(file)
+
+    file.mkdir()
+    assert (tmp_path / "test (1).cbz") == comicapi.utils.unique_file(file)
+
+
+def test_add_to_path(monkeypatch):
+    monkeypatch.setenv("PATH", os.path.abspath("/usr/bin"))
+    comicapi.utils.add_to_path("/bin")
+    assert os.environ["PATH"] == (os.path.abspath("/bin") + os.pathsep + os.path.abspath("/usr/bin"))
+
+    comicapi.utils.add_to_path("/usr/bin")
+    comicapi.utils.add_to_path("/usr/bin/")
+    assert os.environ["PATH"] == (os.path.abspath("/bin") + os.pathsep + os.path.abspath("/usr/bin"))
+
+
+titles = [
+    (("", ""), True),
+    (("Conan el Barbaro", "Conan el Bárbaro"), True),
+    (("鋼の錬金術師", "鋼の錬金術師"), True),
+    (("钢之炼金术师", "鋼の錬金術師"), False),
+    (("batmans grave", "The Batman's Grave"), True),
+    (("batman grave", "The Batman's Grave"), True),
+    (("bats grave", "The Batman's Grave"), False),
+]
+
+
+@pytest.mark.parametrize("value, result", titles)
+def test_titles_match(value, result):
+    assert comicapi.utils.titles_match(value[0], value[1]) == result
+
+
+titles_2 = [
+    ("", ""),
+    ("鋼の錬金術師", "鋼の錬金術師"),
+    ("Conan el Bárbaro", "Conan el Barbaro"),
+    ("The Batman's Grave", "batmans grave"),
+    ("A+X", "ax"),
+    ("ms. marvel", "ms marvel"),
+    ("spider-man/deadpool", "spider man deadpool"),
+]
+
+
+@pytest.mark.parametrize("value, result", titles_2)
+def test_sanitize_title(value, result):
+    assert comicapi.utils.sanitize_title(value) == result.casefold()