From ae542b833d9a08c7a5aa52cc06617fce5c9e85b5 Mon Sep 17 00:00:00 2001 From: elkowar <5300871+elkowar@users.noreply.github.com> Date: Sat, 24 Jul 2021 14:19:06 +0200 Subject: [PATCH] Custom codespan_reporting::files implementation, unload files properly for literal tags --- crates/eww/src/config/eww_config.rs | 8 +- crates/eww/src/error_handling_ctx.rs | 7 +- crates/eww/src/widgets/widget_definitions.rs | 33 +++--- crates/yuck/src/config/config.rs | 15 +-- crates/yuck/src/config/file_provider.rs | 103 +++++++++++++++---- crates/yuck/src/error.rs | 4 + crates/yuck/src/format_diagnostic.rs | 4 + crates/yuck/src/parser/mod.rs | 9 ++ 8 files changed, 136 insertions(+), 47 deletions(-) diff --git a/crates/eww/src/config/eww_config.rs b/crates/eww/src/config/eww_config.rs index 1e18eb3..db70e74 100644 --- a/crates/eww/src/config/eww_config.rs +++ b/crates/eww/src/config/eww_config.rs @@ -1,8 +1,8 @@ use anyhow::*; use eww_shared_util::VarName; -use std::collections::HashMap; +use std::{collections::HashMap, path::Path}; use yuck::config::{ - file_provider::FsYuckFiles, script_var_definition::ScriptVarDefinition, widget_definition::WidgetDefinition, Config, + file_provider::YuckFiles, script_var_definition::ScriptVarDefinition, widget_definition::WidgetDefinition, Config, }; use simplexpr::dynval::DynVal; @@ -19,8 +19,8 @@ pub struct EwwConfig { // files: FsYuckFiles, } impl EwwConfig { - pub fn read_from_file>(files: &mut FsYuckFiles, path: P) -> Result { - let config = Config::generate_from_main_file(files, path.as_ref().to_str().context("Failed to decode path")?)?; + pub fn read_from_file(files: &mut YuckFiles, path: impl AsRef) -> Result { + let config = Config::generate_from_main_file(files, path)?; let Config { widget_definitions, window_definitions, var_definitions, script_vars } = config; Ok(EwwConfig { windows: window_definitions diff --git a/crates/eww/src/error_handling_ctx.rs b/crates/eww/src/error_handling_ctx.rs index 2198efb..6e8e305 100644 --- a/crates/eww/src/error_handling_ctx.rs +++ b/crates/eww/src/error_handling_ctx.rs @@ -4,7 +4,7 @@ use codespan_reporting::diagnostic::Diagnostic; use eww_shared_util::DUMMY_SPAN; use simplexpr::eval::EvalError; use yuck::{ - config::file_provider::FsYuckFiles, + config::file_provider::YuckFiles, error::AstError, format_diagnostic::{eval_error_to_diagnostic, ToDiagnostic}, }; @@ -12,13 +12,14 @@ use yuck::{ use crate::error::DiagError; lazy_static::lazy_static! { - pub static ref ERROR_HANDLING_CTX: Arc> = Arc::new(Mutex::new(FsYuckFiles::new())); + pub static ref ERROR_HANDLING_CTX: Arc> = Arc::new(Mutex::new(YuckFiles::new())); } pub fn clear_files() { - *ERROR_HANDLING_CTX.lock().unwrap() = FsYuckFiles::new(); + *ERROR_HANDLING_CTX.lock().unwrap() = YuckFiles::new(); } + pub fn print_error(err: &anyhow::Error) { let result: anyhow::Result<_> = try { if let Some(err) = err.downcast_ref::() { diff --git a/crates/eww/src/widgets/widget_definitions.rs b/crates/eww/src/widgets/widget_definitions.rs index b14c1c2..a769197 100644 --- a/crates/eww/src/widgets/widget_definitions.rs +++ b/crates/eww/src/widgets/widget_definitions.rs @@ -1,6 +1,6 @@ #![allow(clippy::option_map_unit_fn)] use super::{run_command, BuilderArgs}; -use crate::{enum_parse, eww_state, resolve_block, util::list_difference, widgets::widget_node}; +use crate::{enum_parse, error_handling_ctx, eww_state, resolve_block, util::list_difference, widgets::widget_node}; use anyhow::*; use gdk::WindowExt; use glib; @@ -301,11 +301,7 @@ fn build_gtk_checkbox(bargs: &mut BuilderArgs) -> Result { prop(onchecked: as_string = "", onunchecked: as_string = "") { let old_id = on_change_handler_id.replace(Some( gtk_widget.connect_toggled(move |gtk_widget| { - if gtk_widget.get_active() { - run_command(&onchecked, ""); - } else { - run_command(&onunchecked, ""); - } + run_command(if gtk_widget.get_active() { &onchecked } else { &onunchecked }, ""); }) )); old_id.map(|id| gtk_widget.disconnect(id)); @@ -496,17 +492,11 @@ fn build_gtk_label(bargs: &mut BuilderArgs) -> Result { gtk_widget.set_text(&text); }, // @prop markup - Pango markup to display - prop(markup: as_string) { - gtk_widget.set_markup(&markup); - }, + prop(markup: as_string) { gtk_widget.set_markup(&markup); }, // @prop wrap - Wrap the text. This mainly makes sense if you set the width of this widget. - prop(wrap: as_bool) { - gtk_widget.set_line_wrap(wrap) - }, + prop(wrap: as_bool) { gtk_widget.set_line_wrap(wrap) }, // @prop angle - the angle of rotation for the label (between 0 - 360) - prop(angle: as_f64 = 0) { - gtk_widget.set_angle(angle) - } + prop(angle: as_f64 = 0) { gtk_widget.set_angle(angle) } }); Ok(gtk_widget) } @@ -521,12 +511,23 @@ fn build_gtk_literal(bargs: &mut BuilderArgs) -> Result { let window_name = bargs.window_name.to_string(); let widget_definitions = bargs.widget_definitions.clone(); + // the file id the literal-content has been stored under, for error reporting. + let literal_file_id: Rc>> = Rc::new(RefCell::new(None)); + resolve_block!(bargs, gtk_widget, { // @prop content - inline Eww XML that will be rendered as a widget. prop(content: as_string) { gtk_widget.get_children().iter().for_each(|w| gtk_widget.remove(w)); if !content.is_empty() { - let ast = yuck::parser::parse_string(usize::MAX, &content)?; + let ast = { + let mut yuck_files = error_handling_ctx::ERROR_HANDLING_CTX.lock().unwrap(); + let (span, asts) = yuck_files.load_str("".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)?; let widget_node = &*widget_node::generate_generic_widget_node(&widget_definitions, &HashMap::new(), content_widget_use)?; diff --git a/crates/yuck/src/config/config.rs b/crates/yuck/src/config/config.rs index b07021b..81b2113 100644 --- a/crates/yuck/src/config/config.rs +++ b/crates/yuck/src/config/config.rs @@ -1,4 +1,7 @@ -use std::collections::HashMap; +use std::{ + collections::HashMap, + path::{Path, PathBuf}, +}; use codespan_reporting::files::SimpleFiles; use simplexpr::SimplExpr; @@ -77,7 +80,7 @@ pub struct Config { } impl Config { - fn append_toplevel(&mut self, files: &mut impl YuckFiles, toplevel: TopLevel) -> AstResult<()> { + fn append_toplevel(&mut self, files: &mut YuckFiles, toplevel: TopLevel) -> AstResult<()> { match toplevel { TopLevel::VarDefinition(x) => { self.var_definitions.insert(x.name.clone(), x); @@ -92,7 +95,7 @@ impl Config { self.window_definitions.insert(x.name.clone(), x); } TopLevel::Include(include) => { - let (file_id, toplevels) = files.load(&include.path).map_err(|err| match err { + let (file_id, toplevels) = files.load_file(PathBuf::from(&include.path)).map_err(|err| match err { FilesError::IoError(_) => AstError::IncludedFileNotFound(include), FilesError::AstError(x) => x, })?; @@ -104,7 +107,7 @@ impl Config { Ok(()) } - pub fn generate(files: &mut impl YuckFiles, elements: Vec) -> AstResult { + pub fn generate(files: &mut YuckFiles, elements: Vec) -> AstResult { let mut config = Self { widget_definitions: HashMap::new(), window_definitions: HashMap::new(), @@ -117,8 +120,8 @@ impl Config { Ok(config) } - pub fn generate_from_main_file(files: &mut impl YuckFiles, path: &str) -> AstResult { - let (span, top_levels) = files.load(path).map_err(|err| match err { + pub fn generate_from_main_file(files: &mut YuckFiles, path: impl AsRef) -> AstResult { + let (span, top_levels) = files.load_file(path.as_ref().to_path_buf()).map_err(|err| match err { FilesError::IoError(err) => AstError::Other(None, Box::new(err)), FilesError::AstError(x) => x, })?; diff --git a/crates/yuck/src/config/file_provider.rs b/crates/yuck/src/config/file_provider.rs index cfc5588..1b6c333 100644 --- a/crates/yuck/src/config/file_provider.rs +++ b/crates/yuck/src/config/file_provider.rs @@ -17,46 +17,110 @@ pub enum FilesError { AstError(#[from] AstError), } -pub trait YuckFiles { - fn load(&mut self, path: &str) -> Result<(Span, Vec), FilesError>; +#[derive(Clone, Debug)] +pub enum YuckSource { + File(std::path::PathBuf), + Literal(String), +} + +impl YuckSource { + pub fn read_content(&self) -> std::io::Result { + match self { + YuckSource::File(path) => Ok(std::fs::read_to_string(path)?), + YuckSource::Literal(x) => Ok(x.to_string()), + } + } +} + +#[derive(Clone, Debug)] +pub struct YuckFile { + name: String, + line_starts: Vec, + source: YuckSource, + source_len_bytes: usize, +} + +impl YuckFile { + /// Return the starting byte index of the line with the specified line index. + /// Convenience method that already generates errors if necessary. + fn line_start(&self, line_index: usize) -> Result { + use std::cmp::Ordering; + + match line_index.cmp(&self.line_starts.len()) { + Ordering::Less => Ok(self.line_starts.get(line_index).cloned().expect("failed despite previous check")), + Ordering::Equal => Ok(self.source_len_bytes), + Ordering::Greater => { + Err(codespan_reporting::files::Error::LineTooLarge { given: line_index, max: self.line_starts.len() - 1 }) + } + } + } } #[derive(Debug, Clone)] -pub struct FsYuckFiles { - files: SimpleFiles, +pub struct YuckFiles { + files: HashMap, + latest_id: usize, } -impl FsYuckFiles { +impl YuckFiles { pub fn new() -> Self { - Self { files: SimpleFiles::new() } + Self { files: HashMap::new(), latest_id: 0 } } } -impl YuckFiles for FsYuckFiles { - fn load(&mut self, path: &str) -> Result<(Span, Vec), FilesError> { - let path = PathBuf::from(path); +impl YuckFiles { + fn get_file(&self, id: usize) -> Result<&YuckFile, codespan_reporting::files::Error> { + self.files.get(&id).ok_or(codespan_reporting::files::Error::FileMissing) + } + fn insert_file(&mut self, file: YuckFile) -> usize { + let file_id = self.latest_id; + self.files.insert(file_id, file); + self.latest_id += 1; + file_id + } + + pub fn load_file(&mut self, path: std::path::PathBuf) -> Result<(Span, Vec), FilesError> { let file_content = std::fs::read_to_string(&path)?; - let file_id = self.files.add(path.display().to_string(), file_content.to_string()); + let line_starts = codespan_reporting::files::line_starts(&file_content).collect(); + let yuck_file = YuckFile { + name: path.display().to_string(), + line_starts, + source_len_bytes: file_content.len(), + source: YuckSource::File(path), + }; + let file_id = self.insert_file(yuck_file); Ok(crate::parser::parse_toplevel(file_id, file_content)?) } + + pub fn load_str(&mut self, name: String, content: String) -> Result<(Span, Vec), FilesError> { + 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()) }; + let file_id = self.insert_file(yuck_file); + Ok(crate::parser::parse_toplevel(file_id, content)?) + } + + pub fn unload(&mut self, id: usize) { + self.files.remove(&id); + } } -impl<'a> Files<'a> for FsYuckFiles { +impl<'a> Files<'a> for YuckFiles { type FileId = usize; - type Name = String; - type Source = &'a str; + type Name = &'a str; + type Source = String; - fn name(&self, id: Self::FileId) -> Result { - self.files.name(id) + fn name(&'a self, id: Self::FileId) -> Result { + Ok(&self.get_file(id)?.name) } fn source(&'a self, id: Self::FileId) -> Result { - self.files.source(id) + Ok(self.get_file(id)?.source.read_content().map_err(codespan_reporting::files::Error::Io)?) } fn line_index(&self, id: Self::FileId, byte_index: usize) -> Result { - self.files.line_index(id, byte_index) + Ok(self.get_file(id)?.line_starts.binary_search(&byte_index).unwrap_or_else(|next_line| next_line - 1)) } fn line_range( @@ -64,6 +128,9 @@ impl<'a> Files<'a> for FsYuckFiles { id: Self::FileId, line_index: usize, ) -> Result, codespan_reporting::files::Error> { - self.files.line_range(id, line_index) + let file = self.get_file(id)?; + let line_start = file.line_start(line_index)?; + let next_line_start = file.line_start(line_index + 1)?; + Ok(line_start..next_line_start) } } diff --git a/crates/yuck/src/error.rs b/crates/yuck/src/error.rs index 79e1098..7bff8ad 100644 --- a/crates/yuck/src/error.rs +++ b/crates/yuck/src/error.rs @@ -18,6 +18,9 @@ pub enum AstError { UnknownToplevel(Span, String), #[error("Expected another element, but got nothing")] MissingNode(Span), + #[error("Too many elements, must be exactly {1}")] + TooManyNodes(Span, i32), + #[error("Wrong type of expression: Expected {1} but got {2}")] WrongExprType(Span, AstType, AstType), #[error("Expected to get a value, but got {1}")] @@ -60,6 +63,7 @@ impl AstError { AstError::Other(span, ..) => *span, AstError::ConversionError(err) => err.value.span().map(|x| x.into()), AstError::IncludedFileNotFound(include) => Some(include.path_span), + AstError::TooManyNodes(span, ..) => Some(*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)), } diff --git a/crates/yuck/src/format_diagnostic.rs b/crates/yuck/src/format_diagnostic.rs index e04f917..82de131 100644 --- a/crates/yuck/src/format_diagnostic.rs +++ b/crates/yuck/src/format_diagnostic.rs @@ -104,6 +104,10 @@ impl ToDiagnostic for AstError { 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. Consider nesting the elements in some container element.", + }, AstError::ValidationError(_) => todo!(), } } else { diff --git a/crates/yuck/src/parser/mod.rs b/crates/yuck/src/parser/mod.rs index 8e5499f..62c7291 100644 --- a/crates/yuck/src/parser/mod.rs +++ b/crates/yuck/src/parser/mod.rs @@ -33,6 +33,15 @@ pub fn parse_toplevel(file_id: usize, s: String) -> AstResult<(Span, Vec)> parser.parse(file_id, lexer).map_err(|e| AstError::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) -> AstResult { + 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)), + } +} + macro_rules! test_parser { ($($text:literal),*) => {{ let p = parser::AstParser::new();