Generalised to multiple runtime directories with priorities

This is an implementation for helix-editor#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).

After this change it is possible for all of these directories to be
searched for runtime files. If the same file name appears in multiple runtime
directories, the same priority order is used as above.

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.

helix-loader/src/grammar.rs

@@ -67,8 +67,9 @@ pub fn get_language(name: &str) -> Result<Language> {
 #[cfg(not(target_arch = "wasm32"))]
 pub fn get_language(name: &str) -> Result<Language> {
     use libloading::{Library, Symbol};
-    let mut library_path = crate::runtime_dir().join("grammars").join(name);
-    library_path.set_extension(DYLIB_EXTENSION);
+    let mut rel_library_path = PathBuf::new().join("grammars").join(name);
+    rel_library_path.set_extension(DYLIB_EXTENSION);
+    let library_path = crate::runtime_file(rel_library_path);
 
     let library = unsafe { Library::new(&library_path) }
         .with_context(|| format!("Error opening dynamic library {:?}", library_path))?;
@@ -252,7 +253,9 @@ fn fetch_grammar(grammar: GrammarConfiguration) -> Result<FetchStatus> {
         remote, revision, ..
     } = grammar.source
     {
-        let grammar_dir = crate::runtime_dir()
+        let grammar_dir = crate::runtime_dirs()
+            .first()
+            .expect("No runtime directories provided") // guaranteed by post-condition
             .join("grammars")
             .join("sources")
             .join(&grammar.grammar_id);
@@ -350,7 +353,9 @@ fn build_grammar(grammar: GrammarConfiguration, target: Option<&str>) -> Result<
     let grammar_dir = if let GrammarSource::Local { path } = &grammar.source {
         PathBuf::from(&path)
     } else {
-        crate::runtime_dir()
+        crate::runtime_dirs()
+            .first()
+            .expect("No runtime directories provided") // guaranteed by post-condition
             .join("grammars")
             .join("sources")
             .join(&grammar.grammar_id)
@@ -401,7 +406,10 @@ fn build_tree_sitter_library(
             None
         }
     };
-    let parser_lib_path = crate::runtime_dir().join("grammars");
+    let parser_lib_path = crate::runtime_dirs()
+        .first()
+        .expect("No runtime directories provided") // guaranteed by post-condition
+        .join("grammars");
     let mut library_path = parser_lib_path.join(&grammar.grammar_id);
     library_path.set_extension(DYLIB_EXTENSION);
 
@@ -511,9 +519,6 @@ fn mtime(path: &Path) -> Result<SystemTime> {
 /// Gives the contents of a file from a language's `runtime/queries/<lang>`
 /// directory
 pub fn load_runtime_file(language: &str, filename: &str) -> Result<String, std::io::Error> {
-    let path = crate::RUNTIME_DIR
-        .join("queries")
-        .join(language)
-        .join(filename);
+    let path = crate::runtime_file(&PathBuf::new().join("queries").join(language).join(filename));
     std::fs::read_to_string(&path)
 }

helix-loader/src/lib.rs

@@ -2,11 +2,12 @@ pub mod config;
 pub mod grammar;
 
 use etcetera::base_strategy::{choose_base_strategy, BaseStrategy};
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 
 pub const VERSION_AND_GIT_HASH: &str = env!("VERSION_AND_GIT_HASH");
 
-pub static RUNTIME_DIR: once_cell::sync::Lazy<PathBuf> = once_cell::sync::Lazy::new(runtime_dir);
+static RUNTIME_DIRS: once_cell::sync::Lazy<Vec<PathBuf>> =
+    once_cell::sync::Lazy::new(prioritize_runtime_dirs);
 
 static CONFIG_FILE: once_cell::sync::OnceCell<PathBuf> = once_cell::sync::OnceCell::new();
 
@@ -25,31 +26,83 @@ pub fn initialize_config_file(specified_file: Option<PathBuf>) {
     CONFIG_FILE.set(config_file).ok();
 }
 
-pub fn runtime_dir() -> PathBuf {
+/// A list of runtime directories from highest to lowest priority
+///
+/// The priority is:
+///
+/// 1. `HELIX_RUNTIME` (if environment variable is set)
+/// 2. sibling directory to `CARGO_MANIFEST_DIR` (if environment variable is set)
+/// 3. subdirectory of user config directory (always included)
+/// 4. subdirectory of path to helix executable (always included)
+///
+/// Postcondition: returns at least two paths (they might not exist).
+fn prioritize_runtime_dirs() -> Vec<PathBuf> {
+    // Adding higher priority first
+    let mut rt_dirs = Vec::new();
     if let Ok(dir) = std::env::var("HELIX_RUNTIME") {
-        return dir.into();
+        rt_dirs.push(dir.into());
     }
 
     if let Ok(dir) = std::env::var("CARGO_MANIFEST_DIR") {
         // this is the directory of the crate being run by cargo, we need the workspace path so we take the parent
         let path = std::path::PathBuf::from(dir).parent().unwrap().join(RT_DIR);
         log::debug!("runtime dir: {}", path.to_string_lossy());
-        return path;
+        rt_dirs.push(path);
     }
 
     const RT_DIR: &str = "runtime";
-    let conf_dir = config_dir().join(RT_DIR);
-    if conf_dir.exists() {
-        return conf_dir;
-    }
+    let conf_rt_dir = config_dir().join(RT_DIR);
+    rt_dirs.push(conf_rt_dir);
 
     // fallback to location of the executable being run
     // canonicalize the path in case the executable is symlinked
-    std::env::current_exe()
+    let exe_rt_dir = std::env::current_exe()
         .ok()
         .and_then(|path| std::fs::canonicalize(path).ok())
         .and_then(|path| path.parent().map(|path| path.to_path_buf().join(RT_DIR)))
-        .unwrap()
+        .unwrap();
+    rt_dirs.push(exe_rt_dir);
+    rt_dirs
+}
+
+/// Runtime directories ordered from highest to lowest priority
+///
+/// All directories should be checked when looking for files.
+///
+/// Postcondition: returns at least one path (it might not exist).
+pub fn runtime_dirs() -> &'static [PathBuf] {
+    &RUNTIME_DIRS
+}
+
+/// Find file with path relative to runtime directory
+///
+/// `rel_path` should be the relative path from within the `runtime/` directory.
+/// The valid runtime directories are searched in priority order and the first
+/// file found to exist is returned, otherwise None.
+fn find_runtime_file<P: AsRef<Path>>(rel_path: P) -> Option<PathBuf> {
+    RUNTIME_DIRS.iter().find_map(|rt_dir| {
+        let path = rt_dir.join(rel_path.as_ref());
+        if path.exists() {
+            Some(path)
+        } else {
+            None
+        }
+    })
+}
+
+/// Find file with path relative to runtime directory
+///
+/// `rel_path` should be the relative path from within the `runtime/` directory.
+/// The valid runtime directories are searched in priority order and the first
+/// file found to exist is returned, otherwise the path to the final attempt
+/// that failed.
+pub fn runtime_file<P: AsRef<Path>>(rel_path: P) -> PathBuf {
+    find_runtime_file(&rel_path).unwrap_or_else(|| {
+        RUNTIME_DIRS
+            .last()
+            .and_then(|dir| Some(dir.join(rel_path.as_ref())))
+            .unwrap_or_default()
+    })
 }
 
 pub fn config_dir() -> PathBuf {

helix-term/src/application.rs

@@ -135,10 +135,9 @@ impl Application {
 
         use helix_view::editor::Action;
 
-        let theme_loader = std::sync::Arc::new(theme::Loader::new(
-            &helix_loader::config_dir(),
-            &helix_loader::runtime_dir(),
-        ));
+        let mut theme_parent_dirs = vec![helix_loader::config_dir()];
+        theme_parent_dirs.extend(helix_loader::runtime_dirs().iter().cloned());
+        let theme_loader = std::sync::Arc::new(theme::Loader::new(&theme_parent_dirs));
 
         let true_color = config.editor.true_color || crate::true_color();
         let theme = config
@@ -184,7 +183,7 @@ impl Application {
         compositor.push(editor_view);
 
         if args.load_tutor {
-            let path = helix_loader::runtime_dir().join("tutor");
+            let path = helix_loader::runtime_file("tutor");
             editor.open(&path, Action::VerticalSplit)?;
             // Unset path to prevent accidentally saving to the original tutor file.
             doc_mut!(editor).set_path(None)?;

helix-term/src/commands/typed.rs

@@ -1406,7 +1406,7 @@ fn tutor(
         return Ok(());
     }
 
-    let path = helix_loader::runtime_dir().join("tutor");
+    let path = helix_loader::runtime_file("tutor");
     cx.editor.open(&path, Action::Replace)?;
     // Unset path to prevent accidentally saving to the original tutor file.
     doc_mut!(cx.editor).set_path(None)?;

helix-term/src/health.rs

@@ -52,7 +52,7 @@ pub fn general() -> std::io::Result<()> {
     let config_file = helix_loader::config_file();
     let lang_file = helix_loader::lang_config_file();
     let log_file = helix_loader::log_file();
-    let rt_dir = helix_loader::runtime_dir();
+    let rt_dirs = helix_loader::runtime_dirs();
     let clipboard_provider = get_clipboard_provider();
 
     if config_file.exists() {
@@ -66,17 +66,24 @@ pub fn general() -> std::io::Result<()> {
         writeln!(stdout, "Language file: default")?;
     }
     writeln!(stdout, "Log file: {}", log_file.display())?;
-    writeln!(stdout, "Runtime directory: {}", rt_dir.display())?;
-
-    if let Ok(path) = std::fs::read_link(&rt_dir) {
-        let msg = format!("Runtime directory is symlinked to {}", path.display());
-        writeln!(stdout, "{}", msg.yellow())?;
-    }
-    if !rt_dir.exists() {
-        writeln!(stdout, "{}", "Runtime directory does not exist.".red())?;
+    let msg = format!("Number of Runtime directories: {}", rt_dirs.len());
+    if rt_dirs.len() == 0 {
+        writeln!(stdout, "{}", msg.red())?;
+    } else {
+        writeln!(stdout, "{}", msg)?;
     }
-    if rt_dir.read_dir().ok().map(|it| it.count()) == Some(0) {
-        writeln!(stdout, "{}", "Runtime directory is empty.".red())?;
+    for rt_dir in rt_dirs {
+        writeln!(stdout, "Runtime directory: {}", rt_dir.display())?;
+        if let Ok(path) = std::fs::read_link(&rt_dir) {
+            let msg = format!("Runtime directory is symlinked to {}", path.display());
+            writeln!(stdout, "{}", msg.yellow())?;
+        }
+        if !rt_dir.exists() {
+            writeln!(stdout, "{}", "Runtime directory does not exist.".red())?;
+        }
+        if rt_dir.read_dir().ok().map(|it| it.count()) == Some(0) {
+            writeln!(stdout, "{}", "Runtime directory is empty.".red())?;
+        }
     }
     writeln!(stdout, "Clipboard provider: {}", clipboard_provider.name())?;
 

helix-term/src/ui/mod.rs

@@ -281,10 +281,10 @@ pub mod completers {
     }
 
     pub fn theme(_editor: &Editor, input: &str) -> Vec<Completion> {
-        let mut names = theme::Loader::read_names(&helix_loader::runtime_dir().join("themes"));
-        names.extend(theme::Loader::read_names(
-            &helix_loader::config_dir().join("themes"),
-        ));
+        let mut names = theme::Loader::read_names(&helix_loader::config_dir().join("themes"));
+        for rt_dir in helix_loader::runtime_dirs() {
+            names.extend(theme::Loader::read_names(&rt_dir.join("themes")));
+        }
         names.push("default".into());
         names.push("base16_default".into());
         names.sort();

helix-view/src/theme.rs

@@ -1,5 +1,5 @@
 use std::{
-    collections::HashMap,
+    collections::{HashMap, HashSet},
     path::{Path, PathBuf},
 };
 
@@ -35,19 +35,21 @@ pub static BASE16_DEFAULT_THEME: Lazy<Theme> = 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<PathBuf>,
 }
 impl Loader {
-    /// Creates a new loader that can load themes from two directories.
-    pub fn new<P: AsRef<Path>>(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<Theme> {
         if name == "default" {
             return Ok(self.default());
@@ -56,24 +58,28 @@ 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(),
             ..theme
         })
     }
 
-    // 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_them_name: &str,
-        only_default_dir: bool,
-    ) -> Result<Value> {
-        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<PathBuf>) -> Result<Value> {
+        let path = self
+            .path(name, visited_paths)
+            .ok_or_else(|| anyhow!("Theme: not found or recursively inheriting: {}", name))?;
         let theme_toml = self.load_toml(path)?;
 
         let inherits = theme_toml.get("inherits");
@@ -90,11 +96,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_them_name,
-                    base_them_name == parent_theme_name,
-                )?,
+                _ => self.load_theme(parent_theme_name, visited_paths)?,
             };
 
             self.merge_themes(parent_theme_toml, theme_toml)
@@ -146,32 +148,37 @@ 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<Value> {
         let data = std::fs::read(&path)?;
         let value = toml::from_slice(data.as_slice())?;
 
         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<PathBuf>) -> Option<PathBuf> {
         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)
-        }
+        self.theme_dirs.iter().find_map(|dir| {
+            let path = dir.join(&filename);
+            if path.exists() && !visited_paths.contains(&path) {
+                visited_paths.insert(path.clone());
+                Some(path)
+            } else {
+                None
+            }
+        })
     }
 
-    /// Lists all theme names available in default and user directory
+    /// Lists all theme names available in all directories
     pub fn names(&self) -> Vec<String> {
-        let mut names = Self::read_names(&self.user_dir);
-        names.extend(Self::read_names(&self.default_dir));
+        let mut names = Vec::new();
+        for dir in &self.theme_dirs {
+            names.extend(Self::read_names(dir));
+        }
         names
     }