Add: validation of bundled themes in build workflow

.github/workflows/build.yml

@@ -16,6 +16,7 @@ jobs:
     steps:
       - name: Checkout sources
         uses: actions/checkout@v4
+
       - name: Install stable toolchain
         uses: dtolnay/rust-toolchain@1.70
 
@@ -107,6 +108,9 @@ jobs:
       - name: Validate queries
         run: cargo xtask query-check
 
+      - name: Validate themes
+        run: cargo xtask theme-check
+
       - name: Generate docs
         run: cargo xtask docgen
 

helix-term/src/application.rs

@@ -120,6 +120,12 @@ impl Application {
                         log::warn!("failed to load theme `{}` - {}", theme, e);
                         e
                     })
+                    .map(|(theme, load_errors)| {
+                        for error in load_errors {
+                            log::warn!("{}", error);
+                        }
+                        theme
+                    })
                     .ok()
                     .filter(|theme| (true_color || theme.is_16_color()))
             })
@@ -435,6 +441,12 @@ impl Application {
                         log::warn!("failed to load theme `{}` - {}", theme, e);
                         e
                     })
+                    .map(|(theme, load_errors)| {
+                        for error in load_errors {
+                            log::warn!("{}", error);
+                        }
+                        theme
+                    })
                     .ok()
                     .filter(|theme| (true_color || theme.is_16_color()))
             })

helix-term/src/commands/typed.rs

