From 4327d8accb75e3be09c5fa849b7c8dd25b967aec Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Tue, 16 Jun 2026 10:46:11 +0100 Subject: [PATCH 1/3] Reapply "Allow setting `--debug-model` flag to false" This reverts commit b12a8f312ee9ce2d799a19adc7ddf4ce52ea4bf2. --- src/cli.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 1c85cf56f..37367ce0e 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -45,9 +45,8 @@ pub struct RunOpts { #[arg(long)] pub overwrite: bool, /// Whether to write additional information to CSV files - #[arg(long)] - pub debug_model: bool, - + #[arg(long, value_name = "BOOL", num_args = 0..=1, default_missing_value = "true")] + pub debug_model: Option, /// Whether to skip copying input files to the output folder #[arg(long)] pub no_copy_input_files: bool, @@ -140,8 +139,8 @@ 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 let Some(opt) = opts.debug_model { + settings.debug_model = opt; } if opts.overwrite { settings.overwrite = true; From 417a640df51bb5e67c948ca3cc0adb06d4f3287a Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Tue, 16 Jun 2026 10:50:08 +0100 Subject: [PATCH 2/3] Make `--no-copy-input-files` flag settable with `--copy-input-files=false` This is for consistency, to make it easier to unify CLI and settings file interfaces. --- src/cli.rs | 16 ++++++++-------- tests/cli.rs | 4 ++-- tests/regenerate_test_data.sh | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 37367ce0e..acd13fd90 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -47,9 +47,9 @@ pub struct RunOpts { /// Whether to write additional information to CSV files #[arg(long, value_name = "BOOL", num_args = 0..=1, default_missing_value = "true")] pub debug_model: Option, - /// Whether to skip copying input files to the output folder - #[arg(long)] - pub no_copy_input_files: bool, + /// Whether to copy input files to the output folder + #[arg(long, value_name = "BOOL", num_args = 0..=1, default_missing_value = "true")] + pub copy_input_files: Option, } /// Options for the `graph` command @@ -139,14 +139,14 @@ 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 let Some(opt) = opts.debug_model { - settings.debug_model = opt; - } if opts.overwrite { settings.overwrite = true; } - if opts.no_copy_input_files { - settings.copy_input_files = false; + if let Some(opt) = opts.debug_model { + settings.debug_model = opt; + } + if let Some(opt) = opts.copy_input_files { + settings.copy_input_files = opt; } // Get path to output folder 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 From b7168b234b2ed8cb123660d69af47807f88165a9 Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Tue, 16 Jun 2026 11:20:51 +0100 Subject: [PATCH 3/3] Define macro for unifying options across CLI and settings file Closes #1284. --- src/cli.rs | 38 ++++-------- src/settings.rs | 158 +++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 144 insertions(+), 52 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index acd13fd90..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,15 +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, value_name = "BOOL", num_args = 0..=1, default_missing_value = "true")] - pub debug_model: Option, - /// Whether to copy input files to the output folder - #[arg(long, value_name = "BOOL", num_args = 0..=1, default_missing_value = "true")] - pub copy_input_files: Option, + /// Settings that can be overridden on the command line + #[command(flatten)] + pub overrides: SettingsOverrides, } /// Options for the `graph` command @@ -59,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. @@ -138,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.overwrite { - settings.overwrite = true; - } - if let Some(opt) = opts.debug_model { - settings.debug_model = opt; - } - if let Some(opt) = opts.copy_input_files { - settings.copy_input_files = opt; - } + // Command-line arguments take precedence over the settings file + settings.apply_overrides(&opts.overrides); // Get path to output folder let pathbuf: PathBuf; @@ -219,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); + } }