Improve error messages for literal

crates/eww/src/app.rs

@@ -193,7 +193,7 @@ impl App {
         };
 
         if let Err(err) = result {
-            error_handling_ctx::print_error(&err);
+            error_handling_ctx::print_error(err);
         }
     }
 

crates/eww/src/config/eww_config.rs

@@ -16,8 +16,8 @@ pub struct EwwConfig {
     windows: HashMap<String, EwwWindowDefinition>,
     initial_variables: HashMap<VarName, DynVal>,
     script_vars: HashMap<VarName, ScriptVarDefinition>,
-    // files: FsYuckFiles,
 }
+
 impl EwwConfig {
     pub fn read_from_file(files: &mut YuckFiles, path: impl AsRef<Path>) -> Result<Self> {
         let config = Config::generate_from_main_file(files, path)?;
@@ -30,7 +30,6 @@ impl EwwConfig {
             widgets: widget_definitions,
             initial_variables: var_definitions.into_iter().map(|(k, v)| (k, v.initial_value)).collect(),
             script_vars,
-            // files,
         })
     }
 

crates/eww/src/error_handling_ctx.rs

@@ -1,12 +1,13 @@
 use std::sync::{Arc, Mutex};
 
-use codespan_reporting::diagnostic::Diagnostic;
+use codespan_reporting::{diagnostic::Diagnostic, term, term::Chars};
 use eww_shared_util::DUMMY_SPAN;
 use simplexpr::eval::EvalError;
 use yuck::{
     config::file_provider::YuckFiles,
     error::AstError,
     format_diagnostic::{eval_error_to_diagnostic, ToDiagnostic},
+    gen_diagnostic,
 };
 
 use crate::error::DiagError;
@@ -19,34 +20,44 @@ pub fn clear_files() {
     *ERROR_HANDLING_CTX.lock().unwrap() = YuckFiles::new();
 }
 
+pub fn anyhow_err_to_diagnostic(err: &anyhow::Error) -> Diagnostic<usize> {
+    if let Some(err) = err.downcast_ref::<DiagError>() {
+        err.diag.clone()
+    } else if let Some(err) = err.downcast_ref::<AstError>() {
+        err.to_diagnostic()
+    } else if let Some(err) = err.downcast_ref::<EvalError>() {
+        eval_error_to_diagnostic(err, err.span().unwrap_or(DUMMY_SPAN))
+    } else {
+        gen_diagnostic!(err)
+    }
+}
 