@@ -879,10 +879,13 @@ fn theme(
                 // Ensures that a preview theme gets cleaned up if the user backspaces until the prompt is empty.
                 cx.editor.unset_theme_preview();
             } else if let Some(theme_name) = args.first() {
-                if let Ok(theme) = cx.editor.theme_loader.load(theme_name) {
+                if let Ok((theme, load_errors)) = cx.editor.theme_loader.load(theme_name) {
                     if !(true_color || theme.is_16_color()) {
                         bail!("Unsupported theme: theme requires true color support");
                     }
+                    for error in load_errors {
+                        log::warn!("{}", error);
+                    }
                     cx.editor.set_theme_preview(theme);
                 };
             };
@@ -893,6 +896,12 @@ fn theme(
                     .editor
                     .theme_loader
                     .load(theme_name)
+                    .map(|(theme, load_errors)| {
+                        for error in load_errors {
+                            log::warn!("{}", error);
+                        }
+                        theme
+                    })
                     .map_err(|err| anyhow::anyhow!("Could not load theme: {}", err))?;
                 if !(true_color || theme.is_16_color()) {
                     bail!("Unsupported theme: theme requires true color support");

helix-view/src/theme.rs

@@ -52,21 +52,24 @@ impl Loader {
     }
 
     /// Loads a theme searching directories in priority order.
-    pub fn load(&self, name: &str) -> Result<Theme> {
+    pub fn load(&self, name: &str) -> Result<(Theme, Vec<String>)> {
         if name == "default" {
-            return Ok(self.default());
+            return Ok((self.default(), Vec::new()));
         }
         if name == "base16_default" {
-            return Ok(self.base16_default());
+            return Ok((self.base16_default(), Vec::new()));
         }
 
         let mut visited_paths = HashSet::new();
-        let theme = self.load_theme(name, &mut visited_paths).map(Theme::from)?;
+        let (theme, load_errors) = self
+            .load_theme(name, &mut visited_paths)
+            .map(Theme::from_toml)?;
 
-        Ok(Theme {
+        let theme = Theme {
             name: name.into(),
             ..theme
-        })
+        };
+        Ok((theme, load_errors))
     }
 
     /// Recursively load a theme, merging with any inherited parent themes.
@@ -220,19 +223,11 @@ pub struct Theme {
 
 impl From<Value> for Theme {
     fn from(value: Value) -> Self {
-        if let Value::Table(table) = value {
-            let (styles, scopes, highlights) = build_theme_values(table);
-
-            Self {
-                styles,
-                scopes,
-                highlights,
-                ..Default::default()
-            }
-        } else {
-            warn!("Expected theme TOML value to be a table, found {:?}", value);
-            Default::default()
+        let (theme, load_errors) = Theme::from_toml(value);
+        for error in load_errors {
+            warn!("{}", error);
         }
+        theme
     }
 }
 
@@ -242,31 +237,29 @@ impl<'de> Deserialize<'de> for Theme {
         D: Deserializer<'de>,
     {
         let values = Map::<String, Value>::deserialize(deserializer)?;
-
-        let (styles, scopes, highlights) = build_theme_values(values);
-
-        Ok(Self {
-            styles,
-            scopes,
-            highlights,
-            ..Default::default()
-        })
+        let (theme, load_errors) = Theme::from_keys(values);
+        for error in load_errors {
+            warn!("{}", error);
+        }
+        Ok(theme)
     }
 }
 
 fn build_theme_values(
     mut values: Map<String, Value>,
-) -> (HashMap<String, Style>, Vec<String>, Vec<Style>) {
+) -> (HashMap<String, Style>, Vec<String>, Vec<Style>, Vec<String>) {
     let mut styles = HashMap::new();
     let mut scopes = Vec::new();
     let mut highlights = Vec::new();
 
+    let mut load_errors = Vec::new();
+
     // TODO: alert user of parsing failures in editor
     let palette = values
         .remove("palette")
         .map(|value| {
             ThemePalette::try_from(value).unwrap_or_else(|err| {
-                warn!("{}", err);
+                load_errors.push(err);
                 ThemePalette::default()
             })
         })
@@ -279,7 +272,7 @@ fn build_theme_values(
     for (name, style_value) in values {
         let mut style = Style::default();
         if let Err(err) = palette.parse_style(&mut style, style_value) {
-            warn!("{}", err);
+            load_errors.push(err);
         }
 
         // these are used both as UI and as highlights
@@ -288,7 +281,7 @@ fn build_theme_values(
         highlights.push(style);
     }
 
-    (styles, scopes, highlights)
+    (styles, scopes, highlights, load_errors)
 }
 
 impl Theme {
@@ -354,6 +347,27 @@ impl Theme {
                 .all(|color| !matches!(color, Some(Color::Rgb(..))))
         })
     }
+
+    fn from_toml(value: Value) -> (Self, Vec<String>) {
+        if let Value::Table(table) = value {
+            Theme::from_keys(table)
+        } else {
+            warn!("Expected theme TOML value to be a table, found {:?}", value);
+            Default::default()
+        }
+    }
+
+    fn from_keys(toml_keys: Map<String, Value>) -> (Self, Vec<String>) {
+        let (styles, scopes, highlights, load_errors) = build_theme_values(toml_keys);
+
+        let theme = Self {
+            styles,
+            scopes,
+            highlights,
+            ..Default::default()
+        };
+        (theme, load_errors)
+    }
 }
 
 struct ThemePalette {

runtime/themes/autumn.toml

@@ -68,8 +68,10 @@
 "ui.statusline.select" = { fg = "my_gray7", bg = "my_black", modifiers = ["bold"] }
 "ui.text.focus" = "my_white1"
 "ui.text" = "my_white1"
-"ui.virtual.inlay-hint" = { fg = "my_gray4", bg="my_black", modifiers = ["normal"] }
-"ui.virtual.inlay-hint.parameter" = { fg = "my_gray4", modifiers = ["normal"] }
+# Invalid modifier: "normal". See 'https://github.com/helix-editor/helix/issues/5709'
+"ui.virtual.inlay-hint" = { fg = "my_gray4", bg="my_black" } #, modifiers = ["normal"] }
+# "ui.virtual.inlay-hint.parameter" = { fg = "my_gray4", modifiers = ["normal"] }
+"ui.virtual.inlay-hint.parameter" = "my_gray4"
 "ui.virtual.inlay-hint.type" = { fg = "my_gray4", modifiers = ["italic"] }
 "ui.virtual.jump-label" = { fg = "my_yellow2", modifiers = ["bold"] }
 "ui.virtual.ruler" = { bg = "my_gray1" }

runtime/themes/emacs.toml

@@ -68,7 +68,8 @@
 "ui.menu.selected" = { fg = "dark_red", bg = "light_blue" }
 "ui.selection" = { bg = "lightgoldenrod1" }
 "ui.selection.primary" = { bg = "lightgoldenrod2" }
-"ui.virtual.whitespace" = "highlight"
+# Malformed ANSI: highlight. See 'https://github.com/helix-editor/helix/issues/5709'
+# "ui.virtual.whitespace" = "highlight"
 "ui.virtual.ruler" = { bg = "gray95" }
 "ui.virtual.inlay-hint" = { fg = "gray75" }
 "ui.cursorline.primary" = { bg = "darkseagreen2" }

runtime/themes/flatwhite.toml

@@ -61,8 +61,11 @@
 "ui.virtual" = { fg = "base5", bg = "base6" }
 "ui.virtual.whitespace" = { fg = "base5" }
 "ui.virtual.ruler" = { bg = "base6" }
-"ui.virtual.inlay-hint" = { fg = "base4", modifiers = ["normal"] }
-"ui.virtual.inlay-hint.parameter" = { fg = "base3", modifiers = ["normal"] }
+# Invalid modifier: "normal". See 'https://github.com/helix-editor/helix/issues/5709'
+# "ui.virtual.inlay-hint" = { fg = "base4", modifiers = ["normal"] }
+# "ui.virtual.inlay-hint.parameter" = { fg = "base3", modifiers = ["normal"] }
+"ui.virtual.inlay-hint" = "base4"
+"ui.virtual.inlay-hint.parameter" = "base3"
 "ui.virtual.inlay-hint.type" = { fg = "base3", modifiers = ["italic"] }
 
 "ui.linenr" = { bg = "base6" }

runtime/themes/kanagawa.toml

@@ -25,7 +25,8 @@
 "ui.statusline.normal" = { fg = "sumiInk0", bg = "crystalBlue", modifiers = ["bold"] }
 "ui.statusline.insert" = { fg = "sumiInk0", bg = "autumnGreen", modifiers = ["bold"] }
 "ui.statusline.select" = { fg = "sumiInk0", bg = "oniViolet", modifiers = ["bold"] }
-"ui.statusline.separator" = { fg = "", bg = "" }
+# Malformed ANSI: "". See 'https://github.com/helix-editor/helix/issues/5709'
+# "ui.statusline.separator" = { fg = "", bg = "" }
 
 "ui.bufferline" = { fg = "fujiGray", bg = "sumiInk0" }
 "ui.bufferline.active" = { fg = "oldWhite", bg = "sumiInk0" }

runtime/themes/monokai.toml

@@ -79,7 +79,8 @@
 "ui.statusline" = { fg = "active_text", bg = "#414339" }
 "ui.statusline.inactive" = { fg = "active_text", bg = "#75715e" }
 
-"ui.bufferline" = { fg = "grey2", bg = "bg3" }
+# Malformed ANSI: grey2, bg3. See 'https://github.com/helix-editor/helix/issues/5709'
+# "ui.bufferline" = { fg = "grey2", bg = "bg3" }
 "ui.bufferline.active" = { fg = "active_text", bg = "selection", modifiers = [
   "bold",
 ] }

runtime/themes/monokai_aqua.toml

@@ -8,9 +8,12 @@ inherits = "monokai"
 
 "type" = { fg = "type", modifiers = ["bold"] }
 
-"ui.statusline.normal" = { fg = "light-black", bg = "cyan" }
-"ui.statusline.insert" = { fg = "light-black", bg = "green" }
-"ui.statusline.select" = { fg = "light-black", bg = "purple" }
+# Malformed ANSI: light-black, purple. See 'https://github.com/helix-editor/helix/issues/5709'
+# "ui.statusline.normal" = { fg = "light-black", bg = "cyan" }
+"ui.statusline.normal" = { bg = "cyan" }
+# "ui.statusline.insert" = { fg = "light-black", bg = "green" }
+"ui.statusline.insert" = { bg = "green" }
+# "ui.statusline.select" = { fg = "light-black", bg = "purple" }
 
 "ui.virtual.jump-label" = { fg = "cyan", modifiers = ["bold"] }
 

runtime/themes/zed_onedark.toml

@@ -61,7 +61,8 @@
 "ui.cursor" = { fg = "white", modifiers = ["reversed"] }
 "ui.cursor.primary" = { fg = "white", modifiers = ["reversed"] }
 "ui.cursor.match" = { fg = "blue", modifiers = ["underlined"] }
-"ui.cursor.insert" = { fg = "dark-blue" }
+# Malformed ANSI: dark-blue. See 'https://github.com/helix-editor/helix/issues/5709'
+# "ui.cursor.insert" = { fg = "dark-blue" }
 
 "ui.selection" = { bg = "faint-gray" }
 "ui.selection.primary" = { bg = "#293b5bff" }

xtask/src/main.rs

@@ -2,6 +2,7 @@ mod docgen;
 mod helpers;
 mod path;
 mod querycheck;
+mod theme_check;
 
 use std::{env, error::Error};
 
@@ -11,6 +12,7 @@ pub mod tasks {
     use crate::docgen::{lang_features, typable_commands, write};
     use crate::docgen::{LANG_SUPPORT_MD_OUTPUT, TYPABLE_COMMANDS_MD_OUTPUT};
     use crate::querycheck::query_check;
+    use crate::theme_check::theme_check;
     use crate::DynError;
 
     pub fn docgen() -> Result<(), DynError> {
@@ -23,6 +25,10 @@ pub mod tasks {
         query_check()
     }
 
+    pub fn themecheck() -> Result<(), DynError> {
+        theme_check()
+    }
+
     pub fn print_help() {
         println!(
             "
@@ -43,6 +49,7 @@ fn main() -> Result<(), DynError> {
         Some(t) => match t.as_str() {
             "docgen" => tasks::docgen()?,
             "query-check" => tasks::querycheck()?,
+            "theme-check" => tasks::themecheck()?,
             invalid => return Err(format!("Invalid task name: {}", invalid).into()),
         },
     };

xtask/src/path.rs

@@ -11,8 +11,16 @@ pub fn book_gen() -> PathBuf {
     project_root().join("book/src/generated/")
 }
 
+pub fn runtime() -> PathBuf {
+    project_root().join("runtime")
+}
+
 pub fn ts_queries() -> PathBuf {
-    project_root().join("runtime/queries")
+    runtime().join("queries")
+}
+
+pub fn themes() -> PathBuf {
+    runtime().join("themes")
 }
 
 pub fn lang_config() -> PathBuf {

xtask/src/theme_check.rs

@@ -0,0 +1,33 @@
+use helix_view::theme::Loader;
+
+use crate::{path, DynError};
+
+pub fn theme_check() -> Result<(), DynError> {
+    let theme_names = [
+        vec!["default".to_string(), "base16_default".to_string()],
+        Loader::read_names(&path::themes()),
+    ]
+    .concat();
+    let loader = Loader::new(&[path::runtime()]);
+    let mut errors_present = false;
+
+    for name in theme_names {
+        let (_, load_errors) = loader.load(&name).unwrap();
+
+        if !load_errors.is_empty() {
+            errors_present = true;
+            println!("Theme '{name}' loaded with errors:");
+            for error in load_errors {
+                println!("\t* {}", error);
+            }
+        }
+    }
+
+    match errors_present {
+        true => Err("Errors found when loading bundled themes".into()),
+        false => {
+            println!("Theme check successful!");
+            Ok(())
+        }
+    }
+}