Merge pull request #790 from tweag/nb/config-rework

No longer merge config files by default, use priorities instead

CHANGELOG.md

@@ -51,6 +51,7 @@ This name should be decided amongst the team before the release.
 
 ### Changed
 - [#780](https://github.com/tweag/topiary/pull/780) Measuring scopes are now independent from captures order
+- [#790](https://github.com/tweag/topiary/pull/790) No longer merge config files by default, use priority instead.
 
 ### Fixed
 - [#779](https://github.com/tweag/topiary/pull/779) Load relevant grammars before CLI tests

README.md

@@ -191,6 +191,7 @@ Commands:
 
 Options:
   -C, --configuration <CONFIGURATION>  Configuration file [env: TOPIARY_CONFIG_FILE]
+  -M, --merge-configuration            Enable merging for configuration files
   -v, --verbose...                     Logging verbosity (increased per occurrence)
   -h, --help                           Print help
   -V, --version                        Print version
@@ -231,6 +232,9 @@ Options:
 
           [env: TOPIARY_CONFIG_FILE]
 
+  -M, --merge-configuration
+          Enable merging for configuration files
+
   -v, --verbose...
           Logging verbosity (increased per occurrence)
 
@@ -283,6 +287,9 @@ Options:
 
           [env: TOPIARY_CONFIG_FILE]
 
+  -M, --merge-configuration
+          Enable merging for configuration files
+
   -v, --verbose...
           Logging verbosity (increased per occurrence)
 
@@ -310,6 +317,7 @@ Usage: topiary config [OPTIONS]
 
 Options:
   -C, --configuration <CONFIGURATION>  Configuration file [env: TOPIARY_CONFIG_FILE]
+  -M, --merge-configuration            Enable merging for configuration files
   -v, --verbose...                     Logging verbosity (increased per occurrence)
   -h, --help                           Print help
 ```
@@ -336,6 +344,7 @@ Arguments:
 
 Options:
   -C, --configuration <CONFIGURATION>  Configuration file [env: TOPIARY_CONFIG_FILE]
+  -M, --merge-configuration            Enable merging for configuration files
   -v, --verbose...                     Logging verbosity (increased per occurrence)
   -h, --help                           Print help
 ```
@@ -362,6 +371,7 @@ Usage: topiary prefetch [OPTIONS]
 
 Options:
   -C, --configuration <CONFIGURATION>  Configuration file [env: TOPIARY_CONFIG_FILE]
+  -M, --merge-configuration            Enable merging for configuration files
   -v, --verbose...                     Logging verbosity (increased per occurrence)
   -h, --help                           Print help
 ```
@@ -399,6 +409,9 @@ Options:
 
           [env: TOPIARY_CONFIG_FILE]
 
+  -M, --merge-configuration
+          Enable merging for configuration files
+
   -v, --verbose...
           Logging verbosity (increased per occurrence)
 
@@ -567,9 +580,12 @@ For usage in Nix, a `languages_nix.ncl` file is provided that specifies the
 paths of every language using the `@nickel@` syntax. These can easily be
 replaced with nixpkgs' `substituteAll`.
 
-### Overriding
-If one of the sources listed above attempts to define a language configuration
-already present in the builtin configuration, Topiary will display a Nickel error.
+### Overriding with `--merge-configuration`
+
+By default, Topiary only considers the configuration file with the [highest priority](#configuration-sources). However, if the `-M` or `--merge-configuration` option is provided to the CLI, then all available configurations are merged together, as per the [Nickel specification](https://nickel-lang.org/user-manual/merging).
+
+In that case, if one of the sources listed above attempts to define a language configuration
+already present in the builtin configuration, or if two configuration files have conflicting values, then Topiary will display a Nickel error.
 
 To understand why, one can read the [Nickel documentation on Merging](https://nickel-lang.org/user-manual/merging).
 The short answer is that a priority must be defined. The builtin configuration

topiary-cli/src/cli.rs

@@ -40,6 +40,10 @@ pub struct GlobalArgs {
     )]
     pub configuration: Option<PathBuf>,
 
+    /// Enable merging for configuration files
+    #[arg(short = 'M', long, display_order = 101, global = true)]
+    pub merge_configuration: bool,
+
     /// Logging verbosity (increased per occurrence)
     #[arg(
         short,

topiary-cli/src/main.rs

@@ -35,7 +35,10 @@ async fn main() -> ExitCode {
 async fn run() -> CLIResult<()> {
     let args = cli::get_args()?;
 
-    let config = topiary_config::Configuration::fetch(&args.global.configuration)?;
+    let config = topiary_config::Configuration::fetch(
+        args.global.merge_configuration,
+        &args.global.configuration,
+    )?;
 
     // Delegate by subcommand
     match args.command {

topiary-config/src/lib.rs

@@ -51,19 +51,27 @@ impl Configuration {
     /// with the path that was not found.
     /// If the configuration file exists, but cannot be parsed, this function will return a
     /// `TopiaryConfigError` with the error that occurred.
-    pub fn fetch(file: &Option<PathBuf>) -> TopiaryConfigResult<Self> {
+    pub fn fetch(merge: bool, file: &Option<PathBuf>) -> TopiaryConfigResult<Self> {
         // If we have an explicit file, fail if it doesn't exist
         if let Some(path) = file {
             if !path.exists() {
                 return Err(TopiaryConfigError::FileNotFound(path.to_path_buf()));
             }
         }
 
-        // Otherwise, gather a list of all the files we want to look for
-        let sources: Vec<Source> = Source::fetch(file);
+        if merge {
+            // Get all available configuration sources
+            let sources: Vec<Source> = Source::fetch_all(file);
 
-        // And ask nickel to parse and merge them
-        Self::parse_and_merge(&sources)
+            // And ask Nickel to parse and merge them
+            Self::parse_and_merge(&sources)
+        } else {
+            // Get the available configuration with best priority
+            let source: Source = Source::fetch_one(file);
+
+            // And parse it with Nickel
+            Self::parse(source)
+        }
     }
 
     /// Gets a language configuration from the entire configuration.
@@ -164,6 +172,16 @@ impl Configuration {
 
         Ok(serde_config.into())
     }
+
+    fn parse(source: Source) -> TopiaryConfigResult<Self> {
+        let mut program = Program::<CacheImpl>::new_from_input(source.into(), std::io::stderr())?;
+
+        let term = program.eval_full_for_export()?;
+
+        let serde_config = SerdeConfiguration::deserialize(term)?;
+
+        Ok(serde_config.into())
+    }
 }
 
 impl Default for Configuration {

topiary-config/src/source.rs

@@ -4,7 +4,7 @@ use std::{env::current_dir, ffi::OsString, fmt, io::Cursor, path::PathBuf};
 
 use crate::error::TopiaryConfigError;
 
-/// Sources of TOML configuration
+/// Sources of Nickel configuration
 #[derive(Debug, Clone)]
 pub enum Source {
     Builtin,
@@ -21,13 +21,13 @@ impl From<Source> for nickel_lang_core::program::Input<Cursor<String>, OsString>
 }
 
 impl Source {
-    /// Return the valid sources of configuration, in priority order (lowest to highest):
+    /// Return the valid sources of configuration, in priority order (highest to lowest):
     ///
-    /// 1. Built-in configuration (per `Self::builtin_nickel()`)
-    /// 2. `~/.config/topiary/languages.ncl` (or equivalent)
-    /// 3. `.topiary/languages.ncl` (or equivalent)
-    /// 4. `file`, passed as a CLI argument/environment variable
-    pub fn fetch(file: &Option<PathBuf>) -> Vec<Self> {
+    /// 1. `file`, passed as a CLI argument/environment variable
+    /// 2. `.topiary/languages.ncl` (or equivalent)
+    /// 3. `~/.config/topiary/languages.ncl` (or equivalent)
+    /// 4. Built-in configuration (per `Self::builtin_nickel()`)
+    pub fn fetch_all(file: &Option<PathBuf>) -> Vec<Self> {
         let candidates = [
             Some(find_os_configuration_dir_config()),
             find_workspace_configuration_dir_config(),
@@ -46,6 +46,35 @@ impl Source {
         res
     }
 
+    /// Return the source of configuration that has top priority among available ones.
+    /// The priority order is, from highest to lowest:
+    ///
+    /// 1. `file`, passed as a CLI argument/environment variable
+    /// 2. `.topiary/languages.ncl` (or equivalent)
+    /// 3. `~/.config/topiary/languages.ncl` (or equivalent)
+    /// 4. Built-in configuration (per `Self::builtin_nickel()`)
+    pub fn fetch_one(file: &Option<PathBuf>) -> Self {
+        let cli_specified = Self::find(&file.clone()).map(Self::File);
+        let workspace_specified =
+            Self::find(&find_workspace_configuration_dir_config()).map(Self::File);
+        let os_config_specified =
+            Self::find(&Some(find_os_configuration_dir_config())).map(Self::File);
+
+        if let Some(res) = cli_specified {
+            log::info!("Using CLI-specified configuration: {res}");
+            res
+        } else if let Some(res) = workspace_specified {
+            log::info!("Using workspace-specified configuration: {res}");
+            res
+        } else if let Some(res) = os_config_specified {
+            log::info!("Using global os-specified configuration: {res}");
+            res
+        } else {
+            log::info!("Using built-in configuration");
+            Self::Builtin
+        }
+    }
+
     /// Attempts to find a configuration file, given a `path` parameter. If `path` is `None`, then
     /// the function returns `None`.
     /// Otherwise, if the path is a directory, then it attempts to find a `languages.ncl` file