diff --git a/src/cli.rs b/src/cli.rs index 1c85cf56f..9b718658d 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -3,7 +3,7 @@ use crate::graph::save_commodity_graphs_for_model; use crate::input::{load_commodity_graphs, load_model}; use crate::log; use crate::output::{copy_input_files, create_output_directory, get_graphs_dir, get_output_dir}; -use crate::settings::Settings; +use crate::settings::{Settings, SettingsOverrides}; use ::log::{info, warn}; use anyhow::{Context, Result}; use clap::{Args, CommandFactory, Parser, Subcommand}; @@ -41,16 +41,9 @@ pub struct RunOpts { /// Directory for output files #[arg(short, long)] pub output_dir: Option, - /// Whether to overwrite the output directory if it already exists - #[arg(long)] - pub overwrite: bool, - /// Whether to write additional information to CSV files - #[arg(long)] - pub debug_model: bool, - - /// Whether to skip copying input files to the output folder - #[arg(long)] - pub no_copy_input_files: bool, + /// Settings that can be overridden on the command line + #[command(flatten)] + pub overrides: SettingsOverrides, } /// Options for the `graph` command @@ -60,8 +53,8 @@ pub struct GraphOpts { #[arg(short, long)] pub output_dir: Option, /// Whether to overwrite the output directory if it already exists - #[arg(long)] - pub overwrite: bool, + #[arg(long, value_name = "BOOL", num_args = 0..=1, require_equals = true, default_missing_value = "true")] + pub overwrite: Option, } /// The available commands. @@ -139,16 +132,8 @@ pub fn run_cli() -> Result<()> { pub fn handle_run_command(model_path: &Path, opts: &RunOpts) -> Result<()> { let mut settings = Settings::load_or_default().context("Failed to load settings.")?; - // These settings can be overridden by command-line arguments - if opts.debug_model { - settings.debug_model = true; - } - if opts.overwrite { - settings.overwrite = true; - } - if opts.no_copy_input_files { - settings.copy_input_files = false; - } + // Command-line arguments take precedence over the settings file + settings.apply_overrides(&opts.overrides); // Get path to output folder let pathbuf: PathBuf; @@ -220,9 +205,11 @@ pub fn handle_validate_command(model_path: &Path) -> Result<()> { pub fn handle_save_graphs_command(model_path: &Path, opts: &GraphOpts) -> Result<()> { let mut settings = Settings::load_or_default().context("Failed to load settings.")?; - if opts.overwrite { - settings.overwrite = true; - } + // Command-line arguments take precedence over the settings file + settings.apply_overrides(&SettingsOverrides { + overwrite: opts.overwrite, + ..Default::default() + }); // Get path to output folder let pathbuf: PathBuf; diff --git a/src/settings.rs b/src/settings.rs index 6697b9416..1ce0183cc 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -3,6 +3,7 @@ use crate::get_muse2_config_dir; use crate::input::read_toml; use crate::log::DEFAULT_LOG_LEVEL; use anyhow::Result; +use clap::Args; use documented::DocumentedFields; use serde::{Deserialize, Serialize}; use std::env; @@ -35,38 +36,117 @@ pub fn get_settings_file_path() -> PathBuf { path } -/// Program settings from config file +/// Defines the program [`Settings`] together with their defaults and which of them can be +/// overridden from the command line. /// -/// NOTE: If you add or change a field in this struct, you must also update the schema in -/// `schemas/settings.yaml`. -#[derive(Debug, DocumentedFields, Serialize, Deserialize, PartialEq)] -#[serde(default)] -pub struct Settings { +/// Each setting is declared exactly once: its doc comment, type and default value. Appending +/// `, cli` to a setting generates a matching `Option` field on [`SettingsOverrides`] +/// (reusing the doc comment as the flag's help text) and a merge step in +/// [`Settings::apply_overrides`]. Settings without `cli` can only be set via `settings.toml`. +macro_rules! settings { + ( + $( + $(#[doc = $doc:literal])+ + $field:ident: $ty:ty = $default:expr $(, $cli:ident)? ; + )+ + ) => { + /// Program settings, resolved from the defaults, the settings file and CLI overrides. + /// + /// NOTE: If you add or change a field here, you must also update the schema in + /// `schemas/settings.yaml`. + #[derive(Debug, DocumentedFields, Serialize, Deserialize, PartialEq)] + #[serde(default)] + pub struct Settings { + $( + $(#[doc = $doc])+ + pub $field: $ty, + )+ + } + + impl Default for Settings { + fn default() -> Self { + Self { + $( $field: $default, )+ + } + } + } + + settings!(@overrides { } { } + $( [ $field { $(#[doc = $doc])+ } $($cli)? ] )+ + ); + }; + + // Overridable setting: add an `Option` flag and a merge step. + (@overrides + { $($fields:tt)* } + { $($applies:tt)* } + [ $field:ident { $(#[doc = $doc:literal])+ } cli ] + $($rest:tt)* + ) => { + settings!(@overrides + { + $($fields)* + $(#[doc = $doc])+ + #[arg(long, value_name = "BOOL", num_args = 0..=1, require_equals = true, default_missing_value = "true")] + pub $field: Option, + } + { $($applies)* ($field) } + $($rest)* + ); + }; + + // File-only setting: skip it. + (@overrides + { $($fields:tt)* } + { $($applies:tt)* } + [ $field:ident { $(#[doc = $doc:literal])+ } ] + $($rest:tt)* + ) => { + settings!(@overrides { $($fields)* } { $($applies)* } $($rest)*); + }; + + // No settings left: emit the overrides struct and the merge method. + (@overrides { $($fields:tt)* } { $( ($afield:ident) )* }) => { + /// Settings that can be overridden via command-line arguments. + /// + /// A field is `Some` only when the corresponding flag was passed; otherwise the value from + /// the settings file (or default) is kept. + #[derive(Debug, Default, Args)] + pub struct SettingsOverrides { + $($fields)* + } + + impl Settings { + /// Apply command-line `overrides` on top of these settings. + pub fn apply_overrides(&mut self, overrides: &SettingsOverrides) { + $( + if let Some(value) = overrides.$afield { + self.$afield = value; + } + )* + } + } + }; +} + +settings! { /// The default program log level - pub log_level: String, - /// Whether to overwrite output files by default - pub overwrite: bool, - /// Whether to write additional information to CSV files - pub debug_model: bool, + log_level: String = DEFAULT_LOG_LEVEL.to_string(); + /// Results root path to save MUSE2 results. Defaults to `muse2_results`. - pub results_root: PathBuf, + results_root: PathBuf = PathBuf::from("muse2_results"); + /// Results root path to save MUSE2 graph outputs. Defaults to `muse2_graphs`. - pub graph_results_root: PathBuf, - /// Whether to copy input files to the output folder. - pub copy_input_files: bool, -} + graph_results_root: PathBuf = PathBuf::from("muse2_graphs"); -impl Default for Settings { - fn default() -> Self { - Self { - log_level: DEFAULT_LOG_LEVEL.to_string(), - overwrite: false, - debug_model: false, - results_root: PathBuf::from("muse2_results"), - graph_results_root: PathBuf::from("muse2_graphs"), - copy_input_files: true, - } - } + /// Whether to overwrite output files + overwrite: bool = false, cli; + + /// Whether to write additional information to CSV files + debug_model: bool = false, cli; + + /// Whether to copy input files to the output folder + copy_input_files: bool = true, cli; } impl Settings { @@ -129,6 +209,7 @@ impl Settings { #[cfg(test)] mod tests { use super::*; + use rstest::rstest; use std::fs::File; use std::io::Write; use tempfile::tempdir; @@ -166,4 +247,27 @@ mod tests { fn default_file_contents() { assert!(!Settings::default_file_contents().is_empty()); } + + /// A CLI override should always take precedence over the settings-file value, including the + /// case where the user explicitly opts out of an option that is enabled in the file. + #[rstest] + #[case(false, None, false)] // no flag passed: keep the file value + #[case(true, None, true)] // no flag passed: keep the file value + #[case(false, Some(true), true)] // --overwrite (=true): opt in + #[case(true, Some(false), false)] // --overwrite=false: opt out + fn apply_overrides_takes_precedence( + #[case] file_value: bool, + #[case] cli_value: Option, + #[case] expected: bool, + ) { + let mut settings = Settings { + overwrite: file_value, + ..Settings::default() + }; + settings.apply_overrides(&SettingsOverrides { + overwrite: cli_value, + ..Default::default() + }); + assert_eq!(settings.overwrite, expected); + } } diff --git a/tests/cli.rs b/tests/cli.rs index 4cd5ee69a..c439de2a2 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -85,7 +85,7 @@ fn check_example_info_command() { } #[test] -fn check_no_copy_input_files_flag() { +fn check_copy_input_files_flag() { let tempdir = tempdir().unwrap(); let output_dir = tempdir.path().join("results"); assert_muse2_runs(&[ @@ -93,7 +93,7 @@ fn check_no_copy_input_files_flag() { MODEL_DIR, "--output-dir", &output_dir.to_string_lossy(), - "--no-copy-input-files", + "--copy-input-files=false", ]); // Check that input files were not copied to the output directory diff --git a/tests/regenerate_test_data.sh b/tests/regenerate_test_data.sh index 6387f0ab1..299d1cd9b 100755 --- a/tests/regenerate_test_data.sh +++ b/tests/regenerate_test_data.sh @@ -27,7 +27,7 @@ run_example() { if ! env MUSE2_LOG_LEVEL=error MUSE2_USE_DEFAULT_SETTINGS=1 \ cargo -q run example run -o "data/$example" "$example" \ - --overwrite --no-copy-input-files $@; then + --overwrite --copy-input-files=false $@; then echo ERROR: Failed to run example "$example" > /dev/stderr failed=1 fi