Update plugin settings

Make "runtime" a persistent group, allows normalizing without losing validation
Simplify archiver setting generation
Generate options for setting a url and key for all talkers
Return validated talker settings
Require that the talker id must match the entry point name
Add api_url and api_key as default attributes on talkers
Add default handling of api_url and api_key to register_settings
Update settngs to 0.6.2 to be able to add settings to a group and
  use the display_name attribute
Error if no talkers are loaded
Update talker entry point to comictagger.talker

comictaggerlib/__pyinstaller/hook-comictaggerlib.py

@@ -1,7 +1,7 @@
 from __future__ import annotations
 
-from PyInstaller.utils.hooks import collect_data_files
+from PyInstaller.utils.hooks import collect_data_files, collect_entry_point
 
-datas = []
+datas, hiddenimports = collect_entry_point("comictagger.talker")
 datas += collect_data_files("comictaggerlib.ui")
 datas += collect_data_files("comictaggerlib.graphics")

comictaggerlib/ctsettings/commandline.py

@@ -236,7 +236,7 @@ def register_commands(parser: settngs.Manager) -> None:
 
 def register_commandline_settings(parser: settngs.Manager) -> None:
     parser.add_group("commands", register_commands, True)
-    parser.add_group("runtime", register_settings)
+    parser.add_persistent_group("runtime", register_settings)
 
 
 def validate_commandline_settings(

comictaggerlib/ctsettings/plugin.py

@@ -12,23 +12,39 @@ logger = logging.getLogger("comictagger")
 
 
 def archiver(manager: settngs.Manager) -> None:
-    exe_registered: set[str] = set()
     for archiver in comicapi.comicarchive.archivers:
-        if archiver.exe and archiver.exe not in exe_registered:
+        if archiver.exe:
+            # add_setting will overwrite anything with the same name.
+            # So we only end up with one option even if multiple archivers use the same exe.
             manager.add_setting(
-                f"--{archiver.exe.replace(' ', '-').replace('_', '-').strip().strip('-')}",
+                f"--{settngs.sanitize_name(archiver.exe)}",
                 default=archiver.exe,
                 help="Path to the %(default)s executable\n\n",
             )
-            exe_registered.add(archiver.exe)
 
 
 def register_talker_settings(manager: settngs.Manager) -> None:
-    for talker_name, talker in comictaggerlib.ctsettings.talkers.items():
+    for talker_id, talker in comictaggerlib.ctsettings.talkers.items():
+
+        def api_options(manager: settngs.Manager) -> None:
+            manager.add_setting(
+                f"--{talker_id}-key",
+                default="",
+                display_name="API Key",
+                help=f"API Key for {talker.name} (default: {talker.default_api_key})",
+            )
+            manager.add_setting(
+                f"--{talker_id}-url",
+                default="",
+                display_name="URL",
+                help=f"URL for {talker.name} (default: {talker.default_api_url})",
+            )
+
         try:
-            manager.add_persistent_group("talker_" + talker_name, talker.register_settings, False)
+            manager.add_persistent_group("talker_" + talker_id, api_options, False)
+            manager.add_persistent_group("talker_" + talker_id, talker.register_settings, False)
         except Exception:
-            logger.exception("Failed to register settings for %s", talker_name)
+            logger.exception("Failed to register settings for %s", talker_id)
 
 
 def validate_archive_settings(config: settngs.Config[settngs.Namespace]) -> settngs.Config[settngs.Namespace]:
@@ -37,11 +53,7 @@ def validate_archive_settings(config: settngs.Config[settngs.Namespace]) -> sett
     cfg = settngs.normalize_config(config, file=True, cmdline=True, defaults=False)
     for archiver in comicapi.comicarchive.archivers:
         exe_name = settngs.sanitize_name(archiver.exe)
-        if (
-            exe_name in cfg[0]["archiver"]
-            and cfg[0]["archiver"][exe_name]
-            and cfg[0]["archiver"][exe_name] != archiver.exe
-        ):
+        if exe_name in cfg[0]["archiver"] and cfg[0]["archiver"][exe_name]:
             if os.path.basename(cfg[0]["archiver"][exe_name]) == archiver.exe:
                 comicapi.utils.add_to_path(os.path.dirname(cfg[0]["archiver"][exe_name]))
             else:
@@ -53,15 +65,15 @@ def validate_archive_settings(config: settngs.Config[settngs.Namespace]) -> sett
 def validate_talker_settings(config: settngs.Config[settngs.Namespace]) -> settngs.Config[settngs.Namespace]:
     # Apply talker settings from config file
     cfg = settngs.normalize_config(config, True, True)
-    for talker_name, talker in list(comictaggerlib.ctsettings.talkers.items()):
+    for talker_id, talker in list(comictaggerlib.ctsettings.talkers.items()):
         try:
-            talker.parse_settings(cfg[0]["talker_" + talker_name])
+            cfg[0]["talker_" + talker_id] = talker.parse_settings(cfg[0]["talker_" + talker_id])
         except Exception as e:
             # Remove talker as we failed to apply the settings
-            del comictaggerlib.ctsettings.talkers[talker_name]
+            del comictaggerlib.ctsettings.talkers[talker_id]
             logger.exception("Failed to initialize talker settings: %s", e)
 
-    return config
+    return settngs.get_namespace(cfg)
 
 
 def validate_plugin_settings(config: settngs.Config[settngs.Namespace]) -> settngs.Config[settngs.Namespace]:

comictaggerlib/main.py

@@ -119,6 +119,15 @@ class App:
         # config already loaded
         error = None
 
+        talkers = ctsettings.talkers
+        del ctsettings.talkers
+
+        if len(talkers) < 1:
+            error = error = (
+                f"Failed to load any talkers, please re-install and check the log located in '{self.config[0].runtime_config.user_log_dir}' for more details",
+                True,
+            )
+
         signal.signal(signal.SIGINT, signal.SIG_DFL)
 
         logger.debug("Installed Packages")
@@ -134,7 +143,10 @@ class App:
 
         # manage the CV API key
         # None comparison is used so that the empty string can unset the value
-        if self.config[0].talker_comicvine_cv_api_key is not None or self.config[0].talker_comicvine_cv_url is not None:
+        if not error and (
+            self.config[0].talker_comicvine_comicvine_key is not None
+            or self.config[0].talker_comicvine_comicvine_url is not None
+        ):
             settings_path = self.config[0].runtime_config.user_config_dir / "settings.json"
             if self.config_load_success:
                 self.manager.save_file(self.config[0], settings_path)
@@ -150,9 +162,6 @@ class App:
                 True,
             )
 
