Relative import and recusive fix

CHANGELOG.md

@@ -5,6 +5,10 @@ All notable changes to eww will be listed here, starting at changes since versio
 
 ## [Unreleased]
 
+### BREAKING CHANGES
+- Add relative `(include)`. Includes starting with the `./` or `../` prefixes will now be considered relative and thus may break. 
+  Please update your Yuck files to use the new syntax. (By: max-ishere)
+
 ### Features
 - Add `:namespace` window option
 - Default to building with x11 and wayland support simultaneously
@@ -20,6 +24,12 @@ All notable changes to eww will be listed here, starting at changes since versio
 - Add trigonometric functions (`sin`, `cos`, `tan`, `cot`) and degree/radian conversions (`degtorad`, `radtodeg`) (By: end-4)
 - Add `substring` function to simplexpr
 - Add `--duration` flag to `eww open`
+- Files are only loaded once if they were included multiple times. This prevents recursive imports from crashing `eww`.
+  (By: max-ishere)
+
+### Notable fixes and other changes
+- When `(import)` is used it generates a hint suggesting the use of `(include)` instead (By: max-ishere)
+
 
 ## [0.4.0] (04.09.2022)
 

crates/eww/src/file_database.rs

@@ -39,15 +39,16 @@ impl FileDatabase {
 }
 
 impl YuckFileProvider for FileDatabase {
+    /// Loads a Yuck file from a filesystem. If the file is already loaded returns a dummy [`Span`] and an empty [`Ast`].
     fn load_yuck_file(&mut self, path: std::path::PathBuf) -> Result<(Span, Vec<Ast>), FilesError> {
+        let name = path.display().to_string();
+        if self.files.values().find(|code_file| code_file.name == name).is_some() {
+            return Ok((Span::DUMMY, Vec::new()));
+        }
+
         let file_content = std::fs::read_to_string(&path)?;
         let line_starts = codespan_reporting::files::line_starts(&file_content).collect();
-        let code_file = CodeFile {
-            name: path.display().to_string(),
-            line_starts,
-            source_len_bytes: file_content.len(),
-            source: CodeSource::File(path),
-        };
+        let code_file = CodeFile { name, line_starts, source_len_bytes: file_content.len(), source: CodeSource::File(path) };
         let file_id = self.insert_code_file(code_file);
         Ok(yuck::parser::parse_toplevel(file_id, file_content)?)
     }

crates/yuck/src/config/toplevel.rs

@@ -33,6 +33,25 @@ static TOP_LEVEL_DEFINITION_NAMES: &[&str] = &[
     Include::ELEMENT_NAME,
 ];
 
+/// Defines common ways definitions may be called instead of their official names.
+///
+/// E.g: ~~`import`~~ ➡️ `include`.
+///
+/// This list will be used to generate a hint in the error diagnostic suggesting the use of the correct definition.
+///
+/// **Note:** This is not meant to contain a list of typos for [`TOP_LEVEL_DEFINITION_NAMES`], instead it contains a
+/// list of correctly spelled strings that may be used because they exist in other languages.
+static TOP_LEVEL_COMMON_DEFINITION_ERRORS: &[CommonDefinitionError] = {
+    use CommonDefinitionError as E; // Makes the lines below shorter
+    &[E { wrong: "import", correct: Include::ELEMENT_NAME }]
+};
+
+/// Used to map commonly confused definitions to their correct naming
+struct CommonDefinitionError<'a> {
+    wrong: &'a str,
+    correct: &'a str,
+}
+
 #[derive(Debug, PartialEq, Eq, Clone, serde::Serialize)]
 pub struct Include {
     pub path: String,
@@ -74,11 +93,22 @@ impl FromAst for TopLevel {
             }
             x if x == WindowDefinition::ELEMENT_NAME => Self::WindowDefinition(WindowDefinition::from_tail(span, iter)?),
             x => {
+                for common_error in TOP_LEVEL_COMMON_DEFINITION_ERRORS {
+                    if x == common_error.wrong {
+                        return Err(DiagError(gen_diagnostic! {
+                            msg = format!("Unknown toplevel declaration `{x}`"),
+                            label = sym_span,
+                            note = format!("help: Perhaps you've meant `{}`?", common_error.correct),
+                            note = format!("Must be one of: {}", TOP_LEVEL_DEFINITION_NAMES.iter().join(", ")),
+                        }));
+                    }
+                }
+
                 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(", ")),
-                }))
+                }));
             }
         })
     }