-pub fn print_error(err: &anyhow::Error) {
-    let result: anyhow::Result<_> = try {
-        if let Some(err) = err.downcast_ref::<DiagError>() {
-            eprintln!("{:?}\n{}", err, stringify_diagnostic(&err.diag)?);
-        } else if let Some(err) = err.downcast_ref::<AstError>() {
-            eprintln!("{:?}\n{}", err, stringify_diagnostic(&err.to_diagnostic())?);
-        } else if let Some(err) = err.downcast_ref::<EvalError>() {
-            eprintln!("{:?}\n{}", err, stringify_diagnostic(&eval_error_to_diagnostic(err, err.span().unwrap_or(DUMMY_SPAN)))?);
-        } else {
+pub fn print_error(err: anyhow::Error) {
+    match stringify_diagnostic(&anyhow_err_to_diagnostic(&err)) {
+        Ok(diag) => {
+            eprintln!("{:?}\n{}", err, diag);
+        }
+        Err(_) => {
             log::error!("{:?}", err);
         }
-    };
-    if result.is_err() {
-        log::error!("{:?}", err);
     }
 }
 
 pub fn format_error(err: &anyhow::Error) -> String {
+    for err in err.chain() {
+        format!("chain: {}", err);
+    }
     match err.downcast_ref::<AstError>() {
         Some(err) => stringify_diagnostic(&err.to_diagnostic()).unwrap_or_else(|_| format!("{:?}", err)),
         None => format!("{:?}", err),
     }
 }
 
-pub fn stringify_diagnostic(diagnostic: &Diagnostic<usize>) -> anyhow::Result<String> {
-    use codespan_reporting::term;
-    let config = term::Config::default();
+pub fn stringify_diagnostic(diagnostic: &codespan_reporting::diagnostic::Diagnostic<usize>) -> anyhow::Result<String> {
+    let mut config = term::Config::default();
+    let mut chars = Chars::box_drawing();
+    chars.single_primary_caret = '─';
+    config.chars = chars;
     let mut buf = Vec::new();
     let mut writer = term::termcolor::Ansi::new(&mut buf);
     let files = ERROR_HANDLING_CTX.lock().unwrap();

crates/eww/src/eww_state.rs

@@ -30,12 +30,12 @@ impl StateChangeHandler {
 
         match resolved_attrs {
             Ok(resolved_attrs) => {
-                if let Err(err) = &(self.func)(resolved_attrs).context("Error while updating UI after state change") {
-                    error_handling_ctx::print_error(&err);
+                if let Err(err) = (self.func)(resolved_attrs).context("Error while updating UI after state change") {
+                    error_handling_ctx::print_error(err);
                 }
             }
             Err(err) => {
-                error_handling_ctx::print_error(&err);
+                error_handling_ctx::print_error(err);
             }
         }
     }

crates/eww/src/main.rs

@@ -94,7 +94,7 @@ fn main() {
     };
 
     if let Err(e) = result {
-        error_handling_ctx::print_error(&e);
+        error_handling_ctx::print_error(e);
         std::process::exit(1);
     }
 }

crates/eww/src/script_var_handler.rs

@@ -135,7 +135,9 @@ impl PollVarHandler {
             let result: Result<_> = try {
                 evt_send.send(app::DaemonCommand::UpdateVars(vec![(var.name.clone(), run_poll_once(&var)?)]))?;
             };
-            crate::print_result_err!("while running script-var command", &result);
+            if let Err(err) = result {
+                crate::error_handling_ctx::print_error(err);
+            }
 
             crate::loop_select_exiting! {
                 _ = cancellation_token.cancelled() => break,
@@ -143,7 +145,10 @@ impl PollVarHandler {
                     let result: Result<_> = try {
                         evt_send.send(app::DaemonCommand::UpdateVars(vec![(var.name.clone(), run_poll_once(&var)?)]))?;
                     };
-                    crate::print_result_err!("while running script-var command", &result);
+
+                    if let Err(err) = result {
+                        crate::error_handling_ctx::print_error(err);
+                    }
                 }
             }
         });
@@ -163,9 +168,9 @@ impl PollVarHandler {
 
 fn run_poll_once(var: &PollScriptVar) -> Result<DynVal> {
     match &var.command {
-        VarSource::Shell(span, x) => crate::config::script_var::run_command(x).map_err(|_| {
-            anyhow!(create_script_var_failed_error(*span, &var.name))
-        }),
+        VarSource::Shell(span, x) => {
+            crate::config::script_var::run_command(x).map_err(|_| anyhow!(create_script_var_failed_error(*span, &var.name)))
+        }
         VarSource::Function(x) => x().map_err(|e| anyhow!(e)),
     }
 }

crates/eww/src/widgets/widget_definitions.rs

@@ -6,7 +6,11 @@ use gdk::WindowExt;
 use glib;
 use gtk::{self, prelude::*, ImageExt};
 use std::{cell::RefCell, collections::HashMap, rc::Rc, time::Duration};
-use yuck::{config::validate::ValidationError, error::AstError, parser::from_ast::FromAst};
+use yuck::{
+    config::validate::ValidationError,
+    error::{AstError, AstResult, ResultExt},
+    parser::from_ast::FromAst,
+};
 
 // TODO figure out how to
 // TODO https://developer.gnome.org/gtk3/stable/GtkFixed.html
@@ -510,6 +514,7 @@ fn build_gtk_literal(bargs: &mut BuilderArgs) -> Result<gtk::Box> {
     // TODO these clones here are dumdum
     let window_name = bargs.window_name.to_string();
     let widget_definitions = bargs.widget_definitions.clone();
+    let literal_use_span = bargs.widget.span;
 
     // the file id the literal-content has been stored under, for error reporting.
     let literal_file_id: Rc<RefCell<Option<usize>>> = Rc::new(RefCell::new(None));
@@ -519,19 +524,27 @@ fn build_gtk_literal(bargs: &mut BuilderArgs) -> Result<gtk::Box> {
         prop(content: as_string) {
             gtk_widget.get_children().iter().for_each(|w| gtk_widget.remove(w));
             if !content.is_empty() {
-                let ast = {
-                    let mut yuck_files = error_handling_ctx::ERROR_HANDLING_CTX.lock().unwrap();
-                    let (span, asts) = yuck_files.load_str("<literal-content>".to_string(), content)?;
-                    if let Some(file_id) = literal_file_id.replace(Some(span.2)) {
-                        yuck_files.unload(file_id);
-                    }
-                    yuck::parser::require_single_toplevel(span, asts)?
+                let widget_node_result: AstResult<_> = try {
+                    let ast = {
+                        let mut yuck_files = error_handling_ctx::ERROR_HANDLING_CTX.lock().unwrap();
+                        let (span, asts) = yuck_files.load_str("<literal-content>".to_string(), content)?;
+                        if let Some(file_id) = literal_file_id.replace(Some(span.2)) {
+                            yuck_files.unload(file_id);
+                        }
+                        yuck::parser::require_single_toplevel(span, asts)?
+                    };
+
+                    let content_widget_use = yuck::config::widget_use::WidgetUse::from_ast(ast)?;
+                    widget_node::generate_generic_widget_node(&widget_definitions, &HashMap::new(), content_widget_use)?
                 };
 
-                let content_widget_use = yuck::config::widget_use::WidgetUse::from_ast(ast)?;
-
-                let widget_node = &*widget_node::generate_generic_widget_node(&widget_definitions, &HashMap::new(), content_widget_use)?;
-                let child_widget = widget_node.render(&mut eww_state::EwwState::default(), &window_name, &widget_definitions)?;
+                let widget_node = widget_node_result.context_label(literal_use_span, "Error in the literal used here")?;
+                let child_widget = widget_node.render(&mut eww_state::EwwState::default(), &window_name, &widget_definitions)
+                    .map_err(|e| AstError::ErrorContext {
+                        label_span: literal_use_span,
+                        context: "Error in the literal used here".to_string(),
+                        main_err: Box::new(error_handling_ctx::anyhow_err_to_diagnostic(&e))
+                    })?;
                 gtk_widget.add(&child_widget);
                 child_widget.show();
             }

crates/eww/src/widgets/widget_node.rs

@@ -6,7 +6,7 @@ use simplexpr::SimplExpr;
 use std::collections::HashMap;
 use yuck::{
     config::{validate::ValidationError, widget_definition::WidgetDefinition, widget_use::WidgetUse},
-    error::AstError,
+    error::{AstError, AstResult},
 };
 
 pub trait WidgetNode: std::fmt::Debug + dyn_clone::DynClone + Send + Sync {
@@ -108,16 +108,18 @@ pub fn generate_generic_widget_node(
     defs: &HashMap<String, WidgetDefinition>,
     local_env: &HashMap<VarName, SimplExpr>,
     w: WidgetUse,
-) -> Result<Box<dyn WidgetNode>> {
+) -> AstResult<Box<dyn WidgetNode>> {
     if let Some(def) = defs.get(&w.name) {
-        ensure!(w.children.is_empty(), "User-defined widgets cannot be given children.");
+        if !w.children.is_empty() {
+            Err(AstError::TooManyNodes(w.children_span(), 0).note("User-defined widgets cannot be given children."))?
+        }
 
         let new_local_env = w
             .attrs
             .attrs
             .into_iter()
             .map(|(name, value)| Ok((VarName(name.0), value.value.as_simplexpr()?.resolve_one_level(local_env))))
-            .collect::<Result<HashMap<VarName, _>>>()?;
+            .collect::<AstResult<HashMap<VarName, _>>>()?;
 
         let content = generate_generic_widget_node(defs, &new_local_env, def.widget.clone())?;
         Ok(Box::new(UserDefined { name: w.name, span: Some(w.span), content }))
@@ -131,13 +133,13 @@ pub fn generate_generic_widget_node(
                 .attrs
                 .into_iter()
                 .map(|(name, value)| Ok((name, value.value.as_simplexpr()?.resolve_one_level(local_env))))
-                .collect::<Result<HashMap<_, _>>>()?,
+                .collect::<AstResult<HashMap<_, _>>>()?,
 
             children: w
                 .children
                 .into_iter()
                 .map(|child| generate_generic_widget_node(defs, local_env, child))
-                .collect::<Result<Vec<_>>>()?,
+                .collect::<AstResult<Vec<_>>>()?,
         }))
     }
 }

crates/eww_shared_util/src/span.rs

@@ -21,6 +21,17 @@ impl Span {
         self.1 = self.0;
         self
     }
+
+    /// Turn this span into a span only highlighting the point it ends at, setting the length to 0.
+    pub fn point_span_at_end(mut self) -> Self {
+        self.0 = self.1;
+        self
+    }
+    pub fn shifted(mut self, n: isize) -> Self {
+        self.0 = isize::max(0, self.0 as isize + n) as usize;
+        self.1 = isize::max(0, self.0 as isize + n) as usize;
+        self
+    }
 }
 
 impl std::fmt::Display for Span {

crates/yuck/src/config/file_provider.rs

@@ -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>), FilesError> {
+    pub fn load_str(&mut self, name: String, content: String) -> Result<(Span, Vec<Ast>), AstError> {
         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/validate.rs

@@ -12,7 +12,7 @@ use eww_shared_util::{AttrName, Span, VarName};
 
 #[derive(Debug, thiserror::Error)]
 pub enum ValidationError {
-    #[error("Unknown widget referenced: {1}")]
+    #[error("Unknown widget `{1}` referenced")]
     UnknownWidget(Span, String),
 
     #[error("Missing attribute `{arg_name}` in use of widget `{widget_name}`")]

crates/yuck/src/config/widget_use.rs

@@ -20,6 +20,16 @@ pub struct WidgetUse {
     pub name_span: Span,
 }
 
+impl WidgetUse {
+    pub fn children_span(&self) -> Span {
+        if self.children.is_empty() {
+            self.span.point_span_at_end().shifted(-1)
+        } else {
+            self.children.first().unwrap().span.to(self.children.last().unwrap().span)
+        }
+    }
+}
+
 impl FromAst for WidgetUse {
     fn from_ast(e: Ast) -> AstResult<Self> {
         let span = e.span();

crates/yuck/src/error.rs

@@ -1,5 +1,6 @@
 use crate::{
     config::{attributes::AttrError, config::Include, validate::ValidationError},
+    format_diagnostic::ToDiagnostic,
     parser::{
         ast::{Ast, AstType},
         lexer, parse_error,
@@ -31,6 +32,11 @@ pub enum AstError {
     #[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)]
     ConversionError(#[from] dynval::ConversionError),
 
@@ -47,11 +53,19 @@ pub enum AstError {
     ParseError { file_id: Option<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);
+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 {
+    pub fn note(self, note: &str) -> Self {
+        AstError::ErrorNote(note.to_string(), Box::new(self))
+    }
+
+    pub fn context_label(self, label_span: Span, context: &str) -> Self {
+        AstError::ErrorContext { label_span, context: context.to_string(), main_err: Box::new(self) }
+    }
+
     pub fn get_span(&self) -> Option<Span> {
         match self {
             AstError::UnknownToplevel(span, _) => Some(*span),
@@ -64,8 +78,10 @@ impl AstError {
             AstError::ConversionError(err) => err.value.span().map(|x| x.into()),
             AstError::IncludedFileNotFound(include) => Some(include.path_span),
             AstError::TooManyNodes(span, ..) => Some(*span),
+            AstError::ErrorContext { label_span, .. } => Some(*label_span),
             AstError::ValidationError(error) => None, // TODO none here is stupid
             AstError::ParseError { file_id, source } => file_id.and_then(|id| get_parse_error_span(id, source)),
+            AstError::ErrorNote(_, err) => err.get_span(),
         }
     }
 
@@ -101,3 +117,13 @@ impl<T> OptionAstErrorExt<T> for Option<T> {
         self.ok_or(AstError::MissingNode(span))
     }
 }
+
+pub trait ResultExt<T> {
+    fn context_label(self, label_span: Span, context: &str) -> AstResult<T>;
+}
+
+impl<T> ResultExt<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) })
+    }
+}

crates/yuck/src/format_diagnostic.rs

@@ -3,7 +3,10 @@ use simplexpr::dynval;
 
 use diagnostic::*;
 
-use crate::error::AstError;
+use crate::{
+    config::{attributes::AttrError, validate::ValidationError},
+    error::AstError,
+};
 
 use super::parser::parse_error;
 use eww_shared_util::{AttrName, Span, VarName};
@@ -25,7 +28,7 @@ macro_rules! gen_diagnostic {
                 ::codespan_reporting::diagnostic::Label::primary($span.2, $span.0..$span.1)
                     $(.with_message($label))?
             ]))?
-            $(.with_notes(vec![$note]))?
+            $(.with_notes(vec![$note.to_string()]))?
     };
     ($msg:expr $(, $span:expr $(,)?)?) => {{
         ::codespan_reporting::diagnostic::Diagnostic::error()
@@ -48,26 +51,23 @@ impl DiagnosticExt for Diagnostic<usize> {
     }
 }
 
-pub trait ToDiagnostic {
+pub trait ToDiagnostic: std::fmt::Debug {
     fn to_diagnostic(&self) -> Diagnostic<usize>;
+    fn to_message(&self) -> String {
+        self.to_diagnostic().message
+    }
 }
 
+impl ToDiagnostic for Diagnostic<usize> {
+    fn to_diagnostic(&self) -> Diagnostic<usize> {
+        self.clone()
+    }
+}
 impl ToDiagnostic for AstError {
     fn to_diagnostic(&self) -> Diagnostic<usize> {
+        // TODO this if let should be unnecessary
         if let AstError::ValidationError(error) = self {
-            match error {
-                crate::config::validate::ValidationError::UnknownWidget(span, name) => gen_diagnostic! {
-                    msg = format!("No widget named {} exists", name),
-                    label = span => "Used here",
-                },
-                crate::config::validate::ValidationError::MissingAttr { widget_name, arg_name, arg_list_span, use_span } => {
-                    let diag = gen_diagnostic! {
-                        msg = format!("{}", error),
-                    };
-                    diag.with_opt_label(Some(span_to_secondary_label(*use_span).with_message("Argument missing here")))
-                        .with_opt_label(arg_list_span.map(|s| span_to_secondary_label(s).with_message("but is required here")))
-                }
-            }
+            error.to_diagnostic()
         } else if let Some(span) = self.get_span() {
             match self {
                 AstError::UnknownToplevel(_, name) => gen_diagnostic!(format!("{}", self), span),
@@ -96,6 +96,10 @@ impl ToDiagnostic for AstError {
                     label = span => format!("Expected `{}` here", expected),
                     note = format!("Expected: {}\nGot: {}", expected, got),
                 },
+                AstError::ErrorContext { label_span, context, main_err } => {
+                    main_err.to_diagnostic().with_opt_label(Some(span_to_secondary_label(*label_span).with_message(context)))
+                }
+
                 AstError::ConversionError(err) => conversion_error_to_diagnostic(err, span),
                 AstError::Other(_, source) => gen_diagnostic!(source, span),
                 AstError::AttrError(source) => gen_diagnostic!(source, span),
@@ -106,9 +110,11 @@ impl ToDiagnostic for AstError {
 
                 AstError::TooManyNodes(extra_nodes_span, expected) => gen_diagnostic! {
                     msg = self,
-                    label = extra_nodes_span => "these elements must not be here. Consider nesting the elements in some container element.",
+                    label = extra_nodes_span => "these elements must not be here",
+                    note = "Consider wrapping the elements in some container element",
                 },
-                AstError::ValidationError(_) => todo!(),
+                AstError::ErrorNote(note, source) => source.to_diagnostic().with_notes(vec![note.to_string()]),
+                AstError::ValidationError(source) => source.to_diagnostic(),
             }
         } else {
             Diagnostic::error().with_message(format!("{}", self))
@@ -116,6 +122,32 @@ impl ToDiagnostic for AstError {
     }
 }
 
+impl ToDiagnostic for AttrError {
+    fn to_diagnostic(&self) -> Diagnostic<usize> {
+        match self {
+            AttrError::MissingRequiredAttr(span, attr_name) => {
+                gen_diagnostic!(format!("Missing attribute `{}`", attr_name), span)
+            }
+            AttrError::EvaluationError(span, source) => eval_error_to_diagnostic(source, *span),
+            AttrError::Other(span, source) => gen_diagnostic!(source, span),
+        }
+    }
+}
+
+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 } => gen_diagnostic!(self)
+                .with_opt_label(Some(span_to_secondary_label(*use_span).with_message("Argument missing here")))
+                .with_opt_label(arg_list_span.map(|s| span_to_secondary_label(s).with_message("but is required here"))),
+        }
+    }
+}
+
 fn lalrpop_error_to_diagnostic<T: std::fmt::Display, E>(
     error: &lalrpop_util::ParseError<usize, T, E>,
     span: Span,
@@ -156,7 +188,7 @@ pub fn eval_error_to_diagnostic(error: &simplexpr::eval::EvalError, span: Span)
 
 fn conversion_error_to_diagnostic(error: &dynval::ConversionError, span: Span) -> Diagnostic<usize> {
     let diag = gen_diagnostic! {
-        msg = format!("{}", error),
+        msg = error,
         label = span => format!("{} is not of type `{}`", error.value, error.target_type),
     };
     diag.with_notes(error.source.as_ref().map(|x| vec![format!("{}", x)]).unwrap_or_default())