From 8ff1e3ab7b60694cd402e47e68ad954d702ff898 Mon Sep 17 00:00:00 2001 From: Tim Date: Tue, 3 Sep 2024 19:57:18 +0100 Subject: [PATCH 01/13] Add: xtask to check themes for validation warnings --- .github/workflows/build.yml | 3 ++ Cargo.lock | 2 ++ xtask/Cargo.toml | 2 ++ xtask/src/main.rs | 7 ++++ xtask/src/path.rs | 4 +++ xtask/src/theme_check.rs | 71 +++++++++++++++++++++++++++++++++++++ 6 files changed, 89 insertions(+) create mode 100644 xtask/src/theme_check.rs diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 93fcb98164df..ef69a7c59dff 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -107,6 +107,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 diff --git a/Cargo.lock b/Cargo.lock index d2f90669816b..500a16fda01a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2910,6 +2910,8 @@ dependencies = [ "helix-loader", "helix-term", "helix-view", + "log", + "once_cell", "toml", ] diff --git a/xtask/Cargo.toml b/xtask/Cargo.toml index 25d8955e5a1f..0b6befb83bc4 100644 --- a/xtask/Cargo.toml +++ b/xtask/Cargo.toml @@ -17,3 +17,5 @@ helix-core = { path = "../helix-core" } helix-view = { path = "../helix-view" } helix-loader = { path = "../helix-loader" } toml = "0.8" +log = "~0.4" +once_cell = "1.19" diff --git a/xtask/src/main.rs b/xtask/src/main.rs index aba5c449985e..fcb462f252d5 100644 --- a/xtask/src/main.rs +++ b/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()), }, }; diff --git a/xtask/src/path.rs b/xtask/src/path.rs index 62d884f2eed8..6f4545c274aa 100644 --- a/xtask/src/path.rs +++ b/xtask/src/path.rs @@ -18,3 +18,7 @@ pub fn ts_queries() -> PathBuf { pub fn lang_config() -> PathBuf { project_root().join("languages.toml") } + +pub fn themes() -> PathBuf { + project_root().join("runtime/themes") +} diff --git a/xtask/src/theme_check.rs b/xtask/src/theme_check.rs new file mode 100644 index 000000000000..8bbd896542e6 --- /dev/null +++ b/xtask/src/theme_check.rs @@ -0,0 +1,71 @@ +use std::sync::{Arc, Mutex}; + +use helix_view::theme::Loader; +use log; + +use crate::{path, DynError}; +use once_cell::sync::Lazy; + +static LOGGER: Lazy = Lazy::new(|| MockLog::new()); + +pub fn theme_check() -> Result<(), DynError> { + log::set_logger(&*LOGGER).unwrap(); + log::set_max_level(log::LevelFilter::Warn); + + let theme_names = Loader::read_names(&path::themes()); + let loader = Loader::new(&vec![path::project_root().join("runtime")]); + + let mut issues_found = false; + for name in theme_names { + let _ = loader.load(&name).unwrap(); + + { + let warnings = LOGGER.warnings.lock().unwrap(); + if !warnings.is_empty() { + issues_found = true; + + println!("Theme '{name}' loaded with warnings:"); + for warning in warnings.iter() { + println!("{warning}"); + } + } + } + + LOGGER.clear(); + } + match issues_found { + true => Err("Issues found in bundled themes".to_string().into()), + false => Ok(()), + } +} + +struct MockLog { + warnings: Arc>>, +} + +impl MockLog { + pub fn new() -> Self { + MockLog { + warnings: Arc::new(Mutex::new(Vec::new())), + } + } + + pub fn clear(&self) { + let mut warnings = self.warnings.lock().unwrap(); + warnings.clear(); + } +} + +impl log::Log for MockLog { + fn enabled(&self, _metadata: &log::Metadata) -> bool { + true + } + + fn log(&self, record: &log::Record) { + let mut warnings = self.warnings.lock().unwrap(); + warnings.push(record.args().to_string()); + } + + fn flush(&self) { // Do nothing + } +} From ce7875baa2fa56be850d6f9b977ff69d5af64373 Mon Sep 17 00:00:00 2001 From: Tim Date: Tue, 3 Sep 2024 20:04:10 +0100 Subject: [PATCH 02/13] Update: tidied up runtime paths --- xtask/src/path.rs | 14 +++++++++----- xtask/src/theme_check.rs | 3 ++- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/xtask/src/path.rs b/xtask/src/path.rs index 6f4545c274aa..cf8e8792e9b1 100644 --- a/xtask/src/path.rs +++ b/xtask/src/path.rs @@ -11,14 +11,18 @@ pub fn book_gen() -> PathBuf { project_root().join("book/src/generated/") } -pub fn ts_queries() -> PathBuf { - project_root().join("runtime/queries") +pub fn runtime() -> PathBuf { + project_root().join("runtime") } -pub fn lang_config() -> PathBuf { - project_root().join("languages.toml") +pub fn ts_queries() -> PathBuf { + runtime().join("queries") } pub fn themes() -> PathBuf { - project_root().join("runtime/themes") + runtime().join("themes") +} + +pub fn lang_config() -> PathBuf { + project_root().join("languages.toml") } diff --git a/xtask/src/theme_check.rs b/xtask/src/theme_check.rs index 8bbd896542e6..b1af3dabc434 100644 --- a/xtask/src/theme_check.rs +++ b/xtask/src/theme_check.rs @@ -13,7 +13,7 @@ pub fn theme_check() -> Result<(), DynError> { log::set_max_level(log::LevelFilter::Warn); let theme_names = Loader::read_names(&path::themes()); - let loader = Loader::new(&vec![path::project_root().join("runtime")]); + let loader = Loader::new(&vec![path::runtime()]); let mut issues_found = false; for name in theme_names { @@ -28,6 +28,7 @@ pub fn theme_check() -> Result<(), DynError> { for warning in warnings.iter() { println!("{warning}"); } + println!("\n"); } } From 9ed491a2eb19eb58448b202275676942f72f7756 Mon Sep 17 00:00:00 2001 From: Tim Date: Tue, 3 Sep 2024 20:05:44 +0100 Subject: [PATCH 03/13] Update: test build workflow --- .github/workflows/build.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index ef69a7c59dff..75fd00279d0c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -2,8 +2,8 @@ name: Build on: pull_request: push: - branches: - - master + # branches: + # - master merge_group: schedule: - cron: "00 01 * * *" @@ -16,6 +16,7 @@ jobs: steps: - name: Checkout sources uses: actions/checkout@v4 + - name: Install stable toolchain uses: dtolnay/rust-toolchain@1.70 From af5c69ad0ddc364ccc80b392d4ae7cce7fd8b775 Mon Sep 17 00:00:00 2001 From: Tim Date: Tue, 3 Sep 2024 20:13:20 +0100 Subject: [PATCH 04/13] Update: address clippy lints --- xtask/src/theme_check.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/xtask/src/theme_check.rs b/xtask/src/theme_check.rs index b1af3dabc434..878220561d5b 100644 --- a/xtask/src/theme_check.rs +++ b/xtask/src/theme_check.rs @@ -1,19 +1,18 @@ use std::sync::{Arc, Mutex}; use helix_view::theme::Loader; -use log; use crate::{path, DynError}; use once_cell::sync::Lazy; -static LOGGER: Lazy = Lazy::new(|| MockLog::new()); +static LOGGER: Lazy = Lazy::new(MockLog::new); pub fn theme_check() -> Result<(), DynError> { log::set_logger(&*LOGGER).unwrap(); log::set_max_level(log::LevelFilter::Warn); let theme_names = Loader::read_names(&path::themes()); - let loader = Loader::new(&vec![path::runtime()]); + let loader = Loader::new(&[path::runtime()]); let mut issues_found = false; for name in theme_names { @@ -28,7 +27,6 @@ pub fn theme_check() -> Result<(), DynError> { for warning in warnings.iter() { println!("{warning}"); } - println!("\n"); } } From 232f8e4c60667bb594fe80e8919f04be38d319f6 Mon Sep 17 00:00:00 2001 From: Tim Date: Tue, 3 Sep 2024 20:17:20 +0100 Subject: [PATCH 05/13] Revert: only trigger workflow on push to master branch --- .github/workflows/build.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 75fd00279d0c..c9f198d0c71e 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -2,8 +2,8 @@ name: Build on: pull_request: push: - # branches: - # - master + branches: + - master merge_group: schedule: - cron: "00 01 * * *" From 34c6a353e5b3b925a9c87f89a12751fd8e4346a9 Mon Sep 17 00:00:00 2001 From: Tim Date: Wed, 4 Sep 2024 20:43:18 +0100 Subject: [PATCH 06/13] Add: Theme::from_keys factory method to construct theme from Toml keys --- Cargo.lock | 2 -- helix-view/src/theme.rs | 50 ++++++++++++++++----------- xtask/Cargo.toml | 2 -- xtask/src/theme_check.rs | 73 ++++++++++------------------------------ 4 files changed, 48 insertions(+), 79 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 500a16fda01a..d2f90669816b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2910,8 +2910,6 @@ dependencies = [ "helix-loader", "helix-term", "helix-view", - "log", - "once_cell", "toml", ] diff --git a/helix-view/src/theme.rs b/helix-view/src/theme.rs index 4acc56648aa0..fc628bfad835 100644 --- a/helix-view/src/theme.rs +++ b/helix-view/src/theme.rs @@ -221,14 +221,11 @@ pub struct Theme { impl From 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() + let (theme, validation_failures) = Theme::from_keys(table); + for validation_failure in validation_failures { + warn!("{}", validation_failure); } + theme } else { warn!("Expected theme TOML value to be a table, found {:?}", value); Default::default() @@ -242,31 +239,29 @@ impl<'de> Deserialize<'de> for Theme { D: Deserializer<'de>, { let values = Map::::deserialize(deserializer)?; - - let (styles, scopes, highlights) = build_theme_values(values); - - Ok(Self { - styles, - scopes, - highlights, - ..Default::default() - }) + let (theme, validation_failures) = Theme::from_keys(values); + for validation_failure in validation_failures { + warn!("{}", validation_failure); + } + Ok(theme) } } fn build_theme_values( mut values: Map, -) -> (HashMap, Vec, Vec