config: capture warnings and show them in config error window

This allows deprecated and invalid fields to be surfaced more visibly
in the config error window.

config/src/config.rs

@@ -814,6 +814,7 @@ impl Config {
                         config: Err(err),
                         file_name: Some(path_item.path.clone()),
                         lua: None,
+                        warnings: vec![],
                     }
                 }
                 Ok(None) => continue,
@@ -832,18 +833,23 @@ impl Config {
                 config: Err(err),
                 file_name: None,
                 lua: None,
+                warnings: vec![],
             },
             Ok(cfg) => cfg,
         }
     }
 
     pub fn try_default() -> anyhow::Result<LoadedConfig> {
-        let config = default_config_with_overrides_applied()?.compute_extra_defaults(None);
+        let (config, warnings) =
+            wezterm_dynamic::Error::capture_warnings(|| -> anyhow::Result<Config> {
+                Ok(default_config_with_overrides_applied()?.compute_extra_defaults(None))
+            });
 
         Ok(LoadedConfig {
-            config: Ok(config),
+            config: Ok(config?),
             file_name: None,
             lua: Some(make_lua_context(Path::new(""))?),
+            warnings,
         })
     }
 
@@ -863,40 +869,47 @@ impl Config {
 
         let mut s = String::new();
         file.read_to_string(&mut s)?;
-
-        let cfg: Config;
-
         let lua = make_lua_context(p)?;
-        let config: mlua::Value = smol::block_on(
-            // Skip a potential BOM that Windows software may have placed in the
-            // file. Note that we can't catch this happening for files that are
-            // imported via the lua require function.
-            lua.load(s.trim_start_matches('\u{FEFF}'))
-                .set_name(p.to_string_lossy())?
-                .eval_async(),
-        )?;
-        let config = Config::apply_overrides_to(&lua, config)?;
-        let config = Config::apply_overrides_obj_to(&lua, config, overrides)?;
-        cfg = Config::from_lua(config, &lua).with_context(|| {
-            format!(
-                "Error converting lua value returned by script {} to Config struct",
-                p.display()
-            )
-        })?;
-        cfg.check_consistency()?;
 
-        // Compute but discard the key bindings here so that we raise any
+        let (config, warnings) =
-        // problems earlier than we use them.
+            wezterm_dynamic::Error::capture_warnings(|| -> anyhow::Result<Config> {
-        let _ = cfg.key_bindings();
+                let cfg: Config;
+
+                let config: mlua::Value = smol::block_on(
+                    // Skip a potential BOM that Windows software may have placed in the
+                    // file. Note that we can't catch this happening for files that are
+                    // imported via the lua require function.
+                    lua.load(s.trim_start_matches('\u{FEFF}'))
+                        .set_name(p.to_string_lossy())?
+                        .eval_async(),
+                )?;
+                let config = Config::apply_overrides_to(&lua, config)?;
+                let config = Config::apply_overrides_obj_to(&lua, config, overrides)?;
+                cfg = Config::from_lua(config, &lua).with_context(|| {
+                    format!(
+                        "Error converting lua value returned by script {} to Config struct",
+                        p.display()
+                    )
+                })?;
+                cfg.check_consistency()?;
+
+                // Compute but discard the key bindings here so that we raise any
+                // problems earlier than we use them.
+                let _ = cfg.key_bindings();
+
+                std::env::set_var("WEZTERM_CONFIG_FILE", p);
+                if let Some(dir) = p.parent() {
+                    std::env::set_var("WEZTERM_CONFIG_DIR", dir);
+                }
+                Ok(cfg)
+            });
+        let cfg = config?;
 
-        std::env::set_var("WEZTERM_CONFIG_FILE", p);
-        if let Some(dir) = p.parent() {
-            std::env::set_var("WEZTERM_CONFIG_DIR", dir);
-        }
         Ok(Some(LoadedConfig {
             config: Ok(cfg.compute_extra_defaults(Some(p))),
             file_name: Some(p.to_path_buf()),
             lua: Some(lua),
+            warnings,
         }))
     }
 

config/src/lib.rs

@@ -397,9 +397,16 @@ pub fn configuration_result() -> Result<ConfigHandle, Error> {
     Ok(CONFIG.get())
 }
 
