From b3ae3adbb77c4111366cb59b80d757f361c70237 Mon Sep 17 00:00:00 2001 From: Ivan Molodetskikh Date: Sat, 27 Sep 2025 11:20:43 +0300 Subject: [PATCH] Partially implement config includes Subsequent commits will add merging for all leftover sections. --- niri-config/src/appearance.rs | 10 +- niri-config/src/error.rs | 99 ++++ niri-config/src/lib.rs | 883 +++++++++++++++++++------------ niri-config/src/misc.rs | 11 + niri-config/tests/wiki-parses.rs | 4 +- src/layer/mapped.rs | 4 +- src/layout/mod.rs | 2 +- src/layout/tests.rs | 10 +- src/main.rs | 13 +- src/tests/animations.rs | 4 +- src/tests/floating.rs | 2 +- src/tests/window_opening.rs | 4 +- src/ui/hotkey_overlay.rs | 2 +- src/utils/mod.rs | 2 +- src/utils/watcher.rs | 2 +- 15 files changed, 694 insertions(+), 358 deletions(-) create mode 100644 niri-config/src/error.rs diff --git a/niri-config/src/appearance.rs b/niri-config/src/appearance.rs index bdcbb318..77ee5894 100644 --- a/niri-config/src/appearance.rs +++ b/niri-config/src/appearance.rs @@ -1103,8 +1103,7 @@ mod tests { #[test] fn rule_color_can_override_base_gradient() { - let config = Config::parse( - "test.kdl", + let config = Config::parse_mem( r##" // Start with gradient set. layout { @@ -1127,7 +1126,7 @@ mod tests { ) .unwrap(); - let mut border = config.resolve_layout().border; + let mut border = config.layout.border; for rule in &config.window_rules { border.merge_with(&rule.border); } @@ -1151,8 +1150,7 @@ mod tests { #[test] fn rule_color_can_override_rule_gradient() { - let config = Config::parse( - "test.kdl", + let config = Config::parse_mem( r##" // Start with gradient set. layout { @@ -1196,7 +1194,7 @@ mod tests { ) .unwrap(); - let mut border = config.resolve_layout().border; + let mut border = config.layout.border; let mut tab_indicator_rule = TabIndicatorRule::default(); for rule in &config.window_rules { border.merge_with(&rule.border); diff --git a/niri-config/src/error.rs b/niri-config/src/error.rs new file mode 100644 index 00000000..0210c9c1 --- /dev/null +++ b/niri-config/src/error.rs @@ -0,0 +1,99 @@ +use std::error::Error; +use std::fmt; +use std::path::PathBuf; + +use miette::Diagnostic; + +#[derive(Debug)] +pub struct ConfigParseResult { + pub config: Result, + + // We always try to return includes for the file watcher. + // + // If the main config is valid, but an included file fails to parse, config will be an Err(), + // but includes will still be filled, so that fixing just the included file is enough to + // trigger a reload. + pub includes: Vec, +} + +/// Error type that chains main errors with include errors. +/// +/// Allows miette's Report formatting to have main + include errors all in one. +#[derive(Debug)] +pub struct ConfigIncludeError { + pub main: knuffel::Error, + pub includes: Vec, +} + +impl ConfigParseResult { + pub fn from_err(err: E) -> Self { + Self { + config: Err(err), + includes: Vec::new(), + } + } + + pub fn map_config_res( + self, + f: impl FnOnce(Result) -> Result, + ) -> ConfigParseResult { + ConfigParseResult { + config: f(self.config), + includes: self.includes, + } + } +} + +impl fmt::Display for ConfigIncludeError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(&self.main, f) + } +} + +impl Error for ConfigIncludeError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + self.main.source() + } +} + +impl Diagnostic for ConfigIncludeError { + fn code<'a>(&'a self) -> Option> { + self.main.code() + } + + fn severity(&self) -> Option { + self.main.severity() + } + + fn help<'a>(&'a self) -> Option> { + self.main.help() + } + + fn url<'a>(&'a self) -> Option> { + self.main.url() + } + + fn source_code(&self) -> Option<&dyn miette::SourceCode> { + self.main.source_code() + } + + fn labels(&self) -> Option + '_>> { + self.main.labels() + } + + fn diagnostic_source(&self) -> Option<&dyn Diagnostic> { + self.main.diagnostic_source() + } + + fn related<'a>(&'a self) -> Option + 'a>> { + let main_related = self.main.related(); + let includes_iter = self.includes.iter().map(|err| err as &'a dyn Diagnostic); + + let iter: Box + 'a> = match main_related { + Some(main) => Box::new(main.chain(includes_iter)), + None => Box::new(includes_iter), + }; + + Some(iter) + } +} diff --git a/niri-config/src/lib.rs b/niri-config/src/lib.rs index 4f2cf455..14a93332 100644 --- a/niri-config/src/lib.rs +++ b/niri-config/src/lib.rs @@ -1,12 +1,29 @@ +//! niri config parsing. +//! +//! The config can be constructed from multiple files (includes). To support this, many types are +//! split into two. For example, `Layout` and `LayoutPart` where `Layout` is the final config and +//! `LayoutPart` is one part parsed from one config file. +//! +//! The convention for `Default` impls is to set the initial values before the parsing occurs. +//! Then, parsing will update the values with those parsed from the config. +//! +//! The `Default` values match those from `default-config.kdl` in almost all cases, with a notable +//! exception of `binds {}` and some window rules. + #[macro_use] extern crate tracing; +use std::cell::RefCell; +use std::collections::HashSet; use std::ffi::OsStr; use std::fs::{self, File}; use std::io::Write as _; use std::path::{Path, PathBuf}; +use std::rc::Rc; -use miette::{Context as _, IntoDiagnostic as _}; +use knuffel::errors::DecodeError; +use knuffel::Decode as _; +use miette::{miette, Context as _, IntoDiagnostic as _}; #[macro_use] pub mod macros; @@ -15,6 +32,7 @@ pub mod animations; pub mod appearance; pub mod binds; pub mod debug; +pub mod error; pub mod gestures; pub mod input; pub mod layer_rule; @@ -29,6 +47,7 @@ pub use crate::animations::{Animation, Animations}; pub use crate::appearance::*; pub use crate::binds::*; pub use crate::debug::Debug; +pub use crate::error::{ConfigIncludeError, ConfigParseResult}; pub use crate::gestures::Gestures; pub use crate::input::{Input, ModKey, ScrollMethod, TrackLayout, WarpMouseToFocusMode, Xkb}; pub use crate::layer_rule::LayerRule; @@ -36,61 +55,35 @@ pub use crate::layout::*; pub use crate::misc::*; pub use crate::output::{Output, OutputName, Outputs, Position, Vrr}; pub use crate::utils::FloatOrInt; -use crate::utils::MergeWith as _; +use crate::utils::{Flag, MergeWith as _}; pub use crate::window_rule::{FloatingPosition, RelativeTo, WindowRule}; pub use crate::workspace::{Workspace, WorkspaceLayoutPart}; -#[derive(knuffel::Decode, Debug, PartialEq)] +const RECURSION_LIMIT: u8 = 10; + +#[derive(Debug, Default, PartialEq)] pub struct Config { - #[knuffel(child, default)] pub input: Input, - #[knuffel(children(name = "output"))] pub outputs: Outputs, - #[knuffel(children(name = "spawn-at-startup"))] pub spawn_at_startup: Vec, - #[knuffel(children(name = "spawn-sh-at-startup"))] pub spawn_sh_at_startup: Vec, - #[knuffel(child, default)] - pub layout: LayoutPart, - #[knuffel(child, default)] + pub layout: Layout, pub prefer_no_csd: bool, - #[knuffel(child, default)] pub cursor: Cursor, - #[knuffel( - child, - unwrap(argument), - default = Some(String::from( - "~/Pictures/Screenshots/Screenshot from %Y-%m-%d %H-%M-%S.png" - ))) - ] - pub screenshot_path: Option, - #[knuffel(child, default)] + pub screenshot_path: ScreenshotPath, pub clipboard: Clipboard, - #[knuffel(child, default)] pub hotkey_overlay: HotkeyOverlay, - #[knuffel(child, default)] pub config_notification: ConfigNotification, - #[knuffel(child, default)] pub animations: Animations, - #[knuffel(child, default)] pub gestures: Gestures, - #[knuffel(child, default)] pub overview: Overview, - #[knuffel(child, default)] pub environment: Environment, - #[knuffel(child, default)] pub xwayland_satellite: XwaylandSatellite, - #[knuffel(children(name = "window-rule"))] pub window_rules: Vec, - #[knuffel(children(name = "layer-rule"))] pub layer_rules: Vec, - #[knuffel(child, default)] pub binds: Binds, - #[knuffel(child, default)] pub switch_events: SwitchBinds, - #[knuffel(child, default)] pub debug: Debug, - #[knuffel(children(name = "workspace"))] pub workspaces: Vec, } @@ -113,79 +106,347 @@ pub enum ConfigPath { }, } -impl Config { - pub fn load(path: &Path) -> miette::Result { - let contents = fs::read_to_string(path) - .into_diagnostic() - .with_context(|| format!("error reading {path:?}"))?; +// Newtypes for putting information into the knuffel context. +struct BasePath(PathBuf); +struct RootBase(PathBuf); +struct Recursion(u8); +#[derive(Default)] +struct Includes(Vec); +#[derive(Default)] +struct IncludeErrors(Vec); - let config = Self::parse( - path.file_name() - .and_then(OsStr::to_str) - .unwrap_or("config.kdl"), - &contents, - ) - .context("error parsing")?; - debug!("loaded config from {path:?}"); - Ok(config) - } +// Rather than listing all fields and deriving knuffel::Decode, we implement +// knuffel::DecodeChildren by hand, since we need custom logic for every field anyway: we want to +// merge the values into the config from the context as we go to support the positionality of +// includes. The reason we need this type at all is because knuffel's only entry point that allows +// setting default values on a context is `parse_with_context()` that needs a type to parse. +pub struct ConfigPart; - pub fn parse(filename: &str, text: &str) -> Result { - let _span = tracy_client::span!("Config::parse"); - knuffel::parse(filename, text) - } +impl knuffel::DecodeChildren for ConfigPart +where + S: knuffel::traits::ErrorSpan, +{ + fn decode_children( + nodes: &[knuffel::ast::SpannedNode], + ctx: &mut knuffel::decode::Context, + ) -> Result> { + let _span = tracy_client::span!("parse config file"); - pub fn resolve_layout(&self) -> Layout { - let mut rv = Layout::from_part(&self.layout); + let config = ctx.get::>>().unwrap().clone(); + let includes = ctx.get::>>().unwrap().clone(); + let include_errors = ctx.get::>>().unwrap().clone(); + let recursion = ctx.get::().unwrap().0; - // Preserve the behavior we'd always had for the border section: - // - `layout {}` gives border = off - // - `layout { border {} }` gives border = on - // - `layout { border { off } }` gives border = off - // - // This behavior is inconsistent with the rest of the config where adding an empty section - // generally doesn't change the outcome. Particularly, shadows are also disabled by default - // (like borders), and they always had an `on` instead of an `off` for this reason, so that - // writing `layout { shadow {} }` still results in shadow = off, as it should. - // - // Unfortunately, the default config has always had wording that heavily implies that - // `layout { border {} }` enables the borders. This wording is sure to be present in a lot - // of users' configs by now, which we can't change. - // - // Another way to make things consistent would be to default borders to on. However, that - // is annoying because it would mean changing many tests that rely on borders being off by - // default. This would also contradict the intended default borders value (off). - // - // So, let's just work around the problem here, preserving the original behavior. - if self.layout.border.is_some_and(|x| !x.on && !x.off) { - rv.border.off = false; + let mut seen = HashSet::new(); + + for node in nodes { + let name = &**node.node_name; + + // Within one config file, splitting sections into multiple parts is not allowed to + // reduce confusion. The exceptions here aren't multipart; they all add new values. + if !matches!( + name, + "output" + | "spawn-at-startup" + | "spawn-sh-at-startup" + | "window-rule" + | "layer-rule" + | "workspace" + | "include" + ) && !seen.insert(name) + { + ctx.emit_error(DecodeError::unexpected( + &node.node_name, + "node", + format!("duplicate node `{name}`, single node expected"), + )); + continue; + } + + macro_rules! m_replace { + ($field:ident) => {{ + let part = knuffel::Decode::decode_node(node, ctx)?; + config.borrow_mut().$field = part; + }}; + } + + macro_rules! m_merge { + ($field:ident) => {{ + let part = knuffel::Decode::decode_node(node, ctx)?; + config.borrow_mut().$field.merge_with(&part); + }}; + } + + macro_rules! m_push { + ($field:ident) => {{ + let part = knuffel::Decode::decode_node(node, ctx)?; + config.borrow_mut().$field.push(part); + }}; + } + + match name { + // TODO: most (all?) of these need to be merged instead + "input" => m_replace!(input), + "cursor" => m_replace!(cursor), + "clipboard" => m_replace!(clipboard), + "hotkey-overlay" => m_replace!(hotkey_overlay), + "config-notification" => m_replace!(config_notification), + "animations" => m_replace!(animations), + "gestures" => m_replace!(gestures), + "overview" => m_replace!(overview), + "xwayland-satellite" => m_replace!(xwayland_satellite), + "switch-events" => m_replace!(switch_events), + "debug" => m_replace!(debug), + + // Multipart sections. + "output" => { + let part = Output::decode_node(node, ctx)?; + config.borrow_mut().outputs.0.push(part); + } + "spawn-at-startup" => m_push!(spawn_at_startup), + "spawn-sh-at-startup" => m_push!(spawn_sh_at_startup), + "window-rule" => m_push!(window_rules), + "layer-rule" => m_push!(layer_rules), + "workspace" => m_push!(workspaces), + + // Single-part sections. + "binds" => { + let part = Binds::decode_node(node, ctx)?; + + // We replace conflicting binds, rather than error, to support the use-case + // where you import some preconfigured-dots.kdl, then override some binds with + // your own. + let mut config = config.borrow_mut(); + let binds = &mut config.binds.0; + // Remove existing binds matching any new bind. + binds.retain(|bind| !part.0.iter().any(|new| new.key == bind.key)); + // Add all new binds. + binds.extend(part.0); + } + "environment" => { + let part = Environment::decode_node(node, ctx)?; + config.borrow_mut().environment.0.extend(part.0); + } + + "prefer-no-csd" => { + config.borrow_mut().prefer_no_csd = Flag::decode_node(node, ctx)?.0 + } + + "screenshot-path" => { + let part = knuffel::Decode::decode_node(node, ctx)?; + config.borrow_mut().screenshot_path = part; + } + + "layout" => { + let mut part = LayoutPart::decode_node(node, ctx)?; + + // Preserve the behavior we'd always had for the border section: + // - `layout {}` gives border = off + // - `layout { border {} }` gives border = on + // - `layout { border { off } }` gives border = off + // + // This behavior is inconsistent with the rest of the config where adding an + // empty section generally doesn't change the outcome. Particularly, shadows + // are also disabled by default (like borders), and they always had an `on` + // instead of an `off` for this reason, so that writing `layout { shadow {} }` + // still results in shadow = off, as it should. + // + // Unfortunately, the default config has always had wording that heavily + // implies that `layout { border {} }` enables the borders. This wording is + // sure to be present in a lot of users' configs by now, which we can't change. + // + // Another way to make things consistent would be to default borders to on. + // However, that is annoying because it would mean changing many tests that + // rely on borders being off by default. This would also contradict the + // intended default borders value (off). + // + // So, let's just work around the problem here, preserving the original + // behavior. + if recursion == 0 { + if let Some(border) = part.border.as_mut() { + if !border.on && !border.off { + border.on = true; + } + } + } + + config.borrow_mut().layout.merge_with(&part); + } + + "include" => { + let path: PathBuf = utils::parse_arg_node("include", node, ctx)?; + let base = ctx.get::().unwrap(); + let path = base.0.join(path); + + // We use DecodeError::Missing throughout this block because it results in the + // least confusing error messages while still allowing to provide a span. + + let recursion = ctx.get::().unwrap().0 + 1; + if recursion == RECURSION_LIMIT { + ctx.emit_error(DecodeError::missing( + node, + format!( + "reached the recursion limit; \ + includes cannot be {RECURSION_LIMIT} levels deep" + ), + )); + continue; + } + + let Some(filename) = path.file_name().and_then(OsStr::to_str) else { + ctx.emit_error(DecodeError::missing( + node, + "include path doesn't have a valid file name", + )); + continue; + }; + let base = path.parent().map(Path::to_path_buf).unwrap_or_default(); + + // Store even if the include fails to read or parse, so it gets watched. + includes.borrow_mut().0.push(path.to_path_buf()); + + match fs::read_to_string(&path) { + Ok(text) => { + // Try to get filename relative to the root base config folder for + // clearer error messages. + let root_base = &ctx.get::().unwrap().0; + // Failing to strip prefix usually means absolute path; show it in full. + let relative_path = path.strip_prefix(root_base).ok().unwrap_or(&path); + let filename = relative_path.to_str().unwrap_or(filename); + + let part = knuffel::parse_with_context::< + ConfigPart, + knuffel::span::Span, + _, + >(filename, &text, |ctx| { + ctx.set(BasePath(base)); + ctx.set(RootBase(root_base.clone())); + ctx.set(Recursion(recursion)); + ctx.set(includes.clone()); + ctx.set(include_errors.clone()); + ctx.set(config.clone()); + }); + + match part { + Ok(_) => {} + Err(err) => { + include_errors.borrow_mut().0.push(err); + + ctx.emit_error(DecodeError::missing( + node, + "failed to parse included config", + )); + } + } + } + Err(err) => { + ctx.emit_error(DecodeError::missing( + node, + format!("failed to read included config from {path:?}: {err}"), + )); + } + } + } + + name => { + ctx.emit_error(DecodeError::unexpected( + node, + "node", + format!("unexpected node `{}`", name.escape_default()), + )); + } + } } - rv + Ok(Self) } } -impl Default for Config { - fn default() -> Self { - Config::parse( - "default-config.kdl", +impl Config { + pub fn load_default() -> Self { + let res = Config::parse( + Path::new("default-config.kdl"), include_str!("../../resources/default-config.kdl"), - ) - .unwrap() + ); + + // Includes in the default config can break its parsing at runtime. + assert!( + res.includes.is_empty(), + "default config must not have includes", + ); + + res.config.unwrap() + } + + pub fn load(path: &Path) -> ConfigParseResult { + let contents = match fs::read_to_string(path) { + Ok(x) => x, + Err(err) => { + return ConfigParseResult::from_err( + miette!(err).context(format!("error reading {path:?}")), + ); + } + }; + + Self::parse(path, &contents).map_config_res(|res| { + let config = res.context("error parsing")?; + debug!("loaded config from {path:?}"); + Ok(config) + }) + } + + pub fn parse(path: &Path, text: &str) -> ConfigParseResult { + let base = path.parent().map(Path::to_path_buf).unwrap_or_default(); + let filename = path + .file_name() + .and_then(OsStr::to_str) + .unwrap_or("config.kdl"); + + let config = Rc::new(RefCell::new(Config::default())); + let includes = Rc::new(RefCell::new(Includes(Vec::new()))); + let include_errors = Rc::new(RefCell::new(IncludeErrors(Vec::new()))); + + let part = knuffel::parse_with_context::( + filename, + text, + |ctx| { + ctx.set(BasePath(base.clone())); + ctx.set(RootBase(base)); + ctx.set(Recursion(0)); + ctx.set(includes.clone()); + ctx.set(include_errors.clone()); + ctx.set(config.clone()); + }, + ); + + let includes = includes.take().0; + let include_errors = include_errors.take().0; + let config = part + .map(|_| config.take()) + .map_err(move |err| ConfigIncludeError { + main: err, + includes: include_errors, + }); + + ConfigParseResult { config, includes } + } + + pub fn parse_mem(text: &str) -> Result { + Self::parse(Path::new("config.kdl"), text).config } } impl ConfigPath { /// Loads the config, returns an error if it doesn't exist. - pub fn load(&self) -> miette::Result { + pub fn load(&self) -> ConfigParseResult { let _span = tracy_client::span!("ConfigPath::load"); self.load_inner(|user_path, system_path| { - Err(miette::miette!( + Err(miette!( "no config file found; create one at {user_path:?} or {system_path:?}", )) }) - .context("error loading config") + .map_config_res(|res| res.context("error loading config")) } /// Loads the config, or creates it if it doesn't exist. @@ -194,7 +455,7 @@ impl ConfigPath { /// /// If the config was created, but for some reason could not be read afterwards, /// this may return `(Some(_), Err(_))`. - pub fn load_or_create(&self) -> (Option<&Path>, miette::Result) { + pub fn load_or_create(&self) -> (Option<&Path>, ConfigParseResult) { let _span = tracy_client::span!("ConfigPath::load_or_create"); let mut created_at = None; @@ -205,7 +466,7 @@ impl ConfigPath { .map(|()| user_path) .with_context(|| format!("error creating config at {user_path:?}")) }) - .context("error loading config"); + .map_config_res(|res| res.context("error loading config")); (created_at, result) } @@ -213,7 +474,7 @@ impl ConfigPath { fn load_inner<'a>( &'a self, maybe_create: impl FnOnce(&'a Path, &'a Path) -> miette::Result<&'a Path>, - ) -> miette::Result { + ) -> ConfigParseResult { let path = match self { ConfigPath::Explicit(path) => path.as_path(), ConfigPath::Regular { @@ -225,7 +486,10 @@ impl ConfigPath { } else if system_path.exists() { system_path.as_path() } else { - maybe_create(user_path.as_path(), system_path.as_path())? + match maybe_create(user_path.as_path(), system_path.as_path()) { + Ok(x) => x, + Err(err) => return ConfigParseResult::from_err(miette!(err)), + } } } }; @@ -274,19 +538,19 @@ mod tests { #[test] fn can_create_default_config() { - let _ = Config::default(); + let _ = Config::load_default(); } #[test] fn default_repeat_params() { - let config = Config::parse("config.kdl", "").unwrap(); + let config = Config::parse_mem("").unwrap(); assert_eq!(config.input.keyboard.repeat_delay, 600); assert_eq!(config.input.keyboard.repeat_rate, 25); } #[track_caller] fn do_parse(text: &str) -> Config { - Config::parse("test.kdl", text) + Config::parse_mem(text) .map_err(miette::Report::new) .unwrap() } @@ -809,237 +1073,209 @@ mod tests { command: "qs -c ~/source/qs/MyAwesomeShell", }, ], - layout: LayoutPart { - focus_ring: Some( - BorderRule { - off: false, - on: false, - width: Some( - FloatOrInt( - 5.0, - ), - ), - active_color: Some( - Color { + layout: Layout { + focus_ring: FocusRing { + off: false, + width: 5.0, + active_color: Color { + r: 0.0, + g: 0.39215687, + b: 0.78431374, + a: 1.0, + }, + inactive_color: Color { + r: 1.0, + g: 0.78431374, + b: 0.39215687, + a: 0.0, + }, + urgent_color: Color { + r: 0.60784316, + g: 0.0, + b: 0.0, + a: 1.0, + }, + active_gradient: Some( + Gradient { + from: Color { + r: 0.039215688, + g: 0.078431375, + b: 0.11764706, + a: 1.0, + }, + to: Color { r: 0.0, - g: 0.39215687, - b: 0.78431374, + g: 0.5019608, + b: 1.0, a: 1.0, }, - ), - inactive_color: Some( - Color { - r: 1.0, - g: 0.78431374, - b: 0.39215687, - a: 0.0, + angle: 180, + relative_to: WorkspaceView, + in_: GradientInterpolation { + color_space: Srgb, + hue_interpolation: Shorter, }, - ), - urgent_color: None, - active_gradient: Some( - Gradient { - from: Color { - r: 0.039215688, - g: 0.078431375, - b: 0.11764706, - a: 1.0, - }, - to: Color { - r: 0.0, - g: 0.5019608, - b: 1.0, - a: 1.0, - }, - angle: 180, - relative_to: WorkspaceView, - in_: GradientInterpolation { - color_space: Srgb, - hue_interpolation: Shorter, - }, - }, - ), - inactive_gradient: None, - urgent_gradient: None, + }, + ), + inactive_gradient: None, + urgent_gradient: None, + }, + border: Border { + off: false, + width: 3.0, + active_color: Color { + r: 1.0, + g: 0.78431374, + b: 0.49803922, + a: 1.0, }, - ), - border: Some( - BorderRule { - off: false, - on: false, - width: Some( - FloatOrInt( - 3.0, - ), - ), - active_color: None, - inactive_color: Some( - Color { - r: 1.0, - g: 0.78431374, - b: 0.39215687, - a: 0.0, - }, - ), - urgent_color: None, - active_gradient: None, - inactive_gradient: None, - urgent_gradient: None, + inactive_color: Color { + r: 1.0, + g: 0.78431374, + b: 0.39215687, + a: 0.0, }, - ), - shadow: Some( - ShadowRule { - off: false, - on: false, - offset: Some( - ShadowOffset { - x: FloatOrInt( - 10.0, - ), - y: FloatOrInt( - -20.0, - ), - }, - ), - softness: None, - spread: None, - draw_behind_window: None, - color: None, - inactive_color: None, + urgent_color: Color { + r: 0.60784316, + g: 0.0, + b: 0.0, + a: 1.0, }, - ), - tab_indicator: Some( - TabIndicatorPart { - off: false, - on: false, - hide_when_single_tab: None, - place_within_column: None, - gap: None, - width: Some( - FloatOrInt( - 10.0, - ), + active_gradient: None, + inactive_gradient: None, + urgent_gradient: None, + }, + shadow: Shadow { + on: false, + offset: ShadowOffset { + x: FloatOrInt( + 10.0, ), - length: None, - position: Some( - Top, - ), - gaps_between_tabs: None, - corner_radius: None, - active_color: None, - inactive_color: None, - urgent_color: None, - active_gradient: None, - inactive_gradient: None, - urgent_gradient: None, - }, - ), - insert_hint: Some( - InsertHintPart { - off: false, - on: false, - color: Some( - Color { - r: 1.0, - g: 0.78431374, - b: 0.49803922, - a: 1.0, - }, - ), - gradient: Some( - Gradient { - from: Color { - r: 0.039215688, - g: 0.078431375, - b: 0.11764706, - a: 1.0, - }, - to: Color { - r: 0.0, - g: 0.5019608, - b: 1.0, - a: 1.0, - }, - angle: 180, - relative_to: WorkspaceView, - in_: GradientInterpolation { - color_space: Srgb, - hue_interpolation: Shorter, - }, - }, + y: FloatOrInt( + -20.0, ), }, - ), - preset_column_widths: Some( - [ - Proportion( - 0.25, - ), - Proportion( + softness: 30.0, + spread: 5.0, + draw_behind_window: false, + color: Color { + r: 0.0, + g: 0.0, + b: 0.0, + a: 0.46666667, + }, + inactive_color: None, + }, + tab_indicator: TabIndicator { + off: false, + hide_when_single_tab: false, + place_within_column: false, + gap: 5.0, + width: 10.0, + length: TabIndicatorLength { + total_proportion: Some( 0.5, ), - Fixed( - 960, - ), - Fixed( - 1280, - ), - ], - ), + }, + position: Top, + gaps_between_tabs: 0.0, + corner_radius: 0.0, + active_color: None, + inactive_color: None, + urgent_color: None, + active_gradient: None, + inactive_gradient: None, + urgent_gradient: None, + }, + insert_hint: InsertHint { + off: false, + color: Color { + r: 1.0, + g: 0.78431374, + b: 0.49803922, + a: 1.0, + }, + gradient: Some( + Gradient { + from: Color { + r: 0.039215688, + g: 0.078431375, + b: 0.11764706, + a: 1.0, + }, + to: Color { + r: 0.0, + g: 0.5019608, + b: 1.0, + a: 1.0, + }, + angle: 180, + relative_to: WorkspaceView, + in_: GradientInterpolation { + color_space: Srgb, + hue_interpolation: Shorter, + }, + }, + ), + }, + preset_column_widths: [ + Proportion( + 0.25, + ), + Proportion( + 0.5, + ), + Fixed( + 960, + ), + Fixed( + 1280, + ), + ], default_column_width: Some( - DefaultPresetSize( - Some( - Proportion( - 0.25, - ), - ), + Proportion( + 0.25, ), ), - preset_window_heights: Some( - [ - Proportion( - 0.25, - ), - Proportion( - 0.5, - ), - Fixed( - 960, - ), - Fixed( - 1280, - ), - ], - ), - center_focused_column: Some( - OnOverflow, - ), - always_center_single_column: None, - empty_workspace_above_first: None, - default_column_display: Some( - Tabbed, - ), - gaps: Some( - FloatOrInt( - 8.0, + preset_window_heights: [ + Proportion( + 0.25, ), - ), - struts: Some( - Struts { - left: FloatOrInt( - 1.0, - ), - right: FloatOrInt( - 2.0, - ), - top: FloatOrInt( - 3.0, - ), - bottom: FloatOrInt( - 0.0, - ), - }, - ), - background_color: None, + Proportion( + 0.5, + ), + Fixed( + 960, + ), + Fixed( + 1280, + ), + ], + center_focused_column: OnOverflow, + always_center_single_column: false, + empty_workspace_above_first: false, + default_column_display: Tabbed, + gaps: 8.0, + struts: Struts { + left: FloatOrInt( + 1.0, + ), + right: FloatOrInt( + 2.0, + ), + top: FloatOrInt( + 3.0, + ), + bottom: FloatOrInt( + 0.0, + ), + }, + background_color: Color { + r: 0.25, + g: 0.25, + b: 0.25, + a: 1.0, + }, }, prefer_no_csd: true, cursor: Cursor { @@ -1050,8 +1286,10 @@ mod tests { 3000, ), }, - screenshot_path: Some( - "~/Screenshots/screenshot.png", + screenshot_path: ScreenshotPath( + Some( + "~/Screenshots/screenshot.png", + ), ), clipboard: Clipboard { disable_primary: true, @@ -1855,31 +2093,14 @@ mod tests { // We try to write the config defaults in such a way that empty sections (and an empty // config) give the same outcome as the default config bundled with niri. This test // verifies the actual differences between the two. - let mut default_config = Config::default(); - let empty_config = Config::parse("empty.kdl", "").unwrap(); + let mut default_config = Config::load_default(); + let empty_config = Config::parse_mem("").unwrap(); // Some notable omissions: the default config has some window rules, and an empty config // will not have any binds. Clear them out so they don't spam the diff. default_config.window_rules.clear(); default_config.binds.0.clear(); - let default_layout = default_config.resolve_layout(); - let empty_layout = empty_config.resolve_layout(); - default_config.layout = Default::default(); - assert_snapshot!( - diff_lines( - &format!("{empty_layout:#?}"), - &format!("{default_layout:#?}") - ), - @r" - - 0.3333333333333333, - + 0.33333, - - - 0.6666666666666666, - + 0.66667, - ", - ); - assert_snapshot!( diff_lines( &format!("{empty_config:#?}"), @@ -1903,6 +2124,12 @@ mod tests { + ], + }, + ], + + - 0.3333333333333333, + + 0.33333, + + - 0.6666666666666666, + + 0.66667, "#, ); } diff --git a/niri-config/src/misc.rs b/niri-config/src/misc.rs index 781f51c0..5252c8bf 100644 --- a/niri-config/src/misc.rs +++ b/niri-config/src/misc.rs @@ -36,6 +36,17 @@ impl Default for Cursor { } } +#[derive(knuffel::Decode, Debug, Clone, PartialEq)] +pub struct ScreenshotPath(#[knuffel(argument)] pub Option); + +impl Default for ScreenshotPath { + fn default() -> Self { + Self(Some(String::from( + "~/Pictures/Screenshots/Screenshot from %Y-%m-%d %H-%M-%S.png", + ))) + } +} + #[derive(knuffel::Decode, Debug, Default, Clone, Copy, PartialEq, Eq)] pub struct HotkeyOverlay { #[knuffel(child)] diff --git a/niri-config/tests/wiki-parses.rs b/niri-config/tests/wiki-parses.rs index bc4020d6..9cc6638c 100644 --- a/niri-config/tests/wiki-parses.rs +++ b/niri-config/tests/wiki-parses.rs @@ -1,5 +1,5 @@ use std::fs; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; struct KdlCodeBlock { filename: String, @@ -84,7 +84,7 @@ fn wiki_docs_parses() { must_fail, } in code_blocks { - if let Err(error) = niri_config::Config::parse(&filename, &code) { + if let Err(error) = niri_config::Config::parse(Path::new(&filename), &code).config { if !must_fail { errors.push(format!( "Error parsing wiki KDL code block at {}:{}: {:?}", diff --git a/src/layer/mapped.rs b/src/layer/mapped.rs index 2e9f6ca8..658a0c15 100644 --- a/src/layer/mapped.rs +++ b/src/layer/mapped.rs @@ -59,7 +59,7 @@ impl MappedLayer { clock: Clock, config: &Config, ) -> Self { - let mut shadow_config = config.resolve_layout().shadow; + let mut shadow_config = config.layout.shadow; // Shadows for layer surfaces need to be explicitly enabled. shadow_config.on = false; shadow_config.merge_with(&rules.shadow); @@ -76,7 +76,7 @@ impl MappedLayer { } pub fn update_config(&mut self, config: &Config) { - let mut shadow_config = config.resolve_layout().shadow; + let mut shadow_config = config.layout.shadow; // Shadows for layer surfaces need to be explicitly enabled. shadow_config.on = false; shadow_config.merge_with(&self.rules.shadow); diff --git a/src/layout/mod.rs b/src/layout/mod.rs index c04a72ba..e60828d9 100644 --- a/src/layout/mod.rs +++ b/src/layout/mod.rs @@ -576,7 +576,7 @@ impl HitType { impl Options { fn from_config(config: &Config) -> Self { Self { - layout: config.resolve_layout(), + layout: config.layout.clone(), animations: config.animations.clone(), gestures: config.gestures, overview: config.overview, diff --git a/src/layout/tests.rs b/src/layout/tests.rs index 389408c2..2dc989fd 100644 --- a/src/layout/tests.rs +++ b/src/layout/tests.rs @@ -2357,9 +2357,9 @@ fn removing_all_outputs_preserves_empty_named_workspaces() { #[test] fn config_change_updates_cached_sizes() { let mut config = Config::default(); - let border = config.layout.border.get_or_insert_with(Default::default); + let border = &mut config.layout.border; border.off = false; - border.width = Some(FloatOrInt(2.)); + border.width = 2.; let mut layout = Layout::new(Clock::default(), &config); @@ -2371,7 +2371,7 @@ fn config_change_updates_cached_sizes() { } .apply(&mut layout); - config.layout.border.as_mut().unwrap().width = Some(FloatOrInt(4.)); + config.layout.border.width = 4.; layout.update_config(&config); layout.verify_invariants(); @@ -2380,7 +2380,7 @@ fn config_change_updates_cached_sizes() { #[test] fn preset_height_change_removes_preset() { let mut config = Config::default(); - config.layout.preset_window_heights = Some(vec![PresetSize::Fixed(1), PresetSize::Fixed(2)]); + config.layout.preset_window_heights = vec![PresetSize::Fixed(1), PresetSize::Fixed(2)]; let mut layout = Layout::new(Clock::default(), &config); @@ -2401,7 +2401,7 @@ fn preset_height_change_removes_preset() { } // Leave only one. - config.layout.preset_window_heights = Some(vec![PresetSize::Fixed(1)]); + config.layout.preset_window_heights = vec![PresetSize::Fixed(1)]; layout.update_config(&config); diff --git a/src/main.rs b/src/main.rs index 118fe27a..8e5ce1d0 100644 --- a/src/main.rs +++ b/src/main.rs @@ -24,7 +24,7 @@ use niri::utils::spawning::{ REMOVE_ENV_RUST_BACKTRACE, REMOVE_ENV_RUST_LIB_BACKTRACE, }; use niri::utils::{cause_panic, version, watcher, xwayland, IS_SYSTEMD_SERVICE}; -use niri_config::ConfigPath; +use niri_config::{Config, ConfigPath}; use niri_ipc::socket::SOCKET_PATH_ENV; use portable_atomic::Ordering; use sd_notify::NotifyState; @@ -101,7 +101,7 @@ fn main() -> Result<(), Box> { Sub::Validate { config } => { tracy_client::Client::start(); - config_path(config).load()?; + config_path(config).load().config?; info!("config is valid"); return Ok(()); } @@ -148,10 +148,11 @@ fn main() -> Result<(), Box> { let config_path = config_path(cli.config); env::remove_var("NIRI_CONFIG"); let (config_created_at, config_load_result) = config_path.load_or_create(); - let config_errored = config_load_result.is_err(); - let mut config = config_load_result - .map_err(|err| warn!("{err:?}")) - .unwrap_or_default(); + let config_errored = config_load_result.config.is_err(); + let mut config = config_load_result.config.unwrap_or_else(|err| { + warn!("{err:?}"); + Config::load_default() + }); let spawn_at_startup = mem::take(&mut config.spawn_at_startup); let spawn_sh_at_startup = mem::take(&mut config.spawn_sh_at_startup); diff --git a/src/tests/animations.rs b/src/tests/animations.rs index 4871f2c0..51c57f73 100644 --- a/src/tests/animations.rs +++ b/src/tests/animations.rs @@ -3,7 +3,7 @@ use std::time::Duration; use insta::assert_snapshot; use niri_config::animations::{Curve, EasingParams, Kind}; -use niri_config::{Config, FloatOrInt}; +use niri_config::Config; use niri_ipc::SizeChange; use smithay::utils::{Point, Size}; use wayland_client::protocol::wl_surface::WlSurface; @@ -74,7 +74,7 @@ fn set_up() -> Fixture { }); let mut config = Config::default(); - config.layout.gaps = Some(FloatOrInt(0.0)); + config.layout.gaps = 0.0; config.animations.window_resize.anim.kind = LINEAR; config.animations.window_movement.0.kind = LINEAR; diff --git a/src/tests/floating.rs b/src/tests/floating.rs index 4d0b9383..e1cb4330 100644 --- a/src/tests/floating.rs +++ b/src/tests/floating.rs @@ -795,7 +795,7 @@ window-rule { max-width 300 } "##; - let config = Config::parse("test.kdl", config).unwrap(); + let config = Config::parse_mem(config).unwrap(); let mut f = Fixture::with_config(config); f.add_output(1, (1920, 1080)); f.add_output(2, (1280, 720)); diff --git a/src/tests/window_opening.rs b/src/tests/window_opening.rs index e6210fde..9a2ff6da 100644 --- a/src/tests/window_opening.rs +++ b/src/tests/window_opening.rs @@ -281,7 +281,7 @@ window-rule {{ snapshot_desc.push(format!("config:{config}")); - let config = Config::parse("config.kdl", &config).unwrap(); + let config = Config::parse_mem(&config).unwrap(); let mut f = Fixture::with_config(config); f.add_output(1, (1280, 720)); @@ -574,7 +574,7 @@ layout { snapshot_desc.push(format!("config:{config}")); - let config = Config::parse("config.kdl", &config).unwrap(); + let config = Config::parse_mem(&config).unwrap(); let mut f = Fixture::with_config(config); f.add_output(1, (1280, 720)); diff --git a/src/ui/hotkey_overlay.rs b/src/ui/hotkey_overlay.rs index 783b71ef..67997fc8 100644 --- a/src/ui/hotkey_overlay.rs +++ b/src/ui/hotkey_overlay.rs @@ -607,7 +607,7 @@ mod tests { #[track_caller] fn check(config: &str, action: Action) -> String { - let config = Config::parse("test.kdl", config).unwrap(); + let config = Config::parse_mem(config).unwrap(); if let Some((key, title)) = format_bind(&config.binds.0, &action) { let key = key.map(|key| key_name(false, ModKey::Super, &key)); let key = key.as_deref().unwrap_or("(not bound)"); diff --git a/src/utils/mod.rs b/src/utils/mod.rs index a96c1bfc..8efa8654 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -213,7 +213,7 @@ pub fn expand_home(path: &Path) -> anyhow::Result> { } pub fn make_screenshot_path(config: &Config) -> anyhow::Result> { - let Some(path) = &config.screenshot_path else { + let Some(path) = &config.screenshot_path.0 else { return Ok(None); }; diff --git a/src/utils/watcher.rs b/src/utils/watcher.rs index 034e963a..ecb9007b 100644 --- a/src/utils/watcher.rs +++ b/src/utils/watcher.rs @@ -128,7 +128,7 @@ pub fn setup(state: &mut State, config_path: &ConfigPath) { // Parsing the config actually takes > 20 ms on my beefy machine, so let's do it on the // watcher thread. let process = |path: &ConfigPath| { - path.load().map_err(|err| { + path.load().config.map_err(|err| { warn!("{err:?}"); }) };