-        talkers = ctsettings.talkers
-        del ctsettings.talkers
-
         if self.config[0].runtime_no_gui:
             if error and error[1]:
                 print(f"A fatal error occurred please check the log for more information: {error[0]}")  # noqa: T201

comictaggerlib/settingswindow.py

@@ -320,8 +320,8 @@ class SettingsWindow(QtWidgets.QDialog):
         self.cbxSortByYear.setChecked(self.config[0].talker_sort_series_by_year)
         self.cbxExactMatches.setChecked(self.config[0].talker_exact_series_matches_first)
 
-        self.leKey.setText(self.config[0].talker_comicvine_cv_api_key)
-        self.leURL.setText(self.config[0].talker_comicvine_cv_url)
+        self.leKey.setText(self.config[0].talker_comicvine_comicvine_key)
+        self.leURL.setText(self.config[0].talker_comicvine_comicvine_url)
 
         self.cbxAssumeLoneCreditIsPrimary.setChecked(self.config[0].cbl_assume_lone_credit_is_primary)
         self.cbxCopyCharactersToTags.setChecked(self.config[0].cbl_copy_characters_to_tags)
@@ -436,13 +436,8 @@ class SettingsWindow(QtWidgets.QDialog):
         self.config[0].talker_sort_series_by_year = self.cbxSortByYear.isChecked()
         self.config[0].talker_exact_series_matches_first = self.cbxExactMatches.isChecked()
 
-        if self.leKey.text().strip():
-            self.config[0].talker_comicvine_cv_api_key = self.leKey.text().strip()
-            self.talker.api_key = self.config[0].talker_comicvine_cv_api_key
-
-        if self.leURL.text().strip():
-            self.config[0].talker_comicvine_cv_url = self.leURL.text().strip()
-            self.talker.api_url = self.config[0].talker_comicvine_cv_url
+        self.config[0].talker_comicvine_comicvine_key = self.leKey.text().strip()
+        self.config[0].talker_comicvine_comicvine_url = self.leURL.text().strip()
 
         self.config[0].cbl_assume_lone_credit_is_primary = self.cbxAssumeLoneCreditIsPrimary.isChecked()
         self.config[0].cbl_copy_characters_to_tags = self.cbxCopyCharactersToTags.isChecked()

