From ce1fb9e64c189fb7476b4c72c6774a5bf6cbfd0f Mon Sep 17 00:00:00 2001 From: paul-scott Date: Fri, 10 Mar 2023 01:50:43 +1100 Subject: Generalised to multiple runtime directories with priorities (#5411) * Generalised to multiple runtime directories with priorities This is an implementation for #3346. Previously, one of the following runtime directories were used: 1. `$HELIX_RUNTIME` 2. sibling directory to `$CARGO_MANIFEST_DIR` 3. subdirectory of user config directory 4. subdirectory of path to helix executable The first directory provided / found to exist in this order was used as a root for all runtime file searches (grammars, themes, queries). This change lowers the priority of `$HELIX_RUNTIME` so that the user config runtime has higher priority. More significantly, all of these directories are now searched for runtime files, enabling a user to override default or system-level runtime files. If the same file name appears in multiple runtime directories, the following priority is now used: 1. sibling directory to `$CARGO_MANIFEST_DIR` 2. subdirectory of user config directory 3. `$HELIX_RUNTIME` 4. subdirectory of path to helix executable One exception to this rule is that a user can have a `themes` directory directly in the user config directory that has higher piority to `themes` directories in runtime directories. That behaviour has been preserved. As part of implementing this feature `theme::Loader` was simplified and the cycle detection logic of the theme inheritance was improved to cover more cases and to be more explicit. * Removed AsRef usage to avoid binary growth * Health displaying ;-separated runtime dirs * Changed HELIX_RUNTIME build from src instructions * Updated doc for more detail on runtime directories * Improved health symlink printing and theme cycle errors The health display of runtime symlinks now prints both ends of the link. Separate errors are given when theme file is not found and when the only theme file found would form an inheritence cycle. * Satisfied clippy on passing Path * Clarified highest priority runtime directory purpose * Further clarified multiple runtime details in book Also gave markdown headings to subsections. Fixed a error with table indentation not building table that also appears present on master. --------- Co-authored-by: Paul Scott Co-authored-by: Blaž Hrastnik --- helix-view/src/theme.rs | 95 +++++++++++++++++++++++++++---------------------- 1 file changed, 53 insertions(+), 42 deletions(-) (limited to 'helix-view/src') diff --git a/helix-view/src/theme.rs b/helix-view/src/theme.rs index ce061bab..5d79ff26 100644 --- a/helix-view/src/theme.rs +++ b/helix-view/src/theme.rs @@ -1,5 +1,5 @@ use std::{ - collections::HashMap, + collections::{HashMap, HashSet}, path::{Path, PathBuf}, str, }; @@ -37,19 +37,21 @@ pub static BASE16_DEFAULT_THEME: Lazy = Lazy::new(|| Theme { #[derive(Clone, Debug)] pub struct Loader { - user_dir: PathBuf, - default_dir: PathBuf, + /// Theme directories to search from highest to lowest priority + theme_dirs: Vec, } impl Loader { - /// Creates a new loader that can load themes from two directories. - pub fn new>(user_dir: P, default_dir: P) -> Self { + /// Creates a new loader that can load themes from multiple directories. + /// + /// The provided directories should be ordered from highest to lowest priority. + /// The directories will have their "themes" subdirectory searched. + pub fn new(dirs: &[PathBuf]) -> Self { Self { - user_dir: user_dir.as_ref().join("themes"), - default_dir: default_dir.as_ref().join("themes"), + theme_dirs: dirs.iter().map(|p| p.join("themes")).collect(), } } - /// Loads a theme first looking in the `user_dir` then in `default_dir` + /// Loads a theme searching directories in priority order. pub fn load(&self, name: &str) -> Result { if name == "default" { return Ok(self.default()); @@ -58,7 +60,8 @@ impl Loader { return Ok(self.base16_default()); } - let theme = self.load_theme(name, name, false).map(Theme::from)?; + let mut visited_paths = HashSet::new(); + let theme = self.load_theme(name, &mut visited_paths).map(Theme::from)?; Ok(Theme { name: name.into(), @@ -66,16 +69,18 @@ impl Loader { }) } - // load the theme and its parent recursively and merge them - // `base_theme_name` is the theme from the config.toml, - // used to prevent some circular loading scenarios - fn load_theme( - &self, - name: &str, - base_theme_name: &str, - only_default_dir: bool, - ) -> Result { - let path = self.path(name, only_default_dir); + /// Recursively load a theme, merging with any inherited parent themes. + /// + /// The paths that have been visited in the inheritance hierarchy are tracked + /// to detect and avoid cycling. + /// + /// It is possible for one file to inherit from another file with the same name + /// so long as the second file is in a themes directory with lower priority. + /// However, it is not recommended that users do this as it will make tracing + /// errors more difficult. + fn load_theme(&self, name: &str, visited_paths: &mut HashSet) -> Result { + let path = self.path(name, visited_paths)?; + let theme_toml = self.load_toml(path)?; let inherits = theme_toml.get("inherits"); @@ -92,11 +97,7 @@ impl Loader { // load default themes's toml from const. "default" => DEFAULT_THEME_DATA.clone(), "base16_default" => BASE16_DEFAULT_THEME_DATA.clone(), - _ => self.load_theme( - parent_theme_name, - base_theme_name, - base_theme_name == parent_theme_name, - )?, + _ => self.load_theme(parent_theme_name, visited_paths)?, }; self.merge_themes(parent_theme_toml, theme_toml) @@ -148,7 +149,7 @@ impl Loader { merge_toml_values(theme, palette.into(), 1) } - // Loads the theme data as `toml::Value` first from the user_dir then in default_dir + // Loads the theme data as `toml::Value` fn load_toml(&self, path: PathBuf) -> Result { let data = std::fs::read_to_string(path)?; let value = toml::from_str(&data)?; @@ -156,25 +157,35 @@ impl Loader { Ok(value) } - // Returns the path to the theme with the name - // With `only_default_dir` as false the path will first search for the user path - // disabled it ignores the user path and returns only the default path - fn path(&self, name: &str, only_default_dir: bool) -> PathBuf { + /// Returns the path to the theme with the given name + /// + /// Ignores paths already visited and follows directory priority order. + fn path(&self, name: &str, visited_paths: &mut HashSet) -> Result { let filename = format!("{}.toml", name); - let user_path = self.user_dir.join(&filename); - if !only_default_dir && user_path.exists() { - user_path - } else { - self.default_dir.join(filename) - } - } - - /// Lists all theme names available in default and user directory - pub fn names(&self) -> Vec { - let mut names = Self::read_names(&self.user_dir); - names.extend(Self::read_names(&self.default_dir)); - names + let mut cycle_found = false; // track if there was a path, but it was in a cycle + self.theme_dirs + .iter() + .find_map(|dir| { + let path = dir.join(&filename); + if !path.exists() { + None + } else if visited_paths.contains(&path) { + // Avoiding cycle, continuing to look in lower priority directories + cycle_found = true; + None + } else { + visited_paths.insert(path.clone()); + Some(path) + } + }) + .ok_or_else(|| { + if cycle_found { + anyhow!("Theme: cycle found in inheriting: {}", name) + } else { + anyhow!("Theme: file not found for: {}", name) + } + }) } pub fn default_theme(&self, true_color: bool) -> Theme { -- cgit v1.2.3-70-g09d2