Theme error handling and messaging improvements

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "posting"
-version = "2.3.0"
+version = "2.3.1"
 description = "The modern API client that lives in your terminal."
 authors = [
     { name = "Darren Burns", email = "[email protected]" }

src/posting/app.py

@@ -13,7 +13,7 @@
 from textual.css.query import NoMatches
 from textual.events import Click
 from textual.reactive import Reactive, reactive
-from textual.app import App, ComposeResult, ReturnType
+from textual.app import App, ComposeResult, InvalidThemeError, ReturnType
 from textual.binding import Binding
 from textual.containers import Horizontal, Vertical
 from textual.screen import Screen
@@ -44,7 +44,11 @@
 from posting.jump_overlay import JumpOverlay
 from posting.jumper import Jumper
 from posting.scripts import execute_script, uncache_module, Posting as PostingContext
-from posting.themes import BUILTIN_THEMES, load_user_theme, load_user_themes
+from posting.themes import (
+    BUILTIN_THEMES,
+    load_user_theme,
+    load_user_themes,
+)
 from posting.types import CertTypes, PostingLayout
 from posting.user_host import get_user_host_string
 from posting.variables import SubstitutionError, get_variables, update_variables
@@ -936,16 +940,23 @@ async def watch_collection_files(self) -> None:
     async def watch_themes(self) -> None:
         """Watching the theme directory for changes."""
         async for changes in awatch(self.settings.theme_directory):
-            print("Theme changes detected")
             for change_type, file_path in changes:
                 if file_path.endswith((".yml", ".yaml")):
-                    theme = load_user_theme(Path(file_path))
+                    try:
+                        theme = load_user_theme(Path(file_path))
+                    except Exception as e:
+                        print(f"Couldn't load theme from {str(file_path)}: {e}.")
+                        continue
                     if theme and theme.name == self.theme:
                         self.register_theme(theme)
                         self.set_reactive(App.theme, theme.name)
                         try:
                             self._watch_theme(theme.name)
                         except Exception as e:
+                            # I don't think we want to notify here, as editors often
+                            # use heuristics to determine whether to save a file. This could
+                            # prove jarring if we pop up a notification without the user
+                            # explicitly saving the file in their editor.
                             print(f"Error refreshing CSS: {e}")
 
     def on_mount(self) -> None:
@@ -963,7 +974,17 @@ def on_mount(self) -> None:
             available_themes |= load_xresources_themes()
 
         if settings.load_user_themes:
-            available_themes |= load_user_themes()
+            loaded_themes, failed_themes = load_user_themes()
+            available_themes |= loaded_themes
+
+            # Display a single message for all failed themes.
+            if failed_themes:
+                self.notify(
+                    "\n".join(f"• {path.name}" for path, _ in failed_themes),
+                    title=f"Failed to read {len(failed_themes)} theme{'s' if len(failed_themes) > 1 else ''}",
+                    severity="error",
+                    timeout=8,
+                )
 
         for theme in available_themes.values():
             self.register_theme(theme)
@@ -974,7 +995,19 @@ def on_mount(self) -> None:
         for theme_name in unwanted_themes:
             self.unregister_theme(theme_name)
 
-        self.theme = settings.theme
+        try:
+            self.theme = settings.theme
+        except InvalidThemeError:
+            # This can happen if the user has a custom theme that is invalid,
+            # e.g. a color is invalid or the YAML cannot be parsed.
+            self.theme = "galaxy"
+            self.notify(
+                "Check theme file for syntax errors, invalid colors, etc.\n"
+                "Falling back to [b i]galaxy[/] theme.",
+                title=f"Couldn't apply theme {settings.theme!r}",
+                severity="error",
+                timeout=8,
+            )
 
         self.set_keymap(self.settings.keymap)
         self.jumper = Jumper(

src/posting/themes.py

@@ -1,9 +1,10 @@
 from pathlib import Path
+from typing import NamedTuple
 import uuid
 from pydantic import BaseModel, Field
 from rich.style import Style
+from textual.app import InvalidThemeError
 from textual.color import Color
-from textual.design import ColorSystem
 from textual.theme import Theme as TextualTheme
 from textual.widgets.text_area import TextAreaTheme
 import yaml
@@ -140,20 +141,6 @@ class Theme(BaseModel):
     description: str | None = Field(default=None, exclude=True)
     homepage: str | None = Field(default=None, exclude=True)
 
-    def to_color_system(self) -> ColorSystem:
-        """Convert this theme to a ColorSystem."""
-        return ColorSystem(
-            **self.model_dump(
-                exclude={
-                    "text_area",
-                    "syntax",
-                    "variable",
-                    "url",
-                    "method",
-                }
-            )
-        )
-
     def to_textual_theme(self) -> TextualTheme:
         """Convert this theme to a Textual Theme.
 
@@ -162,6 +149,9 @@ def to_textual_theme(self) -> TextualTheme:
         """
         theme_data = {
             "name": self.name,
+            "dark": self.dark,
+        }
+        colors = {
             "primary": self.primary,
             "secondary": self.secondary,
             "background": self.background,
@@ -171,9 +161,15 @@ def to_textual_theme(self) -> TextualTheme:
             "error": self.error,
             "success": self.success,
             "accent": self.accent,
-            "dark": self.dark,
         }
 
+        # Validate the colors before converting to a Textual theme.
+        for color in colors.values():
+            if color is not None:
+                Color.parse(color)
+
+        theme_data = {**colors, **theme_data}
+
         variables = {}
         if self.url:
             url_styles = self.url.fill_with_defaults(self)
@@ -242,7 +238,8 @@ def to_textual_theme(self) -> TextualTheme:
         theme_data = {k: v for k, v in theme_data.items() if v is not None}
         theme_data["variables"] = {k: v for k, v in variables.items() if v is not None}
 
-        return TextualTheme(**theme_data)
+        textual_theme = TextualTheme(**theme_data)
+        return textual_theme
 
     @staticmethod
     def text_area_theme_from_theme_variables(
@@ -305,37 +302,53 @@ def text_area_theme_from_theme_variables(
         )
 
 
-def load_user_themes() -> dict[str, TextualTheme]:
-    """Load user themes from "~/.config/posting/themes".
+class UserThemeLoadResult(NamedTuple):
+    loaded_themes: dict[str, TextualTheme]
+    """A dictionary mapping theme names to Textual themes."""
+
+    failed_themes: list[tuple[Path, Exception]]
+    """A list of tuples containing the path to the failed theme and the exception that was raised
+    while trying to load the theme."""
+
+
+def load_user_themes() -> UserThemeLoadResult:
+    """Load user themes from the theme directory.
+
+    The theme directory is defined in the settings file as `theme_directory`.
+
+    You can locate it on the command line with `posting locate themes`.
 
     Returns:
         A dictionary mapping theme names to theme objects.
     """
     directory = SETTINGS.get().theme_directory
     themes: dict[str, TextualTheme] = {}
+    failed_themes: list[tuple[Path, Exception]] = []
+
     for path in directory.iterdir():
         path_suffix = path.suffix
         if path_suffix == ".yaml" or path_suffix == ".yml":
-            with path.open() as theme_file:
-                theme_content = yaml.load(theme_file, Loader=yaml.FullLoader) or {}
-                try:
-                    themes[theme_content["name"]] = Theme(
-                        **theme_content
-                    ).to_textual_theme()
-                except KeyError:
-                    raise ValueError(
-                        f"Invalid theme file {path}. A `name` is required."
-                    )
-    return themes
+            try:
+                theme = load_user_theme(path)
+                if theme:
+                    themes[theme.name] = theme
+            except Exception as e:
+                failed_themes.append((path, e))
+
+    return UserThemeLoadResult(loaded_themes=themes, failed_themes=failed_themes)
 
 
 def load_user_theme(path: Path) -> TextualTheme | None:
     with path.open() as theme_file:
-        theme_content = yaml.load(theme_file, Loader=yaml.FullLoader) or {}
+        try:
+            theme_content = yaml.load(theme_file, Loader=yaml.FullLoader) or {}
+        except Exception as e:
+            raise InvalidThemeError(f"Could not parse theme file: {str(e)}.")
+
         try:
             return Theme(**theme_content).to_textual_theme()
-        except KeyError:
-            raise ValueError(f"Invalid theme file {path}. A `name` is required.")
+        except Exception:
+            raise InvalidThemeError(f"Invalid theme file at {str(path)}.")
 
 
 galaxy_primary = Color.parse("#C45AFF")