Cleanup AstError and Error representation in general (#549)

crates/eww/src/config/eww_config.rs

@@ -6,7 +6,8 @@ use yuck::{
         file_provider::YuckFiles, script_var_definition::ScriptVarDefinition, validate::ValidationError,
         widget_definition::WidgetDefinition, window_definition::WindowDefinition, Config,
     },
-    error::AstError,
+    error::DiagError,
+    format_diagnostic::ToDiagnostic,
 };
 
 use simplexpr::dynval::DynVal;
@@ -67,7 +68,7 @@ impl EwwConfig {
         for (name, def) in &config.widget_definitions {
             if widget_definitions::BUILTIN_WIDGET_NAMES.contains(&name.as_str()) {
                 return Err(
-                    AstError::ValidationError(ValidationError::AccidentalBuiltinOverride(def.span, name.to_string())).into()
+                    DiagError(ValidationError::AccidentalBuiltinOverride(def.span, name.to_string()).to_diagnostic()).into()
                 );
             }
         }

crates/eww/src/config/script_var.rs

@@ -6,13 +6,12 @@ use eww_shared_util::{Span, VarName};
 use simplexpr::dynval::DynVal;
 use yuck::{
     config::script_var_definition::{ScriptVarDefinition, VarSource},
+    error::DiagError,
     gen_diagnostic,
 };
 
-use crate::error::DiagError;
-
 pub fn create_script_var_failed_warn(span: Span, var_name: &VarName, error_output: &str) -> DiagError {
-    DiagError::new(gen_diagnostic! {
+    DiagError(gen_diagnostic! {
         kind = Severity::Warning,
         msg = format!("The script for the `{}`-variable exited unsuccessfully", var_name),
         label = span => "Defined here",

crates/eww/src/error.rs

@@ -1,20 +0,0 @@
-use codespan_reporting::diagnostic::Diagnostic;
-
-/// An error that contains a [Diagnostic] for ad-hoc creation of diagnostics.
-#[derive(Debug)]
-pub struct DiagError {
-    pub diag: Diagnostic<usize>,
-}
-
-impl DiagError {
-    pub fn new(diag: Diagnostic<usize>) -> Self {
-        Self { diag }
-    }
-}
-
-impl std::error::Error for DiagError {}
-impl std::fmt::Display for DiagError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{}", self.diag.message)
-    }
-}

crates/eww/src/error_handling_ctx.rs

@@ -12,12 +12,10 @@ use once_cell::sync::Lazy;
 use simplexpr::{dynval::ConversionError, eval::EvalError};
 use yuck::{
     config::{file_provider::YuckFiles, validate::ValidationError},
-    error::AstError,
+    error::DiagError,
     format_diagnostic::ToDiagnostic,
 };
 
-use crate::error::DiagError;
-
 pub static YUCK_FILES: Lazy<Arc<RwLock<YuckFiles>>> = Lazy::new(|| Arc::new(RwLock::new(YuckFiles::new())));
 
 pub fn clear_files() {
@@ -49,9 +47,7 @@ pub fn format_error(err: &anyhow::Error) -> String {
 
 pub fn anyhow_err_to_diagnostic(err: &anyhow::Error) -> Option<Diagnostic<usize>> {
     if let Some(err) = err.downcast_ref::<DiagError>() {
-        Some(err.diag.clone())
+        Some(err.0.clone())
-    } else if let Some(err) = err.downcast_ref::<AstError>() {
-        Some(err.to_diagnostic())
     } else if let Some(err) = err.downcast_ref::<ConversionError>() {
         Some(err.to_diagnostic())
     } else if let Some(err) = err.downcast_ref::<ValidationError>() {

crates/eww/src/main.rs

@@ -4,6 +4,7 @@
 #![feature(box_patterns)]
 #![feature(slice_concat_trait)]
 #![feature(try_blocks)]
+#![feature(hash_drain_filter)]
 #![allow(rustdoc::private_intra_doc_links)]
 
 extern crate gtk;
@@ -24,7 +25,6 @@ mod client;
 mod config;
 mod daemon_response;
 mod display_backend;
-mod error;
 mod error_handling_ctx;
 mod geometry;
 mod ipc_server;

crates/eww/src/widgets/build_widget.rs

@@ -12,14 +12,15 @@ use simplexpr::{dynval::DynVal, SimplExpr};
 use std::{cell::RefCell, collections::HashMap, rc::Rc};
 use yuck::{
     config::{
+        attributes::AttrEntry,
         widget_definition::WidgetDefinition,
         widget_use::{BasicWidgetUse, ChildrenWidgetUse, LoopWidgetUse, WidgetUse},
     },
+    error::DiagError,
     gen_diagnostic,
 };
 
 use crate::{
-    error::DiagError,
     error_handling_ctx,
     state::{
         scope::Listener,
@@ -34,7 +35,7 @@ pub struct BuilderArgs<'a> {
     pub calling_scope: ScopeIndex,
     pub widget_use: BasicWidgetUse,
     pub scope_graph: &'a mut ScopeGraph,
-    pub unhandled_attrs: Vec<AttrName>,
+    pub unhandled_attrs: HashMap<AttrName, AttrEntry>,
     pub widget_defs: Rc<HashMap<String, WidgetDefinition>>,
     pub custom_widget_invocation: Option<Rc<CustomWidgetInvocation>>,
 }
@@ -56,7 +57,7 @@ pub fn build_gtk_widget(
         WidgetUse::Basic(widget_use) => {
             build_basic_gtk_widget(graph, widget_defs, calling_scope, widget_use, custom_widget_invocation)
         }
-        WidgetUse::Loop(_) | WidgetUse::Children(_) => Err(anyhow::anyhow!(DiagError::new(gen_diagnostic! {
+        WidgetUse::Loop(_) | WidgetUse::Children(_) => Err(anyhow::anyhow!(DiagError(gen_diagnostic! {
             msg = "This widget can only be used as a child of some container widget such as box",
             label = widget_use.span(),
             note = "Hint: try wrapping this in a `box`"
@@ -127,7 +128,7 @@ fn build_builtin_gtk_widget(
     custom_widget_invocation: Option<Rc<CustomWidgetInvocation>>,
 ) -> Result<gtk::Widget> {
     let mut bargs = BuilderArgs {
-        unhandled_attrs: widget_use.attrs.attrs.keys().cloned().collect(),
+        unhandled_attrs: widget_use.attrs.attrs.clone(),
         scope_graph: graph,
         calling_scope,
         widget_use,
@@ -161,11 +162,11 @@ fn build_builtin_gtk_widget(
     };
     resolve_widget_attrs(&mut bargs, &gtk_widget)?;
 
-    if !bargs.unhandled_attrs.is_empty() {
+    for (attr_name, attr_entry) in bargs.unhandled_attrs {
         let diag = error_handling_ctx::stringify_diagnostic(gen_diagnostic! {
             kind =  Severity::Warning,
-            msg = format!("Unknown attributes {}", bargs.unhandled_attrs.iter().map(|x| x.to_string()).join(", ")),
+            msg = format!("Unknown attribute {attr_name}"),
-            label = bargs.widget_use.span => "Found in here"
+            label = attr_entry.key_span => "given here"
         })?;
         eprintln!("{}", diag);
     }
@@ -347,7 +348,7 @@ fn validate_container_children_count(container: &gtk::Container, widget_use: &Ba
     }
 
     if container.dynamic_cast_ref::<gtk::Bin>().is_some() && widget_use.children.len() > 1 {
-        Err(DiagError::new(gen_diagnostic! {
+        Err(DiagError(gen_diagnostic! {
             kind =  Severity::Error,
             msg = format!("{} can only have one child", widget_use.name),
             label = widget_use.children_span() => format!("Was given {} children here", widget_use.children.len())

crates/eww/src/widgets/def_widget_macro.rs

@@ -10,8 +10,8 @@ macro_rules! def_widget {
         $({
             $(
                 // explicitly box the function to not cause tons of monomorphization related duplications of Vec::retain
-                let retain_fn: Box<dyn Fn(&eww_shared_util::wrappers::AttrName) -> bool> =
+                let retain_fn: Box<dyn Fn(&eww_shared_util::wrappers::AttrName, &mut yuck::config::attributes::AttrEntry) -> bool> =
-                    Box::new(|a| &a.0 != &::std::stringify!($attr_name).replace('_', "-"));
+                    Box::new(|a, _| &a.0 != &::std::stringify!($attr_name).replace('_', "-"));
                 $args.unhandled_attrs.retain(retain_fn);
             )*
 

crates/eww/src/widgets/widget_definitions.rs

@@ -1,9 +1,7 @@
 #![allow(clippy::option_map_unit_fn)]
 use super::{build_widget::BuilderArgs, circular_progressbar::*, run_command, transform::*};
 use crate::{
-    def_widget, enum_parse,
+    def_widget, enum_parse, error_handling_ctx,
-    error::DiagError,
-    error_handling_ctx,
     util::{list_difference, unindent},
     widgets::build_widget::build_gtk_widget,
 };
@@ -25,8 +23,8 @@ use std::{
     time::Duration,
 };
 use yuck::{
-    config::validate::ValidationError,
+    error::{DiagError, DiagResult},
-    error::{AstError, AstResult},
+    format_diagnostic::{span_to_secondary_label, DiagnosticExt},
     gen_diagnostic,
     parser::from_ast::FromAst,
 };
@@ -109,10 +107,10 @@ pub(super) fn widget_use_to_gtk_widget(bargs: &mut BuilderArgs) -> Result<gtk::W
         WIDGET_NAME_SCROLL => build_gtk_scrolledwindow(bargs)?.upcast(),
         WIDGET_NAME_OVERLAY => build_gtk_overlay(bargs)?.upcast(),
         _ => {
-            return Err(AstError::ValidationError(ValidationError::UnknownWidget(
+            return Err(DiagError(gen_diagnostic! {
-                bargs.widget_use.name_span,
+                msg = format!("referenced unknown widget `{}`", bargs.widget_use.name),
-                bargs.widget_use.name.to_string(),
+                label = bargs.widget_use.name_span => "Used here",
-            ))
+            })
             .into())
         }
     };
@@ -128,13 +126,16 @@ static DEPRECATED_ATTRS: Lazy<HashSet<&str>> =
 /// @desc these properties apply to _all_ widgets, and can be used anywhere!
 pub(super) fn resolve_widget_attrs(bargs: &mut BuilderArgs, gtk_widget: &gtk::Widget) -> Result<()> {
     let deprecated: HashSet<_> = DEPRECATED_ATTRS.to_owned();
-    let contained_deprecated: Vec<_> = bargs.unhandled_attrs.drain_filter(|a| deprecated.contains(&a.0 as &str)).collect();
+    let contained_deprecated: Vec<_> = bargs.unhandled_attrs.drain_filter(|a, _| deprecated.contains(&a.0 as &str)).collect();
     if !contained_deprecated.is_empty() {
         let diag = error_handling_ctx::stringify_diagnostic(gen_diagnostic! {
             kind =  Severity::Error,
             msg = "Unsupported attributes provided",
             label = bargs.widget_use.span => "Found in here",
-            note = format!("The attribute(s) ({}) has/have been removed, as GTK does not support it consistently. Instead, use eventbox to wrap this widget and set the attribute there. See #251 (https://github.com/elkowar/eww/issues/251) for more details.", contained_deprecated.iter().join(", ")),
+            note = format!(
+                "The attribute(s) ({}) has/have been removed, as GTK does not support it consistently. Instead, use eventbox to wrap this widget and set the attribute there. See #251 (https://github.com/elkowar/eww/issues/251) for more details.",
+                contained_deprecated.iter().map(|(x, _)| x).join(", ")
+            ),
         }).unwrap();
         eprintln!("{}", diag);
     }
@@ -546,7 +547,7 @@ fn build_gtk_overlay(bargs: &mut BuilderArgs) -> Result<gtk::Overlay> {
 
     match bargs.widget_use.children.len().cmp(&1) {
         Ordering::Less => {
-            Err(DiagError::new(gen_diagnostic!("overlay must contain at least one element", bargs.widget_use.span)).into())
+            Err(DiagError(gen_diagnostic!("overlay must contain at least one element", bargs.widget_use.span)).into())
         }
         Ordering::Greater | Ordering::Equal => {
             let mut children = bargs.widget_use.children.iter().map(|child| {
@@ -585,18 +586,15 @@ fn build_center_box(bargs: &mut BuilderArgs) -> Result<gtk::Box> {
 
     match bargs.widget_use.children.len().cmp(&3) {
         Ordering::Less => {
-            Err(DiagError::new(gen_diagnostic!("centerbox must contain exactly 3 elements", bargs.widget_use.span)).into())
+            Err(DiagError(gen_diagnostic!("centerbox must contain exactly 3 elements", bargs.widget_use.span)).into())
         }
         Ordering::Greater => {
             let (_, additional_children) = bargs.widget_use.children.split_at(3);
             // we know that there is more than three children, so unwrapping on first and left here is fine.
             let first_span = additional_children.first().unwrap().span();
             let last_span = additional_children.last().unwrap().span();
-            Err(DiagError::new(gen_diagnostic!(
+            Err(DiagError(gen_diagnostic!("centerbox must contain exactly 3 elements, but got more", first_span.to(last_span)))
-                "centerbox must contain exactly 3 elements, but got more",
+                .into())
-                first_span.to(last_span)
-            ))
-            .into())
         }
         Ordering::Equal => {
             let mut children = bargs.widget_use.children.iter().map(|child| {
@@ -868,7 +866,7 @@ fn build_gtk_literal(bargs: &mut BuilderArgs) -> Result<gtk::Box> {
         prop(content: as_string) {
             gtk_widget.children().iter().for_each(|w| gtk_widget.remove(w));
             if !content.is_empty() {
-                let content_widget_use: AstResult<_> = try {
+                let content_widget_use: DiagResult<_> = try {
                     let ast = {
                         let mut yuck_files = error_handling_ctx::YUCK_FILES.write().unwrap();
                         let (span, asts) = yuck_files.load_str("<literal-content>".to_string(), content)?;
@@ -884,10 +882,11 @@ fn build_gtk_literal(bargs: &mut BuilderArgs) -> Result<gtk::Box> {
 
                 // TODO a literal should create a new scope, that I'm not even sure should inherit from root
                 let child_widget = build_gtk_widget(scope_graph, widget_defs.clone(), calling_scope, content_widget_use, None)
-                    .map_err(|e| AstError::ErrorContext {
+                    .map_err(|e| {
-                        label_span: literal_use_span,
+                        let diagnostic = error_handling_ctx::anyhow_err_to_diagnostic(&e)
-                        context: "Error in the literal used here".to_string(),
+                            .unwrap_or_else(|| gen_diagnostic!(e))
-                        main_err: Box::new(error_handling_ctx::anyhow_err_to_diagnostic(&e).unwrap_or_else(|| gen_diagnostic!(e)))
+                            .with_label(span_to_secondary_label(literal_use_span).with_message("Error in the literal used here"));
+                        DiagError(diagnostic)
                     })?;
                 gtk_widget.add(&child_widget);
                 child_widget.show();
@@ -1001,8 +1000,8 @@ fn build_graph(bargs: &mut BuilderArgs) -> Result<super::graph::Graph> {
         // @prop max - the maximum value to show
         prop(min: as_f64 = 0, max: as_f64 = 100) {
             if min > max {
-                return Err(DiagError::new(gen_diagnostic!(
+                return Err(DiagError(gen_diagnostic!(
-                    format!("Graph's min ({}) should never be higher than max ({})",  min, max)
+                    format!("Graph's min ({min}) should never be higher than max ({max})")
                 )).into());
             }
             w.set_property("min", &min);

crates/simplexpr/src/error.rs

@@ -1,60 +1,31 @@
-use crate::{
+use crate::parser::lexer::{self, LexicalError};
-    dynval,
-    parser::lexer::{self, LexicalError},
-};
 use eww_shared_util::{Span, Spanned};
 
-pub type Result<T> = std::result::Result<T, Error>;
 #[derive(thiserror::Error, Debug)]
-pub enum Error {
+#[error("Error parsing expression: {source}")]
-    #[error("Error parsing expression: {source}")]
+pub struct ParseError {
-    ParseError { file_id: usize, source: lalrpop_util::ParseError<usize, lexer::Token, lexer::LexicalError> },
+    pub file_id: usize,
-
+    pub source: lalrpop_util::ParseError<usize, lexer::Token, lexer::LexicalError>,
-    #[error(transparent)]
-    ConversionError(#[from] dynval::ConversionError),
-
-    #[error("{1}")]
-    Spanned(Span, Box<Error>),
-
-    #[error(transparent)]
-    Eval(#[from] crate::eval::EvalError),
-
-    #[error(transparent)]
-    Other(#[from] Box<dyn std::error::Error + Send + Sync>),
 }
 
-impl Error {
+impl ParseError {
     pub fn from_parse_error(file_id: usize, err: lalrpop_util::ParseError<usize, lexer::Token, lexer::LexicalError>) -> Self {
-        Error::ParseError { file_id, source: err }
+        Self { file_id, source: err }
-    }
-
-    pub fn at(self, span: Span) -> Self {
-        Self::Spanned(span, Box::new(self))
     }
 }
 
-impl Spanned for Error {
+impl Spanned for ParseError {
     fn span(&self) -> Span {
-        match self {
+        match &self.source {
-            Self::ParseError { file_id, source } => get_parse_error_span(*file_id, source),
+            lalrpop_util::ParseError::InvalidToken { location } => Span(*location, *location, self.file_id),
-            Self::Spanned(span, _) => *span,
+            lalrpop_util::ParseError::UnrecognizedEOF { location, expected: _ } => Span(*location, *location, self.file_id),
-            Self::Eval(err) => err.span(),
+            lalrpop_util::ParseError::UnrecognizedToken { token, expected: _ } => Span(token.0, token.2, self.file_id),
-            Self::ConversionError(err) => err.span(),
+            lalrpop_util::ParseError::ExtraToken { token } => Span(token.0, token.2, self.file_id),
-            _ => Span::DUMMY,
+            lalrpop_util::ParseError::User { error: LexicalError(span) } => *span,
         }
     }
 }
 
-fn get_parse_error_span(file_id: usize, err: &lalrpop_util::ParseError<usize, lexer::Token, lexer::LexicalError>) -> Span {
-    match err {
-        lalrpop_util::ParseError::InvalidToken { location } => Span(*location, *location, file_id),
-        lalrpop_util::ParseError::UnrecognizedEOF { location, expected: _ } => Span(*location, *location, file_id),
-        lalrpop_util::ParseError::UnrecognizedToken { token, expected: _ } => Span(token.0, token.2, file_id),
-        lalrpop_util::ParseError::ExtraToken { token } => Span(token.0, token.2, file_id),
-        lalrpop_util::ParseError::User { error: LexicalError(span) } => *span,
-    }
-}
-
 #[macro_export]
 macro_rules! spanned {
     ($err:ty, $span:expr, $block:expr) => {{

crates/simplexpr/src/parser/mod.rs

@@ -1,15 +1,12 @@
 pub mod lalrpop_helpers;
 pub mod lexer;
 
-use crate::{
+use crate::{ast::SimplExpr, error::ParseError};
-    ast::SimplExpr,
-    error::{Error, Result},
-};
 
-pub fn parse_string(byte_offset: usize, file_id: usize, s: &str) -> Result<SimplExpr> {
+pub fn parse_string(byte_offset: usize, file_id: usize, s: &str) -> Result<SimplExpr, ParseError> {
     let lexer = lexer::Lexer::new(file_id, byte_offset, s);
     let parser = crate::simplexpr_parser::ExprParser::new();
-    parser.parse(file_id, lexer).map_err(|e| Error::from_parse_error(file_id, e))
+    parser.parse(file_id, lexer).map_err(|e| ParseError::from_parse_error(file_id, e))
 }
 
 #[cfg(test)]

crates/yuck/src/ast_error.rs

@@ -0,0 +1,62 @@
+use eww_shared_util::{AttrName, Span};
+
+use crate::{
+    error::{DiagError, DiagResult},
+    format_diagnostic::ToDiagnostic,
+    gen_diagnostic,
+    parser::ast::AstType,
+};
+
+/// Error type representing errors that occur when trying to access parts of the AST specifically
+#[derive(Debug, thiserror::Error)]
+pub enum AstError {
+    #[error("Did not expect any further elements here. Make sure your format is correct")]
+    NoMoreElementsExpected(Span),
+
+    #[error("Expected more elements")]
+    TooFewElements(Span),
+
+    #[error("Wrong type of expression: Expected {1} but got {2}")]
+    WrongExprType(Span, AstType, AstType),
+
+    #[error("'{0}' is missing a value")]
+    DanglingKeyword(Span, AttrName),
+
+    /// May occur when we need to evaluate an expression when expecting a literal value
+    #[error(transparent)]
+    EvalError(#[from] simplexpr::eval::EvalError),
+}
+
+impl ToDiagnostic for AstError {
+    fn to_diagnostic(&self) -> codespan_reporting::diagnostic::Diagnostic<usize> {
+        match self {
+            AstError::NoMoreElementsExpected(span) => gen_diagnostic!(self, span),
+            AstError::TooFewElements(span) => gen_diagnostic! {
+                msg = self,
+                label = span => "Expected another element here"
+            },
+            AstError::WrongExprType(span, expected, actual) => gen_diagnostic! {
+                msg = "Wrong type of expression",
+                label = span => format!("Expected a `{expected}` here"),
+                note = format!("Expected: {expected}\n     Got: {actual}"),
+            },
+            AstError::DanglingKeyword(span, kw) => gen_diagnostic! {
+                msg = "{kw} is missing a value",
+                label = span => "No value provided for this",
+            },
+            AstError::EvalError(e) => e.to_diagnostic(),
+        }
+    }
+}
+
+impl eww_shared_util::Spanned for AstError {
+    fn span(&self) -> Span {
+        match self {
+            AstError::NoMoreElementsExpected(span) => *span,
+            AstError::TooFewElements(span) => *span,
+            AstError::WrongExprType(span, ..) => *span,
+            AstError::DanglingKeyword(span, _) => *span,
+            AstError::EvalError(e) => e.span(),
+        }
+    }
+}

crates/yuck/src/config/attributes.rs

@@ -10,7 +10,7 @@ use simplexpr::{
 };
 
 use crate::{
-    error::AstError,
+    error::DiagError,
     parser::{ast::Ast, from_ast::FromAst},
 };
 use eww_shared_util::{AttrName, Span, Spanned, VarName};
@@ -37,12 +37,6 @@ impl Spanned for AttrError {
     }
 }
 
-#[derive(Debug)]
-pub struct UnusedAttrs {
-    definition_span: Span,
-    attrs: Vec<(Span, AttrName)>,
-}
-
 #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize)]
 pub struct AttrEntry {
     pub key_span: Span,
@@ -67,7 +61,7 @@ impl Attributes {
         Attributes { span, attrs }
     }
 
-    pub fn ast_required<T: FromAst>(&mut self, key: &str) -> Result<T, AstError> {
+    pub fn ast_required<T: FromAst>(&mut self, key: &str) -> Result<T, DiagError> {
         let key = AttrName(key.to_string());
         match self.attrs.remove(&key) {
             Some(AttrEntry { key_span, value }) => T::from_ast(value),
@@ -75,7 +69,7 @@ impl Attributes {
         }
     }
 
-    pub fn ast_optional<T: FromAst>(&mut self, key: &str) -> Result<Option<T>, AstError> {
+    pub fn ast_optional<T: FromAst>(&mut self, key: &str) -> Result<Option<T>, DiagError> {
         match self.attrs.remove(&AttrName(key.to_string())) {
             Some(AttrEntry { key_span, value }) => T::from_ast(value).map(Some),
             None => Ok(None),
@@ -84,7 +78,7 @@ impl Attributes {
 
     /// Retrieve a required attribute from the set which _must not_ reference any variables,
     /// and is thus known to be static.
-    pub fn primitive_required<T, E>(&mut self, key: &str) -> Result<T, AstError>
+    pub fn primitive_required<T, E>(&mut self, key: &str) -> Result<T, DiagError>
     where
         E: std::error::Error + 'static + Sync + Send,
         T: FromDynVal<Err = E>,
@@ -99,7 +93,7 @@ impl Attributes {
 
     /// Retrieve an optional attribute from the set which _must not_ reference any variables,
     /// and is thus known to be static.
-    pub fn primitive_optional<T, E>(&mut self, key: &str) -> Result<Option<T>, AstError>
+    pub fn primitive_optional<T, E>(&mut self, key: &str) -> Result<Option<T>, DiagError>
     where
         E: std::error::Error + 'static + Sync + Send,
         T: FromDynVal<Err = E>,
@@ -117,8 +111,8 @@ impl Attributes {
     }
 
     /// Consumes the attributes to return a list of unused attributes which may be used to emit a warning.
-    /// TODO actually use this and implement warnings,... lol
+    /// TODO actually use this and emit warnings
-    pub fn get_unused(self, definition_span: Span) -> UnusedAttrs {
+    pub fn get_unused(self) -> impl Iterator<Item = (Span, AttrName)> {
-        UnusedAttrs { definition_span, attrs: self.attrs.into_iter().map(|(k, v)| (v.key_span.to(v.value.span()), k)).collect() }
+        self.attrs.into_iter().map(|(k, v)| (v.key_span.to(v.value.span()), k))
     }
 }

crates/yuck/src/config/backend_window_options.rs

@@ -4,7 +4,7 @@ use anyhow::{anyhow, Result};
 
 use crate::{
     enum_parse,
-    error::AstResult,
+    error::DiagResult,
     parser::{ast::Ast, ast_iterator::AstIterator, from_ast::FromAstElementContent},
     value::NumWithUnit,
 };
@@ -16,6 +16,11 @@ pub use backend::*;
 
 #[cfg(feature = "x11")]
 mod backend {
+    use crate::{
+        error::{DiagError, DiagResultExt},
+        format_diagnostic::ToDiagnostic,
+    };
+
     use super::*;
 
     #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize)]
@@ -27,7 +32,7 @@ mod backend {
     }
 
     impl BackendWindowOptions {
-        pub fn from_attrs(attrs: &mut Attributes) -> AstResult<Self> {
+        pub fn from_attrs(attrs: &mut Attributes) -> DiagResult<Self> {
             let struts = attrs.ast_optional("reserve")?;
             let window_type = attrs.primitive_optional("windowtype")?;
             Ok(Self {
@@ -98,9 +103,9 @@ mod backend {
     impl FromAstElementContent for StrutDefinition {
         const ELEMENT_NAME: &'static str = "struts";
 
-        fn from_tail<I: Iterator<Item = Ast>>(span: Span, mut iter: AstIterator<I>) -> AstResult<Self> {
+        fn from_tail<I: Iterator<Item = Ast>>(span: Span, mut iter: AstIterator<I>) -> DiagResult<Self> {
             let mut attrs = iter.expect_key_values()?;
-            iter.expect_done().map_err(|e| e.note("Check if you are missing a colon in front of a key"))?;
+            iter.expect_done().map_err(DiagError::from).note("Check if you are missing a colon in front of a key")?;
             Ok(StrutDefinition { side: attrs.primitive_required("side")?, dist: attrs.primitive_required("distance")? })
         }
     }
@@ -115,7 +120,7 @@ mod backend {
         pub focusable: bool,
     }
     impl BackendWindowOptions {
-        pub fn from_attrs(attrs: &mut Attributes) -> AstResult<Self> {
+        pub fn from_attrs(attrs: &mut Attributes) -> DiagResult<Self> {
             Ok(Self {
                 exclusive: attrs.primitive_optional("exclusive")?.unwrap_or(false),
                 focusable: attrs.primitive_optional("focusable")?.unwrap_or(false),
@@ -130,7 +135,7 @@ mod backend {
     #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize)]
     pub struct BackendWindowOptions;
     impl BackendWindowOptions {
-        pub fn from_attrs(attrs: &mut Attributes) -> AstResult<Self> {
+        pub fn from_attrs(attrs: &mut Attributes) -> DiagResult<Self> {
             Ok(Self)
         }
     }

crates/yuck/src/config/config.rs

@@ -4,6 +4,7 @@ use std::{
 };
 
 use codespan_reporting::files::SimpleFiles;
+use itertools::Itertools;
 use simplexpr::SimplExpr;
 
 use super::{
@@ -17,16 +18,18 @@ use super::{
 };
 use crate::{
     config::script_var_definition::{ListenScriptVar, PollScriptVar},
-    error::{AstError, AstResult, OptionAstErrorExt},
+    error::{DiagError, DiagResult},
+    format_diagnostic::ToDiagnostic,
+    gen_diagnostic,
     parser::{
         ast::Ast,
         ast_iterator::AstIterator,
         from_ast::{FromAst, FromAstElementContent},
     },
 };
-use eww_shared_util::{AttrName, Span, VarName};
+use eww_shared_util::{AttrName, Span, Spanned, VarName};
 
-pub static TOP_LEVEL_DEFINITION_NAMES: &[&str] = &[
+static TOP_LEVEL_DEFINITION_NAMES: &[&str] = &[
     WidgetDefinition::ELEMENT_NAME,
     WindowDefinition::ELEMENT_NAME,
     VarDefinition::ELEMENT_NAME,
@@ -44,7 +47,7 @@ pub struct Include {
 impl FromAstElementContent for Include {
     const ELEMENT_NAME: &'static str = "include";
 
-    fn from_tail<I: Iterator<Item = Ast>>(span: Span, mut iter: AstIterator<I>) -> AstResult<Self> {
+    fn from_tail<I: Iterator<Item = Ast>>(span: Span, mut iter: AstIterator<I>) -> DiagResult<Self> {
         let (path_span, path) = iter.expect_literal()?;
         iter.expect_done()?;
         Ok(Include { path: path.to_string(), path_span })
@@ -60,7 +63,7 @@ pub enum TopLevel {
 }
 
 impl FromAst for TopLevel {
-    fn from_ast(e: Ast) -> AstResult<Self> {
+    fn from_ast(e: Ast) -> DiagResult<Self> {
         let span = e.span();
         let mut iter = e.try_ast_iter()?;
         let (sym_span, element_name) = iter.expect_symbol()?;
@@ -75,7 +78,13 @@ impl FromAst for TopLevel {
                 Self::ScriptVarDefinition(ScriptVarDefinition::Listen(ListenScriptVar::from_tail(span, iter)?))
             }
             x if x == WindowDefinition::ELEMENT_NAME => Self::WindowDefinition(WindowDefinition::from_tail(span, iter)?),
-            x => return Err(AstError::UnknownToplevel(sym_span, x.to_string())),
+            x => {
+                return Err(DiagError(gen_diagnostic! {
+                    msg = format!("Unknown toplevel declaration `{x}`"),
+                    label = sym_span,
+                    note = format!("Must be one of: {}", TOP_LEVEL_DEFINITION_NAMES.iter().join(", ")),
+                }))
+            }
         })
     }
 }
@@ -89,13 +98,13 @@ pub struct Config {
 }
 
 impl Config {
-    fn append_toplevel(&mut self, files: &mut YuckFiles, toplevel: TopLevel) -> AstResult<()> {
+    fn append_toplevel(&mut self, files: &mut YuckFiles, toplevel: TopLevel) -> DiagResult<()> {
         match toplevel {
             TopLevel::VarDefinition(x) => {
                 if self.var_definitions.contains_key(&x.name) || self.script_vars.contains_key(&x.name) {
-                    return Err(AstError::ValidationError(ValidationError::VariableDefinedTwice {
+                    return Err(DiagError(gen_diagnostic! {
-                        name: x.name.clone(),
+                        msg = format!("Variable {} defined twice", x.name),
-                        span: x.span,
+                        label = x.span => "defined again here",
                     }));
                 } else {
                     self.var_definitions.insert(x.name.clone(), x);
@@ -103,9 +112,9 @@ impl Config {
             }
             TopLevel::ScriptVarDefinition(x) => {
                 if self.var_definitions.contains_key(x.name()) || self.script_vars.contains_key(x.name()) {
-                    return Err(AstError::ValidationError(ValidationError::VariableDefinedTwice {
+                    return Err(DiagError(gen_diagnostic! {
-                        name: x.name().clone(),
+                        msg = format!("Variable {} defined twice", x.name()),
-                        span: x.name_span(),
+                        label = x.name_span() => "defined again here",
                     }));
                 } else {
                     self.script_vars.insert(x.name().clone(), x);
@@ -119,8 +128,11 @@ impl Config {
             }
             TopLevel::Include(include) => {
                 let (file_id, toplevels) = files.load_file(PathBuf::from(&include.path)).map_err(|err| match err {
-                    FilesError::IoError(_) => AstError::IncludedFileNotFound(include),
+                    FilesError::IoError(_) => DiagError(gen_diagnostic! {
-                    FilesError::AstError(x) => x,
+                        msg = format!("Included file `{}` not found", include.path),
+                        label = include.path_span => "Included here",
+                    }),
+                    FilesError::DiagError(x) => x,
                 })?;
                 for element in toplevels {
                     self.append_toplevel(files, TopLevel::from_ast(element)?)?;
@@ -130,7 +142,7 @@ impl Config {
         Ok(())
     }
 
-    pub fn generate(files: &mut YuckFiles, elements: Vec<Ast>) -> AstResult<Self> {
+    pub fn generate(files: &mut YuckFiles, elements: Vec<Ast>) -> DiagResult<Self> {
         let mut config = Self {
             widget_definitions: HashMap::new(),
             window_definitions: HashMap::new(),
@@ -143,10 +155,10 @@ impl Config {
         Ok(config)
     }
 
-    pub fn generate_from_main_file(files: &mut YuckFiles, path: impl AsRef<Path>) -> AstResult<Self> {
+    pub fn generate_from_main_file(files: &mut YuckFiles, path: impl AsRef<Path>) -> DiagResult<Self> {
         let (span, top_levels) = files.load_file(path.as_ref().to_path_buf()).map_err(|err| match err {
-            FilesError::IoError(err) => AstError::Other(Span::DUMMY, Box::new(err)),
+            FilesError::IoError(err) => DiagError(gen_diagnostic!(err)),
-            FilesError::AstError(x) => x,
+            FilesError::DiagError(x) => x,
         })?;
         Self::generate(files, top_levels)
     }

crates/yuck/src/config/file_provider.rs

@@ -4,7 +4,7 @@ use codespan_reporting::files::{Files, SimpleFile, SimpleFiles};
 use eww_shared_util::Span;
 
 use crate::{
-    error::{AstError, AstResult},
+    error::{DiagError, DiagResult},
     parser::ast::Ast,
 };
 
@@ -14,7 +14,7 @@ pub enum FilesError {
     IoError(#[from] std::io::Error),
 
     #[error(transparent)]
-    AstError(#[from] AstError),
+    DiagError(#[from] DiagError),
 }
 
 #[derive(Clone, Debug)]
@@ -93,7 +93,7 @@ impl YuckFiles {
         Ok(crate::parser::parse_toplevel(file_id, file_content)?)
     }
 
-    pub fn load_str(&mut self, name: String, content: String) -> Result<(Span, Vec<Ast>), AstError> {
+    pub fn load_str(&mut self, name: String, content: String) -> Result<(Span, Vec<Ast>), DiagError> {
         let line_starts = codespan_reporting::files::line_starts(&content).collect();
         let yuck_file =
             YuckFile { name, line_starts, source_len_bytes: content.len(), source: YuckSource::Literal(content.to_string()) };

crates/yuck/src/config/script_var_definition.rs

@@ -3,7 +3,8 @@ use std::collections::HashMap;
 use simplexpr::{dynval::DynVal, SimplExpr};
 
 use crate::{
-    error::{AstError, AstResult, AstResultExt},
+    error::{DiagError, DiagResult, DiagResultExt},
+    format_diagnostic::ToDiagnostic,
     parser::{
         ast::Ast,
         ast_iterator::AstIterator,
@@ -65,12 +66,13 @@ pub struct PollScriptVar {
 impl FromAstElementContent for PollScriptVar {
     const ELEMENT_NAME: &'static str = "defpoll";
 
-    fn from_tail<I: Iterator<Item = Ast>>(span: Span, mut iter: AstIterator<I>) -> AstResult<Self> {
+    fn from_tail<I: Iterator<Item = Ast>>(span: Span, mut iter: AstIterator<I>) -> DiagResult<Self> {
-        let result: AstResult<_> = try {
+        let result: DiagResult<_> = try {
             let (name_span, name) = iter.expect_symbol()?;
             let mut attrs = iter.expect_key_values()?;
             let initial_value = Some(attrs.primitive_optional("initial")?.unwrap_or_else(|| DynVal::from_string(String::new())));
-            let interval = attrs.primitive_required::<DynVal, _>("interval")?.as_duration()?;
+            let interval =
+                attrs.primitive_required::<DynVal, _>("interval")?.as_duration().map_err(|e| DiagError(e.to_diagnostic()))?;
             let (script_span, script) = iter.expect_literal()?;
 
             let run_while_expr =
@@ -101,8 +103,8 @@ pub struct ListenScriptVar {
 impl FromAstElementContent for ListenScriptVar {
     const ELEMENT_NAME: &'static str = "deflisten";
 
-    fn from_tail<I: Iterator<Item = Ast>>(span: Span, mut iter: AstIterator<I>) -> AstResult<Self> {
+    fn from_tail<I: Iterator<Item = Ast>>(span: Span, mut iter: AstIterator<I>) -> DiagResult<Self> {
-        let result: AstResult<_> = try {
+        let result: DiagResult<_> = try {
             let (name_span, name) = iter.expect_symbol()?;
             let mut attrs = iter.expect_key_values()?;
             let initial_value = attrs.primitive_optional("initial")?.unwrap_or_else(|| DynVal::from_string(String::new()));

crates/yuck/src/config/validate.rs

@@ -3,7 +3,7 @@ use std::collections::{HashMap, HashSet};
 use simplexpr::SimplExpr;
 
 use crate::{
-    error::AstResult,
+    error::DiagResult,
     parser::{ast::Ast, ast_iterator::AstIterator, from_ast::FromAst},
 };
 
@@ -16,9 +16,6 @@ use eww_shared_util::{AttrName, Span, Spanned, VarName};
 
 #[derive(Debug, thiserror::Error)]
 pub enum ValidationError {
-    #[error("Unknown widget `{1}` referenced")]
-    UnknownWidget(Span, String),
-
     #[error("There is already a builtin widget called `{1}`")]
     AccidentalBuiltinOverride(Span, String),
 
@@ -32,16 +29,11 @@ pub enum ValidationError {
         /// True if the error occurred inside a widget definition, false if it occurred in a window definition
         in_definition: bool,
     },
-
-    #[error("Variable named `{name}` defined twice")]
-    VariableDefinedTwice { span: Span, name: VarName },
 }
 
 impl Spanned for ValidationError {
     fn span(&self) -> Span {
         match self {
-            ValidationError::UnknownWidget(span, _) => *span,
-            ValidationError::VariableDefinedTwice { span, .. } => *span,
             ValidationError::MissingAttr { use_span, .. } => *use_span,
             ValidationError::UnknownVariable { span, .. } => *span,
             ValidationError::AccidentalBuiltinOverride(span, ..) => *span,

crates/yuck/src/config/var_definition.rs

@@ -3,7 +3,7 @@ use std::collections::HashMap;
 use simplexpr::{dynval::DynVal, SimplExpr};
 
 use crate::{
-    error::{AstResult, AstResultExt},
+    error::{DiagResult, DiagResultExt},
     parser::{
         ast::Ast,
         ast_iterator::AstIterator,
@@ -22,8 +22,8 @@ pub struct VarDefinition {
 impl FromAstElementContent for VarDefinition {
     const ELEMENT_NAME: &'static str = "defvar";
 
-    fn from_tail<I: Iterator<Item = Ast>>(span: Span, mut iter: AstIterator<I>) -> AstResult<Self> {
+    fn from_tail<I: Iterator<Item = Ast>>(span: Span, mut iter: AstIterator<I>) -> DiagResult<Self> {
-        let result: AstResult<_> = try {
+        let result: DiagResult<_> = try {
             let (_, name) = iter.expect_symbol()?;
             let (_, initial_value) = iter.expect_literal()?;
             iter.expect_done()?;

crates/yuck/src/config/widget_definition.rs

@@ -3,7 +3,9 @@ use std::collections::HashMap;
 use simplexpr::SimplExpr;
 
 use crate::{
-    error::{AstError::WrongExprType, AstResult, AstResultExt, FormFormatError},
+    error::{DiagError, DiagResult, DiagResultExt},
+    format_diagnostic::{DiagnosticExt, ToDiagnostic},
+    gen_diagnostic,
     parser::{
         ast::Ast,
         ast_iterator::AstIterator,
@@ -22,7 +24,7 @@ pub struct AttrSpec {
 }
 
 impl FromAst for AttrSpec {
-    fn from_ast(e: Ast) -> AstResult<Self> {
+    fn from_ast(e: Ast) -> DiagResult<Self> {
         let span = e.span();
         let symbol = e.as_symbol()?;
         let (name, optional) = if let Some(name) = symbol.strip_prefix('?') { (name.to_string(), true) } else { (symbol, false) };
@@ -42,15 +44,33 @@ pub struct WidgetDefinition {
 impl FromAstElementContent for WidgetDefinition {
     const ELEMENT_NAME: &'static str = "defwidget";
 
-    fn from_tail<I: Iterator<Item = Ast>>(span: Span, mut iter: AstIterator<I>) -> AstResult<Self> {
+    fn from_tail<I: Iterator<Item = Ast>>(span: Span, mut iter: AstIterator<I>) -> DiagResult<Self> {
-        let (name_span, name) = iter.expect_symbol().note(EXPECTED_WIDGET_DEF_FORMAT)?;
+        let (name_span, name) = iter.expect_symbol().map_err(DiagError::from).note(EXPECTED_WIDGET_DEF_FORMAT)?;
         let (args_span, expected_args) = iter
             .expect_array()
-            .wrong_expr_type_to(|_, _| Some(FormFormatError::WidgetDefArglistMissing(name_span.point_span_at_end())))
+            .map_err(|e| {
+                DiagError(match e {
+                    crate::ast_error::AstError::WrongExprType(span, expected, actual) => gen_diagnostic! {
+                        msg = "Widget definition missing argument list",
+                        label = name_span.point_span_at_end() => "Insert the argument list (e.g.: `[]`) here",
+                        note = "This list needs to declare all the non-global variables / attributes used in this widget."
+                    },
+                    other => other.to_diagnostic(),
+                })
+            })
             .note(EXPECTED_WIDGET_DEF_FORMAT)?;
-        let expected_args = expected_args.into_iter().map(AttrSpec::from_ast).collect::<AstResult<_>>()?;
+        let expected_args = expected_args.into_iter().map(AttrSpec::from_ast).collect::<DiagResult<_>>()?;
-        let widget = iter.expect_any().note(EXPECTED_WIDGET_DEF_FORMAT).and_then(WidgetUse::from_ast)?;
+        let widget = iter.expect_any().map_err(DiagError::from).note(EXPECTED_WIDGET_DEF_FORMAT).and_then(WidgetUse::from_ast)?;
-        iter.expect_done().map_err(|e| FormFormatError::WidgetDefMultipleChildren(e.span()))?;
+        iter.expect_done().map_err(|e| {
+            DiagError(gen_diagnostic! {
+                msg = "Widget definition has more than one child widget",
+                label = e.span() => "Found more than one child element here.",
+                note = "A widget-definition may only contain one child element.\n\
+                        To include multiple elements, wrap these elements in a single container widget such as `box`.\n\
+                        This is necessary as eww can't know how you want these elements to be layed out otherwise."
+            })
+        })?;
+
         Ok(Self { name, expected_args, widget, span, args_span })
     }
 }

crates/yuck/src/config/widget_use.rs

@@ -4,7 +4,9 @@ use simplexpr::SimplExpr;
 
 use crate::{
     config::attributes::AttrEntry,
-    error::{AstError, AstResult, AstResultExt, FormFormatError},
+    error::{DiagError, DiagResult, DiagResultExt},
+    format_diagnostic::{DiagnosticExt, ToDiagnostic},
+    gen_diagnostic,
     parser::{
         ast::Ast,
         ast_iterator::AstIterator,
@@ -60,9 +62,9 @@ impl BasicWidgetUse {
         name: String,
         name_span: Span,
         mut iter: AstIterator<I>,
-    ) -> AstResult<Self> {
+    ) -> DiagResult<Self> {
         let attrs = iter.expect_key_values()?;
-        let children = iter.map(WidgetUse::from_ast).collect::<AstResult<Vec<_>>>()?;
+        let children = iter.map(WidgetUse::from_ast).collect::<DiagResult<Vec<_>>>()?;
         Ok(Self { name, attrs, children, span, name_span })
     }
 }
@@ -70,14 +72,17 @@ impl BasicWidgetUse {
 impl FromAstElementContent for LoopWidgetUse {
     const ELEMENT_NAME: &'static str = "for";
 
-    fn from_tail<I: Iterator<Item = Ast>>(span: Span, mut iter: AstIterator<I>) -> AstResult<Self> {
+    fn from_tail<I: Iterator<Item = Ast>>(span: Span, mut iter: AstIterator<I>) -> DiagResult<Self> {
         let (element_name_span, element_name) = iter.expect_symbol()?;
         let (in_string_span, in_string) = iter.expect_symbol()?;
         if in_string != "in" {
-            return Err(AstError::FormFormatError(FormFormatError::ExpectedInInForLoop(in_string_span, in_string)));
+            return Err(DiagError(gen_diagnostic! {
+                msg = "Expected 'in' in this position, but got '{in_string}'",
+                label = in_string_span
+            }));
         }
         let (elements_span, elements_expr) = iter.expect_simplexpr()?;
-        let body = iter.expect_any().note("Expected a loop body").and_then(WidgetUse::from_ast)?;
+        let body = iter.expect_any().map_err(DiagError::from).note("Expected a loop body").and_then(WidgetUse::from_ast)?;
         iter.expect_done()?;
         Ok(Self {
             element_name: VarName(element_name),
@@ -92,7 +97,7 @@ impl FromAstElementContent for LoopWidgetUse {
 impl FromAstElementContent for ChildrenWidgetUse {
     const ELEMENT_NAME: &'static str = "children";
 
-    fn from_tail<I: Iterator<Item = Ast>>(span: Span, mut iter: AstIterator<I>) -> AstResult<Self> {
+    fn from_tail<I: Iterator<Item = Ast>>(span: Span, mut iter: AstIterator<I>) -> DiagResult<Self> {
         let mut attrs = iter.expect_key_values()?;
         let nth_expr = attrs.ast_optional("nth")?;
         iter.expect_done()?;
@@ -101,7 +106,7 @@ impl FromAstElementContent for ChildrenWidgetUse {
 }
 
 impl FromAst for WidgetUse {
-    fn from_ast(e: Ast) -> AstResult<Self> {
+    fn from_ast(e: Ast) -> DiagResult<Self> {
         let span = e.span();
         if let Ok(value) = e.clone().as_simplexpr() {
             Ok(WidgetUse::Basic(label_from_simplexpr(value, span)))

crates/yuck/src/config/window_definition.rs

@@ -4,7 +4,8 @@ use simplexpr::{dynval::DynVal, SimplExpr};
 
 use crate::{
     config::monitor::MonitorIdentifier,
-    error::{AstError, AstResult},
+    error::{DiagError, DiagResult},
+    format_diagnostic::ToDiagnostic,
     parser::{
         ast::Ast,
         ast_iterator::AstIterator,
@@ -30,7 +31,7 @@ pub struct WindowDefinition {
 impl FromAstElementContent for WindowDefinition {
     const ELEMENT_NAME: &'static str = "defwindow";
 
-    fn from_tail<I: Iterator<Item = Ast>>(span: Span, mut iter: AstIterator<I>) -> AstResult<Self> {
+    fn from_tail<I: Iterator<Item = Ast>>(span: Span, mut iter: AstIterator<I>) -> DiagResult<Self> {
         let (_, name) = iter.expect_symbol()?;
         let mut attrs = iter.expect_key_values()?;
         let monitor = attrs.primitive_optional("monitor")?;
@@ -38,7 +39,7 @@ impl FromAstElementContent for WindowDefinition {
         let stacking = attrs.primitive_optional("stacking")?.unwrap_or(WindowStacking::Foreground);
         let geometry = attrs.ast_optional("geometry")?;
         let backend_options = BackendWindowOptions::from_attrs(&mut attrs)?;
-        let widget = iter.expect_any().and_then(WidgetUse::from_ast)?;
+        let widget = iter.expect_any().map_err(DiagError::from).and_then(WidgetUse::from_ast)?;
         iter.expect_done()?;
         Ok(Self { name, monitor, resizable, widget, stacking, geometry, backend_options })
     }

crates/yuck/src/config/window_geometry.rs

@@ -4,7 +4,8 @@ use simplexpr::{dynval::DynVal, SimplExpr};
 
 use crate::{
     enum_parse,
-    error::{AstError, AstResult},
+    error::{DiagError, DiagResult},
+    format_diagnostic::ToDiagnostic,
     parser::{
         ast::Ast,
         ast_iterator::AstIterator,
@@ -119,9 +120,10 @@ pub struct WindowGeometry {
 impl FromAstElementContent for WindowGeometry {
     const ELEMENT_NAME: &'static str = "geometry";
 
-    fn from_tail<I: Iterator<Item = Ast>>(span: Span, mut iter: AstIterator<I>) -> AstResult<Self> {
+    fn from_tail<I: Iterator<Item = Ast>>(span: Span, mut iter: AstIterator<I>) -> DiagResult<Self> {
         let mut attrs = iter.expect_key_values()?;
-        iter.expect_done().map_err(|e| e.note("Check if you are missing a colon in front of a key"))?;
+        iter.expect_done()
+            .map_err(|e| e.to_diagnostic().with_notes(vec!["Check if you are missing a colon in front of a key".to_string()]))?;
         Ok(WindowGeometry {
             anchor_point: attrs.primitive_optional("anchor")?.unwrap_or_default(),
             size: Coords {

crates/yuck/src/error.rs

@@ -1,6 +1,7 @@
 use crate::{
     config::{attributes::AttrError, config::Include, validate::ValidationError},
-    format_diagnostic::ToDiagnostic,
+    format_diagnostic::{lalrpop_error_to_diagnostic, DiagnosticExt, ToDiagnostic},
+    gen_diagnostic,
     parser::{
         ast::{Ast, AstType},
         lexer, parse_error,
@@ -11,112 +12,32 @@ use eww_shared_util::{AttrName, Span, Spanned, VarName};
 use simplexpr::dynval;
 use thiserror::Error;
 
-pub type AstResult<T> = Result<T, AstError>;
+pub type DiagResult<T> = Result<T, DiagError>;
 
 #[derive(Debug, Error)]
-pub enum AstError {
+#[error("{}", .0.to_message())]
-    #[error("Unknown toplevel declaration `{1}`")]
+pub struct DiagError(pub diagnostic::Diagnostic<usize>);
-    UnknownToplevel(Span, String),
-    #[error("Expected another element, but got nothing")]
-    MissingNode(Span),
-    #[error("Too many elements, must be exactly {1}")]
-    TooManyNodes(Span, i32),
-    #[error("Did not expect any further elements here. Make sure your format is correct")]
-    NoMoreElementsExpected(Span),
 
-    #[error(transparent)]
+static_assertions::assert_impl_all!(DiagError: Send, Sync);
-    FormFormatError(#[from] FormFormatError),
-
-    #[error("Wrong type of expression: Expected {1} but got {2}")]
-    WrongExprType(Span, AstType, AstType),
-    #[error("Expected to get a value, but got {1}")]
-    NotAValue(Span, AstType),
-    #[error("Expected element {1}, but read {2}")]
-    MismatchedElementName(Span, String, String),
-
-    #[error("Keyword `{1}` is missing a value")]
-    DanglingKeyword(Span, String),
-
-    #[error("Included file not found {}", .0.path)]
-    IncludedFileNotFound(Include),
-
-    #[error("{}", .main_err.to_message())]
-    ErrorContext { label_span: Span, context: String, main_err: Box<dyn ToDiagnostic + Send + Sync + 'static> },
-    #[error("{1}")]
-    ErrorNote(String, #[source] Box<AstError>),
-
-    #[error(transparent)]
-    SimplExpr(#[from] simplexpr::error::Error),
-
-    #[error(transparent)]
-    ConversionError(#[from] dynval::ConversionError),
-
-    #[error("{1}")]
-    Other(Span, Box<dyn std::error::Error + Sync + Send + 'static>),
-
-    #[error(transparent)]
-    AttrError(#[from] AttrError),
-
-    #[error(transparent)]
-    ValidationError(#[from] ValidationError),
-
-    #[error("Parse error: {source}")]
-    ParseError { file_id: usize, source: lalrpop_util::ParseError<usize, lexer::Token, parse_error::ParseError> },
-}
-
-static_assertions::assert_impl_all!(AstError: Send, Sync);
 static_assertions::assert_impl_all!(dynval::ConversionError: Send, Sync);
 static_assertions::assert_impl_all!(lalrpop_util::ParseError < usize, lexer::Token, parse_error::ParseError>: Send, Sync);
 
-impl AstError {
+impl<T: ToDiagnostic> From<T> for DiagError {
-    pub fn note(self, note: &str) -> Self {
+    fn from(x: T) -> Self {
-        AstError::ErrorNote(note.to_string(), Box::new(self))
+        Self(x.to_diagnostic())
     }
+}
 
-    pub fn context_label(self, label_span: Span, context: &str) -> Self {
+impl DiagError {
-        AstError::ErrorContext { label_span, context: context.to_string(), main_err: Box::new(self) }
+    pub fn note(self, note: &str) -> Self {
+        DiagError(self.0.with_note(note.to_string()))
     }
 
     pub fn from_parse_error(
         file_id: usize,
         err: lalrpop_util::ParseError<usize, lexer::Token, parse_error::ParseError>,
-    ) -> AstError {
+    ) -> DiagError {
-        AstError::ParseError { file_id, source: err }
+        DiagError(lalrpop_error_to_diagnostic(&err, file_id))
-    }
-
-    pub fn wrong_expr_type_to<T: Into<AstError>>(self, f: impl FnOnce(Span, AstType) -> Option<T>) -> AstError {
-        match self {
-            AstError::WrongExprType(span, expected, got) => {
-                f(span.point_span(), got).map(|x| x.into()).unwrap_or_else(|| AstError::WrongExprType(span, expected, got))
-            }
-            AstError::ErrorNote(s, err) => AstError::ErrorNote(s, Box::new(err.wrong_expr_type_to(f))),
-            other => other,
-        }
-    }
-}
-
-impl Spanned for AstError {
-    fn span(&self) -> Span {
-        match self {
-            AstError::UnknownToplevel(span, _) => *span,
-            AstError::MissingNode(span) => *span,
-            AstError::WrongExprType(span, ..) => *span,
-            AstError::NotAValue(span, ..) => *span,
-            AstError::MismatchedElementName(span, ..) => *span,
-            AstError::DanglingKeyword(span, _) => *span,
-            AstError::AttrError(err) => err.span(),
-            AstError::Other(span, ..) => *span,
-            AstError::ConversionError(err) => err.value.span(),
-            AstError::IncludedFileNotFound(include) => include.path_span,
-            AstError::TooManyNodes(span, ..) => *span,
-            AstError::ErrorContext { label_span, .. } => *label_span,
-            AstError::ValidationError(error) => error.span(),
-            AstError::ParseError { file_id, source } => get_parse_error_span(*file_id, source),
-            AstError::ErrorNote(_, err) => err.span(),
-            AstError::NoMoreElementsExpected(span) => *span,
-            AstError::SimplExpr(err) => err.span(),
-            AstError::FormFormatError(err) => err.span(),
-        }
     }
 }
 
@@ -131,56 +52,13 @@ pub fn get_parse_error_span<T, E: Spanned>(file_id: usize, err: &lalrpop_util::P
     }
 }
 
-pub trait OptionAstErrorExt<T> {
+pub trait DiagResultExt<T> {
-    fn or_missing(self, span: Span) -> Result<T, AstError>;
+    fn note(self, note: &str) -> DiagResult<T>;
-}
-impl<T> OptionAstErrorExt<T> for Option<T> {
-    fn or_missing(self, span: Span) -> Result<T, AstError> {
-        self.ok_or(AstError::MissingNode(span))
-    }
 }
 
-pub trait AstResultExt<T> {
+impl<T> DiagResultExt<T> for DiagResult<T> {
-    fn context_label(self, label_span: Span, context: &str) -> AstResult<T>;
+    fn note(self, note: &str) -> DiagResult<T> {
-    fn note(self, note: &str) -> AstResult<T>;
-
-    /// Map any [AstError::WrongExprType]s error to any other Into<AstError> (such as a [FormFormatError])
-    /// If the provided closure returns `None`, the error will be kept unmodified
-    fn wrong_expr_type_to<E: Into<AstError>>(self, f: impl FnOnce(Span, AstType) -> Option<E>) -> AstResult<T>;
-}
-
-impl<T> AstResultExt<T> for AstResult<T> {
-    fn context_label(self, label_span: Span, context: &str) -> AstResult<T> {
-        self.map_err(|e| AstError::ErrorContext { label_span, context: context.to_string(), main_err: Box::new(e) })
-    }
-
-    fn note(self, note: &str) -> AstResult<T> {
         self.map_err(|e| e.note(note))
     }
-
-    fn wrong_expr_type_to<E: Into<AstError>>(self, f: impl FnOnce(Span, AstType) -> Option<E>) -> AstResult<T> {
-        self.map_err(|err| err.wrong_expr_type_to(f))
-    }
 }
 
-#[derive(Debug, Error)]
-pub enum FormFormatError {
-    #[error("Widget definition missing argument list")]
-    WidgetDefArglistMissing(Span),
-
-    #[error("Widget definition has more than one child widget")]
-    WidgetDefMultipleChildren(Span),
-
-    #[error("Expected 'in' in this position, but got '{}'", .1)]
-    ExpectedInInForLoop(Span, String),
-}
-
-impl Spanned for FormFormatError {
-    fn span(&self) -> Span {
-        match self {
-            FormFormatError::WidgetDefArglistMissing(span)
-            | FormFormatError::WidgetDefMultipleChildren(span)
-            | FormFormatError::ExpectedInInForLoop(span, _) => *span,
-        }
-    }
-}

crates/yuck/src/format_diagnostic.rs

@@ -1,5 +1,4 @@
 use codespan_reporting::{diagnostic, files};
-use config::TOP_LEVEL_DEFINITION_NAMES;
 use itertools::Itertools;
 use simplexpr::dynval;
 
@@ -7,19 +6,28 @@ use diagnostic::*;
 
 use crate::{
     config::{attributes::AttrError, config, validate::ValidationError},
-    error::{get_parse_error_span, AstError, FormFormatError},
+    error::{get_parse_error_span, DiagError},
 };
 
 use super::parser::parse_error;
 use eww_shared_util::{AttrName, Span, Spanned, VarName};
 
-fn span_to_primary_label(span: Span) -> Label<usize> {
+pub fn span_to_primary_label(span: Span) -> Label<usize> {
     Label::primary(span.2, span.0..span.1)
 }
-fn span_to_secondary_label(span: Span) -> Label<usize> {
+pub fn span_to_secondary_label(span: Span) -> Label<usize> {
     Label::secondary(span.2, span.0..span.1)
 }
 
+/// Generate a nicely formatted diagnostic
+/// ```rs
+/// gen_diagnostic! {
+///     kind = Severity::Error,
+///     msg = format!("Expected value, but got `{}`", actual),
+///     label = span => "Expected some value here",
+///     note = format!("Got: {}", actual),
+/// }
+/// ```
 #[macro_export]
 macro_rules! gen_diagnostic {
     ( $(kind = $kind:expr,)?
@@ -54,12 +62,17 @@ macro_rules! gen_diagnostic {
 
 pub trait DiagnosticExt: Sized {
     fn with_label(self, label: Label<usize>) -> Self;
+    fn with_note(self, note: String) -> Self;
 }
 
 impl DiagnosticExt for Diagnostic<usize> {
     fn with_label(self, label: Label<usize>) -> Self {
         self.with_labels(vec![label])
     }
+
+    fn with_note(self, note: String) -> Self {
+        self.with_notes(vec![note])
+    }
 }
 
 pub trait ToDiagnostic: std::fmt::Debug {
@@ -74,69 +87,11 @@ impl ToDiagnostic for Diagnostic<usize> {
         self.clone()
     }
 }
-impl ToDiagnostic for AstError {
-    fn to_diagnostic(&self) -> Diagnostic<usize> {
-        match self {
-            AstError::UnknownToplevel(span, name) => gen_diagnostic! {
-                msg = self,
-                label = span,
-                note = format!("Must be one of: {}", TOP_LEVEL_DEFINITION_NAMES.iter().join(", "))
-            },
-            AstError::MissingNode(span) => gen_diagnostic! {
-                msg = "Expected another element",
-                label = span => "Expected another element here",
-            },
-            AstError::WrongExprType(span, expected, actual) => gen_diagnostic! {
-                msg = "Wrong type of expression",
-                label = span => format!("Expected a `{}` here", expected),
-                note = format!("Expected: {}\n     Got: {}", expected, actual),
-            },
-            AstError::NotAValue(span, actual) => gen_diagnostic! {
-                msg = format!("Expected value, but got `{}`", actual),
-                label = span => "Expected some value here",
-                note = format!("Got: {}", actual),
-            },
-
-            AstError::ParseError { file_id, source } => lalrpop_error_to_diagnostic(source, *file_id),
-            AstError::MismatchedElementName(span, expected, got) => gen_diagnostic! {
-                msg = format!("Expected element `{}`, but found `{}`", expected, got),
-                label = span => format!("Expected `{}` here", expected),
-                note = format!("Expected: {}\n     Got: {}", expected, got),
-            },
-            AstError::ErrorContext { label_span, context, main_err } => {
-                main_err.to_diagnostic().with_label(span_to_secondary_label(*label_span).with_message(context))
-            }
-
-            AstError::ConversionError(source) => source.to_diagnostic(),
-            AstError::Other(span, source) => gen_diagnostic!(source, span),
-            AstError::AttrError(source) => source.to_diagnostic(),
-            AstError::IncludedFileNotFound(include) => gen_diagnostic!(
-                msg = format!("Included file `{}` not found", include.path),
-                label = include.path_span => "Included here",
-            ),
-
-            AstError::TooManyNodes(extra_nodes_span, expected) => gen_diagnostic! {
-                msg = self,
-                label = extra_nodes_span => "these elements must not be here",
-                note = "Consider wrapping the elements in some container element",
-            },
-            AstError::DanglingKeyword(span, keyword) => gen_diagnostic! {
-                msg = self,
-                label = span => "No value provided for this",
-            },
-            AstError::ErrorNote(note, source) => source.to_diagnostic().with_notes(vec![note.to_string()]),
-            AstError::ValidationError(source) => source.to_diagnostic(),
-            AstError::NoMoreElementsExpected(span) => gen_diagnostic!(self, span),
-            AstError::SimplExpr(source) => source.to_diagnostic(),
-            AstError::FormFormatError(error) => error.to_diagnostic(),
-        }
-    }
-}
 
 impl ToDiagnostic for parse_error::ParseError {
     fn to_diagnostic(&self) -> Diagnostic<usize> {
         match self {
-            parse_error::ParseError::SimplExpr(error) => error.to_diagnostic(),
+            parse_error::ParseError::SimplExpr(source) => lalrpop_error_to_diagnostic(&source.source, source.file_id),
             parse_error::ParseError::LexicalError(span) => generate_lexical_error_diagnostic(*span),
         }
     }
@@ -157,10 +112,6 @@ impl ToDiagnostic for AttrError {
 impl ToDiagnostic for ValidationError {
     fn to_diagnostic(&self) -> Diagnostic<usize> {
         match self {
-            ValidationError::UnknownWidget(span, name) => gen_diagnostic! {
-                msg = self,
-                label = span => "Used here",
-            },
             ValidationError::MissingAttr { widget_name, arg_name, arg_list_span, use_span } => {
                 let mut diag = Diagnostic::error()
                     .with_message(self.to_string())
@@ -199,10 +150,6 @@ impl ToDiagnostic for ValidationError {
                 label = span => "Defined here",
                 note = "Hint: Give your widget a different name. You could call it \"John\" for example. That's a cool name."
             },
-            ValidationError::VariableDefinedTwice { span, name } => gen_diagnostic! {
-                msg = self,
-                label = span => "Defined again here"
-            },
         }
     }
 }
@@ -212,7 +159,7 @@ fn variable_deprecation_note(var_name: String) -> Option<String> {
         .then(|| "Note: EWW_CPU_USAGE has recently been removed, and has now been renamed to EWW_CPU".to_string())
 }
 
-fn lalrpop_error_to_diagnostic<T: std::fmt::Display, E: Spanned + ToDiagnostic>(
+pub fn lalrpop_error_to_diagnostic<T: std::fmt::Display, E: Spanned + ToDiagnostic>(
     error: &lalrpop_util::ParseError<usize, T, E>,
     file_id: usize,
 ) -> Diagnostic<usize> {
@@ -232,19 +179,6 @@ fn lalrpop_error_to_diagnostic<T: std::fmt::Display, E: Spanned + ToDiagnostic>(
     }
 }
 
-impl ToDiagnostic for simplexpr::error::Error {
-    fn to_diagnostic(&self) -> Diagnostic<usize> {
-        use simplexpr::error::Error::*;
-        match self {
-            ParseError { source, file_id } => lalrpop_error_to_diagnostic(source, *file_id),
-            ConversionError(error) => error.to_diagnostic(),
-            Eval(error) => error.to_diagnostic(),
-            Other(error) => gen_diagnostic!(error),
-            Spanned(span, error) => error.to_diagnostic().with_label(span_to_primary_label(*span)),
-        }
-    }
-}
-
 impl ToDiagnostic for simplexpr::parser::lexer::LexicalError {
     fn to_diagnostic(&self) -> Diagnostic<usize> {
         generate_lexical_error_diagnostic(self.span())
@@ -290,27 +224,3 @@ fn generate_lexical_error_diagnostic(span: Span) -> Diagnostic<usize> {
         label = span => "Invalid token"
     }
 }
-
-impl ToDiagnostic for FormFormatError {
-    fn to_diagnostic(&self) -> diagnostic::Diagnostic<usize> {
-        match self {
-            FormFormatError::WidgetDefArglistMissing(span) => gen_diagnostic! {
-                msg = self,
-                label = span => "Insert the argument list (e.g.: `[]`) here",
-                note = "This list will in the future need to declare all the non-global variables / attributes used in this widget.\n\
-                        This is not yet neccessary, but is still considered good style.",
-            },
-            FormFormatError::WidgetDefMultipleChildren(span) => gen_diagnostic! {
-                msg = self,
-                label = span => "Found more than one child element here.",
-                note = "A widget-definition may only contain one child element.\n\
-                        To include multiple elements, wrap these elements in a single container widget such as `box`.\n\
-                        This is necessary as eww can't know how you want these elements to be layed out otherwise."
-            },
-            FormFormatError::ExpectedInInForLoop(span, got) => gen_diagnostic! {
-                msg = self,
-                label = span,
-            },
-        }
-    }
-}

crates/yuck/src/lib.rs

@@ -7,3 +7,4 @@ pub mod error;
 pub mod format_diagnostic;
 pub mod parser;
 pub mod value;
+pub mod ast_error;

crates/yuck/src/parser/ast.rs

@@ -2,13 +2,14 @@ use itertools::Itertools;
 use simplexpr::{ast::SimplExpr, dynval::DynVal};
 use std::collections::HashMap;
 
-use eww_shared_util::{Span, VarName};
+use eww_shared_util::{Span, Spanned, VarName};
 use std::fmt::Display;
 
 use super::{ast_iterator::AstIterator, from_ast::FromAst};
 use crate::{
+    ast_error::AstError,
     config::attributes::{AttrEntry, Attributes},
-    error::{AstError, AstResult, OptionAstErrorExt},
+    error::{DiagError, DiagResult},
 };
 
 #[derive(Debug, PartialEq, Eq, Copy, Clone)]
@@ -36,11 +37,17 @@ impl Display for AstType {
 
 #[derive(PartialEq, Eq, Clone, serde::Serialize)]
 pub enum Ast {
+    /// I.e.: `(foo bar baz)`
     List(Span, Vec<Ast>),
+    /// I.e.: `[foo bar baz]`
     Array(Span, Vec<Ast>),
+    /// I.e.: `:foo`
     Keyword(Span, String),
+    /// I.e.: `foo`
     Symbol(Span, String),
+    /// I.e.: `{1 + 2}`
     SimplExpr(Span, SimplExpr),
+    /// I.e.: `// foo`
     Comment(Span),
 }
 
@@ -80,18 +87,7 @@ impl Ast {
         }
     }
 
-    pub fn span(&self) -> Span {
+    pub fn as_simplexpr(&self) -> Result<SimplExpr, AstError> {
-        match self {
-            Ast::List(span, _) => *span,
-            Ast::Array(span, _) => *span,
-            Ast::Keyword(span, _) => *span,
-            Ast::Symbol(span, _) => *span,
-            Ast::SimplExpr(span, _) => *span,
-            Ast::Comment(span) => *span,
-        }
-    }
-
-    pub fn as_simplexpr(&self) -> AstResult<SimplExpr> {
         match self {
             // TODO do I do this?
             // Ast::Array(span, elements) => todo!()
@@ -101,7 +97,7 @@ impl Ast {
         }
     }
 
-    pub fn try_ast_iter(self) -> AstResult<AstIterator<impl Iterator<Item = Ast>>> {
+    pub fn try_ast_iter(self) -> Result<AstIterator<impl Iterator<Item = Ast>>, AstError> {
         let span = self.span();
         let list = self.as_list()?;
         Ok(AstIterator::new(span, list.into_iter()))
@@ -127,3 +123,16 @@ impl std::fmt::Debug for Ast {
         write!(f, "{}", self)
     }
 }
+
+impl Spanned for Ast {
+    fn span(&self) -> Span {
+        match self {
+            Ast::List(span, _) => *span,
+            Ast::Array(span, _) => *span,
+            Ast::Keyword(span, _) => *span,
+            Ast::Symbol(span, _) => *span,
+            Ast::SimplExpr(span, _) => *span,
+            Ast::Comment(span) => *span,
+        }
+    }
+}

crates/yuck/src/parser/ast_iterator.rs

@@ -9,11 +9,15 @@ use super::{
     from_ast::FromAst,
 };
 use crate::{
+    ast_error::AstError,
     config::attributes::{AttrEntry, Attributes},
-    error::{AstError, AstResult, OptionAstErrorExt},
+    error::{DiagError, DiagResult},
+    format_diagnostic::ToDiagnostic,
+    gen_diagnostic,
 };
-use eww_shared_util::{AttrName, Span, VarName};
+use eww_shared_util::{AttrName, Span, Spanned, VarName};
 
+/// Iterator over [`crate::parser::ast::Ast`] nodes which allows to explicitly expect specific types of items
 pub struct AstIterator<I: Iterator<Item = Ast>> {
     remaining_span: Span,
     iter: itertools::PutBack<I>,
@@ -22,8 +26,9 @@ pub struct AstIterator<I: Iterator<Item = Ast>> {
 macro_rules! return_or_put_back {
     ($(fn $name:ident -> $expr_type:expr, $t:ty = $p:pat => $ret:expr)*) => {
         $(
-            pub fn $name(&mut self) -> AstResult<$t> {
+            pub fn $name(&mut self) -> Result<$t, AstError> {
                 let expr_type = $expr_type;
+                use eww_shared_util::Spanned;
                 match self.expect_any()? {
                     $p => Ok($ret),
                     other => {
@@ -45,11 +50,11 @@ impl<I: Iterator<Item = Ast>> AstIterator<I> {
         fn expect_array     -> AstType::Array,     (Span, Vec<Ast>)  = Ast::Array(span, x)     => (span, x)
     }
 
-    pub fn expect_literal(&mut self) -> AstResult<(Span, DynVal)> {
+    pub fn expect_literal(&mut self) -> Result<(Span, DynVal), AstError> {
         // TODO add some others
         match self.expect_any()? {
             // Ast::Array(_, _) => todo!(),
-            Ast::SimplExpr(span, expr) => Ok((span, expr.eval_no_vars().map_err(|e| AstError::SimplExpr(e.into()))?)),
+            Ast::SimplExpr(span, expr) => Ok((span, expr.eval_no_vars()?)),
             other => {
                 let span = other.span();
                 let actual_type = other.expr_type();
@@ -63,11 +68,11 @@ impl<I: Iterator<Item = Ast>> AstIterator<I> {
         AstIterator { remaining_span: span, iter: itertools::put_back(iter) }
     }
 
-    pub fn expect_any(&mut self) -> AstResult<Ast> {
+    pub fn expect_any(&mut self) -> Result<Ast, AstError> {
-        self.next().or_missing(self.remaining_span.point_span())
+        self.next().ok_or_else(|| AstError::TooFewElements(self.remaining_span.point_span()))
     }
 
-    pub fn expect_simplexpr(&mut self) -> AstResult<(Span, SimplExpr)> {
+    pub fn expect_simplexpr(&mut self) -> Result<(Span, SimplExpr), AstError> {
         let expr_type = AstType::SimplExpr;
         match self.expect_any()? {
             Ast::SimplExpr(span, expr) => Ok((span, expr)),
@@ -81,7 +86,7 @@ impl<I: Iterator<Item = Ast>> AstIterator<I> {
         }
     }
 
-    pub fn expect_done(&mut self) -> AstResult<()> {
+    pub fn expect_done(&mut self) -> Result<(), AstError> {
         if let Some(next) = self.next() {
             self.put_back(next);
             Err(AstError::NoMoreElementsExpected(self.remaining_span))
@@ -90,7 +95,7 @@ impl<I: Iterator<Item = Ast>> AstIterator<I> {
         }
     }
 
-    pub fn expect_key_values(&mut self) -> AstResult<Attributes> {
+    pub fn expect_key_values(&mut self) -> Result<Attributes, AstError> {
         parse_key_values(self, true)
     }
 
@@ -112,7 +117,10 @@ impl<I: Iterator<Item = Ast>> Iterator for AstIterator<I> {
 }
 
 /// Parse consecutive `:keyword value` pairs from an expression iterator into an [Attributes].
-fn parse_key_values(iter: &mut AstIterator<impl Iterator<Item = Ast>>, fail_on_dangling_kw: bool) -> AstResult<Attributes> {
+fn parse_key_values(
+    iter: &mut AstIterator<impl Iterator<Item = Ast>>,
+    fail_on_dangling_kw: bool,
+) -> Result<Attributes, AstError> {
     let mut data = HashMap::new();
     let mut attrs_span = iter.remaining_span.point_span();
     loop {
@@ -125,7 +133,7 @@ fn parse_key_values(iter: &mut AstIterator<impl Iterator<Item = Ast>>, fail_on_d
                 }
                 None => {
                     if fail_on_dangling_kw {
-                        return Err(AstError::DanglingKeyword(key_span, kw));
+                        return Err(AstError::DanglingKeyword(key_span, kw.into()));
                     } else {
                         iter.iter.put_back(Ast::Keyword(key_span, kw));
                         attrs_span.1 = iter.remaining_span.0;

crates/yuck/src/parser/from_ast.rs

@@ -2,8 +2,8 @@ use super::{
     ast::{Ast, AstType},
     ast_iterator::AstIterator,
 };
-use crate::{error::*, parser};
+use crate::{error::*, format_diagnostic::ToDiagnostic, gen_diagnostic, parser};
-use eww_shared_util::{AttrName, Span, VarName};
+use eww_shared_util::{AttrName, Span, Spanned, VarName};
 use itertools::Itertools;
 use simplexpr::{ast::SimplExpr, dynval::DynVal};
 use std::{
@@ -13,18 +13,18 @@ use std::{
 };
 
 pub trait FromAst: Sized {
-    fn from_ast(e: Ast) -> AstResult<Self>;
+    fn from_ast(e: Ast) -> DiagResult<Self>;
 }
 
 impl FromAst for Ast {
-    fn from_ast(e: Ast) -> AstResult<Self> {
+    fn from_ast(e: Ast) -> DiagResult<Self> {
         Ok(e)
     }
 }
 
 impl FromAst for String {
-    fn from_ast(e: Ast) -> AstResult<Self> {
+    fn from_ast(e: Ast) -> DiagResult<Self> {
-        Ok(e.as_simplexpr()?.eval_no_vars().map_err(simplexpr::error::Error::Eval)?.to_string())
+        Ok(e.as_simplexpr()?.eval_no_vars().map_err(|e| DiagError(e.to_diagnostic()))?.to_string())
     }
 }
 
@@ -32,27 +32,35 @@ impl FromAst for String {
 /// I.e. to parse (foo [a b] (c d)), [`FromAstElementContent::from_tail`] would just get [a b] (c d).
 pub trait FromAstElementContent: Sized {
     const ELEMENT_NAME: &'static str;
-    fn from_tail<I: Iterator<Item = Ast>>(span: Span, iter: AstIterator<I>) -> AstResult<Self>;
+    fn from_tail<I: Iterator<Item = Ast>>(span: Span, iter: AstIterator<I>) -> DiagResult<Self>;
 }
 
 impl<T: FromAstElementContent> FromAst for T {
-    fn from_ast(e: Ast) -> AstResult<Self> {
+    fn from_ast(e: Ast) -> DiagResult<Self> {
         let span = e.span();
         let mut iter = e.try_ast_iter()?;
         let (element_name_span, element_name) = iter.expect_symbol()?;
         if Self::ELEMENT_NAME != element_name {
-            return Err(AstError::MismatchedElementName(element_name_span, Self::ELEMENT_NAME.to_string(), element_name));
+            return Err(DiagError(gen_diagnostic! {
+                msg = format!("Expected element `{}`, but found `{element_name}`", Self::ELEMENT_NAME),
+                label = element_name_span => format!("Expected `{}` here", Self::ELEMENT_NAME),
+                note = format!("Expected: {}\n     Got: {element_name}", Self::ELEMENT_NAME),
+            }));
         }
         Self::from_tail(span, iter)
     }
 }
 
 impl FromAst for SimplExpr {
-    fn from_ast(e: Ast) -> AstResult<Self> {
+    fn from_ast(e: Ast) -> DiagResult<Self> {
         match e {
             Ast::Symbol(span, x) => Ok(SimplExpr::var_ref(span, x)),
             Ast::SimplExpr(span, x) => Ok(x),
-            _ => Err(AstError::NotAValue(e.span(), e.expr_type())),
+            _ => Err(DiagError(gen_diagnostic! {
+                msg = format!("Expected value, but got `{}`", e.expr_type()),
+                label = e.span() => "Expected some value here",
+                note = format!("Got: {}", e.expr_type()),
+            })),
         }
     }
 }

crates/yuck/src/parser/mod.rs

@@ -1,7 +1,9 @@
-use eww_shared_util::Span;
+use eww_shared_util::{Span, Spanned};
 use lalrpop_util::lalrpop_mod;
 
-use super::error::{AstError, AstResult};
+use crate::gen_diagnostic;
+
+use super::error::{DiagError, DiagResult};
 use ast::Ast;
 
 use std::{fmt::Display, ops::Deref};
@@ -20,25 +22,32 @@ lalrpop_mod!(
     "/parser/parser.rs"
 );
 
-pub fn parse_string(file_id: usize, s: &str) -> AstResult<Ast> {
+pub fn parse_string(file_id: usize, s: &str) -> DiagResult<Ast> {
     let lexer = lexer::Lexer::new(file_id, s.to_string());
     let parser = parser::AstParser::new();
-    parser.parse(file_id, lexer).map_err(|e| AstError::from_parse_error(file_id, e))
+    parser.parse(file_id, lexer).map_err(|e| DiagError::from_parse_error(file_id, e))
 }
 
 /// Parse multiple toplevel nodes into a list of [Ast]
-pub fn parse_toplevel(file_id: usize, s: String) -> AstResult<(Span, Vec<Ast>)> {
+pub fn parse_toplevel(file_id: usize, s: String) -> DiagResult<(Span, Vec<Ast>)> {
     let lexer = lexer::Lexer::new(file_id, s);
     let parser = parser::ToplevelParser::new();
-    parser.parse(file_id, lexer).map_err(|e| AstError::from_parse_error(file_id, e))
+    parser.parse(file_id, lexer).map_err(|e| DiagError::from_parse_error(file_id, e))
 }
 
 /// get a single ast node from a list of asts, returning an Err if the length is not exactly 1.
-pub fn require_single_toplevel(span: Span, mut asts: Vec<Ast>) -> AstResult<Ast> {
+pub fn require_single_toplevel(span: Span, mut asts: Vec<Ast>) -> DiagResult<Ast> {
     match asts.len() {
-        0 => Err(AstError::MissingNode(span)),
         1 => Ok(asts.remove(0)),
-        _ => Err(AstError::TooManyNodes(asts.get(1).unwrap().span().to(asts.last().unwrap().span()), 1)),
+        0 => Err(DiagError(gen_diagnostic! {
+            msg = "Expected exactly one element, but got none",
+            label = span
+        })),
+        n => Err(DiagError(gen_diagnostic! {
+            msg = "Expected exactly one element, but but got {n}",
+            label = asts.get(1).unwrap().span().to(asts.last().unwrap().span()) => "these elements must not be here",
+            note = "Consider wrapping the elements in some container element",
+        })),
     }
 }
 

crates/yuck/src/parser/parse_error.rs

@@ -3,7 +3,7 @@ use eww_shared_util::{AttrName, Span, Spanned, VarName};
 #[derive(Debug, thiserror::Error)]
 pub enum ParseError {
     #[error("{0}")]
-    SimplExpr(simplexpr::error::Error),
+    SimplExpr(simplexpr::error::ParseError),
     #[error("Unknown token")]
     LexicalError(Span),
 }

crates/yuck/src/parser/parser.lalrpop

@@ -53,7 +53,7 @@ SimplExpr: SimplExpr = {
         let parser = simplexpr::simplexpr_parser::ExprParser::new();
         parser.parse(file_id, x.into_iter().map(Ok))
             .map_err(|e| ParseError::User {
-                error: parse_error::ParseError::SimplExpr(simplexpr::error::Error::from_parse_error(file_id, e))
+                error: parse_error::ParseError::SimplExpr(simplexpr::error::ParseError::from_parse_error(file_id, e))
             })
    }
 }