Make config code highlighter error more friendly (#152)

* Change opts test layout

* Show existing opaque error

* Improve error message for config code highlighter

src/color.rs

@@ -107,14 +107,21 @@ impl PartialEq for Theme {
     }
 }
 
-// TODO: the error message here degraded when switching this to allow for custom themes. It'd be
-//       good to still list all the default themes when passing in something else. Add a test for
-//       the error message
-#[derive(Deserialize, Clone, Debug, PartialEq, Eq)]
-#[serde(untagged)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub enum SyntaxTheme {
     Defaults(ThemeDefaults),
-    Custom { path: PathBuf },
+    Custom(ThemeCustom),
+}
+
+#[derive(Deserialize, Clone, Debug, PartialEq, Eq)]
+pub struct ThemeCustom {
+    path: PathBuf,
+}
+
+impl SyntaxTheme {
+    pub fn custom(path: PathBuf) -> Self {
+        Self::Custom(ThemeCustom { path })
+    }
 }
 
 impl TryFrom<SyntaxTheme> for SyntectTheme {
@@ -123,7 +130,7 @@ impl TryFrom<SyntaxTheme> for SyntectTheme {
     fn try_from(syntax_theme: SyntaxTheme) -> Result<Self, Self::Error> {
         match syntax_theme {
             SyntaxTheme::Defaults(default) => Ok(SyntectTheme::from(default)),
-            SyntaxTheme::Custom { path } => {
+            SyntaxTheme::Custom(ThemeCustom { path }) => {
                 let mut reader = BufReader::new(File::open(&path).with_context(|| {
                     format!("Failed opening theme from path {}", path.display())
                 })?);
@@ -134,8 +141,51 @@ impl TryFrom<SyntaxTheme> for SyntectTheme {
     }
 }
 
-#[derive(Deserialize, Clone, Copy, Debug, PartialEq, Eq)]
-#[serde(rename_all = "kebab-case")]
+// Give better error messages than regular `#[serde(untagged)]`
+impl<'de> Deserialize<'de> for SyntaxTheme {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        #[derive(Deserialize)]
+        #[serde(untagged)]
+        enum Untagged {
+            Defaults(String),
+            Custom(ThemeCustom),
+        }
+
+        let Ok(untagged) = Untagged::deserialize(deserializer) else {
+            return Err(serde::de::Error::custom(
+                "Expects either a default theme name or a path to a custom theme. E.g.\n\
+                default: \"inspired-github\"\n\
+                custom:  { path = \"/path/to/custom.tmTheme\" }"
+            ))
+        };
+
+        match untagged {
+            // Unfortunately #[serde(untagged)] uses private internals to reuse a deserializer
+            // mutliple times. We can't so now we have to fall back to other means to give a good
+            // error message ;-;
+            Untagged::Defaults(theme_name) => match ThemeDefaults::from_kebab(&theme_name) {
+                Some(theme) => Ok(Self::Defaults(theme)),
+                None => {
+                    let variants = ThemeDefaults::kebab_pairs()
+                        .iter()
+                        .map(|(kebab, _)| format!("\"{kebab}\""))
+                        .collect::<Vec<_>>()
+                        .join(", ");
+                    let msg = format!(
+                        "\"{theme_name}\" didn't match any of the expected variants: [{variants}]"
+                    );
+                    Err(serde::de::Error::custom(msg))
+                }
+            },
+            Untagged::Custom(custom) => Ok(Self::Custom(custom)),
+        }
+    }
+}
+
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub enum ThemeDefaults {
     Base16OceanDark,
     Base16EightiesDark,
@@ -147,6 +197,24 @@ pub enum ThemeDefaults {
 }
 
 impl ThemeDefaults {
+    fn kebab_pairs() -> &'static [(&'static str, Self)] {
+        &[
+            ("base16-ocean-dark", Self::Base16OceanDark),
+            ("base16-eighties-dark", Self::Base16EightiesDark),
+            ("base16-mocha-dark", Self::Base16MochaDark),
+            ("base16-ocean-light", Self::Base16OceanLight),
+            ("inspired-github", Self::InspiredGithub),
+            ("solarized-dark", Self::SolarizedDark),
+            ("solarized-light", Self::SolarizedLight),
+        ]
+    }
+
+    fn from_kebab(kebab: &str) -> Option<Self> {
+        Self::kebab_pairs()
+            .iter()
+            .find_map(|&(hay, var)| (kebab == hay).then_some(var))
+    }
+
     pub fn as_syntect_name(self) -> &'static str {
         match self {
             Self::Base16OceanDark => "base16-ocean.dark",

src/opts/tests/error_msg.rs

@@ -0,0 +1,24 @@
+macro_rules! snapshot_config_parse_error {
+    ( $( ($test_name:ident, $md_text:ident) ),* $(,)? ) => {
+        $(
+            #[test]
+            fn $test_name() {
+                let err = ::toml::from_str::<$crate::opts::Config>($md_text).unwrap_err();
+
+                ::insta::with_settings!({
+                    description => $md_text,
+                }, {
+                    ::insta::assert_display_snapshot!(err);
+                });
+            }
+        )*
+    }
+}
+
+const UNKNOWN_THEME: &str = r#"light-theme.code-highlighter = "doesnt-exist""#;
+const INVALID_THEME_TY: &str = "light-theme.code-highlighter = []";
+
+snapshot_config_parse_error!(
+    (unknown_theme, UNKNOWN_THEME),
+    (invalid_theme_ty, INVALID_THEME_TY),
+);

src/opts/tests/mod.rs

@@ -0,0 +1,2 @@
+mod error_msg;
+mod parse;

src/opts/tests.rs → src/opts/tests/parse.rs

@@ -1,10 +1,12 @@
 use std::{ffi::OsString, path::PathBuf};
 
-use super::{cli, config, Opts, ResolvedTheme, ThemeType};
 use crate::color::{SyntaxTheme, Theme, ThemeDefaults};
 use crate::keybindings::Keybindings;
-use crate::opts::config::{FontOptions, LinesToScroll};
-use crate::opts::Args;
+use crate::opts::{
+    cli,
+    config::{self, FontOptions, LinesToScroll},
+    Args, Opts, ResolvedTheme, ThemeType,
+};
 
 use pretty_assertions::assert_eq;
 
@@ -200,7 +202,7 @@ fn custom_syntax_theme() {
     fn config_with_theme_at(path: PathBuf) -> config::Config {
         let mut config = config::Config::default();
         config.light_theme = Some(config::OptionalTheme {
-            code_highlighter: Some(SyntaxTheme::Custom { path }),
+            code_highlighter: Some(SyntaxTheme::custom(path)),
             ..Default::default()
         });
         config

src/opts/tests/snapshots/inlyne__opts__tests__error_msg__invalid_theme_ty.snap

@@ -0,0 +1,13 @@
+---
+source: src/opts/tests/error_msg.rs
+description: "light-theme.code-highlighter = []"
+expression: err
+---
+TOML parse error at line 1, column 32
+  |
+1 | light-theme.code-highlighter = []
+  |                                ^^
+Expects either a default theme name or a path to a custom theme. E.g.
+default: "inspired-github"
+custom:  { path = "/path/to/custom.tmTheme" }
+

src/opts/tests/snapshots/inlyne__opts__tests__error_msg__unknown_theme.snap

@@ -0,0 +1,11 @@
+---
+source: src/opts/tests/error_msg.rs
+description: "light-theme.code-highlighter = \"doesnt-exist\""
+expression: err
+---
+TOML parse error at line 1, column 32
+  |
+1 | light-theme.code-highlighter = "doesnt-exist"
+  |                                ^^^^^^^^^^^^^^
+"doesnt-exist" didn't match any of the expected variants: ["base16-ocean-dark", "base16-eighties-dark", "base16-mocha-dark", "base16-ocean-light", "inspired-github", "solarized-dark", "solarized-light"]
+