comictalker/__init__.py

@@ -26,13 +26,16 @@ def get_talkers(version: str, cache: pathlib.Path) -> dict[str, ComicTalker]:
     """Returns all comic talker instances"""
     talkers: dict[str, ComicTalker] = {}
 
-    for talker in entry_points(group="comictagger.talkers"):
+    for talker in entry_points(group="comictagger.talker"):
         try:
             talker_cls = talker.load()
             obj = talker_cls(version, cache)
-            talkers[obj.id] = obj
+            if obj.id != talker.name:
+                logger.error("Talker ID must be the same as the entry point name")
+                continue
+            talkers[talker.name] = obj
+
         except Exception:
             logger.exception("Failed to load talker: %s", talker.name)
-            raise TalkerError(source=talker.name, code=4, desc="Failed to initialise talker")
 
     return talkers

comictalker/comictalker.py

@@ -21,6 +21,7 @@ import settngs
 
 from comicapi.genericmetadata import GenericMetadata
 from comictalker.resulttypes import ComicIssue, ComicSeries
+from comictalker.talker_utils import fix_url
 
 logger = logging.getLogger(__name__)
 
@@ -107,15 +108,15 @@ class ComicTalker:
 
     name: str = "Example"
     id: str = "example"
-    logo_url: str = "https://example.com/logo.png"
-    website: str = "https://example.com/"
+    website: str = "https://example.com"
+    logo_url: str = f"{website}/logo.png"
     attribution: str = f"Metadata provided by <a href='{website}'>{name}</a>"
 
     def __init__(self, version: str, cache_folder: pathlib.Path) -> None:
         self.cache_folder = cache_folder
         self.version = version
-        self.api_key: str = ""
-        self.api_url: str = ""
+        self.api_key = self.default_api_key = ""
+        self.api_url = self.default_api_url = ""
 
     def register_settings(self, parser: settngs.Manager) -> None:
         """Allows registering settings using the settngs package with an argparse like interface"""