+/// Returns the combined set of errors + warnings encountered
+/// while loading the preferred configuration
+pub fn configuration_warnings_and_errors() -> Vec<String> {
+    CONFIG.get_warnings_and_errors()
+}
+
 struct ConfigInner {
     config: Arc<Config>,
     error: Option<String>,
+    warnings: Vec<String>,
     generation: usize,
     watcher: Option<notify::RecommendedWatcher>,
     subscribers: HashMap<usize, Box<dyn Fn() -> bool + Send>>,
@@ -410,6 +417,7 @@ impl ConfigInner {
         Self {
             config: Arc::new(Config::default_config()),
             error: None,
+            warnings: vec![],
             generation: 0,
             watcher: None,
             subscribers: HashMap::new(),
@@ -508,8 +516,11 @@ impl ConfigInner {
             config,
             file_name,
             lua,
+            warnings,
         } = Config::load();
 
+        self.warnings = warnings;
+
         // Before we process the success/failure, extract and update
         // any paths that we should be watching
         let mut watch_paths = vec![];
@@ -680,6 +691,18 @@ impl Configuration {
         inner.error.as_ref().cloned()
     }
 
+    pub fn get_warnings_and_errors(&self) -> Vec<String> {
+        let mut result = vec![];
+        let inner = self.inner.lock().unwrap();
+        if let Some(error) = &inner.error {
+            result.push(error.clone());
+        }
+        for warning in &inner.warnings {
+            result.push(warning.clone());
+        }
+        result
+    }
+
     /// Returns any captured error message, and clears
     /// it from the config state.
     #[allow(dead_code)]
@@ -723,6 +746,7 @@ pub struct LoadedConfig {
     pub config: anyhow::Result<Config>,
     pub file_name: Option<PathBuf>,
     pub lua: Option<mlua::Lua>,
+    pub warnings: Vec<String>,
 }
 
 fn default_one_point_oh_f64() -> f64 {

config/src/lua.rs

@@ -154,7 +154,7 @@ impl ConfigHelper {
                                 break;
                             }
                         }
-                        log::warn!("{message}");
+                        wezterm_dynamic::Error::warn(message);
                     }
                     Some(value) => {
                         self.object.insert(DynValue::String(key), value.clone());

docs/changelog.md

@@ -111,6 +111,9 @@ As features stabilize some brief notes about them will accumulate here.
   font. [#2932](https://github.com/wez/wezterm/issues/2932)
 * Color schemes: `Gruvbox Dark` was renamed to `GruvboxDark` and adjusted in
   the upstream iTerm2-Color-Schemes repo
+* Config warnings, such as using deprecated or invalid fields will now cause
+  the configuration error window to be shown. Previously, only hard errors were
+  shown, which meant that a number of minor config issues could be overlooked.
 
 #### Updated
 * Bundled harfbuzz updated to version 6.0.0

wezterm-dynamic/src/error.rs

@@ -1,8 +1,18 @@
 use crate::fromdynamic::{FromDynamicOptions, UnknownFieldAction};
 use crate::object::Object;
 use crate::value::Value;
+use std::cell::RefCell;
+use std::rc::Rc;
 use thiserror::Error;
 
+pub trait WarningCollector {
+    fn warn(&self, message: String);
+}
+
+thread_local! {
+    static WARNING_COLLECTOR: RefCell<Option<Box<dyn WarningCollector>>> = RefCell::new(None);
+}
+
 #[derive(Error, Debug)]
 #[non_exhaustive]
 pub enum Error {
@@ -60,6 +70,58 @@ pub enum Error {
 }
 
 impl Error {
+    /// Log a warning; if a warning collector is set for the current thread,
+    /// use it, otherwise, log a regular warning message.
+    pub fn warn(message: String) {
+        WARNING_COLLECTOR.with(|collector| {
+            let collector = collector.borrow();
+            if let Some(collector) = collector.as_ref() {
+                collector.warn(message);
+            } else {
+                log::warn!("{message}");
+            }
+        });
+    }
+
+    pub fn capture_warnings<F: FnOnce() -> T, T>(f: F) -> (T, Vec<String>) {
+        let warnings = Rc::new(RefCell::new(vec![]));
+
+        struct Collector {
+            warnings: Rc<RefCell<Vec<String>>>,
+        }
+
+        impl WarningCollector for Collector {
+            fn warn(&self, message: String) {
+                self.warnings.borrow_mut().push(message);
+            }
+        }
+
+        Self::set_warning_collector(Collector {
+            warnings: Rc::clone(&warnings),
+        });
+        let result = f();
+        Self::clear_warning_collector();
+        let warnings = match Rc::try_unwrap(warnings) {
+            Ok(warnings) => warnings.into_inner(),
+            Err(warnings) => (*warnings).clone().into_inner(),
+        };
+        (result, warnings)
+    }
+
+    /// Replace the warning collector for the current thread
+    fn set_warning_collector<T: WarningCollector + 'static>(c: T) {
+        WARNING_COLLECTOR.with(|collector| {
+            collector.borrow_mut().replace(Box::new(c));
+        });
+    }
+
+    /// Clear the warning collector for the current thread
+    fn clear_warning_collector() {
+        WARNING_COLLECTOR.with(|collector| {
+            collector.borrow_mut().take();
+        });
+    }
+
     fn compute_unknown_fields(
         type_name: &'static str,
         object: &crate::Object,
@@ -108,7 +170,7 @@ impl Error {
         match options.deprecated_fields {
             UnknownFieldAction::Deny => Err(err),
             UnknownFieldAction::Warn => {
-                log::warn!("{:#}", err);
+                Self::warn(format!("{:#}", err));
                 Ok(())
             }
             UnknownFieldAction::Ignore => unreachable!(),
@@ -134,7 +196,7 @@ impl Error {
 
         if show_warning {
             for err in &errors {
-                log::warn!("{:#}", err);
+                Self::warn(format!("{:#}", err));
             }
         }
 

wezterm-gui/src/main.rs

@@ -778,8 +778,9 @@ fn main() {
 }
 
 fn maybe_show_configuration_error_window() {
-    if let Err(err) = config::configuration_result() {
+    let warnings = config::configuration_warnings_and_errors();
-        let err = format!("{:#}", err);
+    if !warnings.is_empty() {
+        let err = warnings.join("\n");
         mux::connui::show_configuration_error_message(&err);
     }
 }