diff --git a/docs/next/CHANGELOG.md b/docs/next/CHANGELOG.md index 277ff405..973241bc 100644 --- a/docs/next/CHANGELOG.md +++ b/docs/next/CHANGELOG.md @@ -8,6 +8,7 @@ - The plugin marketplace now discovers valid manifests at repository roots and subdirectories, groups multiple plugins under each repository, and publishes their versions and exact default-branch commits. ### Fixed +- `herdr config check` now reports unknown built-in theme names instead of silently accepting them. (#2452) - macOS `herdr --remote` clients now keep the accepted bridge socket blocking, preventing an immediate disconnect after the protocol handshake. (#2478, thanks @mathijshenquet) - Prefix keybindings now preserve Shift in WezTerm Kitty keyboard mode, so commands such as config reload no longer trigger their unshifted action. (#2435) - Stable direct installs, self-updates, and remote helper downloads now require and verify the SHA-256 digest published for each GitHub release asset. diff --git a/src/app/state.rs b/src/app/state.rs index 0e1a8439..ca1b919b 100644 --- a/src/app/state.rs +++ b/src/app/state.rs @@ -558,24 +558,24 @@ impl Palette { /// Resolve a theme by name. Returns None for unknown names. pub fn from_name(name: &str) -> Option { - match name.to_lowercase().replace([' ', '_'], "-").as_str() { - "catppuccin" | "catppuccin-mocha" => Some(Self::catppuccin()), - "catppuccin-latte" | "latte" | "light" => Some(Self::catppuccin_latte()), + match crate::config::canonical_theme_name(name)? { + "catppuccin" => Some(Self::catppuccin()), + "catppuccin-latte" => Some(Self::catppuccin_latte()), "terminal" => Some(Self::terminal()), - "tokyo-night" | "tokyonight" => Some(Self::tokyo_night()), - "tokyo-night-day" | "tokyo-day" | "tokyonight-day" => Some(Self::tokyo_night_day()), + "tokyo-night" => Some(Self::tokyo_night()), + "tokyo-night-day" => Some(Self::tokyo_night_day()), "dracula" => Some(Self::dracula()), "nord" => Some(Self::nord()), - "gruvbox" | "gruvbox-dark" => Some(Self::gruvbox()), + "gruvbox" => Some(Self::gruvbox()), "gruvbox-light" => Some(Self::gruvbox_light()), - "one-dark" | "onedark" => Some(Self::one_dark()), - "one-light" | "onelight" => Some(Self::one_light()), - "solarized" | "solarized-dark" => Some(Self::solarized()), + "one-dark" => Some(Self::one_dark()), + "one-light" => Some(Self::one_light()), + "solarized" => Some(Self::solarized()), "solarized-light" => Some(Self::solarized_light()), "kanagawa" => Some(Self::kanagawa()), - "kanagawa-lotus" | "lotus" => Some(Self::kanagawa_lotus()), - "rose-pine" | "rosepine" => Some(Self::rose_pine()), - "rose-pine-dawn" | "rosepine-dawn" | "dawn" => Some(Self::rose_pine_dawn()), + "kanagawa-lotus" => Some(Self::kanagawa_lotus()), + "rose-pine" => Some(Self::rose_pine()), + "rose-pine-dawn" => Some(Self::rose_pine_dawn()), "vesper" => Some(Self::vesper()), _ => None, } @@ -1044,26 +1044,7 @@ impl SettingsSection { } /// All built-in theme names in display order. -pub const THEME_NAMES: &[&str] = &[ - "catppuccin", - "catppuccin-latte", - "terminal", - "tokyo-night", - "tokyo-night-day", - "dracula", - "nord", - "gruvbox", - "gruvbox-light", - "one-dark", - "one-light", - "solarized", - "solarized-light", - "kanagawa", - "kanagawa-lotus", - "rose-pine", - "rose-pine-dawn", - "vesper", -]; +pub const THEME_NAMES: &[&str] = crate::config::THEME_NAMES; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct MenuListState { diff --git a/src/config.rs b/src/config.rs index 83562c48..858ca632 100644 --- a/src/config.rs +++ b/src/config.rs @@ -30,11 +30,11 @@ pub use self::{ SpaceSidebarToken, SpacesSidebarConfig, }, sound::SoundConfig, - theme::{parse_color, CustomThemeColors, ThemeConfig}, + theme::{parse_color, CustomThemeColors, ThemeConfig, THEME_NAMES}, }; -pub(crate) use self::io::upsert_top_level_bool; pub(crate) use self::keybinds::parse_key_combo; +pub(crate) use self::{io::upsert_top_level_bool, theme::canonical_theme_name}; pub const CONFIG_PATH_ENV_VAR: &str = "HERDR_CONFIG_PATH"; pub const DEFAULT_SCROLLBACK_LIMIT_BYTES: usize = 10_000_000; @@ -72,6 +72,7 @@ impl Config { .into_iter() .chain(keybind_diags) .chain(self.remote_image_paste_key().err()) + .chain(self.theme.diagnostics()) .chain(self.ui.sound.diagnostics()) .chain(self.invalid_sidebar_bounds_diagnostic()) .collect() diff --git a/src/config/io.rs b/src/config/io.rs index b6a70427..7ed82395 100644 --- a/src/config/io.rs +++ b/src/config/io.rs @@ -341,6 +341,8 @@ fn load_live_config_from_str(content: &str) -> Result> |section| config.remote = section, ); + diagnostics.extend(config.theme.diagnostics()); + Ok(LoadedConfig { config, diagnostics, @@ -863,6 +865,20 @@ resume_agents_on_restore = true assert!(loaded.invalid_sections.is_empty()); } + #[test] + fn load_live_config_warns_about_unknown_theme_names() { + let loaded = load_live_config_from_str( + r#" +[theme] +name = "catppucin" +"#, + ) + .unwrap(); + + assert_eq!(loaded.diagnostics.len(), 1); + assert!(loaded.diagnostics[0].contains("theme.name = \"catppucin\"")); + } + #[test] fn load_live_config_warns_about_unknown_top_level_sections() { let loaded = load_live_config_from_str( diff --git a/src/config/theme.rs b/src/config/theme.rs index e98c9dfb..e89fe99c 100644 --- a/src/config/theme.rs +++ b/src/config/theme.rs @@ -1,6 +1,51 @@ use serde::Deserialize; use tracing::warn; +pub const THEME_NAMES: &[&str] = &[ + "catppuccin", + "catppuccin-latte", + "terminal", + "tokyo-night", + "tokyo-night-day", + "dracula", + "nord", + "gruvbox", + "gruvbox-light", + "one-dark", + "one-light", + "solarized", + "solarized-light", + "kanagawa", + "kanagawa-lotus", + "rose-pine", + "rose-pine-dawn", + "vesper", +]; + +pub(crate) fn canonical_theme_name(name: &str) -> Option<&'static str> { + match name.to_lowercase().replace([' ', '_'], "-").as_str() { + "catppuccin" | "catppuccin-mocha" => Some("catppuccin"), + "catppuccin-latte" | "latte" | "light" => Some("catppuccin-latte"), + "terminal" => Some("terminal"), + "tokyo-night" | "tokyonight" => Some("tokyo-night"), + "tokyo-night-day" | "tokyo-day" | "tokyonight-day" => Some("tokyo-night-day"), + "dracula" => Some("dracula"), + "nord" => Some("nord"), + "gruvbox" | "gruvbox-dark" => Some("gruvbox"), + "gruvbox-light" => Some("gruvbox-light"), + "one-dark" | "onedark" => Some("one-dark"), + "one-light" | "onelight" => Some("one-light"), + "solarized" | "solarized-dark" => Some("solarized"), + "solarized-light" => Some("solarized-light"), + "kanagawa" => Some("kanagawa"), + "kanagawa-lotus" | "lotus" => Some("kanagawa-lotus"), + "rose-pine" | "rosepine" => Some("rose-pine"), + "rose-pine-dawn" | "rosepine-dawn" | "dawn" => Some("rose-pine-dawn"), + "vesper" => Some("vesper"), + _ => None, + } +} + /// Theme configuration: pick a built-in or override individual tokens. /// /// ```toml @@ -26,6 +71,31 @@ pub struct ThemeConfig { pub custom: Option, } +impl ThemeConfig { + pub(crate) fn diagnostics(&self) -> Vec { + let valid = THEME_NAMES.join(", "); + [ + ("theme.name", self.name.as_deref(), "catppuccin"), + ("theme.dark_name", self.dark_name.as_deref(), "catppuccin"), + ( + "theme.light_name", + self.light_name.as_deref(), + "catppuccin-latte", + ), + ] + .into_iter() + .filter_map(|(field, value, fallback)| { + let value = value?; + canonical_theme_name(value).is_none().then(|| { + format!( + "unknown theme name {field} = {value:?}; using {fallback:?}; valid themes: {valid}" + ) + }) + }) + .collect() + } +} + /// Per-token color overrides. All fields optional — only set what you want to change. #[derive(Debug, Clone, Default, Deserialize)] #[serde(default)] @@ -132,6 +202,34 @@ name = "dracula" assert_eq!(config.theme.name.as_deref(), Some("dracula")); } + #[test] + fn unknown_theme_names_are_diagnosed() { + let config: Config = toml::from_str( + r#" +[theme] +name = "catppucin" +dark_name = "tokio-night" +light_name = "lattee" +"#, + ) + .unwrap(); + + let diagnostics = config.theme.diagnostics(); + assert_eq!(diagnostics.len(), 3); + assert!(diagnostics[0].contains("theme.name = \"catppucin\"")); + assert!(diagnostics[0].contains("using \"catppuccin\"")); + assert!(diagnostics[1].contains("theme.dark_name = \"tokio-night\"")); + assert!(diagnostics[2].contains("theme.light_name = \"lattee\"")); + assert!(diagnostics[2].contains("using \"catppuccin-latte\"")); + } + + #[test] + fn theme_name_aliases_are_valid() { + for name in ["catppuccin-mocha", "tokyonight", "gruvbox-dark", "dawn"] { + assert!(canonical_theme_name(name).is_some(), "alias: {name}"); + } + } + #[test] fn parse_color_accepts_reset_aliases() { use ratatui::style::Color; diff --git a/tests/cli/workspace.rs b/tests/cli/workspace.rs index 8c435b2d..4153f3b2 100644 --- a/tests/cli/workspace.rs +++ b/tests/cli/workspace.rs @@ -496,6 +496,33 @@ new_tabb = "ctrl+t" cleanup_test_base(&base); } +#[test] +fn config_check_reports_unknown_theme_names() { + let base = unique_test_dir(); + let config_home = base.join("config"); + let runtime_dir = base.join("runtime"); + let config_dir = config_home.join(app_dir_name()); + fs::create_dir_all(&config_dir).unwrap(); + fs::write( + config_dir.join("config.toml"), + "[theme]\nname = \"catppucin\"\n", + ) + .unwrap(); + + let checked = run_named_cli(&config_home, &runtime_dir, &["config", "check"]); + + assert_eq!(checked.status.code(), Some(1)); + let stdout = String::from_utf8_lossy(&checked.stdout); + assert!(stdout.contains("config: issues found"), "{stdout}"); + assert!( + stdout.contains("unknown theme name theme.name = \"catppucin\"; using \"catppuccin\""), + "{stdout}" + ); + assert!(stdout.contains("valid themes: catppuccin"), "{stdout}"); + + cleanup_test_base(&base); +} + #[test] fn config_check_reports_unreadable_config_path() { let base = unique_test_dir();