diff --git a/build.rs b/build.rs index 696f7eed..5e9307b2 100644 --- a/build.rs +++ b/build.rs @@ -88,7 +88,7 @@ fn write_man_page(data: &[u8], dir: &Path, page_name: &str) -> std::io::Result

Result<(), Box> { +const fn verify_public_api_symbols() { // Exercise CLI localization, config merge, and host pattern symbols so the // shared modules remain linked when the build script is compiled without // tests. @@ -107,8 +107,9 @@ fn main() -> Result<(), Box> { const _: fn(&cli::Cli) -> bool = cli::Cli::progress_enabled; const _: fn(&str) -> Result = HostPattern::parse; const _: fn(&HostPattern, host_pattern::HostCandidate<'_>) -> bool = HostPattern::matches; +} - // Regenerate the manual page when the CLI or metadata changes. +fn emit_rerun_directives() { println!("cargo:rerun-if-changed=src/cli/mod.rs"); println!("cargo:rerun-if-changed=src/cli/config.rs"); println!("cargo:rerun-if-changed=src/cli/merge.rs"); @@ -125,13 +126,9 @@ fn main() -> Result<(), Box> { println!("cargo:rerun-if-changed=src/localization/keys.rs"); println!("cargo:rerun-if-changed=locales/en-US/messages.ftl"); println!("cargo:rerun-if-changed=locales/es-ES/messages.ftl"); +} - build_l10n_audit::audit_localization_keys()?; - - // Packagers expect man pages under target/generated-man//. - let out_dir = out_dir_for_target_profile(); - - // The top-level page documents the entire command interface. +fn generate_man_page(out_dir: &Path) -> Result<(), Box> { let cmd = cli::Cli::command(); let name = cmd .get_bin_name() @@ -149,7 +146,6 @@ fn main() -> Result<(), Box> { let version = env::var("CARGO_PKG_VERSION").map_err( |_| "CARGO_PKG_VERSION must be set by Cargo; cannot render manual page without it.", )?; - let man = Man::new(cmd) .section("1") .source(format!("{cargo_bin} {version}")) @@ -157,7 +153,7 @@ fn main() -> Result<(), Box> { let mut buf = Vec::new(); man.render(&mut buf)?; let page_name = format!("{cargo_bin}.1"); - write_man_page(&buf, &out_dir, &page_name)?; + write_man_page(&buf, out_dir, &page_name)?; if let Some(extra_dir) = env::var_os("OUT_DIR") { let extra_dir_path = PathBuf::from(extra_dir); if let Err(err) = write_man_page(&buf, &extra_dir_path, &page_name) { @@ -167,6 +163,13 @@ fn main() -> Result<(), Box> { ); } } - Ok(()) } + +fn main() -> Result<(), Box> { + verify_public_api_symbols(); + emit_rerun_directives(); + build_l10n_audit::audit_localization_keys()?; + let out_dir = out_dir_for_target_profile(); + generate_man_page(&out_dir) +} diff --git a/docs/execplans/3-10-1-guarantee-status-message-ordering.md b/docs/execplans/3-10-1-guarantee-status-message-ordering.md index 33674739..29fcb3c6 100644 --- a/docs/execplans/3-10-1-guarantee-status-message-ordering.md +++ b/docs/execplans/3-10-1-guarantee-status-message-ordering.md @@ -276,8 +276,7 @@ model provides the necessary ordering guarantees. `NINJA_STDERR_MARKER`) to avoid coupling to localized UI strings. 2. **Status messages do not contaminate stdout in standard mode**: Verifies - stream - routing in non-accessible mode using the same stable markers. + stream routing in non-accessible mode using the same stable markers. 3. **Build artifacts can be captured via stdout redirection**: Verifies that `netsuke manifest -` output goes to stdout without status contamination. @@ -285,12 +284,10 @@ model provides the necessary ordering guarantees. Supporting infrastructure added: - `FakeNinjaConfig` struct in `tests/bdd/steps/progress_output.rs` for - configurable - fixture generation with optional stderr markers. + configurable fixture generation with optional stderr markers. - `install_fake_ninja_with_config()` function for flexible fixture setup. - Updated `fake_ninja_emits_stdout_output` fixture to emit both stdout and - stderr - markers for comprehensive stream routing verification. + stderr markers for comprehensive stream routing verification. ### Stage E: Documentation (completed) diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index ef64855a..4990c38d 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -2034,12 +2034,11 @@ layered configuration lives in a dedicated `CliConfig` struct derived with OrthoConfig in `src/cli/config.rs`. The top-level `src/cli/mod.rs` module re-exports that public CLI surface. This separation keeps parsing, configuration discovery, and runtime command selection as distinct concerns -while preserving the existing command syntax. -Invoking `netsuke` with no explicit subcommand still resolves to `build`, and -the `build` command can now take default `emit` and `targets` values from -`[cmds.build]` in configuration files or `NETSUKE_CMDS__BUILD__*` environment -variables. Explicit CLI targets or `--emit` values still override those -defaults. +while preserving the existing command syntax. Invoking `netsuke` with no +explicit subcommand still resolves to `build`, and the `build` command can now +take default `emit` and `targets` values from `[cmds.build]` in configuration +files or `NETSUKE_CMDS__BUILD__*` environment variables. Explicit CLI targets +or `--emit` values still override those defaults. Configuration is layered in the order defaults -> configuration files -> environment variables -> CLI overrides. Discovery honours `NETSUKE_CONFIG_PATH` diff --git a/src/cli/config.rs b/src/cli/config.rs index 1373fb73..587fc55f 100644 --- a/src/cli/config.rs +++ b/src/cli/config.rs @@ -4,6 +4,7 @@ //! and merging. It captures global CLI settings plus per-subcommand defaults //! under the `cmds` namespace. +use clap::ValueEnum; use ortho_config::{OrthoConfig, OrthoResult, PostMergeContext, PostMergeHook}; use serde::{Deserialize, Serialize}; use std::path::PathBuf; @@ -12,7 +13,7 @@ use super::validation_error; use crate::host_pattern::HostPattern; /// Colour-output policy accepted by layered configuration. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, ValueEnum, Default)] #[serde(rename_all = "kebab-case")] pub enum ColourPolicy { /// Follow the host environment. @@ -25,7 +26,7 @@ pub enum ColourPolicy { } /// Spinner and progress rendering policy. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, ValueEnum, Default)] #[serde(rename_all = "kebab-case")] pub enum SpinnerMode { /// Follow Netsuke's default progress behaviour. @@ -38,7 +39,7 @@ pub enum SpinnerMode { } /// Top-level diagnostics and output format. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, ValueEnum, Default)] #[serde(rename_all = "kebab-case")] pub enum OutputFormat { /// Human-readable terminal output. @@ -49,7 +50,7 @@ pub enum OutputFormat { } /// Presentation theme for semantic prefixes and glyph choices. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, ValueEnum, Default)] #[serde(rename_all = "kebab-case")] pub enum Theme { /// Follow the host environment. diff --git a/src/cli/merge.rs b/src/cli/merge.rs index 99b84b9c..03105b80 100644 --- a/src/cli/merge.rs +++ b/src/cli/merge.rs @@ -155,6 +155,10 @@ fn cli_overrides_from_matches(cli: &Cli, matches: &ArgMatches) -> OrthoResult, - /// Resolved colour policy from layered configuration. - #[arg(skip)] + /// Override colour policy for terminal output. + #[arg(long, value_name = "POLICY")] pub colour_policy: Option, - /// Resolved spinner mode from layered configuration. - #[arg(skip)] + /// Override spinner animation mode. + #[arg(long, value_name = "MODE")] pub spinner_mode: Option, - /// Resolved output format from layered configuration. - #[arg(skip)] + /// Override output format style. + #[arg(long, value_name = "FORMAT")] pub output_format: Option, - /// Resolved presentation theme from layered configuration. - #[arg(skip)] + /// Override presentation theme. + #[arg(long, value_name = "THEME")] pub theme: Option, /// Optional subcommand to execute; defaults to `build` when omitted. diff --git a/tests/bdd/steps/configuration_preferences.rs b/tests/bdd/steps/configuration_preferences.rs index 11d14fad..a245c7c7 100644 --- a/tests/bdd/steps/configuration_preferences.rs +++ b/tests/bdd/steps/configuration_preferences.rs @@ -4,7 +4,8 @@ use crate::bdd::fixtures::{RefCellOptionExt, TestWorld}; use crate::bdd::helpers::parse_store::store_parse_outcome; use crate::bdd::helpers::tokens::build_tokens; use anyhow::{Context, Result, anyhow, bail, ensure}; -use netsuke::cli::{Cli, Commands, Theme}; +use clap::ValueEnum as _; +use netsuke::cli::{Cli, ColourPolicy, Commands, OutputFormat, SpinnerMode, Theme}; use netsuke::cli_localization; use netsuke::output_prefs; use rstest_bdd_macros::{given, then, when}; @@ -39,7 +40,7 @@ fn write_config(world: &TestWorld, contents: &str) -> Result<()> { fs::write(&path, contents).with_context(|| format!("write {}", path.display()))?; let previous = std::env::var_os(CONFIG_ENV_VAR); // SAFETY: `EnvLock` is held in `world.env_lock` for the lifetime of the scenario. - unsafe { std::env::set_var(CONFIG_ENV_VAR, OsStr::new(path.as_os_str())) }; + unsafe { std::env::set_var(CONFIG_ENV_VAR, path.as_os_str()) }; world.track_env_var(CONFIG_ENV_VAR.to_owned(), previous); Ok(()) } @@ -61,6 +62,93 @@ fn merge_cli(world: &TestWorld, args: &str) { store_parse_outcome(&world.cli, &world.cli_error, outcome); } +/// Convert a clap `ValueEnum` to its canonical name string. +/// +/// # Panics +/// +/// Panics if the enum variant does not have a possible value (which should +/// never happen for well-formed `ValueEnum` implementations). +fn enum_name(value: &T) -> String { + value + .to_possible_value() + .unwrap_or_else(|| panic!("all ValueEnum variants must have a possible value")) + .get_name() + .to_owned() +} + +/// Write a theme value to the config file. +fn write_theme_config(world: &TestWorld, theme: Theme) -> Result<()> { + write_config(world, &format!("theme = \"{}\"\n", enum_name(&theme))) +} + +/// Write a colour policy value to the config file. +fn write_colour_policy_config(world: &TestWorld, policy: ColourPolicy) -> Result<()> { + write_config( + world, + &format!("colour_policy = \"{}\"\n", enum_name(&policy)), + ) +} + +/// Write a spinner mode value to the config file. +fn write_spinner_mode_config(world: &TestWorld, mode: SpinnerMode) -> Result<()> { + write_config(world, &format!("spinner_mode = \"{}\"\n", enum_name(&mode))) +} + +/// Write an output format value to the config file. +fn write_output_format_config(world: &TestWorld, format: OutputFormat) -> Result<()> { + write_config( + world, + &format!("output_format = \"{}\"\n", enum_name(&format)), + ) +} + +/// Set the `NETSUKE_THEME` environment variable. +fn set_env_theme(world: &TestWorld, theme: Theme) { + ensure_env_lock(world); + let previous = std::env::var_os("NETSUKE_THEME"); + let value = enum_name(&theme); + // SAFETY: `EnvLock` is held in `world.env_lock` for the lifetime of the scenario. + unsafe { std::env::set_var("NETSUKE_THEME", OsStr::new(&value)) }; + world.track_env_var("NETSUKE_THEME".to_owned(), previous); +} + +/// Set the `NETSUKE_COLOUR_POLICY` environment variable. +fn set_env_colour_policy(world: &TestWorld, policy: ColourPolicy) { + ensure_env_lock(world); + let previous = std::env::var_os("NETSUKE_COLOUR_POLICY"); + let value = enum_name(&policy); + // SAFETY: `EnvLock` is held in `world.env_lock` for the lifetime of the scenario. + unsafe { std::env::set_var("NETSUKE_COLOUR_POLICY", OsStr::new(&value)) }; + world.track_env_var("NETSUKE_COLOUR_POLICY".to_owned(), previous); +} + +/// Set the `NETSUKE_SPINNER_MODE` environment variable. +fn set_env_spinner_mode(world: &TestWorld, mode: SpinnerMode) { + ensure_env_lock(world); + let previous = std::env::var_os("NETSUKE_SPINNER_MODE"); + let value = enum_name(&mode); + // SAFETY: `EnvLock` is held in `world.env_lock` for the lifetime of the scenario. + unsafe { std::env::set_var("NETSUKE_SPINNER_MODE", OsStr::new(&value)) }; + world.track_env_var("NETSUKE_SPINNER_MODE".to_owned(), previous); +} + +/// Assert a merged CLI field value matches the expected value. +fn assert_merged_field(world: &TestWorld, extract: F, expected: T, label: &str) -> Result<()> +where + T: Copy + PartialEq + std::fmt::Debug, + F: FnOnce(&Cli) -> Option, +{ + let value = world + .cli + .with_ref(|cli| extract(cli)) + .context("expected merged CLI to be available")?; + ensure!( + value == Some(expected), + "expected merged {label} to be {expected:?}, got {value:?}", + ); + Ok(()) +} + #[given("the Netsuke config file sets build targets to {target:string}")] fn config_sets_build_targets(world: &TestWorld, target: &str) -> Result<()> { write_config(world, &format!("[cmds.build]\ntargets = [\"{target}\"]\n")) @@ -87,7 +175,9 @@ fn set_environment_locale_override(world: &TestWorld, locale: &str) -> Result<() #[given("the Netsuke config file sets output format to {format:string}")] fn config_sets_output_format(world: &TestWorld, format: &str) -> Result<()> { - write_config(world, &format!("output_format = \"{format}\"\n")) + let typed = OutputFormat::from_str(format, true) + .map_err(|err| anyhow!("invalid output format '{format}': {err}"))?; + write_output_format_config(world, typed) } #[given("the Netsuke config file sets no_emoji to true")] @@ -95,6 +185,51 @@ fn config_sets_no_emoji(world: &TestWorld) -> Result<()> { write_config(world, "no_emoji = true\n") } +#[given("the Netsuke config file sets theme to {theme:string}")] +fn config_sets_theme(world: &TestWorld, theme: &str) -> Result<()> { + let typed = + Theme::from_str(theme, true).map_err(|err| anyhow!("invalid theme '{theme}': {err}"))?; + write_theme_config(world, typed) +} + +#[given("the Netsuke config file sets colour policy to {policy:string}")] +fn config_sets_colour_policy(world: &TestWorld, policy: &str) -> Result<()> { + let typed = ColourPolicy::from_str(policy, true) + .map_err(|err| anyhow!("invalid colour policy '{policy}': {err}"))?; + write_colour_policy_config(world, typed) +} + +#[given("the Netsuke config file sets spinner mode to {mode:string}")] +fn config_sets_spinner_mode(world: &TestWorld, mode: &str) -> Result<()> { + let typed = SpinnerMode::from_str(mode, true) + .map_err(|err| anyhow!("invalid spinner mode '{mode}': {err}"))?; + write_spinner_mode_config(world, typed) +} + +#[given("the NETSUKE_THEME environment variable is {theme:string}")] +fn set_environment_theme_override(world: &TestWorld, theme: &str) -> Result<()> { + let typed = + Theme::from_str(theme, true).map_err(|err| anyhow!("invalid theme '{theme}': {err}"))?; + set_env_theme(world, typed); + Ok(()) +} + +#[given("the NETSUKE_COLOUR_POLICY environment variable is {policy:string}")] +fn set_environment_colour_policy_override(world: &TestWorld, policy: &str) -> Result<()> { + let typed = ColourPolicy::from_str(policy, true) + .map_err(|err| anyhow!("invalid colour policy '{policy}': {err}"))?; + set_env_colour_policy(world, typed); + Ok(()) +} + +#[given("the NETSUKE_SPINNER_MODE environment variable is {mode:string}")] +fn set_environment_spinner_mode_override(world: &TestWorld, mode: &str) -> Result<()> { + let typed = SpinnerMode::from_str(mode, true) + .map_err(|err| anyhow!("invalid spinner mode '{mode}': {err}"))?; + set_env_spinner_mode(world, typed); + Ok(()) +} + #[expect( clippy::unnecessary_wraps, reason = "rstest-bdd macro generates Result wrapper; FIXME: https://github.com/leynos/rstest-bdd/issues/381" @@ -158,15 +293,7 @@ fn merged_verbose_enabled(world: &TestWorld) -> Result<()> { #[then("the merged theme is ascii")] fn merged_theme_is_ascii(world: &TestWorld) -> Result<()> { - let theme = world - .cli - .with_ref(|cli| cli.theme) - .context("expected merged CLI to be available")?; - ensure!( - theme == Some(Theme::Ascii), - "expected merged theme to be ASCII, got {theme:?}", - ); - Ok(()) + assert_merged_field(world, |cli| cli.theme, Theme::Ascii, "theme") } #[then("the merge error should contain {fragment:string}")] @@ -181,3 +308,28 @@ fn merge_error_contains(world: &TestWorld, fragment: &str) -> Result<()> { ); Ok(()) } + +#[then("the merged theme is unicode")] +fn merged_theme_is_unicode(world: &TestWorld) -> Result<()> { + assert_merged_field(world, |cli| cli.theme, Theme::Unicode, "theme") +} + +#[then("the merged colour policy is always")] +fn merged_colour_policy_is_always(world: &TestWorld) -> Result<()> { + assert_merged_field( + world, + |cli| cli.colour_policy, + ColourPolicy::Always, + "colour policy", + ) +} + +#[then("the merged spinner mode is enabled")] +fn merged_spinner_mode_is_enabled(world: &TestWorld) -> Result<()> { + assert_merged_field( + world, + |cli| cli.spinner_mode, + SpinnerMode::Enabled, + "spinner mode", + ) +} diff --git a/tests/features/configuration_preferences.feature b/tests/features/configuration_preferences.feature index 7bc656ad..187a7c37 100644 --- a/tests/features/configuration_preferences.feature +++ b/tests/features/configuration_preferences.feature @@ -32,3 +32,69 @@ Feature: Configuration preferences Then parsing succeeds And the merged theme is ascii And the prefix contains no non-ASCII characters + + Scenario: CLI theme flag overrides configuration file + Given an empty workspace + And the Netsuke config file sets theme to "unicode" + When the CLI is parsed and merged with "--theme ascii" + Then parsing succeeds + And the merged theme is ascii + + Scenario: CLI theme flag overrides environment variable + Given an empty workspace + And the NETSUKE_THEME environment variable is "unicode" + When the CLI is parsed and merged with "--theme ascii" + Then parsing succeeds + And the merged theme is ascii + + Scenario: CLI theme flag has highest precedence over env and config + Given an empty workspace + And the Netsuke config file sets theme to "unicode" + And the NETSUKE_THEME environment variable is "auto" + When the CLI is parsed and merged with "--theme ascii" + Then parsing succeeds + And the merged theme is ascii + + Scenario: CLI colour policy flag overrides configuration file + Given an empty workspace + And the Netsuke config file sets colour policy to "never" + When the CLI is parsed and merged with "--colour-policy always" + Then parsing succeeds + And the merged colour policy is always + + Scenario: CLI colour policy flag overrides environment variable + Given an empty workspace + And the NETSUKE_COLOUR_POLICY environment variable is "never" + When the CLI is parsed and merged with "--colour-policy always" + Then parsing succeeds + And the merged colour policy is always + + Scenario: CLI spinner mode flag overrides configuration file + Given an empty workspace + And the Netsuke config file sets spinner mode to "disabled" + When the CLI is parsed and merged with "--spinner-mode enabled" + Then parsing succeeds + And the merged spinner mode is enabled + + Scenario: CLI spinner mode flag overrides environment variable + Given an empty workspace + And the NETSUKE_SPINNER_MODE environment variable is "disabled" + When the CLI is parsed and merged with "--spinner-mode enabled" + Then parsing succeeds + And the merged spinner mode is enabled + + Scenario: Environment variable overrides configuration for theme + Given an empty workspace + And the Netsuke config file sets theme to "ascii" + And the NETSUKE_THEME environment variable is "unicode" + When the CLI is parsed and merged with "" + Then parsing succeeds + And the merged theme is unicode + + Scenario: Environment variable overrides configuration for colour policy + Given an empty workspace + And the Netsuke config file sets colour policy to "auto" + And the NETSUKE_COLOUR_POLICY environment variable is "always" + When the CLI is parsed and merged with "" + Then parsing succeeds + And the merged colour policy is always