Custom codespan_reporting::files implementation, unload files properly for literal tags

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<P: AsRef<std::path::Path>>(files: &mut FsYuckFiles, path: P) -> Result<Self> {
+    pub fn read_from_file(files: &mut YuckFiles, path: impl AsRef<Path>) -> Result<Self> {
-        let config = Config::generate_from_main_file(files, path.as_ref().to_str().context("Failed to decode path")?)?;
+        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

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<Mutex<FsYuckFiles>> = Arc::new(Mutex::new(FsYuckFiles::new()));
+    pub static ref ERROR_HANDLING_CTX: Arc<Mutex<YuckFiles>> = 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::<DiagError>() {

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<gtk::CheckButton> {
        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(if gtk_widget.get_active() { &onchecked } else { &onunchecked }, "");
-                        run_command(&onchecked, "");
-                    } else {
-                        run_command(&onunchecked, "");
-                    }
                 })
             ));
             old_id.map(|id| gtk_widget.disconnect(id));
@@ -496,17 +492,11 @@ fn build_gtk_label(bargs: &mut BuilderArgs) -> Result<gtk::Label> {
             gtk_widget.set_text(&text);
         },
         // @prop markup - Pango markup to display
-        prop(markup: as_string) {
+        prop(markup: as_string) { gtk_widget.set_markup(&markup); },
-            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) {
+        prop(wrap: as_bool) { gtk_widget.set_line_wrap(wrap) },
-            gtk_widget.set_line_wrap(wrap)
-        },
         // @prop angle - the angle of rotation for the label (between 0 - 360)
-        prop(angle: as_f64 = 0) {
+        prop(angle: as_f64 = 0) { gtk_widget.set_angle(angle) }
-            gtk_widget.set_angle(angle)
-        }
     });
     Ok(gtk_widget)
 }
@@ -521,12 +511,23 @@ fn build_gtk_literal(bargs: &mut BuilderArgs) -> Result<gtk::Box> {
     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<RefCell<Option<usize>>> = 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("<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)?;
 
                 let widget_node = &*widget_node::generate_generic_widget_node(&widget_definitions, &HashMap::new(), content_widget_use)?;

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<Ast>) -> AstResult<Self> {
+    pub fn generate(files: &mut YuckFiles, elements: Vec<Ast>) -> AstResult<Self> {
         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<Self> {
+    pub fn generate_from_main_file(files: &mut YuckFiles, path: impl AsRef<Path>) -> AstResult<Self> {
-        let (span, top_levels) = files.load(path).map_err(|err| match err {
+        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,
         })?;

crates/yuck/src/config/file_provider.rs

@@ -17,46 +17,110 @@ pub enum FilesError {
     AstError(#[from] AstError),
 }
 
-pub trait YuckFiles {
+#[derive(Clone, Debug)]
-    fn load(&mut self, path: &str) -> Result<(Span, Vec<Ast>), FilesError>;
+pub enum YuckSource {
+    File(std::path::PathBuf),
+    Literal(String),
+}
+
+impl YuckSource {
+    pub fn read_content(&self) -> std::io::Result<String> {
+        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<usize>,
+    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<usize, codespan_reporting::files::Error> {
+        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 {
+pub struct YuckFiles {
-    files: SimpleFiles<String, String>,
+    files: HashMap<usize, YuckFile>,
+    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 {
+impl YuckFiles {
-    fn load(&mut self, path: &str) -> Result<(Span, Vec<Ast>), FilesError> {
+    fn get_file(&self, id: usize) -> Result<&YuckFile, codespan_reporting::files::Error> {
-        let path = PathBuf::from(path);
+        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<Ast>), 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<Ast>), 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 Name = &'a str;
-    type Source = &'a str;
+    type Source = String;
 
-    fn name(&self, id: Self::FileId) -> Result<Self::Name, codespan_reporting::files::Error> {
+    fn name(&'a self, id: Self::FileId) -> Result<Self::Name, codespan_reporting::files::Error> {
-        self.files.name(id)
+        Ok(&self.get_file(id)?.name)
     }
 
     fn source(&'a self, id: Self::FileId) -> Result<Self::Source, codespan_reporting::files::Error> {
-        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<usize, codespan_reporting::files::Error> {
-        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<std::ops::Range<usize>, 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)
     }
 }

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)),
         }

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 {

crates/yuck/src/parser/mod.rs

@@ -33,6 +33,15 @@ pub fn parse_toplevel(file_id: usize, s: String) -> AstResult<(Span, Vec<Ast>)>
     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<Ast>) -> AstResult<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)),
+    }
+}
+
 macro_rules! test_parser {
     ($($text:literal),*) => {{
         let p = parser::AstParser::new();