@@ -126,6 +127,15 @@ class ComicTalker:
         settings is a dictionary of settings defined in register_settings.
         It is only guaranteed that the settings defined in register_settings will be present.
         """
+        if settings[f"{self.id}_key"]:
+            self.api_key = settings[f"{self.id}_key"]
+        if settings[f"{self.id}_url"]:
+            self.api_url = fix_url(settings[f"{self.id}_url"])
+
+        if self.api_key == "":
+            self.api_key = self.default_api_key
+        if self.api_url == "":
+            self.api_url = self.default_api_url
         return settings
 
     def check_api_key(self, key: str, url: str) -> bool:

comictalker/talker_utils.py

@@ -15,6 +15,7 @@ from __future__ import annotations
 
 import logging
 import re
+from urllib.parse import urlsplit
 
 from bs4 import BeautifulSoup
 
@@ -26,6 +27,14 @@ from comictalker.resulttypes import ComicIssue
 logger = logging.getLogger(__name__)
 
 
+def fix_url(url: str) -> str:
+    tmp_url = urlsplit(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 + "/")
+    return tmp_url.geturl()
+
+
 def map_comic_issue_to_metadata(
     issue_results: ComicIssue, source: str, remove_html_tables: bool = False, use_year_volume: bool = False
 ) -> GenericMetadata:

comictalker/talkers/comicvine.py

@@ -22,7 +22,7 @@ import logging
 import pathlib
 import time
 from typing import Any, Callable, Generic, TypeVar
-from urllib.parse import urljoin, urlsplit
+from urllib.parse import urljoin
 
 import requests
 import settngs
@@ -156,33 +156,36 @@ CV_STATUS_RATELIMIT = 107
 class ComicVineTalker(ComicTalker):
     name: str = "Comic Vine"
     id: str = "comicvine"
-    logo_url: str = "https://comicvine.gamespot.com/a/bundles/comicvinesite/images/logo.png"
-    website: str = "https://comicvine.gamespot.com/"
+    website: str = "https://comicvine.gamespot.com"
+    logo_url: str = f"{website}/a/bundles/comicvinesite/images/logo.png"
     attribution: str = f"Metadata provided by <a href='{website}'>{name}</a>"
 
     def __init__(self, version: str, cache_folder: pathlib.Path):
         super().__init__(version, cache_folder)
         # Default settings
-        self.api_url: str = "https://comicvine.gamespot.com/api"
-        self.api_key: str = "27431e6787042105bd3e47e169a624521f89f3a4"
+        self.default_api_url = self.api_url = f"{self.website}/api/"
+        self.default_api_key = self.api_key = "27431e6787042105bd3e47e169a624521f89f3a4"
         self.remove_html_tables: bool = False
         self.use_series_start_as_volume: bool = False
         self.wait_on_ratelimit: bool = False
 
-        tmp_url = urlsplit(self.api_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_url = tmp_url.geturl()
-
         # NOTE: This was hardcoded before which is why it isn't in settings
         self.wait_on_ratelimit_time: int = 20
 
     def register_settings(self, parser: settngs.Manager) -> None:
-        parser.add_setting("--cv-api-key", help="Use the given Comic Vine API Key.")
-        parser.add_setting("--cv-url", help="Use the given Comic Vine URL.")
+        # The empty string being the default allows this setting to be unset, allowing the default to change
+        parser.add_setting(
+            f"--{self.id}-key",
+            default="",
+            display_name="API Key",
+            help=f"Use the given Comic Vine API Key. (default: {self.default_api_key})",
+        )
+        parser.add_setting(
+            f"--{self.id}-url",
+            default="",
+            display_name="API URL",
+            help=f"Use the given Comic Vine URL. (default: {self.default_api_url})",
+        )
         parser.add_setting("--cv-use-series-start-as-volume", default=False, action=argparse.BooleanOptionalAction)
         parser.add_setting("--cv-wait-on-ratelimit", default=False, action=argparse.BooleanOptionalAction)
         parser.add_setting(
@@ -193,35 +196,24 @@ class ComicVineTalker(ComicTalker):
         )
 
     def parse_settings(self, settings: dict[str, Any]) -> dict[str, Any]:
-        if settings["cv_api_key"]:
-            self.api_key = settings["cv_api_key"]
-        if settings["cv_url"]:
-            tmp_url = urlsplit(settings["cv_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_url = tmp_url.geturl()
+        settings = super().parse_settings(settings)
 
         self.use_series_start_as_volume = settings["cv_use_series_start_as_volume"]
         self.wait_on_ratelimit = settings["cv_wait_on_ratelimit"]
         self.remove_html_tables = settings["cv_remove_html_tables"]
-        return settngs
+        return settings
 
     def check_api_key(self, key: str, url: str) -> bool:
+        url = talker_utils.fix_url(url)
         if not url:
-            url = self.api_url
+            url = self.default_api_url
         try:
-            tmp_url = urlsplit(url)
-            if tmp_url.path and tmp_url.path[-1] != "/":
-                tmp_url = tmp_url._replace(path=tmp_url.path + "/")
-            url = tmp_url.geturl()
             test_url = urljoin(url, "issue/1/")
 
             cv_response: CVResult = requests.get(
                 test_url,
                 headers={"user-agent": "comictagger/" + self.version},
-                params={"api_key": key, "format": "json", "field_list": "name"},
+                params={"api_key": key or self.default_api_key, "format": "json", "field_list": "name"},
             ).json()
 
             # Bogus request, but if the key is wrong, you get error 100: "Invalid API Key"

requirements.txt

@@ -8,7 +8,7 @@ pycountry
 #pyicu; sys_platform == 'linux' or sys_platform == 'darwin'
 rapidfuzz>=2.12.0
 requests==2.*
-settngs==0.5.0
+settngs==0.6.2
 text2digits
 typing_extensions
 wordninja

setup.py

@@ -66,7 +66,7 @@ setup(
             "rar = comicapi.archivers.rar:RarArchiver",
             "folder = comicapi.archivers.folder:FolderArchiver",
         ],
-        "comictagger.talkers": [
+        "comictagger.talker": [
             "comicvine = comictalker.talkers.comicvine:ComicVineTalker",
         ],
     },