@@ -93,7 +123,12 @@ pub struct Config {
 }
 
 impl Config {
-    fn append_toplevel(&mut self, files: &mut impl YuckFileProvider, toplevel: TopLevel) -> DiagResult<()> {
+    fn append_toplevel(
+        &mut self,
+        files: &mut impl YuckFileProvider,
+        toplevel: TopLevel,
+        path: impl AsRef<Path>,
+    ) -> DiagResult<()> {
         match toplevel {
             TopLevel::VarDefinition(x) => {
                 if self.var_definitions.contains_key(&x.name) || self.script_vars.contains_key(&x.name) {
@@ -122,30 +157,48 @@ impl Config {
                 self.window_definitions.insert(x.name.clone(), x);
             }
             TopLevel::Include(include) => {
-                let (_, toplevels) = files.load_yuck_file(PathBuf::from(&include.path)).map_err(|err| match err {
+                // Resolve the potentially relative path to it's target
+                let mut include_path = PathBuf::from(&include.path);
+                if include_path.starts_with("./") || include_path.starts_with("../") {
+                    // Allows relative paths to go beyond the config directory.
+                    //
+                    // Should not panic unless the file we just read doesn't exist anymore or the path points to a file
+                    // that is indexed as a directory (`eww.yuck/test.txt`, where `eww.yuck` is a file).
+                    //
+                    // Since both cases are extremly rare to happen there is no point in making overly verbose
+                    // diagnostics.
+                    let canonical_path =
+                        path.as_ref().canonicalize().expect("Failed to canonicalize `{path}` due to a filesystem error.");
+
+                    include_path = util::resolve_relative_file(canonical_path, include_path);
+                }
+
+                let (_, toplevels) = files.load_yuck_file(include_path.clone()).map_err(|err| match err {
                     FilesError::IoError(_) => DiagError(gen_diagnostic! {
-                        msg = format!("Included file `{}` not found", include.path),
-                        label = include.path_span => "Included here",
+                    msg = format!("Included file `{}` not found", include.path),
+                    label = include.path_span => "Included here",
+                    note = format!("Hint: Resolved to `{}`", include_path.to_string_lossy()),
                     }),
                     FilesError::DiagError(x) => x,
                 })?;
+
                 for element in toplevels {
-                    self.append_toplevel(files, TopLevel::from_ast(element)?)?;
+                    self.append_toplevel(files, TopLevel::from_ast(element)?, &include_path)?;
                 }
             }
         }
         Ok(())
     }
 
-    pub fn generate(files: &mut impl YuckFileProvider, elements: Vec<Ast>) -> DiagResult<Self> {
+    pub fn generate(files: &mut impl YuckFileProvider, elements: Vec<Ast>, path: impl AsRef<Path>) -> DiagResult<Self> {
         let mut config = Self {
             widget_definitions: HashMap::new(),
             window_definitions: HashMap::new(),
             var_definitions: HashMap::new(),
             script_vars: HashMap::new(),
         };
         for element in elements {
-            config.append_toplevel(files, TopLevel::from_ast(element)?)?;
+            config.append_toplevel(files, TopLevel::from_ast(element)?, &path)?;
         }
         Ok(config)
     }
@@ -155,6 +208,31 @@ impl Config {
             FilesError::IoError(err) => DiagError(gen_diagnostic!(err)),
             FilesError::DiagError(x) => x,
         })?;
-        Self::generate(files, top_levels)
+        Self::generate(files, top_levels, path)
+    }
+}
+
+//‌‌/‌ Contains code that makes assumptions about how it is used. Thus it is only avaliable to this module.
+mod util {
+    use std::path::{Path, PathBuf};
+
+    /// Takes two paths and retuns location of `path_offset` relative to `base_file`.
+    /// Both cases of `base_file` being a file or a directory are handled.
+    ///
+    /// The resulting location could be invalid, a directory or a file.
+    pub fn resolve_relative_file(base_file: impl AsRef<Path>, path_offset: impl AsRef<Path>) -> PathBuf {
+        let base_file = base_file.as_ref();
+        let path_offset = path_offset.as_ref();
+
+        // if "" then path_offset is already the thing we need
+        if base_file.is_file() && base_file != Path::new("") {
+            // `.parent()` panics when:
+            // - "/" is a directory, so this is not an issue
+            // - "" is not an issue because of the `if` above
+            // Thus this should never panic!
+            base_file.parent().unwrap().join(path_offset)
+        } else {
+            base_file.join(path_offset)
+        }
     }
 }

crates/yuck/src/format_diagnostic.rs

@@ -22,6 +22,7 @@ pub fn span_to_secondary_label(span: Span) -> Label<usize> {
 ///     kind = Severity::Error,
 ///     msg = format!("Expected value, but got `{}`", actual),
 ///     label = span => "Expected some value here",
+///     note = "help: you can have many notes in the diagnostic",
 ///     note = format!("Got: {}", actual),
 /// }
 /// ```
@@ -30,7 +31,7 @@ macro_rules! gen_diagnostic {
     ( $(kind = $kind:expr,)?
       $(msg = $msg:expr)?
       $(, label = $span:expr $(=> $label:expr)?)?
-      $(, note = $note:expr)? $(,)?
+      $(, note = $note:expr)* $(,)?
     ) => {
         ::codespan_reporting::diagnostic::Diagnostic::new(gen_diagnostic! {
             @macro_fallback $({$kind})? {::codespan_reporting::diagnostic::Severity::Error}
@@ -40,7 +41,7 @@ macro_rules! gen_diagnostic {
                 ::codespan_reporting::diagnostic::Label::primary($span.2, $span.0..$span.1)
                     $(.with_message($label))?
             ]))?
-            $(.with_notes(vec![$note.to_string()]))?
+            .with_notes(vec![$($note.to_string(),)*])
     };
     ($msg:expr $(, $span:expr $(,)?)?) => {{
         ::codespan_reporting::diagnostic::Diagnostic::error()

docs/src/configuration.md

@@ -273,11 +273,25 @@ There are two options to achieve this:
 
 ### Using `include`
 
+A single yuck file may import the contents of any other yuck file. For this, make use of the `include` directive.
+
+Imports are resolved in a similar way to shell paths. This is explained below:
+
 ```lisp
-(include "./path/to/your/file.yuck")
+; These imports are located inside ~/.config/eww/example/example.yuck
+
+;       Prefix            Resolves to
+
+(import "file.yuck")    ; ~/.config/eww/file.yuck
+(import "/file.yuck")   ; /file.yuck
+(import "./file.yuck") ‌ ; ~/.config/eww/example/file.yuck
+(import "../file.yuck") ; ~/.config/eww/file.yuck
 ```
 
-A single yuck file may import the contents of any other yuck file. For this, make use of the `include` directive.
+As you can see they behave the same way as paths would in the shell. The only rule is that when there is no `‌/`‌, `./‌` or `../` the
+path is relative to the config directory. You should try to use relative paths as much as possible for better modularity.
+
+Some older yuck imports may break if they used the `./` prefix.
 
 ### Using a separate eww configuration directory