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).

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.

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 {
-    if let Ok(dir) = std::env::var("HELIX_RUNTIME") {
-        return dir.into();
-    }
-
+/// A list of runtime directories from highest to lowest priority
+///
+/// The priority is:
+///
+/// 1. sibling directory to `CARGO_MANIFEST_DIR` (if environment variable is set)
+/// 2. subdirectory of user config directory (always included)
+/// 3. `HELIX_RUNTIME` (if environment variable is set)
+/// 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> {
+    const RT_DIR: &str = "runtime";
+    // Adding higher priority first
+    let mut rt_dirs = Vec::new();
     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);
+
+    if let Ok(dir) = std::env::var("HELIX_RUNTIME") {
+        rt_dirs.push(dir.into());
     }
 
     // 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()
+            .map(|dir| 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,19 @@ 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())?;
-    }
-    if rt_dir.read_dir().ok().map(|it| it.count()) == Some(0) {
-        writeln!(stdout, "{}", "Runtime directory is empty.".red())?;
+    writeln!(stdout, "Number of Runtime directories: {}", rt_dirs.len())?;
+    for (i, rt_dir) in rt_dirs.iter().enumerate() {
+        writeln!(stdout, "Runtime directory {}: {}", i + 1, 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
     }