diff --git a/Cargo.lock b/Cargo.lock index 727c9106..9dc49fd6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -490,7 +490,7 @@ dependencies = [ "libc", "option-ext", "redox_users 0.5.2", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -1511,11 +1511,12 @@ checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" [[package]] name = "ortho_config" -version = "0.7.0" +version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2ee9d60c6be312d7c6ce47b7146e3c9b8464c54fc304ef8f7656e29c268faeb1" +checksum = "9314c7a4be184287f3f9a66fcbcec054ac215d4975152df86b7aadeae8d5e019" dependencies = [ "camino", + "cap-std", "clap", "clap-dispatch", "directories", @@ -1537,9 +1538,9 @@ dependencies = [ [[package]] name = "ortho_config_macros" -version = "0.7.0" +version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "992d8fc0b33823adb993fe8968614ac0df3964d9e3564f7a147fd253e27a3acd" +checksum = "a3941ab7c592a0b93aadf12ce30e0ef3d01af87b46c8f363d8157dc33273ba83" dependencies = [ "heck 0.5.0", "proc-macro2", diff --git a/Cargo.toml b/Cargo.toml index 080d7015..dd06398f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -52,13 +52,13 @@ time = { version = "0.3.44", features = ["formatting", "macros", "parsing", "ser ureq = { version = "2.10.5" } wait-timeout = "0.2" url = "^2.5.0" -ortho_config = { version = "0.7.0", features = ["serde_json"] } +ortho_config = { version = "0.8.0", features = ["serde_json"] } sys-locale = "0.3.2" [build-dependencies] clap = { version = "4.5.0", features = ["derive"] } clap_mangen = "0.2.29" -ortho_config = { version = "0.7.0", features = ["serde_json"] } +ortho_config = { version = "0.8.0", features = ["serde_json"] } serde = { version = "1", features = ["derive"] } serde_json = { version = "1", features = ["preserve_order"] } thiserror = "1" diff --git a/build.rs b/build.rs index 30e5b77d..5e9307b2 100644 --- a/build.rs +++ b/build.rs @@ -88,10 +88,13 @@ 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. + const _: usize = std::mem::size_of::(); + const _: usize = std::mem::size_of::(); + const _: usize = std::mem::size_of::(); const _: usize = std::mem::size_of::(); const _: fn(&[OsString]) -> Option = cli::locale_hint_from_args; const _: fn(&[OsString]) -> Option = cli::diag_json_hint_from_args; @@ -100,11 +103,17 @@ fn main() -> Result<(), Box> { const _: fn(&cli::Cli, &ArgMatches) -> ortho_config::OrthoResult = cli::merge_with_config; const _: LocalizedParseFn = cli::parse_with_localizer_from; + const _: fn(&cli::Cli) -> Option = cli::Cli::no_emoji_override; + 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"); + println!("cargo:rerun-if-changed=src/cli/parser.rs"); println!("cargo:rerun-if-changed=src/cli/parsing.rs"); println!("cargo:rerun-if-env-changed=CARGO_PKG_VERSION"); println!("cargo:rerun-if-env-changed=CARGO_PKG_NAME"); @@ -117,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() @@ -141,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}")) @@ -149,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) { @@ -159,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/execplans/3-11-1-cli-config-struct.md b/docs/execplans/3-11-1-cli-config-struct.md new file mode 100644 index 00000000..b47b9d88 --- /dev/null +++ b/docs/execplans/3-11-1-cli-config-struct.md @@ -0,0 +1,549 @@ +# Introduce `CliConfig` as the layered CLI configuration schema + +This ExecPlan (execution plan) is a living document. The sections +`Constraints`, `Tolerances`, `Risks`, `Progress`, `Surprises & Discoveries`, +`Decision Log`, and `Outcomes & Retrospective` must be kept up to date as work +proceeds. + +Status: COMPLETED + +No `PLANS.md` file exists in this repository. + +## Purpose / big picture + +Roadmap item `3.11.1` asks for a dedicated `CliConfig` struct derived with +`OrthoConfig` so Netsuke has one explicit, typed schema for layered CLI +configuration. This plan now targets `ortho_config` `0.8.0` and uses the +repository copy of `docs/ortho-config-users-guide.md`, which has been replaced +with the upstream `v0.8.0` guide. At planning time the repository already had +partial layered configuration, but it was centered on `src/cli/mod.rs`, where +`Cli` served three roles at once: + +1. Clap parser. +2. OrthoConfig merge target. +3. Runtime command model passed into the runner. + +That coupling makes the code difficult to extend. It also leaves roadmap fields +such as colour policy, spinner mode, output format, default targets, and theme +either implicit or missing. + +After this change, a novice should be able to point at one config schema and +see how Netsuke resolves: + +- verbosity +- locale +- colour policy +- spinner mode +- progress and accessible output behaviour +- output format +- default build targets +- theme selection + +Observable success means: + +1. `CliConfig` exists and is the authoritative OrthoConfig-derived schema. +2. Clap parsing still works, but parsing and layered configuration are no + longer the same type. +3. Configuration files, environment variables, and CLI flags resolve through + the same typed fields with documented precedence. +4. Unit tests (`rstest`) and behavioural tests (`rstest-bdd` v0.5.0) cover + happy paths, unhappy paths, and precedence edge cases. +5. `docs/users-guide.md`, `docs/netsuke-design.md`, and `docs/roadmap.md` + reflect the final behaviour. + +## Constraints + +- Keep existing top-level commands stable: `build`, `clean`, `graph`, and + `manifest` must continue to parse and dispatch. +- Upgrade every in-repo `ortho_config` dependency to `0.8.0`, and add or + upgrade `ortho_config_macros` to `0.8.0` if the final implementation uses + derive-generated selected-subcommand merging. +- Keep the toolchain at Rust `1.88` or newer. The current repository already + uses Rust `1.89.0`, so this is a compatibility floor rather than a required + toolchain migration. +- Do not regress localized Clap help or localized runtime diagnostics. +- Use `ortho_config` as the primary merge mechanism rather than adding another + bespoke loader. +- When derive-generated code touches `figment`, `uncased`, or `xdg`, prefer the + `ortho_config::...` re-exports described by the `0.8.0` guide unless the + application source genuinely needs those crates directly. +- Preserve the current configuration discovery behaviour for this milestone. + Do not expose the visible `--config` / `NETSUKE_CONFIG` interface here; that + belongs to roadmap item `3.11.3`. +- Preserve existing user-facing flags where feasible. If a new canonical field + supersedes an old one, keep a compatibility path unless the user explicitly + approves a breaking change. +- If `cli_default_as_absent` is used on any new or refactored field, use typed + Clap defaults (`default_value_t` / `default_values_t`) rather than + string-based `default_value`, and pass `ArgMatches` into merge flows so + explicit CLI overrides remain distinguishable from inferred defaults. +- No source file may exceed 400 lines. This is a hard constraint, not a style + preference. +- All new or changed public types and modules require Rustdoc and module-level + documentation. +- Behaviour must be validated with both `rstest` unit tests and + `rstest-bdd` v0.5.0 behavioural tests. +- The implementation must end with successful `make check-fmt`, `make lint`, + and `make test` runs captured via `tee` and `set -o pipefail`. +- Documentation updates are part of the feature, not follow-up work. + +## Tolerances (exception triggers) + +- Scope: if implementation requires more than 18 files or more than 900 net + new lines, stop and escalate. +- Interfaces: if the runner or subcommand API must change in a way that breaks + existing tests or user-facing CLI syntax, stop and escalate. +- Dependencies: if implementation requires a new runtime dependency beyond + `ortho_config` facilities already in use, stop and escalate. +- Compatibility: if preserving `--no-emoji` compatibility while introducing a + canonical theme field becomes impossible without ambiguous precedence, stop + and escalate. +- Investigation: if configuration merging still behaves unexpectedly after + three red/green cycles, stop and document the blocking case before proceeding. + +## Risks + +- Risk: `src/cli/mod.rs` was already 398 lines at planning time, so any + additive work there would violate the file-size limit. Severity: high. + Likelihood: high. Mitigation: split parser definitions, config schema, and + merge helpers into separate modules before adding new fields. + +- Risk: the repository already has partial OrthoConfig support, manual config + discovery, and merge tests. A careless refactor could duplicate logic rather + than simplifying it. Severity: medium. Likelihood: high. Mitigation: treat + this as a consolidation task and delete superseded merge code as part of the + same change. + +- Risk: the repository currently depends on `ortho_config 0.7.0`, while this + plan now targets `0.8.0`. Severity: medium. Likelihood: high. Mitigation: + treat the crate upgrade as part of the same branch and apply the documented + migration rules up front instead of retrofitting them after the schema + refactor lands. + +- Risk: roadmap item `3.11.1` reaches into future roadmap items by naming + output format and theme before `3.10.3` and `3.12.*` are complete. Severity: + medium. Likelihood: high. Mitigation: introduce typed config fields now, but + limit behaviour to what the current product can honour and fail clearly on + unsupported values. + +- Risk: build default targets are subcommand-specific, while most other config + is global. Severity: medium. Likelihood: medium. Mitigation: use + OrthoConfig's subcommand configuration support for `build` instead of forcing + targets into the global namespace. + +## Progress + +- [x] (2026-03-07) Reviewed roadmap `3.11.1`, the OrthoConfig guide, current + CLI/config code, and nearby execplans. +- [x] (2026-03-07) Drafted this ExecPlan. +- [x] (2026-03-09) Continued implementation from the existing branch state. +- [x] (2026-03-09) Stage A: split parser, config schema, and merge + responsibilities. +- [x] (2026-03-09) Stage B: introduce typed config groups and compatibility + mapping. +- [x] (2026-03-09) Stage C: wire global and subcommand merges through + `CliConfig`. +- [x] (2026-03-09) Stage D: add unit and behavioural coverage. +- [x] (2026-03-09) Stage E: update user/design docs, mark roadmap item done, + and run all quality gates. + +## Surprises & Discoveries + +- The codebase already derives `OrthoConfig` on `Cli`; roadmap `3.11.1` is + therefore a refactor-and-expansion task, not a greenfield introduction. +- The repository is already on Rust `1.89.0`, which satisfies the + `ortho_config 0.8.0` minimum of Rust `1.88`. The crate version is the real + migration step; the compiler floor is not. +- The repository already documented configuration discovery in + `docs/users-guide.md` and already had merge tests in + `tests/cli_tests/merge.rs`. The implementation needed to preserve these + guarantees while changing the type layout. +- `rstest-bdd` feature-file edits may require touching `tests/bdd_tests.rs` to + force Cargo to rebuild generated scenarios. +- The new configuration-preferences behaviour-driven development (BDD) + coverage initially flaked only in the full suite because + `NETSUKE_CONFIG_PATH` is process-global. Holding + `test_support/src/env_lock.rs`'s `EnvLock` for the whole scenario fixed the + race without weakening the coverage. + +## Decision Log + +- Decision: introduce a dedicated `CliConfig` type and treat the existing CLI + parser as a separate concern. Rationale: parsing tokens and merging layered + configuration are related but not identical jobs, and combining them has + already pushed the current module to the file-size limit. Date/Author: + 2026-03-07 (Codex, plan draft) + +- Decision: use OrthoConfig subcommand configuration for build defaults rather + than a global `default_targets` field. Rationale: target lists only make + sense for `build`; placing them under `cmds.build` matches the OrthoConfig + user's guide and avoids leaking command-specific semantics into global + config. Date/Author: 2026-03-07 (Codex, plan draft) + +- Decision: keep current hidden config-path override behaviour in this + milestone and defer the visible `--config` / `NETSUKE_CONFIG` surface to + roadmap item `3.11.3`. Rationale: the roadmap splits schema introduction from + config-path UX, and this plan should not silently complete later work. + Date/Author: 2026-03-07 (Codex, plan draft) + +- Decision: make `theme` the canonical presentation setting and treat + `no_emoji` as a compatibility alias that resolves to the ASCII theme. + Rationale: roadmap `3.12.*` already talks about themes, while the current + implementation only exposes `no_emoji`; a compatibility bridge avoids + breaking existing users. Date/Author: 2026-03-07 (Codex, plan draft) + +- Decision: add typed enums for colour policy, spinner mode, output format, + and theme even where runtime behaviour is still limited. Rationale: the + schema should become explicit now so future milestones extend behaviour + without reworking config names a second time. Unsupported combinations must + fail with actionable diagnostics. Date/Author: 2026-03-07 (Codex, plan draft) + +- Decision: treat `ortho_config 0.8.0` as the baseline for this work and align + the implementation with its migration rules. Rationale: the local guide now + reflects `v0.8.0`, and the implementation plan should not be written against + `0.7.x` semantics. The repository does not currently alias the dependency + name, so `#[ortho_config(crate = "...")]` is not required unless that changes + during implementation. Date/Author: 2026-03-08 (Codex, plan revision) + +- Decision: keep `Cli` as the parser/runtime command carrier while moving all + layered schema responsibilities into `CliConfig`. Rationale: this achieved + the roadmap separation with a smaller, safer surface-area change while + preserving runner tests and command-dispatch call sites. Date/Author: + 2026-03-09 (Codex, implementation) + +- Decision: validate `output_format = "json"` as unsupported for now instead of + silently accepting it. Rationale: roadmap item `3.10.3` is still open, so + accepting JSON output configuration without delivering the behaviour would be + misleading. Date/Author: 2026-03-09 (Codex, implementation) + +These decisions must be recorded in `docs/netsuke-design.md` during +implementation if they remain unchanged after coding begins. + +## Outcomes & Retrospective + +Completed on 2026-03-09. + +Implemented results: + +- Added `src/cli/config.rs`'s `CliConfig` as the authoritative + OrthoConfig-derived schema and split the CLI module into parser, config, and + merge submodules. +- Upgraded `ortho_config` to `0.8.0`. +- Kept `Cli` as the parser/runtime command carrier while rooting configuration + merge in `CliConfig`. +- Added typed config fields for `colour_policy`, `spinner_mode`, + `output_format`, and `theme`. +- Canonicalized `no_emoji = true` to the ASCII theme while rejecting + contradictory combinations. +- Wired `[cmds.build] targets` and `emit` defaults into the runtime build + command when the user does not supply explicit CLI values. +- Added unit coverage in `tests/cli_tests/merge.rs` plus behavioural coverage + in `tests/features/configuration_preferences.feature`. +- Updated the user guide, design document, and roadmap entry for `3.11.1`. + +Quality-gate evidence: + +- `make check-fmt` +- `make lint` +- `make test` +- `make markdownlint` (with `markdownlint-cli2` available on `PATH`) +- `make nixie` + +Lessons learned: + +- Keeping the parser/runtime type stable while introducing a separate merge + schema is a pragmatic migration path when downstream code already consumes + the parser type pervasively. +- Behaviour-driven development (BDD) coverage that touches process-wide + environment variables must hold `EnvLock` for the full scenario, not only for + individual mutations. + +## Context and orientation + +Historical planning context for this change was spread across the following +files: + +- `src/cli/mod.rs`: then contained the `Cli` type, Clap parser, OrthoConfig + derive, validation parsers, and merge logic. +- `Cargo.toml`: then pinned `ortho_config = "0.7.0"` and + `rust-version = "1.89.0"`. +- `rust-toolchain.toml`: then pinned toolchain `1.89.0`, which already + satisfied the `0.8.0` minimum. +- `src/main.rs`: startup parse/merge flow and runtime localization bootstrap. +- `src/output_mode.rs`: accessible versus standard output mode resolution. +- `src/output_prefs.rs`: emoji-aware semantic prefixes and then-current + `no_emoji` handling. +- `src/runner/mod.rs`: uses merged CLI state to choose output mode, progress + behaviour, and build targets. +- `tests/cli_tests/merge.rs`: merge precedence coverage. +- `tests/cli_tests/parsing.rs` and `tests/features/cli.feature`: parse-only + coverage. + +Two important facts shape this plan: + +1. The repo already has a layered configuration story. +2. The missing piece is a stable, explicit schema that separates merged config + from raw CLI parsing and adds the roadmap fields that do not yet exist. + +## Target architecture + +The implementation should converge on three layers of types. + +1. A Clap-facing parser model, still responsible for token parsing and + user-facing command syntax. +2. A layered configuration model rooted at `CliConfig`, derived with + `OrthoConfig`, `Serialize`, and `Deserialize`, using `0.8.0` semantics. +3. A runtime model passed into the runner after configuration and subcommand + selection have been resolved. + +The preferred shape is: + +- `src/cli/mod.rs`: parser entry points and minimal top-level glue. +- `src/cli/config.rs`: `CliConfig` plus nested typed config groups. +- `src/cli/merge.rs` or similar: conversion from parsed CLI overrides plus + OrthoConfig layer composition into the merged runtime shape. +- `BuildArgs` (or a renamed build-config type) derived with `OrthoConfig` so + `cmds.build` configuration can supply default targets and optional emit-path + defaults. + +The config schema should cover at least these concepts: + +- `verbose` +- `locale` +- `colour_policy` +- `spinner_mode` +- `output_format` +- `theme` +- `progress` +- `accessible` +- current fetch-policy settings +- build default targets through subcommand configuration + +For this milestone, a valid example config should look like this: + +```toml +verbose = true +locale = "es-ES" +colour_policy = "auto" +spinner_mode = "auto" +output_format = "human" +theme = "ascii" +progress = true +accessible = false + +[cmds.build] +targets = ["all"] +``` + +The final user guide must explain the actual accepted values and any +compatibility aliases such as `no_emoji = true`. + +## Plan of work + +### Stage A: split responsibilities before adding new fields + +Start by reducing the blast radius in `src/cli/mod.rs`. Move the merge logic +and the future `CliConfig` definition out of that file first. The goal is to +make later edits mechanical instead of risky. This stage also establishes the +`ortho_config 0.8.0` baseline before higher-level schema refactors pile on. + +Concrete work in this stage: + +1. Extract the current OrthoConfig-driven merge helpers into a dedicated module. +2. Introduce a parser-only root type if needed (`Cli`, `CliArgs`, or similar), + while keeping command syntax unchanged. +3. Upgrade `ortho_config` to `0.8.0` and confirm the repository still builds + against Rust `1.89.0` without toolchain changes. +4. Keep existing parsing tests green before introducing new schema fields. + +Acceptance for Stage A: + +- CLI parsing tests still pass unchanged. +- No file exceeds 400 lines. +- No behaviour changes are visible yet. + +### Stage B: introduce `CliConfig` and typed config groups + +Create `CliConfig` as the authoritative layered configuration schema. Use typed +enums or newtypes where values are semantic rather than free-form text. + +Concrete work in this stage: + +1. Define `CliConfig` and any nested groups needed to keep modules readable. +2. Add typed enums for: + - `ColourPolicy` + - `SpinnerMode` + - `OutputFormat` + - `Theme` +3. Decide and implement compatibility handling for the current `no_emoji` + surface. +4. Use OrthoConfig discovery attributes on `CliConfig` to preserve the current + `NETSUKE_CONFIG_PATH` and hidden config-path behaviour. +5. Keep global fields optional where layered precedence requires absence to be + meaningful. +6. Where Clap defaults are required, use typed defaults so + `cli_default_as_absent` remains valid under `0.8.0`. + +Acceptance for Stage B: + +- `CliConfig` can be merged from defaults, file, env, and CLI layers in unit + tests without invoking the full parser. +- Invalid enum values fail with actionable errors. +- Existing config-discovery paths still work. + +### Stage C: merge global config plus selected subcommand defaults + +This stage makes `CliConfig` drive runtime behaviour instead of the current +all-in-one `Cli` type. + +Concrete work in this stage: + +1. Replace `Cli::merge_with_config` with a merge path rooted in `CliConfig`. +2. Merge `build` subcommand defaults using OrthoConfig's subcommand support so + `[cmds.build] targets = [...]` becomes the default target list when the user + does not pass targets explicitly. +3. Convert the merged config plus parsed command into the runtime shape + consumed by `src/runner/mod.rs`. +4. Update output-mode and output-preference resolution to consume the new typed + fields rather than a loose collection of booleans. +5. Keep startup locale resolution intact so localized Clap help still works + before full merge. +6. If selected-subcommand merge derives are introduced, add + `ortho_config_macros 0.8.0` and pass `ArgMatches` where required by + `cli_default_as_absent`. + +Acceptance for Stage C: + +- Running `netsuke` with no explicit targets can pick up configured build + targets from `cmds.build`. +- CLI overrides still beat environment and file values. +- Existing commands continue to dispatch correctly. + +### Stage D: add test coverage for happy and unhappy paths + +Unit tests should prove the schema and merge logic. Behavioural tests should +prove the user-visible outcomes. + +Unit coverage to add with `rstest`: + +- defaults < file < env < CLI precedence for representative global fields +- `theme` / `no_emoji` compatibility mapping +- enum parsing failures for `colour_policy`, `spinner_mode`, and + `output_format` +- build-target defaulting through `cmds.build` +- unsupported but syntactically valid combinations fail clearly when required +- any `cli_default_as_absent` fields continue to prefer file/env values when + the user did not explicitly supply the CLI flag + +Behavioural coverage to add with `rstest-bdd` v0.5.0: + +- config file supplies default build targets and `netsuke` uses them +- CLI `--locale` or `--verbose` overrides file and env values +- invalid config values produce user-facing diagnostics +- compatibility alias (`no_emoji`) still produces ASCII-themed output + +Prefer a dedicated feature file such as +`tests/features/configuration_preferences.feature` plus matching step +definitions, rather than overloading the existing CLI-parsing feature with +merge semantics. + +### Stage E: document the final contract and close the roadmap item + +After behaviour is working and tested, update the docs as part of the same +change. + +Documentation work: + +1. Update `docs/users-guide.md` with the new config schema, precedence rules, + accepted values, and example TOML. +2. Update `docs/netsuke-design.md` to record: + - separation of parser and config schema + - subcommand config for default build targets + - canonical theme handling and `no_emoji` compatibility + - which output-format values are supported in this milestone +3. Mark roadmap item `3.11.1` as done in `docs/roadmap.md`. + +Acceptance for Stage E: + +- User docs match the shipped behaviour. +- Design docs capture the decisions from this plan that survived + implementation. +- Only roadmap item `3.11.1` is marked done unless later work is intentionally + completed and validated too. + +## Concrete steps + +Run all commands from the repository root. + +Before editing feature files, remember the existing `rstest-bdd` gotcha: + +```sh +touch tests/bdd_tests.rs +``` + +Use `tee` and `set -o pipefail` for every long-running gate: + +```sh +set -o pipefail +make check-fmt 2>&1 | tee /tmp/netsuke-check-fmt.log +``` + +```sh +set -o pipefail +make lint 2>&1 | tee /tmp/netsuke-lint.log +``` + +```sh +set -o pipefail +make test 2>&1 | tee /tmp/netsuke-test.log +``` + +Because docs will change, also run: + +```sh +set -o pipefail +make markdownlint 2>&1 | tee /tmp/netsuke-markdownlint.log +``` + +```sh +set -o pipefail +make nixie 2>&1 | tee /tmp/netsuke-nixie.log +``` + +```sh +set -o pipefail +make fmt 2>&1 | tee /tmp/netsuke-fmt.log +``` + +After `make fmt`, inspect `git status --short` and remove incidental edits in +unrelated files before finalizing the change. + +The local OrthoConfig reference for this task is now the `v0.8.0` guide at +`docs/ortho-config-users-guide.md`. Do not rely on older `0.7.x` examples when +the two guides disagree. + +## Validation and acceptance + +The feature is complete only when all of the following are true: + +1. `CliConfig` is the authoritative OrthoConfig-derived schema. +2. Parser-only code and merge-only code are separated enough to keep files + under the 400-line limit. +3. `build` default targets can be supplied through configuration without + breaking explicit CLI targets. +4. Typed config values are documented and validated with clear unhappy-path + errors. +5. Unit tests and behavioural tests cover both precedence and failure cases. +6. `make check-fmt`, `make lint`, `make test`, `make markdownlint`, and + `make nixie` succeed. +7. `docs/users-guide.md`, `docs/netsuke-design.md`, and `docs/roadmap.md` are + updated. + +## Idempotence and recovery + +This work should be implemented in small, reversible steps. If a stage fails, +return the tree to the last green checkpoint, update this ExecPlan's +`Decision Log` and `Progress`, and retry with a narrower diff rather than +stacking speculative fixes. If formatting tools touch unrelated Markdown files, +restore only the incidental edits created during the current turn before +proceeding. diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index d5c9ff23..4990c38d 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -2029,34 +2029,52 @@ the targets listed in the `defaults` section of the manifest are built. ### 8.4 Design Decisions -The CLI is implemented using clap's derive API in `src/cli/mod.rs`. Netsuke -applies `Cli::with_default_command` after parsing so invoking `netsuke` with no -explicit command still triggers a build. Configuration is layered with -OrthoConfig (defaults, configuration files, environment variables, then CLI -overrides) while treating clap defaults as absent so file or environment values -are not masked. Configuration discovery honours `NETSUKE_CONFIG_PATH` and the -standard OrthoConfig search order; environment variables use the `NETSUKE_` -prefix with `__` as a nesting separator. CLI help and clap errors are localized -via Fluent resources; locale resolution is handled in -`src/locale_resolution.rs` with the precedence `--locale` -> `NETSUKE_LOCALE` --> configuration `locale` -> system default. System locale strings are -normalized by stripping encoding suffixes (such as `.UTF-8`), removing variant -suffixes (such as `@latin`), and replacing underscores with hyphens before -validation. English plus Spanish catalogues ship in `locales/`; unsupported -locales fall back to `en-US`. Runtime diagnostics (for example manifest -parsing, stdlib template errors, and runner failures) use the same Fluent -localizer so the locale selection is consistent across user-facing output. A -build-time audit in `build.rs` validates that all referenced Fluent message -keys exist in the bundled catalogues, ensuring missing strings fail CI before -release. CLI execution and dispatch live in `src/runner.rs`, keeping `main.rs` -focused on parsing. Process management, Ninja invocation, argument redaction, -and the temporary file helpers reside in `src/runner/process.rs`, allowing the -runner entry point to delegate low-level concerns. The working directory flag -mirrors Ninja's `-C` option but is resolved internally: Netsuke runs Ninja with -a configured working directory and resolves relative output paths (for example -`build --emit` and `manifest`) under the same directory so behaviour matches a -real directory change. Error scenarios are validated using clap's `ErrorKind` -enumeration in unit tests and via Cucumber steps for behavioural coverage. +The parser-facing `Cli` type is now defined in `src/cli/parser.rs`, while +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. + +Configuration is layered in the order defaults -> configuration files -> +environment variables -> CLI overrides. Discovery honours `NETSUKE_CONFIG_PATH` +and the standard OrthoConfig search order; environment variables use the +`NETSUKE_` prefix with `__` as a nesting separator. The schema now explicitly +covers verbosity, locale, accessible mode, progress, colour policy, spinner +mode, output format, theme selection, fetch policy, and build defaults. `theme` +is the canonical presentation setting; the older `no_emoji` field remains as a +compatibility alias that canonicalizes to the ASCII theme. Conflicting +combinations such as `theme = "unicode"` together with `no_emoji = true` fail +during merge. `spinner_mode` likewise validates against the legacy `progress` +boolean so contradictory inputs are rejected early. `output_format` is typed +now, but only `human` is accepted until the future JSON diagnostics milestone +lands. + +CLI help and clap errors are localized via Fluent resources; locale resolution +is handled in `src/locale_resolution.rs` with the precedence `--locale` -> +`NETSUKE_LOCALE` -> configuration `locale` -> system default. System locale +strings are normalized by stripping encoding suffixes (such as `.UTF-8`), +removing variant suffixes (such as `@latin`), and replacing underscores with +hyphens before validation. English plus Spanish catalogues ship in `locales/`; +unsupported locales fall back to `en-US`. Runtime diagnostics (for example +manifest parsing, stdlib template errors, and runner failures) use the same +Fluent localizer so the locale selection is consistent across user-facing +output. A build-time audit in `build.rs` validates that all referenced Fluent +message keys exist in the bundled catalogues, ensuring missing strings fail CI +before release. CLI execution and dispatch live in `src/runner.rs`, keeping +`main.rs` focused on parsing. Process management, Ninja invocation, argument +redaction, and the temporary file helpers reside in `src/runner/process.rs`, +allowing the runner entry point to delegate low-level concerns. The working +directory flag mirrors Ninja's `-C` option but is resolved internally: Netsuke +runs Ninja with a configured working directory and resolves relative output +paths (for example `build --emit` and `manifest`) under the same directory so +behaviour matches a real directory change. Error scenarios are validated using +clap's `ErrorKind` enumeration in unit tests and via Cucumber steps for +behavioural coverage. Real-time stage reporting now uses a six-stage model in `src/status.rs` backed by `indicatif::MultiProgress` for standard terminals. The reporter keeps one diff --git a/docs/ortho-config-users-guide.md b/docs/ortho-config-users-guide.md index 2553cdd2..801d8805 100644 --- a/docs/ortho-config-users-guide.md +++ b/docs/ortho-config-users-guide.md @@ -141,7 +141,7 @@ CLI layers. This validates every precedence permutation without copy-pasting setup. Every derived configuration also exposes `compose_layers()` and -`compose_layers_from_iter(..)`. These helpers discover configuration files, +`compose_layers_from_iter(...)`. These helpers discover configuration files, serialize environment variables, and capture CLI input as a `LayerComposition`, keeping discovery separate from merging. The returned composition includes both the ordered layers and any collected errors, letting callers push additional @@ -273,7 +273,7 @@ Add `ortho_config` as a dependency in `Cargo.toml` along with `serde`: ```toml [dependencies] -ortho_config = "0.7.0" # replace with the latest version +ortho_config = "0.8.0" # replace with the latest version serde = { version = "1.0", features = ["derive"] } clap = { version = "4", features = ["derive"] } # required for CLI support ``` @@ -284,7 +284,7 @@ corresponding cargo features: ```toml [dependencies] -ortho_config = { version = "0.7.0", features = ["json5", "yaml"] } +ortho_config = { version = "0.8.0", features = ["json5", "yaml"] } # Enabling these features expands file formats; precedence stays: defaults < file < env < CLI. ``` @@ -301,6 +301,62 @@ Redox targets), and the optional parsers (`figment_json5`, `json5`, is enabled by default because the crate relies on it internally; disable default features only when explicitly opting back into `serde_json`. +### Dependency architecture for derive macro users + +The `#[derive(OrthoConfig)]` macro emits fully qualified paths rooted at +`ortho_config`. For example, generated code references +`ortho_config::figment::Figment` and `ortho_config::uncased::Uncased` rather +than `figment::...` or `uncased::...`. Those paths resolve because +`ortho_config` re-exports these crates. + +For screen readers: The following diagram shows that generated code references +re-exported crates through `ortho_config`, so consumer crates can rely on the +runtime crate dependency. + +```mermaid +flowchart TD + A[Consumer crate] -->|depends on| B[ortho_config] + C[derive OrthoConfig] -->|generates| D[ortho_config::figment::...] + C -->|generates| E[ortho_config::uncased::...] + B -->|re-exports| F[figment] + B -->|re-exports| G[uncased] + B -->|re-exports on Unix/Redox| H[xdg] +``` + +_Figure 1: Derive output resolves parser crates through `ortho_config`._ + +In the common case, `Cargo.toml` does not need direct `figment`, `uncased`, or +`xdg` dependencies: + +```toml +[dependencies] +ortho_config = "0.8.0" +serde = { version = "1.0", features = ["derive"] } +clap = { version = "4", features = ["derive"] } +``` + +### Troubleshooting dependency errors + +- If source code imports `figment`, `uncased`, or `xdg` directly, either switch + imports to `ortho_config::figment` / `ortho_config::uncased` / + `ortho_config::xdg`, or keep explicit dependencies for that direct usage. +- If derive output fails with unresolved `ortho_config::...` paths, ensure the + dependency key is named `ortho_config` in `Cargo.toml` or use the + `#[ortho_config(crate = "...")]` attribute to specify the alias. +- **Dependency aliasing** is supported via the `crate` attribute. When + renaming the dependency in `Cargo.toml` (for example, + `my_cfg = { package = "ortho_config", ... }`), add + `#[ortho_config(crate = "my_cfg")]` to the struct so generated code + references the correct crate path. +- If dependency resolution reports conflicts, inspect duplicates with + `cargo tree -d` and prefer the versions selected through `ortho_config` + unless direct usage requires something else. + +### FAQ: should `figment`, `uncased`, or `xdg` be direct dependencies? + +No for derive-generated code. Yes, only when application code directly imports +those crates without going through the `ortho_config::` re-exports. + YAML parsing is handled by the pure-Rust `serde-saphyr` crate. It adheres to the YAML 1.2 specification, so unquoted scalars such as `yes`, `on`, and `off` remain strings. The provider enables `Options::strict_booleans`, ensuring only @@ -378,13 +434,13 @@ environment variables like `APP_PORT` and file names such as `.app.toml`. Field attributes modify how a field is sourced or merged: -| Attribute | Behaviour | -| --------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `default = expr` | Supplies a default value when no source provides one. The expression can be a literal or a function path. | -| `cli_long = "name"` | Overrides the automatically generated long CLI flag (kebab-case). | -| `cli_short = 'c'` | Adds a single-letter short flag for the field. | -| `merge_strategy = "append"` | For `Vec` fields, specifies that values from different sources should be concatenated. This is currently the only supported strategy and is the default for vector fields. | -| `cli_default_as_absent` | Treats clap's `default_value_t` as absent during subcommand merging. File and environment values take precedence over clap defaults, while explicit CLI overrides still win. | +| Attribute | Behaviour | +| --------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `default = expr` | Supplies a default value when no source provides one. The expression can be a literal or a function path. | +| `cli_long = "name"` | Overrides the automatically generated long CLI flag (kebab-case). | +| `cli_short = 'c'` | Adds a single-letter short flag for the field. | +| `merge_strategy = "append"` | For `Vec` fields, specifies that values from different sources should be concatenated. This is currently the only supported strategy and is the default for vector fields. | +| `cli_default_as_absent` | Treats typed clap defaults (`default_value_t`, `default_values_t`) as absent during configuration merging. File and environment values take precedence, while explicit CLI overrides still win. | Unrecognized keys are ignored by the derive macro for forwards compatibility. Unknown keys will therefore silently do nothing. Developers who require @@ -872,21 +928,32 @@ when the user does not explicitly provide a value on the command line, the field is excluded from the CLI layer so that file and environment values take precedence. -Add the attribute alongside the matching `default` attribute: +Add `cli_default_as_absent` and define the default in clap. The derive macro +now infers the struct default from clap's default metadata, so the default only +needs to be declared once: ```rust #[derive(Parser, Deserialize, Serialize, OrthoConfig)] #[ortho_config(prefix = "APP_")] struct GreetArgs { #[arg(long, default_value_t = String::from("!"))] - #[ortho_config(default = String::from("!"), cli_default_as_absent)] + #[ortho_config(cli_default_as_absent)] punctuation: String, } ``` +`default_value_t` and `default_values_t` are supported for inferred defaults. +`default_value` inference is intentionally unsupported for now; use +`default_value_t` or add an explicit `#[ortho_config(default = ...)]` to avoid +string-parser mismatches. Parser-faithful `default_value` inference is planned +as a day-2 follow-up. + +If `#[ortho_config(default = ...)]` is still provided, that explicit value +remains available for generated defaults/documentation metadata. + **Precedence with the attribute (lowest to highest):** -1. Struct default (`#[ortho_config(default = ...)]`) +1. Struct default (`#[ortho_config(default = ...)]` or inferred from clap) 2. Configuration file 3. Environment variable 4. Explicit CLI override (e.g. `--punctuation "?"`) @@ -1048,6 +1115,135 @@ fn interop(r: ortho_config::OrthoResult) -> Result } ``` +## Documentation metadata (OrthoConfigDocs) + +The derive macro now emits an `OrthoConfigDocs` implementation alongside the +runtime loader. This lets tooling such as `cargo-orthohelp` serialize a stable, +clap-agnostic intermediate representation (IR) for man pages and PowerShell +help. + +```rust +use ortho_config::docs::OrthoConfigDocs; + +#[derive(serde::Deserialize, serde::Serialize, ortho_config::OrthoConfig)] +#[ortho_config(prefix = "APP")] +struct AppConfig { + #[ortho_config(default = 8080)] + port: u16, +} + +let ir = AppConfig::get_doc_metadata(); +let json = ortho_config::serde_json::to_string_pretty(&ir)?; +println!("{json}"); +``` + +When IDs are not supplied, the macro generates deterministic defaults such as +`{app}.about` for the CLI overview and `{app}.fields.{field}.help` for field +descriptions. Field-level metadata can be refined with `help_id`, +`long_help_id`, `value(type = "...")`, `deprecated(note_id = "...")`, +`env(name = "...")`, and `file(key_path = "...")`. These documentation +attributes affect only the emitted IR; they do not change runtime naming or +loading behaviour. + +### Generating IR with cargo-orthohelp + +`cargo-orthohelp` compiles a tiny bridge binary that calls +`OrthoConfigDocs::get_doc_metadata()`, resolves Fluent messages per locale, and +writes localized IR JSON into the chosen output directory. Add metadata to the +package `Cargo.toml` so the tool knows which config type to load: + +```toml +[package.metadata.ortho_config] +root_type = "hello_world::cli::HelloWorldCli" +locales = ["en-US", "ja"] +``` + +Run the tool from the project root: + +```bash +cargo orthohelp --out-dir target/orthohelp --locale en-US +``` + +`--cache` reuses any previously generated IR cached under +`target/orthohelp//ir.json`, while `--no-build` skips the bridge build +and fails if the cache is missing. The generated per-locale JSON lives under +`/ir/.json` and is ready for downstream generators. + +### Generating man pages + +`cargo-orthohelp` can generate roff-formatted man pages from the localized IR. +Use `--format man` to produce `man/man/.` files suitable for +installation via `make install` or packaging: + +```bash +cargo orthohelp --format man --out-dir target/man --locale en-US +``` + +The generator produces standard man page sections in the canonical order: + +1. **NAME** – binary name and one-line description +2. **SYNOPSIS** – usage pattern with flags +3. **DESCRIPTION** – expanded about text +4. **OPTIONS** – CLI flags with types, defaults, and possible values +5. **ENVIRONMENT** – environment variables mapped to fields +6. **FILES** – configuration file paths and discovery locations +7. **PRECEDENCE** – source priority order (defaults → file → env → CLI) +8. **EXAMPLES** – usage examples from the IR +9. **SEE ALSO** – related commands and documentation links +10. **EXIT STATUS** – standard exit codes + +Additional options: + +- `--man-section ` – man page section number (default: 1) +- `--man-date ` – override the date shown in the footer +- `--man-split-subcommands` – generate separate man pages for each subcommand + +Text is automatically escaped for roff: backslashes are doubled, and leading +dashes, periods, and single quotes are escaped to prevent macro interpretation. +Enum fields list their possible values in the OPTIONS description. + +### Generating PowerShell help + +`cargo-orthohelp` can generate PowerShell external help in Microsoft Assistance +Markup Language (MAML) alongside a wrapper module, so +`Get-Help {BinName} -Full` surfaces the same configuration metadata as the man +page generator. Use the `ps` format to emit the module layout under +`powershell/`: + +```bash +cargo orthohelp --format ps --out-dir target/orthohelp --locale en-US +``` + +The generator produces: + +- `powershell//.psm1` – wrapper module. +- `powershell//.psd1` – module manifest. +- `powershell///-help.xml` – MAML help. +- `powershell///about_.help.txt` – about topic. + +`en-US` help is always generated. If only other locales are rendered, the +generator copies the first locale into `en-US` unless fallback generation is +disabled with `--ensure-en-us false`. + +PowerShell options: + +- `--ps-module-name ` – override the module name (defaults to the binary + name). +- `--ps-split-subcommands ` – emit wrapper functions for subcommands. +- `--ps-include-common-parameters ` – include CommonParameters in MAML. +- `--ps-help-info-uri ` – set `HelpInfoUri` for Update-Help payloads. +- `--ensure-en-us ` – control the `en-US` fallback behaviour. + +To set defaults in `Cargo.toml`, use the Windows metadata table: + +```toml +[package.metadata.ortho_config.windows] +module_name = "MyModule" +include_common_parameters = true +split_subcommands_into_functions = false +help_info_uri = "https://example.com/help/MyModule" +``` + ## Additional notes - **Vector merging** – For `Vec` fields the default merge strategy is @@ -1069,10 +1265,10 @@ fn interop(r: ortho_config::OrthoResult) -> Result while still requiring the CLI to provide a value when defaults are absent; see the `vk` example above. -- **Changing naming conventions** – Currently, only the default - snake/hyphenated (underscores → hyphens)/upper snake mappings are supported. - Future versions may introduce attributes such as `file_key` or `env` to - customize names further. +- **Changing naming conventions** – Runtime naming continues to use the + default snake/hyphenated (underscores → hyphens)/upper snake mappings. For + documentation output, use `env(name = "...")` and `file(key_path = "...")` to + override IR metadata without altering runtime behaviour. - **Testing** – Because the CLI and environment variables are merged at runtime, integration tests should set environment variables and construct CLI diff --git a/docs/roadmap.md b/docs/roadmap.md index d76d6e04..5a9c2ee1 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -282,11 +282,11 @@ library, and CLI ergonomics. ### 3.11. Configuration and preferences -- [ ] 3.11.1. Introduce `CliConfig` struct derived with `OrthoConfig`. See +- [x] 3.11.1. Introduce `CliConfig` struct derived with `OrthoConfig`. See [ortho-config-users-guide.md](ortho-config-users-guide.md). - - [ ] Share schema across Clap integration, configuration files, and + - [x] Share schema across Clap integration, configuration files, and environment variables. - - [ ] Cover verbosity, colour policy, locale, spinner mode, output format, + - [x] Cover verbosity, colour policy, locale, spinner mode, output format, default targets, and theme. - [ ] 3.11.2. Discover configuration files in project and user scopes. - [ ] Honour env overrides and CLI precedence. diff --git a/docs/users-guide.md b/docs/users-guide.md index caa36718..65e3c316 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -557,6 +557,66 @@ Environment variables use the `NETSUKE_` prefix (for example, `NETSUKE_JOBS=8`). Use `__` to separate nested keys when matching structured configuration. +The layered schema is rooted in `CliConfig`. Netsuke currently accepts these +top-level configuration keys: + +- `file = "Netsukefile"` +- `jobs = 8` +- `verbose = true|false` +- `locale = "en-US"` +- `fetch_allow_scheme = ["https"]` +- `fetch_allow_host = ["example.com"]` +- `fetch_block_host = ["blocked.example.com"]` +- `fetch_default_deny = true|false` +- `accessible = true|false` +- `progress = true|false` +- `theme = "auto"|"unicode"|"ascii"` +- `no_emoji = true|false` +- `spinner_mode = "auto"|"enabled"|"disabled"` +- `colour_policy = "auto"|"always"|"never"` +- `output_format = "human"` + +Build-only defaults live under `[cmds.build]`: + +- `emit = "out.ninja"` +- `targets = ["hello"]` + +Example: + +```toml +verbose = true +locale = "es-ES" +colour_policy = "auto" +spinner_mode = "auto" +output_format = "human" +theme = "ascii" +progress = true +accessible = false + +[cmds.build] +targets = ["hello"] +``` + +`[cmds.build].targets` is used only when the user does not pass explicit build +targets on the command line. Explicit CLI targets always win. + +`theme` is the canonical presentation setting. `no_emoji = true` remains as a +compatibility alias and resolves to the ASCII theme. Conflicting settings such +as `theme = "unicode"` with `no_emoji = true` are rejected during configuration +merge. + +`spinner_mode = "disabled"` is equivalent to disabling progress output unless +the user explicitly sets `progress = true`, which is treated as a conflict. +Likewise, `spinner_mode = "enabled"` conflicts with `progress = false`. + +`output_format = "json"` is intentionally rejected for now. Roadmap item +`3.10.3` will add JSON diagnostics; until then, the only supported value is +`"human"`. + +`colour_policy` is accepted and layered today, so users can standardize their +preferred setting, but Netsuke does not yet emit coloured terminal output, so +this value currently has no visible effect. + Use `--locale `, `NETSUKE_LOCALE`, or a `locale = "..."` entry in a configuration file to select localized CLI copy and error messages. Locale precedence is: command-line flag, environment variable, configuration file, diff --git a/src/cli/config.rs b/src/cli/config.rs new file mode 100644 index 00000000..587fc55f --- /dev/null +++ b/src/cli/config.rs @@ -0,0 +1,234 @@ +//! Layered CLI configuration schema. +//! +//! [`CliConfig`] is the single typed schema used for configuration discovery +//! 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; + +use super::validation_error; +use crate::host_pattern::HostPattern; + +/// Colour-output policy accepted by layered configuration. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, ValueEnum, Default)] +#[serde(rename_all = "kebab-case")] +pub enum ColourPolicy { + /// Follow the host environment. + #[default] + Auto, + /// Force colour output on when available. + Always, + /// Force colour output off. + Never, +} + +/// Spinner and progress rendering policy. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, ValueEnum, Default)] +#[serde(rename_all = "kebab-case")] +pub enum SpinnerMode { + /// Follow Netsuke's default progress behaviour. + #[default] + Auto, + /// Force progress summaries on. + Enabled, + /// Disable progress summaries. + Disabled, +} + +/// Top-level diagnostics and output format. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, ValueEnum, Default)] +#[serde(rename_all = "kebab-case")] +pub enum OutputFormat { + /// Human-readable terminal output. + #[default] + Human, + /// Machine-readable JSON diagnostics. + Json, +} + +/// Presentation theme for semantic prefixes and glyph choices. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, ValueEnum, Default)] +#[serde(rename_all = "kebab-case")] +pub enum Theme { + /// Follow the host environment. + #[default] + Auto, + /// Prefer the Unicode/emoji presentation. + Unicode, + /// Prefer ASCII-only output. + Ascii, +} + +/// Layered defaults for the `build` subcommand. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)] +pub struct BuildConfig { + /// Optional default path for the emitted Ninja manifest. + pub emit: Option, + /// Default targets used when the user does not pass any targets. + #[serde(default)] + pub targets: Vec, +} + +/// Subcommand-specific layered defaults. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)] +pub struct CommandConfigs { + /// Configuration that applies only to the `build` subcommand. + #[serde(default)] + pub build: BuildConfig, +} + +/// Authoritative schema for layered CLI configuration. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, OrthoConfig)] +#[ortho_config(prefix = "NETSUKE", post_merge_hook)] +pub struct CliConfig { + /// Path to the Netsuke manifest file to use. + #[ortho_config(default = default_manifest_path())] + pub file: PathBuf, + + /// Set the number of parallel build jobs. + pub jobs: Option, + + /// Enable verbose diagnostic logging and completion timing summaries. + #[ortho_config(default = false)] + pub verbose: bool, + + /// Locale tag for CLI copy (for example: en-US, es-ES). + pub locale: Option, + + /// Additional URL schemes allowed for the `fetch` helper. + #[ortho_config(merge_strategy = "append")] + #[serde(default)] + pub fetch_allow_scheme: Vec, + + /// Hostnames permitted when default deny is enabled. + #[ortho_config(merge_strategy = "append")] + #[serde(default)] + pub fetch_allow_host: Vec, + + /// Hostnames that are always blocked. + #[ortho_config(merge_strategy = "append")] + #[serde(default)] + pub fetch_block_host: Vec, + + /// Deny all hosts by default; only allow the declared allowlist. + #[ortho_config(default = false)] + pub fetch_default_deny: bool, + + /// Force accessible output mode on or off. + pub accessible: Option, + + /// Compatibility alias for requesting the ASCII theme. + pub no_emoji: Option, + + /// Emit machine-readable diagnostics in JSON on stderr. + #[ortho_config(default = false)] + pub diag_json: bool, + + /// Force progress summaries on or off. + pub progress: Option, + + /// Preferred colour policy. + #[ortho_config(skip_cli)] + pub colour_policy: Option, + + /// Preferred spinner or progress mode. + #[ortho_config(skip_cli)] + pub spinner_mode: Option, + + /// Preferred diagnostics/output format. + #[ortho_config(skip_cli)] + pub output_format: Option, + + /// Preferred terminal theme. + #[ortho_config(skip_cli)] + pub theme: Option, + + /// Per-subcommand defaults. + #[ortho_config(skip_cli)] + #[serde(default)] + pub cmds: CommandConfigs, +} + +impl Default for CliConfig { + fn default() -> Self { + Self { + file: default_manifest_path(), + jobs: None, + verbose: false, + locale: None, + fetch_allow_scheme: Vec::new(), + fetch_allow_host: Vec::new(), + fetch_block_host: Vec::new(), + fetch_default_deny: false, + accessible: None, + no_emoji: None, + diag_json: false, + progress: None, + colour_policy: None, + spinner_mode: None, + output_format: None, + theme: None, + cmds: CommandConfigs::default(), + } + } +} + +impl CliConfig { + pub(super) fn default_manifest_path() -> PathBuf { + default_manifest_path() + } +} + +impl PostMergeHook for CliConfig { + fn post_merge(&mut self, _ctx: &PostMergeContext) -> OrthoResult<()> { + validate_theme_compatibility(self)?; + validate_spinner_mode_compatibility(self)?; + validate_output_format_support(self)?; + Ok(()) + } +} + +fn default_manifest_path() -> PathBuf { + PathBuf::from("Netsukefile") +} + +fn validate_theme_compatibility(config: &CliConfig) -> OrthoResult<()> { + match (config.theme, config.no_emoji) { + (Some(Theme::Unicode), Some(true)) => Err(validation_error( + "theme", + "theme = \"unicode\" conflicts with no_emoji = true; use theme = \"ascii\" instead", + )), + (Some(Theme::Ascii), Some(false)) => Err(validation_error( + "no_emoji", + "no_emoji = false conflicts with theme = \"ascii\"; remove the alias or choose theme = \"unicode\"", + )), + _ => Ok(()), + } +} + +fn validate_spinner_mode_compatibility(config: &CliConfig) -> OrthoResult<()> { + match (config.spinner_mode, config.progress) { + (Some(SpinnerMode::Disabled), Some(true)) => Err(validation_error( + "spinner_mode", + "spinner_mode = \"disabled\" conflicts with progress = true", + )), + (Some(SpinnerMode::Enabled), Some(false)) => Err(validation_error( + "progress", + "progress = false conflicts with spinner_mode = \"enabled\"", + )), + _ => Ok(()), + } +} + +fn validate_output_format_support(config: &CliConfig) -> OrthoResult<()> { + if matches!(config.output_format, Some(OutputFormat::Json)) { + return Err(validation_error( + "output_format", + "output_format = \"json\" is not supported yet; use \"human\" for this milestone", + )); + } + Ok(()) +} diff --git a/src/cli/merge.rs b/src/cli/merge.rs new file mode 100644 index 00000000..03105b80 --- /dev/null +++ b/src/cli/merge.rs @@ -0,0 +1,251 @@ +//! Layer-composition and conversion helpers for CLI configuration. + +use clap::ArgMatches; +use clap::parser::ValueSource; +use ortho_config::declarative::LayerComposition; +use ortho_config::figment::{Figment, providers::Env}; +use ortho_config::uncased::Uncased; +use ortho_config::{ConfigDiscovery, MergeComposer, OrthoMergeExt, OrthoResult, sanitize_value}; +use serde::Serialize; +use serde_json::{Map, Value, json}; +use std::path::PathBuf; + +use super::config::{BuildConfig, CliConfig, Theme}; +use super::parser::{BuildArgs, Cli, Commands}; +use super::validation_error; +const CONFIG_ENV_VAR: &str = "NETSUKE_CONFIG_PATH"; +const ENV_PREFIX: &str = "NETSUKE_"; + +/// Merge discovered configuration layers over parsed CLI input. +/// +/// # Errors +/// +/// Returns an [`ortho_config::OrthoError`] if layer composition or merging +/// fails. +pub fn merge_with_config(cli: &Cli, matches: &ArgMatches) -> OrthoResult { + let mut errors = Vec::new(); + let mut composer = MergeComposer::with_capacity(4); + + match sanitize_value(&CliConfig::default()) { + Ok(value) => composer.push_defaults(value), + Err(err) => errors.push(err), + } + + let discovery = config_discovery(cli.directory.as_ref()); + let mut file_layers = discovery.compose_layers(); + errors.append(&mut file_layers.required_errors); + if file_layers.value.is_empty() { + errors.append(&mut file_layers.optional_errors); + } + for layer in file_layers.value { + composer.push_layer(layer); + } + + let env_provider = env_provider() + .map(|key| Uncased::new(key.as_str().to_ascii_uppercase())) + .split("__"); + match Figment::from(env_provider) + .extract::() + .into_ortho_merge() + { + Ok(value) => composer.push_environment(value), + Err(err) => errors.push(err), + } + + match cli_overrides_from_matches(cli, matches) { + Ok(value) if !is_empty_value(&value) => composer.push_cli(value), + Ok(_) => {} + Err(err) => errors.push(err), + } + + let composition = LayerComposition::new(composer.layers(), errors); + let merged = composition.into_merge_result(CliConfig::merge_from_layers)?; + Ok(apply_config(cli, merged)) +} + +fn env_provider() -> Env { + Env::prefixed(ENV_PREFIX) +} + +fn config_discovery(directory: Option<&PathBuf>) -> ConfigDiscovery { + let mut builder = ConfigDiscovery::builder("netsuke").env_var(CONFIG_ENV_VAR); + if let Some(dir) = directory { + builder = builder.clear_project_roots().add_project_root(dir); + } + builder.build() +} + +fn is_empty_value(value: &Value) -> bool { + matches!(value, Value::Object(map) if map.is_empty()) +} + +fn diag_json_from_layer(value: &Value) -> Option { + value + .as_object() + .and_then(|map| map.get("diag_json")) + .and_then(Value::as_bool) +} + +/// Resolve the effective diagnostic JSON preference from the raw config layers. +/// +/// This is used before full config merging so startup and merge-time failures +/// can still honour `diag_json` values sourced from config files or the +/// environment. +#[must_use] +pub fn resolve_merged_diag_json(cli: &Cli, matches: &ArgMatches) -> bool { + let mut diag_json = CliConfig::default().diag_json; + + let discovery = config_discovery(cli.directory.as_ref()); + let file_layers = discovery.compose_layers(); + for layer in file_layers.value { + let layer_value = layer.into_value(); + if let Some(layer_diag_json) = diag_json_from_layer(&layer_value) { + diag_json = layer_diag_json; + } + } + + let env_provider = env_provider() + .map(|key| Uncased::new(key.as_str().to_ascii_uppercase())) + .split("__"); + if let Ok(value) = Figment::from(env_provider).extract::() + && let Some(env_diag_json) = diag_json_from_layer(&value) + { + diag_json = env_diag_json; + } + + if matches.value_source("diag_json") == Some(ValueSource::CommandLine) { + cli.diag_json + } else { + diag_json + } +} + +fn cli_overrides_from_matches(cli: &Cli, matches: &ArgMatches) -> OrthoResult { + let mut root = Map::new(); + + maybe_insert_explicit(matches, "file", &cli.file, &mut root)?; + maybe_insert_explicit(matches, "jobs", &cli.jobs, &mut root)?; + maybe_insert_explicit(matches, "verbose", &cli.verbose, &mut root)?; + maybe_insert_explicit(matches, "locale", &cli.locale, &mut root)?; + maybe_insert_explicit( + matches, + "fetch_allow_scheme", + &cli.fetch_allow_scheme, + &mut root, + )?; + maybe_insert_explicit( + matches, + "fetch_allow_host", + &cli.fetch_allow_host, + &mut root, + )?; + maybe_insert_explicit( + matches, + "fetch_block_host", + &cli.fetch_block_host, + &mut root, + )?; + maybe_insert_explicit( + matches, + "fetch_default_deny", + &cli.fetch_default_deny, + &mut root, + )?; + maybe_insert_explicit(matches, "accessible", &cli.accessible, &mut root)?; + maybe_insert_explicit(matches, "progress", &cli.progress, &mut root)?; + maybe_insert_explicit(matches, "no_emoji", &cli.no_emoji, &mut root)?; + maybe_insert_explicit(matches, "diag_json", &cli.diag_json, &mut root)?; + maybe_insert_explicit(matches, "colour_policy", &cli.colour_policy, &mut root)?; + maybe_insert_explicit(matches, "spinner_mode", &cli.spinner_mode, &mut root)?; + maybe_insert_explicit(matches, "output_format", &cli.output_format, &mut root)?; + maybe_insert_explicit(matches, "theme", &cli.theme, &mut root)?; + + if let Some(Commands::Build(args)) = cli.command.as_ref() + && let Some(build_matches) = matches.subcommand_matches("build") + { + let build = build_cli_overrides(args, build_matches)?; + if !build.is_empty() { + root.insert("cmds".to_owned(), json!({ "build": Value::Object(build) })); + } + } + + Ok(Value::Object(root)) +} + +fn build_cli_overrides(args: &BuildArgs, matches: &ArgMatches) -> OrthoResult> { + let mut build = Map::new(); + maybe_insert_explicit(matches, "emit", &args.emit, &mut build)?; + maybe_insert_explicit(matches, "targets", &args.targets, &mut build)?; + Ok(build) +} + +fn maybe_insert_explicit( + matches: &ArgMatches, + field: &str, + value: &T, + target: &mut Map, +) -> OrthoResult<()> +where + T: Serialize, +{ + if matches.value_source(field) == Some(ValueSource::CommandLine) { + target.insert(field.to_owned(), serialize_value(field, value)?); + } + Ok(()) +} + +fn serialize_value(field: &str, value: &T) -> OrthoResult +where + T: Serialize, +{ + serde_json::to_value(value).map_err(|err| validation_error(field, &err.to_string())) +} + +fn apply_config(parsed: &Cli, config: CliConfig) -> Cli { + Cli { + file: config.file, + directory: parsed.directory.clone(), + jobs: config.jobs, + verbose: config.verbose, + locale: config.locale, + fetch_allow_scheme: config.fetch_allow_scheme, + fetch_allow_host: config.fetch_allow_host, + fetch_block_host: config.fetch_block_host, + fetch_default_deny: config.fetch_default_deny, + accessible: config.accessible, + no_emoji: config.no_emoji, + diag_json: config.diag_json, + progress: config.progress, + colour_policy: config.colour_policy, + spinner_mode: config.spinner_mode, + output_format: config.output_format, + theme: canonical_theme(config.theme, config.no_emoji), + command: Some(resolve_command(parsed.command.as_ref(), &config.cmds.build)), + } +} + +fn resolve_command(parsed: Option<&Commands>, build_defaults: &BuildConfig) -> Commands { + match parsed { + Some(Commands::Build(args)) => Commands::Build(BuildArgs { + emit: args.emit.clone().or_else(|| build_defaults.emit.clone()), + targets: if args.targets.is_empty() { + build_defaults.targets.clone() + } else { + args.targets.clone() + }, + }), + Some(other) => other.clone(), + None => Commands::Build(BuildArgs { + emit: build_defaults.emit.clone(), + targets: build_defaults.targets.clone(), + }), + } +} + +const fn canonical_theme(theme: Option, no_emoji: Option) -> Option { + match (theme, no_emoji) { + (Some(value), _) => Some(value), + (None, Some(true)) => Some(Theme::Ascii), + _ => None, + } +} diff --git a/src/cli/mod.rs b/src/cli/mod.rs index d9ab7a26..4b37d7d8 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -1,457 +1,28 @@ -//! Command line interface definition using clap. +//! Command-line parsing plus layered CLI configuration support. //! -//! This module defines the [`Cli`] structure and its subcommands. -//! It mirrors the design described in `docs/netsuke-design.md`. +//! The parser-facing [`Cli`] type remains responsible for user-facing command +//! syntax, while [`CliConfig`] is the authoritative OrthoConfig-derived schema +//! used to merge defaults, configuration files, environment variables, and CLI +//! overrides into the runtime shape consumed by the runner. -use clap::builder::{TypedValueParser, ValueParser}; -use clap::error::ErrorKind; -use clap::parser::ValueSource; -use clap::{ArgMatches, Args, CommandFactory, FromArgMatches, Parser, Subcommand}; -use ortho_config::declarative::LayerComposition; -use ortho_config::figment::{Figment, providers::Env}; -use ortho_config::localize_clap_error_with_command; -use ortho_config::uncased::Uncased; -use ortho_config::{ - ConfigDiscovery, LocalizationArgs, Localizer, MergeComposer, OrthoConfig, OrthoMergeExt, - OrthoResult, sanitize_value, -}; -use serde::{Deserialize, Serialize}; -use std::ffi::OsString; -use std::path::PathBuf; +use ortho_config::OrthoError; use std::sync::Arc; -use crate::cli_l10n::localize_command; -use crate::host_pattern::HostPattern; -use crate::localization::{self, keys}; +mod config; +mod merge; +mod parser; mod parsing; -use parsing::{parse_host_pattern, parse_jobs, parse_locale, parse_scheme}; - -/// Maximum number of jobs accepted by the CLI. -const MAX_JOBS: usize = 64; -const CONFIG_ENV_VAR: &str = "NETSUKE_CONFIG_PATH"; -const ENV_PREFIX: &str = "NETSUKE_"; - -#[derive(Clone)] -struct LocalizedValueParser { - localizer: Arc, - parser: F, -} - -impl LocalizedValueParser { - fn new(localizer: Arc, parser: F) -> Self { - Self { localizer, parser } - } -} - -impl TypedValueParser for LocalizedValueParser -where - F: Fn(&dyn Localizer, &str) -> Result + Clone + Send + Sync + 'static, - T: Send + Sync + Clone + 'static, -{ - type Value = T; - - fn parse_ref( - &self, - cmd: &clap::Command, - _arg: Option<&clap::Arg>, - value: &std::ffi::OsStr, - ) -> Result { - let mut command = cmd.clone(); - let Some(raw_value) = value.to_str() else { - return Err(command.error(ErrorKind::InvalidUtf8, "invalid UTF-8")); - }; - (self.parser)(self.localizer.as_ref(), raw_value) - .map_err(|err| command.error(ErrorKind::ValueValidation, err)) - } -} - -fn validation_message( - localizer: &dyn Localizer, - key: &'static str, - args: Option<&LocalizationArgs<'_>>, - fallback: &str, -) -> String { - localizer.message(key, args, fallback) -} - -/// A modern, friendly build system that uses YAML and Jinja, powered by Ninja. -#[derive(Debug, Parser, Serialize, Deserialize, OrthoConfig)] -#[command(author, version, about, long_about = None)] -#[ortho_config(prefix = "NETSUKE")] -pub struct Cli { - /// Path to the Netsuke manifest file to use. - #[arg(short, long, value_name = "FILE", default_value = "Netsukefile")] - #[ortho_config(default = default_manifest_path())] - pub file: PathBuf, - - /// Run as if started in this directory. - /// - /// This affects manifest lookup, output paths, and config discovery. - #[arg(short = 'C', long, value_name = "DIR")] - pub directory: Option, - - /// Set the number of parallel build jobs. - /// - /// Values must be between 1 and 64. - #[arg(short, long, value_name = "N")] - pub jobs: Option, - - /// Enable verbose diagnostic logging and completion timing summaries. - #[arg(short, long)] - #[ortho_config(default = false)] - pub verbose: bool, - - /// Locale tag for CLI copy (for example: en-US, es-ES). - #[arg(long, value_name = "LOCALE")] - pub locale: Option, - - /// Additional URL schemes allowed for the `fetch` helper. - #[arg(long = "fetch-allow-scheme", value_name = "SCHEME")] - #[ortho_config(merge_strategy = "append")] - pub fetch_allow_scheme: Vec, - - /// Hostnames that are permitted when default deny is enabled. - /// - /// Supports wildcards such as `*.example.com`. - #[arg(long = "fetch-allow-host", value_name = "HOST")] - #[ortho_config(merge_strategy = "append")] - pub fetch_allow_host: Vec, - - /// Hostnames that are always blocked, even when allowed elsewhere. - /// - /// Supports wildcards such as `*.example.com`. - #[arg(long = "fetch-block-host", value_name = "HOST")] - #[ortho_config(merge_strategy = "append")] - pub fetch_block_host: Vec, - - /// Deny all hosts by default; only allow the declared allowlist. - #[arg(long = "fetch-default-deny")] - #[ortho_config(default = false)] - pub fetch_default_deny: bool, - - /// Force accessible output mode on or off (overrides auto-detection). - #[arg(long)] - pub accessible: Option, - - /// Suppress emoji glyphs in output (overrides auto-detection). - #[arg(long)] - pub no_emoji: Option, - - /// Emit machine-readable diagnostics in JSON on stderr. - #[arg(long)] - #[ortho_config(default = false)] - pub diag_json: bool, - - /// Force standard progress summaries on or off. - /// - /// When omitted, Netsuke enables progress summaries in standard mode. - #[arg(long)] - pub progress: Option, - - /// Optional subcommand to execute; defaults to `build` when omitted. - /// - /// `OrthoConfig` merging ignores this field; CLI parsing supplies it. - #[serde(skip)] - #[command(subcommand)] - #[ortho_config(skip_cli)] - pub command: Option, -} - -impl Cli { - /// Apply the default command if none was specified. - #[must_use] - pub fn with_default_command(mut self) -> Self { - if self.command.is_none() { - self.command = Some(Commands::Build(BuildArgs { - emit: None, - targets: Vec::new(), - })); - } - self - } -} - -impl Default for Cli { - fn default() -> Self { - Self { - file: default_manifest_path(), - directory: None, - jobs: None, - verbose: false, - locale: None, - fetch_allow_scheme: Vec::new(), - fetch_allow_host: Vec::new(), - fetch_block_host: Vec::new(), - fetch_default_deny: false, - accessible: None, - progress: None, - no_emoji: None, - diag_json: false, - command: None, - } - .with_default_command() - } -} - -/// Arguments accepted by the `build` command. -#[derive(Debug, Args, PartialEq, Eq, Clone, Serialize, Deserialize)] -pub struct BuildArgs { - /// Write the generated Ninja manifest to this path and retain it. - #[arg(long, value_name = "FILE")] - pub emit: Option, - - /// A list of specific targets to build. - #[serde(default)] - pub targets: Vec, -} - -/// Available top-level commands for Netsuke. -#[derive(Debug, Subcommand, PartialEq, Eq, Clone, Serialize, Deserialize)] -#[serde(rename_all = "kebab-case")] -pub enum Commands { - /// Build specified targets (or default targets if none are given). - Build(BuildArgs), - - /// Remove build artefacts and intermediate files. - Clean, - - /// Display the build dependency graph in DOT format for visualisation. - Graph, - - /// Write the Ninja manifest to the specified file without invoking Ninja. - Manifest { - /// Output path for the generated Ninja file. - /// - /// Use `-` to write to stdout. - #[arg(value_name = "FILE")] - file: PathBuf, - }, -} - -/// Return the default manifest filename when none is provided. -fn default_manifest_path() -> PathBuf { - PathBuf::from("Netsukefile") -} - -/// Inspect raw arguments and extract the requested locale before full parsing. -#[must_use] -pub fn locale_hint_from_args(args: &[OsString]) -> Option { - crate::cli_l10n::locale_hint_from_args(args) -} - -/// Inspect raw arguments and extract the requested `--diag-json` state. -#[must_use] -pub fn diag_json_hint_from_args(args: &[OsString]) -> Option { - crate::cli_l10n::diag_json_hint_from_args(args) -} - -/// Parse CLI arguments with localized clap output. -/// -/// Returns both the parsed CLI struct and the `ArgMatches` required for -/// configuration merging. -/// -/// # Errors -/// -/// Returns a `clap::Error` with localization applied when parsing fails. -pub fn parse_with_localizer_from( - iter: I, - localizer: &Arc, -) -> Result<(Cli, ArgMatches), clap::Error> -where - I: IntoIterator, - T: Into + Clone, -{ - let mut command = localize_command(Cli::command(), localizer.as_ref()); - command = configure_validation_parsers(command, localizer); - let matches = command - .try_get_matches_from_mut(iter) - .map_err(|err| localize_clap_error_with_command(err, localizer.as_ref(), Some(&command)))?; - // Clone matches before from_arg_matches_mut consumes the values. - let matches_for_merge = matches.clone(); - let mut matches_for_parse = matches; - let cli = Cli::from_arg_matches_mut(&mut matches_for_parse).map_err(|clap_err| { - let with_cmd = clap_err.with_cmd(&command); - localize_clap_error_with_command(with_cmd, localizer.as_ref(), Some(&command)) - })?; - Ok((cli, matches_for_merge)) -} - -/// Return the prefixed environment provider for CLI configuration. -fn env_provider() -> Env { - Env::prefixed(ENV_PREFIX) -} - -/// Build configuration discovery rooted in the optional working directory. -fn config_discovery(directory: Option<&PathBuf>) -> ConfigDiscovery { - let mut builder = ConfigDiscovery::builder("netsuke").env_var(CONFIG_ENV_VAR); - if let Some(dir) = directory { - builder = builder.clear_project_roots().add_project_root(dir); - } - builder.build() -} - -/// Return `true` when no CLI overrides were supplied. -/// -/// The merge pipeline treats an empty JSON object as "no overrides". -fn is_empty_value(value: &serde_json::Value) -> bool { - matches!(value, serde_json::Value::Object(map) if map.is_empty()) -} - -fn diag_json_from_layer(value: &serde_json::Value) -> Option { - value - .as_object() - .and_then(|map| map.get("diag_json")) - .and_then(serde_json::Value::as_bool) -} - -/// Resolve the effective diagnostic JSON preference from the raw config layers. -/// -/// This is used before full config merging so startup and merge-time failures -/// can still honour `diag_json` values sourced from config files or the -/// environment. -#[must_use] -pub fn resolve_merged_diag_json(cli: &Cli, matches: &ArgMatches) -> bool { - let mut diag_json = Cli::default().diag_json; - - let discovery = config_discovery(cli.directory.as_ref()); - let file_layers = discovery.compose_layers(); - for layer in file_layers.value { - let layer_value = layer.into_value(); - if let Some(layer_diag_json) = diag_json_from_layer(&layer_value) { - diag_json = layer_diag_json; - } - } - - let env_provider = env_provider() - .map(|key| Uncased::new(key.as_str().to_ascii_uppercase())) - .split("__"); - if let Ok(value) = Figment::from(env_provider).extract::() - && let Some(env_diag_json) = diag_json_from_layer(&value) - { - diag_json = env_diag_json; - } - - if matches.value_source("diag_json") == Some(ValueSource::CommandLine) { - cli.diag_json - } else { - diag_json - } -} - -fn cli_overrides_from_matches(cli: &Cli, matches: &ArgMatches) -> OrthoResult { - let value = sanitize_value(cli)?; - let mut map = match value { - serde_json::Value::Object(map) => map, - other => { - let mut args = LocalizationArgs::default(); - args.insert("value", format!("{other:?}").into()); - let localizer = localization::localizer(); - return Err(Arc::new(ortho_config::OrthoError::Validation { - key: String::from("cli"), - message: validation_message( - localizer.as_ref(), - keys::CLI_CONFIG_EXPECTED_OBJECT, - Some(&args), - &format!("expected parsed CLI values to serialize to an object, got {other:?}"), - ), - })); - } - }; - - map.remove("command"); - for field in [ - "file", - "verbose", - "fetch_default_deny", - "fetch_allow_scheme", - "fetch_allow_host", - "fetch_block_host", - "accessible", - "progress", - "no_emoji", - "diag_json", - ] { - if matches.value_source(field) != Some(ValueSource::CommandLine) { - map.remove(field); - } - } - - Ok(serde_json::Value::Object(map)) -} - -fn configure_validation_parsers( - mut command: clap::Command, - localizer: &Arc, -) -> clap::Command { - let jobs_parser = LocalizedValueParser::new(Arc::clone(localizer), parse_jobs); - let locale_parser = LocalizedValueParser::new(Arc::clone(localizer), parse_locale); - let scheme_parser = LocalizedValueParser::new(Arc::clone(localizer), parse_scheme); - let host_parser = LocalizedValueParser::new(Arc::clone(localizer), parse_host_pattern); - - command = command.mut_arg("jobs", |arg| { - arg.value_parser(ValueParser::new(jobs_parser)) - }); - command = command.mut_arg("locale", |arg| { - arg.value_parser(ValueParser::new(locale_parser)) - }); - command = command.mut_arg("fetch_allow_scheme", |arg| { - arg.value_parser(ValueParser::new(scheme_parser.clone())) - }); - command = command.mut_arg("fetch_allow_host", |arg| { - arg.value_parser(ValueParser::new(host_parser.clone())) - }); - command = command.mut_arg("fetch_block_host", |arg| { - arg.value_parser(ValueParser::new(host_parser)) - }); - command -} - -/// Merge configuration layers over the parsed CLI values. -/// -/// # Errors -/// -/// Returns an [`ortho_config::OrthoError`] if layer composition or merging -/// fails. -pub fn merge_with_config(cli: &Cli, matches: &ArgMatches) -> OrthoResult { - let command = cli.command.clone(); - let mut errors = Vec::new(); - let mut composer = MergeComposer::with_capacity(4); - - match sanitize_value(&Cli::default()) { - Ok(value) => composer.push_defaults(value), - Err(err) => errors.push(err), - } - - let discovery = config_discovery(cli.directory.as_ref()); - let mut file_layers = discovery.compose_layers(); - errors.append(&mut file_layers.required_errors); - if file_layers.value.is_empty() { - errors.append(&mut file_layers.optional_errors); - } - for layer in file_layers.value { - composer.push_layer(layer); - } - - let env_provider = env_provider() - .map(|key| Uncased::new(key.as_str().to_ascii_uppercase())) - .split("__"); - match Figment::from(env_provider) - .extract::() - .into_ortho_merge() - { - Ok(value) => composer.push_environment(value), - Err(err) => errors.push(err), - } - - match cli_overrides_from_matches(cli, matches) { - Ok(value) if !is_empty_value(&value) => composer.push_cli(value), - Ok(_) => {} - Err(err) => errors.push(err), - } +pub use config::{CliConfig, ColourPolicy, OutputFormat, SpinnerMode, Theme}; +pub use merge::{merge_with_config, resolve_merged_diag_json}; +pub use parser::{ + BuildArgs, Cli, Commands, diag_json_hint_from_args, locale_hint_from_args, + parse_with_localizer_from, +}; - let composition = LayerComposition::new(composer.layers(), errors); - let mut merged = composition.into_merge_result(Cli::merge_from_layers)?; - merged.command = command; - Ok(merged) +pub(super) fn validation_error(key: &str, message: &str) -> Arc { + Arc::new(OrthoError::Validation { + key: key.to_owned(), + message: message.to_owned(), + }) } diff --git a/src/cli/parser.rs b/src/cli/parser.rs new file mode 100644 index 00000000..9a07b727 --- /dev/null +++ b/src/cli/parser.rs @@ -0,0 +1,312 @@ +//! Clap-facing parser types and localized parsing helpers. + +use clap::builder::{TypedValueParser, ValueParser}; +use clap::error::ErrorKind; +use clap::{ArgMatches, Args, CommandFactory, FromArgMatches, Parser, Subcommand}; +use ortho_config::localize_clap_error_with_command; +use ortho_config::{LocalizationArgs, Localizer}; +use serde::{Deserialize, Serialize}; +use std::ffi::OsString; +use std::path::PathBuf; +use std::sync::Arc; + +use super::config::CliConfig; +use super::parsing::{parse_host_pattern, parse_jobs, parse_locale, parse_scheme}; +use super::{ColourPolicy, OutputFormat, SpinnerMode, Theme}; +use crate::cli_l10n::localize_command; +pub use crate::cli_l10n::{diag_json_hint_from_args, locale_hint_from_args}; +use crate::host_pattern::HostPattern; + +#[derive(Clone)] +struct LocalizedValueParser { + localizer: Arc, + parser: F, +} + +impl LocalizedValueParser { + fn new(localizer: Arc, parser: F) -> Self { + Self { localizer, parser } + } +} + +impl TypedValueParser for LocalizedValueParser +where + F: Fn(&dyn Localizer, &str) -> Result + Clone + Send + Sync + 'static, + T: Send + Sync + Clone + 'static, +{ + type Value = T; + + fn parse_ref( + &self, + cmd: &clap::Command, + _arg: Option<&clap::Arg>, + value: &std::ffi::OsStr, + ) -> Result { + let mut command = cmd.clone(); + let Some(raw_value) = value.to_str() else { + return Err(command.error(ErrorKind::InvalidUtf8, "invalid UTF-8")); + }; + (self.parser)(self.localizer.as_ref(), raw_value) + .map_err(|err| command.error(ErrorKind::ValueValidation, err)) + } +} + +pub(super) fn validation_message( + localizer: &dyn Localizer, + key: &'static str, + args: Option<&LocalizationArgs<'_>>, + fallback: &str, +) -> String { + localizer.message(key, args, fallback) +} + +/// A modern, friendly build system that uses YAML and Jinja, powered by Ninja. +#[derive(Debug, Parser, Serialize, Deserialize)] +#[command(author, version, about, long_about = None)] +pub struct Cli { + /// Path to the Netsuke manifest file to use. + #[arg( + short, + long, + value_name = "FILE", + default_value_os_t = CliConfig::default_manifest_path() + )] + pub file: PathBuf, + + /// Run as if started in this directory. + /// + /// This affects manifest lookup, output paths, and config discovery. + #[arg(short = 'C', long, value_name = "DIR")] + pub directory: Option, + + /// Set the number of parallel build jobs. + /// + /// Values must be between 1 and 64. + #[arg(short, long, value_name = "N")] + pub jobs: Option, + + /// Enable verbose diagnostic logging and completion timing summaries. + #[arg(short, long)] + pub verbose: bool, + + /// Locale tag for CLI copy (for example: en-US, es-ES). + #[arg(long, value_name = "LOCALE")] + pub locale: Option, + + /// Additional URL schemes allowed for the `fetch` helper. + #[arg(long = "fetch-allow-scheme", value_name = "SCHEME")] + pub fetch_allow_scheme: Vec, + + /// Hostnames that are permitted when default deny is enabled. + /// + /// Supports wildcards such as `*.example.com`. + #[arg(long = "fetch-allow-host", value_name = "HOST")] + pub fetch_allow_host: Vec, + + /// Hostnames that are always blocked, even when allowed elsewhere. + /// + /// Supports wildcards such as `*.example.com`. + #[arg(long = "fetch-block-host", value_name = "HOST")] + pub fetch_block_host: Vec, + + /// Deny all hosts by default; only allow the declared allowlist. + #[arg(long = "fetch-default-deny")] + pub fetch_default_deny: bool, + + /// Force accessible output mode on or off (overrides auto-detection). + #[arg(long)] + pub accessible: Option, + + /// Suppress emoji glyphs in output (overrides auto-detection). + #[arg(long)] + pub no_emoji: Option, + + /// Emit machine-readable diagnostics in JSON on stderr. + #[arg(long)] + pub diag_json: bool, + + /// Force standard progress summaries on or off. + /// + /// When omitted, Netsuke enables progress summaries in standard mode. + #[arg(long)] + pub progress: Option, + + /// Override colour policy for terminal output. + #[arg(long, value_name = "POLICY")] + pub colour_policy: Option, + + /// Override spinner animation mode. + #[arg(long, value_name = "MODE")] + pub spinner_mode: Option, + + /// Override output format style. + #[arg(long, value_name = "FORMAT")] + pub output_format: Option, + + /// Override presentation theme. + #[arg(long, value_name = "THEME")] + pub theme: Option, + + /// Optional subcommand to execute; defaults to `build` when omitted. + #[serde(skip)] + #[command(subcommand)] + pub command: Option, +} + +impl Cli { + /// Apply the default command if none was specified. + #[must_use] + pub fn with_default_command(mut self) -> Self { + if self.command.is_none() { + self.command = Some(Commands::Build(BuildArgs::default())); + } + self + } + + /// Return the effective emoji override for output preference resolution. + #[must_use] + pub const fn no_emoji_override(&self) -> Option { + match self.theme { + Some(Theme::Ascii) => Some(true), + Some(Theme::Unicode) => Some(false), + _ => { + if matches!(self.no_emoji, Some(true)) { + Some(true) + } else { + None + } + } + } + } + + /// Return whether progress summaries should be enabled. + #[must_use] + pub const fn progress_enabled(&self) -> bool { + match (self.progress, self.spinner_mode) { + (Some(value), _) => value, + (None, Some(SpinnerMode::Disabled)) => false, + _ => true, + } + } +} + +impl Default for Cli { + fn default() -> Self { + Self { + file: CliConfig::default_manifest_path(), + directory: None, + jobs: None, + verbose: false, + locale: None, + fetch_allow_scheme: Vec::new(), + fetch_allow_host: Vec::new(), + fetch_block_host: Vec::new(), + fetch_default_deny: false, + accessible: None, + progress: None, + no_emoji: None, + diag_json: false, + colour_policy: None, + spinner_mode: None, + output_format: None, + theme: None, + command: None, + } + .with_default_command() + } +} + +/// Arguments accepted by the `build` command. +#[derive(Debug, Args, PartialEq, Eq, Clone, Serialize, Deserialize, Default)] +pub struct BuildArgs { + /// Write the generated Ninja manifest to this path and retain it. + #[arg(long, value_name = "FILE")] + pub emit: Option, + + /// A list of specific targets to build. + #[serde(default)] + pub targets: Vec, +} + +/// Available top-level commands for Netsuke. +#[derive(Debug, Subcommand, PartialEq, Eq, Clone, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub enum Commands { + /// Build specified targets (or default targets if none are given). + Build(BuildArgs), + + /// Remove build artefacts and intermediate files. + Clean, + + /// Display the build dependency graph in DOT format for visualisation. + Graph, + + /// Write the Ninja manifest to the specified file without invoking Ninja. + Manifest { + /// Output path for the generated Ninja file. + /// + /// Use `-` to write to stdout. + #[arg(value_name = "FILE")] + file: PathBuf, + }, +} + +/// Parse CLI arguments with localized clap output. +/// +/// Returns both the parsed CLI struct and the `ArgMatches` required for +/// configuration merging. +/// +/// # Errors +/// +/// Returns a `clap::Error` with localization applied when parsing fails. +pub fn parse_with_localizer_from( + iter: I, + localizer: &Arc, +) -> Result<(Cli, ArgMatches), clap::Error> +where + I: IntoIterator, + T: Into + Clone, +{ + let mut command = localize_command(Cli::command(), localizer.as_ref()); + command = configure_validation_parsers(command, localizer); + let matches = command + .try_get_matches_from_mut(iter) + .map_err(|err| localize_clap_error_with_command(err, localizer.as_ref(), Some(&command)))?; + let matches_for_merge = matches.clone(); + let mut matches_for_parse = matches; + let cli = Cli::from_arg_matches_mut(&mut matches_for_parse).map_err(|clap_err| { + let with_cmd = clap_err.with_cmd(&command); + localize_clap_error_with_command(with_cmd, localizer.as_ref(), Some(&command)) + })?; + Ok((cli, matches_for_merge)) +} + +fn configure_validation_parsers( + mut command: clap::Command, + localizer: &Arc, +) -> clap::Command { + let jobs_parser = LocalizedValueParser::new(Arc::clone(localizer), parse_jobs); + let locale_parser = LocalizedValueParser::new(Arc::clone(localizer), parse_locale); + let scheme_parser = LocalizedValueParser::new(Arc::clone(localizer), parse_scheme); + let host_parser = LocalizedValueParser::new(Arc::clone(localizer), parse_host_pattern); + + command = command.mut_arg("jobs", |arg| { + arg.value_parser(ValueParser::new(jobs_parser)) + }); + command = command.mut_arg("locale", |arg| { + arg.value_parser(ValueParser::new(locale_parser)) + }); + command = command.mut_arg("fetch_allow_scheme", |arg| { + arg.value_parser(ValueParser::new(scheme_parser.clone())) + }); + command = command.mut_arg("fetch_allow_host", |arg| { + arg.value_parser(ValueParser::new(host_parser.clone())) + }); + command = command.mut_arg("fetch_block_host", |arg| { + arg.value_parser(ValueParser::new(host_parser)) + }); + command +} + +/// Maximum number of jobs accepted by the CLI. +pub(super) const MAX_JOBS: usize = 64; diff --git a/src/cli/parsing.rs b/src/cli/parsing.rs index 147d4243..fca65bce 100644 --- a/src/cli/parsing.rs +++ b/src/cli/parsing.rs @@ -10,24 +10,24 @@ pub(super) fn parse_jobs(localizer: &dyn Localizer, s: &str) -> Result Result Result { let trimmed = s.trim(); if trimmed.is_empty() { - return Err(super::validation_message( + return Err(super::parser::validation_message( localizer, keys::CLI_SCHEME_EMPTY, None, @@ -50,7 +50,7 @@ pub(super) fn parse_scheme(localizer: &dyn Localizer, s: &str) -> Result Result Result Result { let trimmed = s.trim(); if trimmed.is_empty() { - return Err(super::validation_message( + return Err(super::parser::validation_message( localizer, keys::CLI_LOCALE_EMPTY, None, @@ -85,7 +85,7 @@ pub(super) fn parse_locale(localizer: &dyn Localizer, s: &str) -> Result ExitCode::SUCCESS, Err(err) => handle_runner_error(err, prefs, runtime_mode), diff --git a/src/runner/mod.rs b/src/runner/mod.rs index 0cc6d8c6..416c4821 100644 --- a/src/runner/mod.rs +++ b/src/runner/mod.rs @@ -136,7 +136,7 @@ const fn should_force_text_task_updates(mode: OutputMode, stdout_is_tty: bool) - /// Returns an error if manifest generation or the Ninja process fails. pub fn run(cli: &Cli, prefs: OutputPrefs) -> Result<()> { let mode = output_mode::resolve(cli.accessible); - let progress_enabled = cli.progress.unwrap_or(true) && !cli.diag_json; + let progress_enabled = cli.progress_enabled() && !cli.diag_json; let stdout_is_tty = std::io::stdout().is_terminal(); let reporter = make_reporter(ReporterOptions { mode, diff --git a/test_support/src/netsuke.rs b/test_support/src/netsuke.rs index c6f3b3f8..bd4a4299 100644 --- a/test_support/src/netsuke.rs +++ b/test_support/src/netsuke.rs @@ -53,10 +53,15 @@ pub struct NetsukeRun { /// Returns an error when `netsuke` cannot be located or the process cannot be /// spawned. pub fn run_netsuke_in(current_dir: &Path, args: &[&str]) -> Result { + let isolated_config_home = current_dir.join(".config"); let mut cmd = assert_cmd::Command::new(netsuke_executable()?); let output = cmd .current_dir(current_dir) .env("PATH", "") + .env_remove("NETSUKE_CONFIG_PATH") + .env_remove("NETSUKE_OUTPUT_FORMAT") + .env("HOME", current_dir) + .env("XDG_CONFIG_HOME", &isolated_config_home) .args(args) .output() .context("run netsuke command")?; diff --git a/tests/bdd/fixtures/mod.rs b/tests/bdd/fixtures/mod.rs index 97a405c0..61a252f2 100644 --- a/tests/bdd/fixtures/mod.rs +++ b/tests/bdd/fixtures/mod.rs @@ -27,6 +27,7 @@ use std::path::PathBuf; use std::sync::MutexGuard; use test_support::PathGuard; use test_support::env::{NinjaEnvGuard, restore_many}; +use test_support::env_lock::EnvLock; use test_support::http::HttpServer; /// Combined test world for all BDD scenarios. @@ -148,6 +149,8 @@ pub struct TestWorld { // Environment state /// Snapshot of pre-scenario values for environment variables that were overridden. pub env_vars: RefCell>>, + /// Scenario-scoped guard that serialises environment mutations when needed. + pub env_lock: RefCell>, } impl TestWorld { @@ -190,6 +193,7 @@ impl Drop for TestWorld { self.ninja_env_guard.borrow_mut().take(); self.localization_guard.borrow_mut().take(); self.localization_lock.borrow_mut().take(); + self.env_lock.borrow_mut().take(); self.restore_environment(); self.stdlib_text.clear(); } diff --git a/tests/bdd/steps/configuration_preferences.rs b/tests/bdd/steps/configuration_preferences.rs new file mode 100644 index 00000000..a245c7c7 --- /dev/null +++ b/tests/bdd/steps/configuration_preferences.rs @@ -0,0 +1,335 @@ +//! Step definitions for layered CLI configuration preferences. + +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 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}; +use std::ffi::OsStr; +use std::fs; +use std::path::PathBuf; +use std::sync::Arc; +use test_support::display_error_chain; +use test_support::env_lock::EnvLock; + +const CONFIG_ENV_VAR: &str = "NETSUKE_CONFIG_PATH"; +const LOCALE_ENV_VAR: &str = "NETSUKE_LOCALE"; + +fn workspace_path(world: &TestWorld) -> Result { + let temp = world.temp_dir.borrow(); + let dir = temp + .as_ref() + .context("temp dir has not been initialised for configuration steps")?; + Ok(dir.path().to_path_buf()) +} + +fn ensure_env_lock(world: &TestWorld) { + if world.env_lock.borrow().is_none() { + *world.env_lock.borrow_mut() = Some(EnvLock::acquire()); + } +} + +fn write_config(world: &TestWorld, contents: &str) -> Result<()> { + ensure_env_lock(world); + let workspace = workspace_path(world)?; + let path = workspace.join("netsuke.toml"); + 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, path.as_os_str()) }; + world.track_env_var(CONFIG_ENV_VAR.to_owned(), previous); + Ok(()) +} + +fn merge_cli(world: &TestWorld, args: &str) { + let tokens = build_tokens(args); + let localizer = Arc::from(cli_localization::build_localizer(None)); + let outcome = netsuke::cli::parse_with_localizer_from(tokens, &localizer) + .and_then(|(cli, matches)| { + netsuke::cli::merge_with_config(&cli, &matches).map_err(|err| { + clap::Error::raw( + clap::error::ErrorKind::InvalidValue, + display_error_chain(err.as_ref()), + ) + }) + }) + .map(Cli::with_default_command) + .map_err(|err| err.to_string()); + 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")) +} + +#[given("the Netsuke config file sets locale to {locale:string}")] +fn config_sets_locale(world: &TestWorld, locale: &str) -> Result<()> { + write_config(world, &format!("locale = \"{locale}\"\n")) +} + +#[given("the NETSUKE_LOCALE environment variable is {locale:string}")] +#[expect( + clippy::unnecessary_wraps, + reason = "rstest-bdd macro generates Result wrapper; FIXME: https://github.com/leynos/rstest-bdd/issues/381" +)] +fn set_environment_locale_override(world: &TestWorld, locale: &str) -> Result<()> { + ensure_env_lock(world); + let previous = std::env::var_os(LOCALE_ENV_VAR); + // SAFETY: `EnvLock` is held in `world.env_lock` for the lifetime of the scenario. + unsafe { std::env::set_var(LOCALE_ENV_VAR, OsStr::new(locale)) }; + world.track_env_var(LOCALE_ENV_VAR.to_owned(), previous); + Ok(()) +} + +#[given("the Netsuke config file sets output format to {format:string}")] +fn config_sets_output_format(world: &TestWorld, format: &str) -> Result<()> { + 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")] +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" +)] +#[when("the CLI is parsed and merged with {args:string}")] +fn parse_and_merge_cli(world: &TestWorld, args: &str) -> Result<()> { + merge_cli(world, args); + Ok(()) +} + +#[when("merged output preferences are resolved")] +fn resolve_merged_output_prefs(world: &TestWorld) -> Result<()> { + let prefs = world + .cli + .with_ref(|cli| output_prefs::resolve(cli.no_emoji_override())) + .ok_or_else(|| anyhow!("expected merged CLI before resolving output prefs"))?; + world.output_prefs.set(prefs); + Ok(()) +} + +#[then("the merged CLI uses build target {target:string}")] +fn merged_cli_uses_build_target(world: &TestWorld, target: &str) -> Result<()> { + let command = world + .cli + .with_ref(|cli| cli.command.clone()) + .context("expected merged CLI to be available")? + .context("expected merged CLI command to be set")?; + match command { + Commands::Build(args) => ensure!( + args.targets.first().map(String::as_str) == Some(target), + "expected first merged build target '{target}', got {:?}", + args.targets, + ), + other => bail!("expected merged build command, got {other:?}"), + } + Ok(()) +} + +#[then("the merged locale is {locale:string}")] +fn merged_locale_is(world: &TestWorld, locale: &str) -> Result<()> { + let merged_locale = world + .cli + .with_ref(|cli| cli.locale.clone()) + .context("expected merged CLI to be available")?; + ensure!( + merged_locale.as_deref() == Some(locale), + "expected merged locale '{locale}', got {merged_locale:?}", + ); + Ok(()) +} + +#[then("verbose mode is enabled in the merged CLI")] +fn merged_verbose_enabled(world: &TestWorld) -> Result<()> { + let verbose = world + .cli + .with_ref(|cli| cli.verbose) + .context("expected merged CLI to be available")?; + ensure!(verbose, "expected merged verbose mode to be enabled"); + Ok(()) +} + +#[then("the merged theme is ascii")] +fn merged_theme_is_ascii(world: &TestWorld) -> Result<()> { + assert_merged_field(world, |cli| cli.theme, Theme::Ascii, "theme") +} + +#[then("the merge error should contain {fragment:string}")] +fn merge_error_contains(world: &TestWorld, fragment: &str) -> Result<()> { + let error = world + .cli_error + .get() + .context("expected a merge error to be captured")?; + ensure!( + error.contains(fragment), + "expected merge error to contain '{fragment}', got '{error}'", + ); + 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/bdd/steps/locale_resolution.rs b/tests/bdd/steps/locale_resolution.rs index 934180bd..d8ddee5c 100644 --- a/tests/bdd/steps/locale_resolution.rs +++ b/tests/bdd/steps/locale_resolution.rs @@ -6,7 +6,7 @@ use crate::bdd::fixtures::TestWorld; use crate::bdd::helpers::tokens::build_tokens; use anyhow::{Context, Result, ensure}; -use netsuke::cli::Cli; +use netsuke::cli::{Cli, CliConfig}; use netsuke::cli_localization; use netsuke::locale_resolution; use netsuke::localization::keys; @@ -17,7 +17,7 @@ use test_support::locale_stubs::{StubEnv, StubSystemLocale}; fn merge_locale_layers(world: &TestWorld) -> Result { let mut composer = MergeComposer::new(); - let defaults = sanitize_value(&Cli::default())?; + let defaults = sanitize_value(&CliConfig::default())?; composer.push_defaults(defaults); if let Some(locale) = world.locale_config.get() { @@ -32,7 +32,11 @@ fn merge_locale_layers(world: &TestWorld) -> Result { composer.push_cli(json!({ "locale": locale })); } - Cli::merge_from_layers(composer.layers()).context("merge locale layers") + let merged = CliConfig::merge_from_layers(composer.layers()).context("merge locale layers")?; + Ok(Cli { + locale: merged.locale, + ..Cli::default() + }) } fn record_resolved_locale(world: &TestWorld, resolved: Option<&str>) { diff --git a/tests/bdd/steps/mod.rs b/tests/bdd/steps/mod.rs index e145f266..a8b084a2 100644 --- a/tests/bdd/steps/mod.rs +++ b/tests/bdd/steps/mod.rs @@ -16,6 +16,7 @@ mod accessibility_preferences; mod accessible_output; mod cli; +mod configuration_preferences; #[cfg(unix)] mod fs; mod ir; diff --git a/tests/cli_tests/merge.rs b/tests/cli_tests/merge.rs index 32e65c5f..a0628250 100644 --- a/tests/cli_tests/merge.rs +++ b/tests/cli_tests/merge.rs @@ -4,7 +4,7 @@ //! CLI) and list-value appending. use anyhow::{Context, Result, ensure}; -use netsuke::cli::Cli; +use netsuke::cli::{CliConfig, Theme}; use netsuke::cli_localization; use ortho_config::{MergeComposer, sanitize_value}; use rstest::{fixture, rstest}; @@ -18,7 +18,101 @@ use test_support::{EnvVarGuard, env_lock::EnvLock}; #[fixture] fn default_cli_json() -> Result { - sanitize_value(&Cli::default()) + sanitize_value(&CliConfig::default()) +} + +fn with_config_file(toml_content: &str, cli_args: &[&str], f: F) -> anyhow::Result +where + F: FnOnce(netsuke::cli::Cli) -> anyhow::Result, +{ + let _env_lock = test_support::env_lock::EnvLock::acquire(); + let temp_dir = tempfile::tempdir().context("create temporary config directory")?; + let config_path = temp_dir.path().join("netsuke.toml"); + std::fs::write(&config_path, toml_content).context("write netsuke.toml")?; + let _config_guard = + test_support::EnvVarGuard::set("NETSUKE_CONFIG_PATH", config_path.as_os_str()); + let localizer = std::sync::Arc::from(netsuke::cli_localization::build_localizer(None)); + let (cli, matches) = netsuke::cli::parse_with_localizer_from(cli_args, &localizer) + .context("parse CLI args for merge")?; + let merged = netsuke::cli::merge_with_config(&cli, &matches) + .context("merge CLI and configuration layers")?; + f(merged) +} + +fn assert_build_targets( + toml_content: &str, + cli_args: &[&str], + expected_targets: &[String], +) -> anyhow::Result<()> { + with_config_file(toml_content, cli_args, |merged| { + let Some(netsuke::cli::Commands::Build(args)) = merged.command else { + anyhow::bail!("expected merged command to be build"); + }; + ensure!( + args.targets == expected_targets, + "build targets mismatch: got {:?}, expected {:?}", + args.targets, + expected_targets, + ); + Ok(()) + }) +} + +#[derive(Debug)] +enum ExpectedValidationError { + ThemeUnicodeAliasConflict, + ThemeAsciiAliasConflict, + SpinnerDisabledConflict, + SpinnerEnabledConflict, + UnsupportedOutputFormat, +} + +impl ExpectedValidationError { + fn expected_fragment(&self) -> &'static str { + match self { + Self::ThemeUnicodeAliasConflict => { + "theme = \"unicode\" conflicts with no_emoji = true" + } + Self::ThemeAsciiAliasConflict => { + "no_emoji = false conflicts with theme = \"ascii\"" + } + Self::SpinnerDisabledConflict => { + "spinner_mode = \"disabled\" conflicts with progress = true" + } + Self::SpinnerEnabledConflict => { + "progress = false conflicts with spinner_mode = \"enabled\"" + } + Self::UnsupportedOutputFormat => { + "output_format = \"json\" is not supported yet" + } + } + } +} + +fn merge_defaults_with_file_layer( + defaults: serde_json::Value, + file_layer: serde_json::Value, +) -> anyhow::Result { + let mut composer = ortho_config::MergeComposer::new(); + composer.push_defaults(defaults); + composer.push_file(file_layer, None); + netsuke::cli::CliConfig::merge_from_layers(composer.layers()).map_err(anyhow::Error::from) +} + +fn assert_merge_rejects( + defaults: serde_json::Value, + file_layer: serde_json::Value, + expected_error: ExpectedValidationError, +) -> anyhow::Result<()> { + let err = match merge_defaults_with_file_layer(defaults, file_layer) { + Ok(value) => anyhow::bail!("merge should have failed, got {value:#?}"), + Err(err) => err, + }; + ensure!( + err.to_string().contains(expected_error.expected_fragment()), + "unexpected error text: {err}", + ); + Ok(()) } #[rstest] @@ -59,7 +153,7 @@ fn cli_merge_layers_respects_precedence_and_appends_lists( "diag_json": true, "verbose": true })); - let merged = Cli::merge_from_layers(composer.layers())?; + let merged = CliConfig::merge_from_layers(composer.layers())?; ensure!( merged.file.as_path() == Path::new("Configfile"), "file layer should override defaults", @@ -164,10 +258,78 @@ fn cli_merge_layers_prefers_cli_then_env_then_file_for_locale( composer.push_environment(json!({ "locale": "es-ES" })); composer.push_cli(json!({ "locale": "en-US" })); - let merged = Cli::merge_from_layers(composer.layers())?; + let merged = CliConfig::merge_from_layers(composer.layers())?; ensure!( merged.locale.as_deref() == Some("en-US"), "CLI locale should override env and file layers", ); Ok(()) } + +#[rstest] +fn cli_config_build_defaults_apply_when_cli_targets_are_absent() -> Result<()> { + assert_build_targets( + r#" +[cmds.build] +targets = ["all", "docs"] +"#, + &["netsuke"], + &[String::from("all"), String::from("docs")], + ) +} + +#[rstest] +fn cli_config_explicit_targets_override_configured_build_defaults() -> Result<()> { + assert_build_targets( + r#" +[cmds.build] +targets = ["all"] +"#, + &["netsuke", "build", "lint"], + &[String::from("lint")], + ) +} + +#[rstest] +#[case( + json!({ "theme": "unicode", "no_emoji": true }), + ExpectedValidationError::ThemeUnicodeAliasConflict, +)] +#[case( + json!({ "theme": "ascii", "no_emoji": false }), + ExpectedValidationError::ThemeAsciiAliasConflict, +)] +#[case( + json!({ "spinner_mode": "disabled", "progress": true }), + ExpectedValidationError::SpinnerDisabledConflict, +)] +#[case( + json!({ "spinner_mode": "enabled", "progress": false }), + ExpectedValidationError::SpinnerEnabledConflict, +)] +#[case( + json!({ "output_format": "json" }), + ExpectedValidationError::UnsupportedOutputFormat, +)] +fn cli_config_rejects_conflicting_or_unsupported_settings( + default_cli_json: Result, + #[case] file_layer: serde_json::Value, + #[case] expected_error: ExpectedValidationError, +) -> Result<()> { + assert_merge_rejects(default_cli_json?, file_layer, expected_error) +} + +#[rstest] +fn cli_runtime_canonicalizes_ascii_theme_from_no_emoji_alias() -> Result<()> { + with_config_file("no_emoji = true\n", &["netsuke"], |merged| { + ensure!( + merged.theme == Some(Theme::Ascii), + "no_emoji compatibility alias should canonicalize to the ASCII theme", + ); + ensure!( + merged.no_emoji == Some(true), + "no_emoji alias should remain available in the runtime CLI", + ); + Ok(()) + }) +} diff --git a/tests/cli_tests/parsing.rs b/tests/cli_tests/parsing.rs index b77980ac..2f880575 100644 --- a/tests/cli_tests/parsing.rs +++ b/tests/cli_tests/parsing.rs @@ -3,8 +3,9 @@ use anyhow::{Context, Result, ensure}; use clap::Parser; use clap::error::ErrorKind; -use netsuke::cli::{BuildArgs, Cli, Commands}; +use netsuke::cli::{BuildArgs, Cli, Commands, Theme}; use netsuke::host_pattern::HostPattern; +use netsuke::output_prefs; use rstest::rstest; use std::path::PathBuf; @@ -209,3 +210,41 @@ fn parse_cli_errors(#[case] argv: Vec<&str>, #[case] expected_error: ErrorKind) ); Ok(()) } + +#[test] +fn no_emoji_override_false_defers_to_environment_suppression() -> Result<()> { + let cli = Cli { + no_emoji: Some(false), + ..Cli::default() + }; + + let prefs = output_prefs::resolve_with(cli.no_emoji_override(), |key| match key { + "NETSUKE_NO_EMOJI" => Some(String::from("1")), + _ => None, + }); + + ensure!( + !prefs.emoji_allowed(), + "no_emoji = false should defer to environment suppression", + ); + Ok(()) +} + +#[test] +fn no_emoji_override_honours_unicode_theme_over_environment_suppression() -> Result<()> { + let cli = Cli { + theme: Some(Theme::Unicode), + ..Cli::default() + }; + + let prefs = output_prefs::resolve_with(cli.no_emoji_override(), |key| match key { + "NETSUKE_NO_EMOJI" => Some(String::from("1")), + _ => None, + }); + + ensure!( + prefs.emoji_allowed(), + "theme = unicode should remain authoritative over environment suppression", + ); + Ok(()) +} diff --git a/tests/features/configuration_preferences.feature b/tests/features/configuration_preferences.feature new file mode 100644 index 00000000..187a7c37 --- /dev/null +++ b/tests/features/configuration_preferences.feature @@ -0,0 +1,100 @@ +Feature: Configuration preferences + + Scenario: Configured build targets become the default build command targets + Given a minimal Netsuke workspace + And the Netsuke config file sets build targets to "hello" + When the CLI is parsed and merged with "" + Then parsing succeeds + And the merged CLI uses build target "hello" + + Scenario: CLI locale and verbose flags override configuration and environment + Given an empty workspace + And the Netsuke config file sets locale to "es-ES" + And the NETSUKE_LOCALE environment variable is "fr-FR" + When the CLI is parsed and merged with "--locale en-US --verbose" + Then parsing succeeds + And the merged locale is "en-US" + And verbose mode is enabled in the merged CLI + + Scenario: Unsupported output format fails during configuration merge + Given an empty workspace + And the Netsuke config file sets output format to "json" + When the CLI is parsed and merged with "" + Then an error should be returned + And the merge error should contain "output_format = " + + Scenario: no_emoji compatibility alias resolves to the ASCII theme + Given a minimal Netsuke workspace + And the Netsuke config file sets no_emoji to true + When the CLI is parsed and merged with "" + And merged output preferences are resolved + And the success prefix is rendered + 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