From 9f70a22cf0a9d6fe79a7ebc8b30252a13d710446 Mon Sep 17 00:00:00 2001 From: elkowar <5300871+elkowar@users.noreply.github.com> Date: Fri, 23 Jul 2021 13:45:30 +0200 Subject: [PATCH] significantly better error message handling everywhere --- crates/eww/src/app.rs | 45 ++++++---------- crates/eww/src/error_handling_ctx.rs | 43 ++++++++-------- crates/eww/src/eww_state.rs | 10 +++- crates/eww/src/main.rs | 3 +- crates/eww/src/server.rs | 54 ++++++++++++-------- crates/eww/src/util.rs | 15 ------ crates/eww/src/widgets/mod.rs | 17 ++---- crates/eww/src/widgets/widget_definitions.rs | 20 ++++---- crates/eww/src/widgets/widget_node.rs | 33 ++++++++---- crates/simplexpr/src/dynval.rs | 8 ++- crates/simplexpr/src/lib.rs | 4 +- crates/simplexpr/src/parser/lexer.rs | 10 ++-- crates/simplexpr/src/parser/mod.rs | 6 +-- crates/yuck/src/config/config.rs | 5 +- crates/yuck/src/config/validate.rs | 4 +- crates/yuck/src/config/widget_use.rs | 14 ++--- crates/yuck/src/error.rs | 14 ----- crates/yuck/src/format_diagnostic.rs | 34 +++++++++--- crates/yuck/src/parser/ast.rs | 20 ++------ crates/yuck/src/parser/lexer.rs | 4 +- crates/yuck/src/parser/parser.lalrpop | 5 +- 21 files changed, 181 insertions(+), 187 deletions(-) diff --git a/crates/eww/src/app.rs b/crates/eww/src/app.rs index e8e0fe7..7db2642 100644 --- a/crates/eww/src/app.rs +++ b/crates/eww/src/app.rs @@ -1,9 +1,4 @@ -use crate::{ - config::{self, EwwConfig}, - display_backend, error_handling_ctx, eww_state, - script_var_handler::*, - EwwPaths, -}; +use crate::{config, display_backend, error_handling_ctx, eww_state, script_var_handler::*, EwwPaths}; use anyhow::*; use debug_stub_derive::*; use eww_shared_util::VarName; @@ -14,10 +9,7 @@ use simplexpr::dynval::DynVal; use std::collections::HashMap; use tokio::sync::mpsc::UnboundedSender; use yuck::{ - config::{ - file_provider::FsYuckFiles, - window_geometry::{AnchorPoint, WindowGeometry}, - }, + config::window_geometry::{AnchorPoint, WindowGeometry}, value::Coords, }; @@ -123,20 +115,18 @@ impl App { &mut error_handling_ctx::ERROR_HANDLING_CTX.lock().unwrap(), &self.paths.get_yuck_path(), ); - match config_result { - Ok(new_config) => self.handle_command(DaemonCommand::UpdateConfig(new_config)), - Err(e) => { - errors.push(e); - } - } - - let css_result = crate::util::parse_scss_from_file(&self.paths.get_eww_scss_path()); - match css_result { - Ok(new_css) => self.handle_command(DaemonCommand::UpdateCss(new_css)), + match config_result.and_then(|new_config| self.load_config(new_config)) { + Ok(()) => {} Err(e) => errors.push(e), } - let errors = errors.into_iter().map(|e| error_handling_ctx::format_error(e)).join("\n"); + let css_result = crate::util::parse_scss_from_file(&self.paths.get_eww_scss_path()); + match css_result.and_then(|css| self.load_css(&css)) { + Ok(()) => {} + Err(e) => errors.push(e), + } + + let errors = errors.into_iter().map(|e| error_handling_ctx::format_error(&e)).join("\n"); if errors.is_empty() { sender.send(DaemonResponse::Success(String::new()))?; } else { @@ -202,14 +192,9 @@ impl App { } }; - // if let Err(err) = result { - // if let Some(ast_error) = err.root_cause().downcast_ref::() { - // println!("ast error: {:?}", ast_error); - //} else { - // dbg!(err.root_cause()); - //} - - crate::print_result_err!("while handling event", &result); + if let Err(err) = result { + error_handling_ctx::print_error(&err); + } } fn stop_application(&mut self) { @@ -397,7 +382,7 @@ fn get_monitor_geometry(n: i32) -> gdk::Rectangle { fn respond_with_error(sender: DaemonResponseSender, result: Result) -> Result<()> { match result { Ok(_) => sender.send(DaemonResponse::Success(String::new())), - Err(e) => sender.send(DaemonResponse::Failure(error_handling_ctx::format_error(e))), + Err(e) => sender.send(DaemonResponse::Failure(error_handling_ctx::format_error(&e))), } .context("sending response from main thread") } diff --git a/crates/eww/src/error_handling_ctx.rs b/crates/eww/src/error_handling_ctx.rs index 4a8999a..04ce248 100644 --- a/crates/eww/src/error_handling_ctx.rs +++ b/crates/eww/src/error_handling_ctx.rs @@ -1,6 +1,13 @@ use std::sync::{Arc, Mutex}; -use yuck::{config::file_provider::FsYuckFiles, error::AstError, format_diagnostic::ToDiagnostic}; +use codespan_reporting::diagnostic::Diagnostic; +use eww_shared_util::DUMMY_SPAN; +use simplexpr::eval::EvalError; +use yuck::{ + config::file_provider::FsYuckFiles, + error::AstError, + format_diagnostic::{eval_error_to_diagnostic, ToDiagnostic}, +}; lazy_static::lazy_static! { pub static ref ERROR_HANDLING_CTX: Arc> = Arc::new(Mutex::new(FsYuckFiles::new())); @@ -10,41 +17,35 @@ pub fn clear_files() { *ERROR_HANDLING_CTX.lock().unwrap() = FsYuckFiles::new(); } -pub fn print_error(err: anyhow::Error) { +pub fn print_error(err: &anyhow::Error) { match err.downcast_ref::() { Some(err) => { - print_ast_error(err); - } - None => { - log::error!("{:?}", err); + eprintln!("{:?}\n{}", err, stringify_diagnostic(err.to_diagnostic())); } + None => match err.downcast_ref::() { + Some(err) => { + eprintln!("{:?}\n{}", err, stringify_diagnostic(eval_error_to_diagnostic(err, err.span().unwrap_or(DUMMY_SPAN)))); + } + None => { + log::error!("{:?}", err); + } + }, } } -pub fn print_ast_error(err: &AstError) { - let diag = err.to_diagnostic(); - use codespan_reporting::term; - let config = term::Config::default(); - let mut writer = term::termcolor::StandardStream::stderr(term::termcolor::ColorChoice::Always); - let files = ERROR_HANDLING_CTX.lock().unwrap(); - term::emit(&mut writer, &config, &*files, &diag).unwrap(); -} - -pub fn format_error(err: anyhow::Error) -> String { +pub fn format_error(err: &anyhow::Error) -> String { match err.downcast_ref::() { - Some(err) => format_ast_error(err), + Some(err) => stringify_diagnostic(err.to_diagnostic()), None => format!("{:?}", err), } } -pub fn format_ast_error(err: &AstError) -> String { - let diag = err.to_diagnostic(); +pub fn stringify_diagnostic(diagnostic: Diagnostic) -> String { use codespan_reporting::term; let config = term::Config::default(); - // let mut writer = term::termcolor::StandardStream::stderr(term::termcolor::ColorChoice::Always); let mut buf = Vec::new(); let mut writer = term::termcolor::Ansi::new(&mut buf); let files = ERROR_HANDLING_CTX.lock().unwrap(); - term::emit(&mut writer, &config, &*files, &diag).unwrap(); + term::emit(&mut writer, &config, &*files, &diagnostic).unwrap(); String::from_utf8(buf).unwrap() } diff --git a/crates/eww/src/eww_state.rs b/crates/eww/src/eww_state.rs index 92aeeee..25c6640 100644 --- a/crates/eww/src/eww_state.rs +++ b/crates/eww/src/eww_state.rs @@ -4,6 +4,8 @@ use std::{collections::HashMap, sync::Arc}; use simplexpr::{dynval::DynVal, SimplExpr}; +use crate::error_handling_ctx; + /// Handler that gets executed to apply the necessary parts of the eww state to /// a gtk widget. These are created and initialized in EwwState::resolve. pub struct StateChangeHandler { @@ -28,9 +30,13 @@ impl StateChangeHandler { match resolved_attrs { Ok(resolved_attrs) => { - crate::print_result_err!("while updating UI based after state change", &(self.func)(resolved_attrs)) + 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); } - Err(err) => log::error!("Error while resolving attributes: {:?}", err), } } } diff --git a/crates/eww/src/main.rs b/crates/eww/src/main.rs index fde5ad8..9cccbaf 100644 --- a/crates/eww/src/main.rs +++ b/crates/eww/src/main.rs @@ -38,7 +38,6 @@ fn main() { let log_level_filter = if opts.log_debug { log::LevelFilter::Debug } else { log::LevelFilter::Info }; if std::env::var("RUST_LOG").is_ok() { - println!("hey"); pretty_env_logger::init_timed(); } else { pretty_env_logger::formatted_timed_builder().filter(Some("eww"), log_level_filter).init(); @@ -94,7 +93,7 @@ fn main() { }; if let Err(e) = result { - error_handling_ctx::print_error(e); + error_handling_ctx::print_error(&e); std::process::exit(1); } } diff --git a/crates/eww/src/server.rs b/crates/eww/src/server.rs index fe4802c..20f9b77 100644 --- a/crates/eww/src/server.rs +++ b/crates/eww/src/server.rs @@ -1,7 +1,12 @@ -use crate::{EwwPaths, app, config, error_handling_ctx, eww_state::*, ipc_server, script_var_handler, util}; +use crate::{app, config, error_handling_ctx, eww_state::*, ipc_server, script_var_handler, util, EwwPaths}; use anyhow::*; -use yuck::config::file_provider::FsYuckFiles; -use std::{collections::HashMap, os::unix::io::AsRawFd, path::Path}; + +use std::{ + collections::HashMap, + os::unix::io::AsRawFd, + path::Path, + sync::{atomic::Ordering, Arc}, +}; use tokio::sync::mpsc::*; pub fn initialize_server(paths: EwwPaths) -> Result<()> { @@ -29,10 +34,10 @@ pub fn initialize_server(paths: EwwPaths) -> Result<()> { log::info!("Loading paths: {}", &paths); - + // disgusting global state, I hate this, but https://github.com/buffet told me that this is what I should do for peak maintainability error_handling_ctx::clear_files(); - - let eww_config = config::EwwConfig::read_from_file(&mut error_handling_ctx::ERROR_HANDLING_CTX.lock().unwrap(), &paths.get_yuck_path())?; + let eww_config = + config::EwwConfig::read_from_file(&mut error_handling_ctx::ERROR_HANDLING_CTX.lock().unwrap(), &paths.get_yuck_path())?; gtk::init()?; @@ -109,27 +114,36 @@ fn init_async_part(paths: EwwPaths, ui_send: UnboundedSender /// Watch configuration files for changes, sending reload events to the eww app when the files change. async fn run_filewatch>(config_dir: P, evt_send: UnboundedSender) -> Result<()> { - use notify::Watcher; + use notify::{RecommendedWatcher, RecursiveMode, Watcher}; let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel(); - let mut watcher: notify::RecommendedWatcher = - notify::Watcher::new_immediate(move |res: notify::Result| match res { - Ok(event) => { - if let Err(err) = tx.send(event.paths) { + let mut watcher: RecommendedWatcher = Watcher::new_immediate(move |res: notify::Result| match res { + Ok(event) => { + let relevant_files_changed = event.paths.iter().any(|path| { + let ext = path.extension().unwrap_or_default(); + ext == "yuck" || ext == "scss" + }); + if !relevant_files_changed { + if let Err(err) = tx.send(()) { log::warn!("Error forwarding file update event: {:?}", err); } } - Err(e) => log::error!("Encountered Error While Watching Files: {}", e), - })?; - watcher.watch(&config_dir, notify::RecursiveMode::Recursive)?; + } + Err(e) => log::error!("Encountered Error While Watching Files: {}", e), + })?; + watcher.watch(&config_dir, RecursiveMode::Recursive)?; + + // make sure to not trigger reloads too much by only accepting one reload every 500ms. + let debounce_done = Arc::new(std::sync::atomic::AtomicBool::new(true)); crate::loop_select_exiting! { - Some(paths) = rx.recv() => { - for path in paths { - let extension = path.extension().unwrap_or_default(); - if extension != "xml" && extension != "scss" { - continue; - } + Some(()) = rx.recv() => { + let debounce_done = debounce_done.clone(); + if debounce_done.swap(false, Ordering::SeqCst) { + tokio::spawn(async move { + tokio::time::sleep(std::time::Duration::from_millis(500)).await; + debounce_done.store(true, Ordering::SeqCst); + }); let (daemon_resp_sender, mut daemon_resp_response) = tokio::sync::mpsc::unbounded_channel(); evt_send.send(app::DaemonCommand::ReloadConfigAndCss(daemon_resp_sender))?; diff --git a/crates/eww/src/util.rs b/crates/eww/src/util.rs index 4f684f7..5d853ed 100644 --- a/crates/eww/src/util.rs +++ b/crates/eww/src/util.rs @@ -122,21 +122,6 @@ impl> T { } } -pub fn parse_duration(s: &str) -> Result { - use std::time::Duration; - if s.ends_with("ms") { - Ok(Duration::from_millis(s.trim_end_matches("ms").parse()?)) - } else if s.ends_with('s') { - Ok(Duration::from_secs(s.trim_end_matches('s').parse()?)) - } else if s.ends_with('m') { - Ok(Duration::from_secs(s.trim_end_matches('m').parse::()? * 60)) - } else if s.ends_with('h') { - Ok(Duration::from_secs(s.trim_end_matches('h').parse::()? * 60 * 60)) - } else { - Err(anyhow!("unrecognized time format: {}", s)) - } -} - pub trait IterAverage { fn avg(self) -> f32; } diff --git a/crates/eww/src/widgets/mod.rs b/crates/eww/src/widgets/mod.rs index 7598ff7..a918390 100644 --- a/crates/eww/src/widgets/mod.rs +++ b/crates/eww/src/widgets/mod.rs @@ -64,18 +64,7 @@ fn build_builtin_gtk_widget( ) -> Result> { let mut bargs = BuilderArgs { eww_state, widget, window_name, unhandled_attrs: widget.attrs.keys().collect(), widget_definitions }; - let gtk_widget = match widget_to_gtk_widget(&mut bargs) { - Ok(Some(gtk_widget)) => gtk_widget, - result => { - return result.with_context(|| { - format!( - "{}Error building widget {}", - bargs.widget.span.map(|x| format!("{} |", x)).unwrap_or_default(), - bargs.widget.name, - ) - }) - } - }; + let gtk_widget = widget_to_gtk_widget(&mut bargs)?; // run resolve functions for superclasses such as range, orientable, and widget @@ -85,7 +74,7 @@ fn build_builtin_gtk_widget( let child_widget = child.render(bargs.eww_state, window_name, widget_definitions).with_context(|| { format!( "{}error while building child '{:#?}' of '{}'", - widget.span.map(|x| format!("{} |", x)).unwrap_or_default(), + format!("{} | ", widget.span), &child, >k_widget.get_widget_name() ) @@ -106,7 +95,7 @@ fn build_builtin_gtk_widget( if !bargs.unhandled_attrs.is_empty() { log::error!( "{}: Unknown attribute used in {}: {}", - widget.span.map(|x| format!("{} | ", x)).unwrap_or_default(), + format!("{} | ", widget.span), widget.name, bargs.unhandled_attrs.iter().map(|x| x.to_string()).join(", ") ) diff --git a/crates/eww/src/widgets/widget_definitions.rs b/crates/eww/src/widgets/widget_definitions.rs index f4cd54c..b14c1c2 100644 --- a/crates/eww/src/widgets/widget_definitions.rs +++ b/crates/eww/src/widgets/widget_definitions.rs @@ -1,23 +1,19 @@ #![allow(clippy::option_map_unit_fn)] use super::{run_command, BuilderArgs}; -use crate::{ - enum_parse, eww_state, resolve_block, - util::{list_difference, parse_duration}, - widgets::widget_node, -}; +use crate::{enum_parse, eww_state, resolve_block, util::list_difference, widgets::widget_node}; use anyhow::*; use gdk::WindowExt; use glib; use gtk::{self, prelude::*, ImageExt}; -use std::{cell::RefCell, collections::HashMap, rc::Rc}; -use yuck::parser::from_ast::FromAst; +use std::{cell::RefCell, collections::HashMap, rc::Rc, time::Duration}; +use yuck::{config::validate::ValidationError, error::AstError, parser::from_ast::FromAst}; // TODO figure out how to // TODO https://developer.gnome.org/gtk3/stable/GtkFixed.html //// widget definitions -pub(super) fn widget_to_gtk_widget(bargs: &mut BuilderArgs) -> Result> { +pub(super) fn widget_to_gtk_widget(bargs: &mut BuilderArgs) -> Result { let gtk_widget = match bargs.widget.name.as_str() { "box" => build_gtk_box(bargs)?.upcast(), "scale" => build_gtk_scale(bargs)?.upcast(), @@ -35,9 +31,11 @@ pub(super) fn widget_to_gtk_widget(bargs: &mut BuilderArgs) -> Result build_gtk_checkbox(bargs)?.upcast(), "revealer" => build_gtk_revealer(bargs)?.upcast(), "if-else" => build_if_else(bargs)?.upcast(), - _ => return Ok(None), + _ => { + Err(AstError::ValidationError(ValidationError::UnknownWidget(bargs.widget.name_span, bargs.widget.name.to_string())))? + } }; - Ok(Some(gtk_widget)) + Ok(gtk_widget) } /// attributes that apply to all widgets @@ -287,7 +285,7 @@ fn build_gtk_revealer(bargs: &mut BuilderArgs) -> Result { // @prop reveal - sets if the child is revealed or not prop(reveal: as_bool) { gtk_widget.set_reveal_child(reveal); }, // @prop duration - the duration of the reveal transition - prop(duration: as_string = "500ms") { gtk_widget.set_transition_duration(parse_duration(&duration)?.as_millis() as u32); }, + prop(duration: as_duration = Duration::from_millis(500)) { gtk_widget.set_transition_duration(duration.as_millis() as u32); }, }); Ok(gtk_widget) } diff --git a/crates/eww/src/widgets/widget_node.rs b/crates/eww/src/widgets/widget_node.rs index 82a367f..215689c 100644 --- a/crates/eww/src/widgets/widget_node.rs +++ b/crates/eww/src/widgets/widget_node.rs @@ -4,7 +4,10 @@ use dyn_clone; use eww_shared_util::{AttrName, Span, VarName}; use simplexpr::SimplExpr; use std::collections::HashMap; -use yuck::config::{widget_definition::WidgetDefinition, widget_use::WidgetUse}; +use yuck::{ + config::{validate::ValidationError, widget_definition::WidgetDefinition, widget_use::WidgetUse}, + error::AstError, +}; pub trait WidgetNode: std::fmt::Debug + dyn_clone::DynClone + Send + Sync { fn get_name(&self) -> &str; @@ -14,9 +17,8 @@ pub trait WidgetNode: std::fmt::Debug + dyn_clone::DynClone + Send + Sync { /// /// Also registers all the necessary state-change handlers in the eww_state. /// - /// This may return `Err` in case there was an actual error while parsing or - /// resolving the widget, Or `Ok(None)` if the widget_use just didn't match any - /// widget name. + /// This may return `Err` in case there was an actual error while parsing + /// or when the widget_use did not match any widget name fn render( &self, eww_state: &mut EwwState, @@ -56,14 +58,23 @@ impl WidgetNode for UserDefined { #[derive(Debug, Clone)] pub struct Generic { pub name: String, - pub span: Option, + pub name_span: Span, + pub span: Span, pub children: Vec>, pub attrs: HashMap, } impl Generic { pub fn get_attr(&self, key: &str) -> Result<&SimplExpr> { - self.attrs.get(key).context(format!("attribute '{}' missing from use of '{}'", key, &self.name)) + Ok(self.attrs.get(key).ok_or_else(|| { + AstError::ValidationError(ValidationError::MissingAttr { + widget_name: self.name.to_string(), + arg_name: AttrName(key.to_string()), + use_span: self.span, + // TODO set this when available + arg_list_span: None, + }) + })?) } /// returns all the variables that are referenced in this widget @@ -78,7 +89,7 @@ impl WidgetNode for Generic { } fn get_span(&self) -> Option { - self.span + Some(self.span) } fn render( @@ -87,8 +98,9 @@ impl WidgetNode for Generic { window_name: &str, widget_definitions: &HashMap, ) -> Result { - crate::widgets::build_builtin_gtk_widget(eww_state, window_name, widget_definitions, self)? - .with_context(|| format!("Unknown widget '{}'", self.get_name())) + Ok(crate::widgets::build_builtin_gtk_widget(eww_state, window_name, widget_definitions, self)?.ok_or_else(|| { + AstError::ValidationError(ValidationError::UnknownWidget(self.name_span, self.get_name().to_string())) + })?) } } @@ -112,7 +124,8 @@ pub fn generate_generic_widget_node( } else { Ok(Box::new(Generic { name: w.name, - span: Some(w.span), + name_span: w.name_span, + span: w.span, attrs: w .attrs .attrs diff --git a/crates/simplexpr/src/dynval.rs b/crates/simplexpr/src/dynval.rs index f72248d..62b71bb 100644 --- a/crates/simplexpr/src/dynval.rs +++ b/crates/simplexpr/src/dynval.rs @@ -6,7 +6,7 @@ use std::{fmt, iter::FromIterator, str::FromStr}; pub type Result = std::result::Result; #[derive(Debug, thiserror::Error)] -#[error("Failed to turn {value} into a value of type {target_type}")] +#[error("Failed to turn `{value}` into a value of type {target_type}")] pub struct ConversionError { pub value: DynVal, pub target_type: &'static str, @@ -93,6 +93,12 @@ macro_rules! impl_dynval_from { impl_dynval_from!(bool, i32, u32, f32, u8, f64, &str); +impl From for DynVal { + fn from(d: std::time::Duration) -> Self { + DynVal(format!("{}ms", d.as_millis()), None) + } +} + impl From<&serde_json::Value> for DynVal { fn from(v: &serde_json::Value) -> Self { DynVal( diff --git a/crates/simplexpr/src/lib.rs b/crates/simplexpr/src/lib.rs index 48156f4..b4eefc1 100644 --- a/crates/simplexpr/src/lib.rs +++ b/crates/simplexpr/src/lib.rs @@ -19,6 +19,4 @@ lalrpop_mod!( pub simplexpr_parser ); -pub fn parse_string(file_id: usize, s: &str) -> Result { - parser::parse_string(file_id, s) -} +pub use parser::parse_string; diff --git a/crates/simplexpr/src/parser/lexer.rs b/crates/simplexpr/src/parser/lexer.rs index e3c2447..2d3394d 100644 --- a/crates/simplexpr/src/parser/lexer.rs +++ b/crates/simplexpr/src/parser/lexer.rs @@ -56,11 +56,12 @@ pub type SpannedResult = Result<(Loc, Tok, Loc), Error>; pub struct Lexer<'input> { lexer: logos::SpannedIter<'input, Token>, + byte_offset: usize, } impl<'input> Lexer<'input> { - pub fn new(text: &'input str) -> Self { - Lexer { lexer: logos::Lexer::new(text).spanned() } + pub fn new(byte_offset: usize, text: &'input str) -> Self { + Lexer { lexer: logos::Lexer::new(text).spanned(), byte_offset } } } @@ -69,10 +70,11 @@ impl<'input> Iterator for Lexer<'input> { fn next(&mut self) -> Option { let (token, range) = self.lexer.next()?; + let range = (range.start + self.byte_offset, range.end + self.byte_offset); if token == Token::Error { - Some(Err(LexicalError(range.start, range.end))) + Some(Err(LexicalError(range.0, range.1))) } else { - Some(Ok((range.start, token, range.end))) + Some(Ok((range.0, token, range.1))) } } } diff --git a/crates/simplexpr/src/parser/mod.rs b/crates/simplexpr/src/parser/mod.rs index f833b61..169490a 100644 --- a/crates/simplexpr/src/parser/mod.rs +++ b/crates/simplexpr/src/parser/mod.rs @@ -6,8 +6,8 @@ use crate::{ error::{Error, Result}, }; -pub fn parse_string(file_id: usize, s: &str) -> Result { - let lexer = lexer::Lexer::new(s); +pub fn parse_string(byte_offset: usize, file_id: usize, s: &str) -> Result { + let lexer = lexer::Lexer::new(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)) } @@ -20,7 +20,7 @@ mod tests { use crate::parser::lexer::Lexer; ::insta::with_settings!({sort_maps => true}, { $( - ::insta::assert_debug_snapshot!(p.parse(0, Lexer::new($text))); + ::insta::assert_debug_snapshot!(p.parse(0, Lexer::new(0, $text))); )* }); }} diff --git a/crates/yuck/src/config/config.rs b/crates/yuck/src/config/config.rs index bb5bce9..0063d0a 100644 --- a/crates/yuck/src/config/config.rs +++ b/crates/yuck/src/config/config.rs @@ -118,7 +118,10 @@ impl Config { } pub fn generate_from_main_file(files: &mut impl YuckFiles, path: &str) -> AstResult { - let (span, top_levels) = files.load(path).map_err(|err| AstError::Other(None, Box::new(err)))?; + let (span, top_levels) = files.load(path).map_err(|err| match err { + FilesError::IoError(err) => AstError::Other(None, Box::new(err)), + FilesError::AstError(x) => x, + })?; Self::generate(files, top_levels) } } diff --git a/crates/yuck/src/config/validate.rs b/crates/yuck/src/config/validate.rs index 093e2c5..73a4630 100644 --- a/crates/yuck/src/config/validate.rs +++ b/crates/yuck/src/config/validate.rs @@ -16,7 +16,7 @@ pub enum ValidationError { UnknownWidget(Span, String), #[error("Missing attribute `{arg_name}` in use of widget `{widget_name}`")] - MissingAttr { widget_name: String, arg_name: AttrName, arg_list_span: Span, use_span: Span }, + MissingAttr { widget_name: String, arg_name: AttrName, arg_list_span: Option, use_span: Span }, } pub fn validate(defs: &HashMap, content: &WidgetUse) -> Result<(), ValidationError> { @@ -26,7 +26,7 @@ pub fn validate(defs: &HashMap, content: &WidgetUse) - return Err(ValidationError::MissingAttr { widget_name: def.name.to_string(), arg_name: expected.clone(), - arg_list_span: def.args_span, + arg_list_span: Some(def.args_span), use_span: content.span, }); } diff --git a/crates/yuck/src/config/widget_use.rs b/crates/yuck/src/config/widget_use.rs index 18efcc0..efea534 100644 --- a/crates/yuck/src/config/widget_use.rs +++ b/crates/yuck/src/config/widget_use.rs @@ -17,20 +17,22 @@ pub struct WidgetUse { pub attrs: Attributes, pub children: Vec, pub span: Span, + pub name_span: Span, } impl FromAst for WidgetUse { fn from_ast(e: Ast) -> AstResult { let span = e.span(); - if let Ok(text) = e.as_literal_ref() { + if let Ok(value) = e.clone().as_simplexpr() { Ok(Self { name: "label".to_string(), + name_span: span.point_span(), attrs: Attributes::new( - span.into(), + span, maplit::hashmap! { AttrName("text".to_string()) => AttrEntry::new( - span.into(), - Ast::Literal(span.into(), text.clone()) + span, + Ast::SimplExpr(span.into(), value.clone()) ) }, ), @@ -39,10 +41,10 @@ impl FromAst for WidgetUse { }) } else { let mut iter = e.try_ast_iter()?; - let (_, name) = iter.expect_symbol()?; + let (name_span, name) = iter.expect_symbol()?; let attrs = iter.expect_key_values()?; let children = iter.map(WidgetUse::from_ast).collect::>>()?; - Ok(Self { name, attrs, children, span }) + Ok(Self { name, attrs, children, span, name_span }) } } } diff --git a/crates/yuck/src/error.rs b/crates/yuck/src/error.rs index 2c4662b..79e1098 100644 --- a/crates/yuck/src/error.rs +++ b/crates/yuck/src/error.rs @@ -89,20 +89,6 @@ fn get_parse_error_span( } } -// pub fn spanned(span: Span, err: impl Into) -> AstError { -// use AstError::*; -// match err.into() { -// UnknownToplevel(s, x) => UnknownToplevel(Some(s.unwrap_or(span)), x), -// MissingNode(s) => MissingNode(Some(s.unwrap_or(span))), -// WrongExprType(s, x, y) => WrongExprType(Some(s.unwrap_or(span)), x, y), -// UnknownToplevel(s, x) => UnknownToplevel(Some(s.unwrap_or(span)), x), -// MissingNode(s) => MissingNode(Some(s.unwrap_or(span))), -// NotAValue(s, x) => NotAValue(Some(s.unwrap_or(span)), x), -// MismatchedElementName(s, expected, got) => MismatchedElementName(Some(s.unwrap_or(span)), expected, got), -// Other(s, x) => Other(Some(s.unwrap_or(span)), x), -// x @ ConversionError(_) | x @ AttrError(_) | x @ ValidationError(_) | x @ ParseError { .. } => x, -//} - pub trait OptionAstErrorExt { fn or_missing(self, span: Span) -> Result; } diff --git a/crates/yuck/src/format_diagnostic.rs b/crates/yuck/src/format_diagnostic.rs index f244344..d96586c3 100644 --- a/crates/yuck/src/format_diagnostic.rs +++ b/crates/yuck/src/format_diagnostic.rs @@ -33,6 +33,20 @@ macro_rules! gen_diagnostic { }}; } +pub trait DiagnosticExt: Sized { + fn with_opt_label(self, label: Option>) -> Self; +} + +impl DiagnosticExt for Diagnostic { + fn with_opt_label(self, label: Option>) -> Self { + if let Some(label) = label { + self.with_labels(vec![label]) + } else { + self + } + } +} + pub trait ToDiagnostic { fn to_diagnostic(&self) -> Diagnostic; } @@ -49,10 +63,8 @@ impl ToDiagnostic for AstError { let diag = gen_diagnostic! { msg = format!("{}", error), }; - diag.with_labels(vec![ - Label::secondary(use_span.2, use_span.0..use_span.1).with_message("Argument missing here"), - Label::secondary(arg_list_span.2, arg_list_span.0..arg_list_span.1).with_message("but is required here"), - ]) + 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"))) } } } else if let Some(span) = self.get_span() { @@ -76,7 +88,7 @@ impl ToDiagnostic for AstError { AstError::ParseError { file_id, source } => lalrpop_error_to_diagnostic(source, span, |error| match error { parse_error::ParseError::SimplExpr(_, error) => simplexpr_error_to_diagnostic(error, span), - parse_error::ParseError::LexicalError(_) => lexical_error_to_diagnostic(span), + parse_error::ParseError::LexicalError(span) => lexical_error_to_diagnostic(*span), }), AstError::MismatchedElementName(_, expected, got) => gen_diagnostic! { msg = format!("Expected element `{}`, but found `{}`", expected, got), @@ -120,17 +132,23 @@ fn lalrpop_error_to_diagnostic( } } -fn simplexpr_error_to_diagnostic(error: &simplexpr::error::Error, span: Span) -> Diagnostic { +// TODO this needs a lot of improvement +pub fn simplexpr_error_to_diagnostic(error: &simplexpr::error::Error, span: Span) -> Diagnostic { use simplexpr::error::Error::*; match error { ParseError { source, .. } => lalrpop_error_to_diagnostic(source, span, move |error| lexical_error_to_diagnostic(span)), ConversionError(error) => conversion_error_to_diagnostic(error, span), - Eval(error) => gen_diagnostic!(error, span), + Eval(error) => eval_error_to_diagnostic(error, span), Other(error) => gen_diagnostic!(error, span), - Spanned(_, error) => gen_diagnostic!(error, span), + Spanned(span, error) => gen_diagnostic!(error, span), } } +// TODO this needs a lot of improvement +pub fn eval_error_to_diagnostic(error: &simplexpr::eval::EvalError, span: Span) -> Diagnostic { + gen_diagnostic!(error, error.span().unwrap_or(span)) +} + fn conversion_error_to_diagnostic(error: &dynval::ConversionError, span: Span) -> Diagnostic { let diag = gen_diagnostic! { msg = format!("{}", error), diff --git a/crates/yuck/src/parser/ast.rs b/crates/yuck/src/parser/ast.rs index bcaa402..a1efb33 100644 --- a/crates/yuck/src/parser/ast.rs +++ b/crates/yuck/src/parser/ast.rs @@ -2,7 +2,7 @@ use itertools::Itertools; use simplexpr::{ast::SimplExpr, dynval::DynVal}; use std::collections::HashMap; -use eww_shared_util::Span; +use eww_shared_util::{Span, VarName}; use std::fmt::Display; use super::{ast_iterator::AstIterator, from_ast::FromAst}; @@ -94,10 +94,10 @@ impl Ast { pub fn as_simplexpr(self) -> AstResult { match self { - // do I do this? - // Ast::Array(_, _) => todo!(), - // Ast::Symbol(_, _) => todo!(), - Ast::Literal(span, x) => Ok(SimplExpr::Literal(span.into(), x)), + // TODO do I do this? + // Ast::Array(span, elements) => todo!() + Ast::Symbol(span, x) => Ok(SimplExpr::VarRef(span, VarName(x))), + Ast::Literal(span, x) => Ok(SimplExpr::Literal(span, x)), Ast::SimplExpr(span, x) => Ok(x), _ => Err(AstError::WrongExprType(self.span(), AstType::IntoPrimitive, self.expr_type())), } @@ -126,16 +126,6 @@ impl std::fmt::Display for Ast { } impl std::fmt::Debug for Ast { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - use Ast::*; write!(f, "{}", self) - // match self { - // List(span, x) => f.debug_tuple(&format!("List<{}>", span)).field(x).finish(), - // Array(span, x) => f.debug_tuple(&format!("Array<{}>", span)).field(x).finish(), - // Keyword(span, x) => write!(f, "Number<{}>({})", span, x), - // Symbol(span, x) => write!(f, "Symbol<{}>({})", span, x), - // Value(span, x) => write!(f, "Value<{}>({})", span, x), - // SimplExpr(span, x) => write!(f, "SimplExpr<{}>({})", span, x), - // Comment(span) => write!(f, "Comment<{}>", span), - //} } } diff --git a/crates/yuck/src/parser/lexer.rs b/crates/yuck/src/parser/lexer.rs index 322abdf..c8b5012 100644 --- a/crates/yuck/src/parser/lexer.rs +++ b/crates/yuck/src/parser/lexer.rs @@ -97,7 +97,7 @@ impl Iterator for Lexer { let string = &self.source[self.pos..]; if string.starts_with('{') { - self.pos += 1; + // self.pos += 1; let expr_start = self.pos; let mut in_string = false; loop { @@ -107,8 +107,8 @@ impl Iterator for Lexer { let string = &self.source[self.pos..]; if string.starts_with('}') && !in_string { - let tok_str = &self.source[expr_start..self.pos]; self.pos += 1; + let tok_str = &self.source[expr_start..self.pos]; return Some(Ok((expr_start, Token::SimplExpr(tok_str.to_string()), self.pos - 1))); } else if string.starts_with('"') { self.pos += 1; diff --git a/crates/yuck/src/parser/parser.lalrpop b/crates/yuck/src/parser/parser.lalrpop index a7d05d4..848120d 100644 --- a/crates/yuck/src/parser/parser.lalrpop +++ b/crates/yuck/src/parser/parser.lalrpop @@ -59,9 +59,8 @@ StrLit: String = { SimplExpr: SimplExpr = { =>? { let expr = x[1..x.len() - 1].to_string(); - simplexpr::parse_string(file_id, &expr).map_err(|e| { - let span = e.get_span().map(|Span(simpl_l, simpl_r, file_id)| Span(1 + l + simpl_l, 1 + l + simpl_r, file_id)); - ParseError::User { error: parse_error::ParseError::SimplExpr(span, e) }}) + simplexpr::parse_string(l + 1, file_id, &expr).map_err(|e| { + ParseError::User { error: parse_error::ParseError::SimplExpr(e.get_span(), e) }}) } }