Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No longer merge config files by default, use priorities instead #790

Merged
merged 4 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 19 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -231,6 +232,9 @@ Options:

[env: TOPIARY_CONFIG_FILE]

-M, --merge-configuration
Enable merging for configuration files

-v, --verbose...
Logging verbosity (increased per occurrence)

Expand Down Expand Up @@ -283,6 +287,9 @@ Options:

[env: TOPIARY_CONFIG_FILE]

-M, --merge-configuration
Enable merging for configuration files

-v, --verbose...
Logging verbosity (increased per occurrence)

Expand Down Expand Up @@ -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
```
Expand All @@ -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
```
Expand All @@ -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
```
Expand Down Expand Up @@ -399,6 +409,9 @@ Options:

[env: TOPIARY_CONFIG_FILE]

-M, --merge-configuration
Enable merging for configuration files

-v, --verbose...
Logging verbosity (increased per occurrence)

Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions topiary-cli/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 4 additions & 1 deletion topiary-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
28 changes: 23 additions & 5 deletions topiary-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
43 changes: 36 additions & 7 deletions topiary-config/src/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(),
Expand All @@ -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
Expand Down
Loading