From 00d6759bd41f53427c224dfae9485102ce6b771f Mon Sep 17 00:00:00 2001 From: Leynos Date: Fri, 2 Jan 2026 02:33:35 +0000 Subject: [PATCH 1/9] docs(execplans): add comprehensive ExecPlan for refining CLI output clarity Introduce a new ExecPlan in docs/execplans/cli-output-clarity.md that outlines the purpose, progress checklist, decision log, and concrete steps for enhancing the Netsuke CLI output. This includes integrating OrthoConfig for configuration layering, localizing CLI help and error messages, refining user-facing messages, adding tests, and updating documentation to improve usability and consistency across CLI commands. Co-authored-by: terragon-labs[bot] --- docs/execplans/cli-output-clarity.md | 222 +++++++++++++++++++++++++++ 1 file changed, 222 insertions(+) create mode 100644 docs/execplans/cli-output-clarity.md diff --git a/docs/execplans/cli-output-clarity.md b/docs/execplans/cli-output-clarity.md new file mode 100644 index 00000000..1453b7d9 --- /dev/null +++ b/docs/execplans/cli-output-clarity.md @@ -0,0 +1,222 @@ +# Refine CLI Output With OrthoConfig Localised Help + +This ExecPlan is a living document. The sections `Progress`, +`Surprises & Discoveries`, `Decision Log`, and `Outcomes & Retrospective` must +be kept up to date as work proceeds. + +No `PLANS.md` exists in the repository root at the time of writing. + +## Purpose / Big Picture + +Users should see clear, descriptive CLI help and intuitive command feedback +when running Netsuke. Configuration should be layered ergonomically via +`ortho_config` so defaults, config files, environment variables, and CLI flags +behave consistently. Localised help strings should be used for CLI usage and +error output. Success is observable by running `netsuke --help`, subcommand +help, and core commands (build, clean, graph, manifest) and seeing clear +messages, plus passing the new unit and behavioural tests that assert these +outputs and configuration precedence. + +## Progress + +- [x] (2026-01-02 00:00Z) Drafted ExecPlan skeleton and captured repository + context. +- [ ] Inventory current CLI output and help messages (help text, errors, + subcommand feedback) and record gaps. +- [ ] Implement OrthoConfig-backed CLI configuration and localised help + plumbing. +- [ ] Refine user-facing CLI output and update docs. +- [ ] Add unit tests and rstest-bdd behavioural tests for happy/unhappy paths. +- [ ] Run formatting, lint, and test gates; update roadmap entry to done. + +## Surprises & Discoveries + +- Observation: None yet. + Evidence: N/A. + +## Decision Log + +- Decision: (pending) + Rationale: (pending) + Date/Author: (pending) + +## Outcomes & Retrospective + +- Outcome: Not started. + Notes: Initial plan captured; implementation pending. + +## Context and Orientation + +Key runtime entry points and CLI definitions live in these files: + +- `src/cli.rs` defines the clap CLI (flags, subcommands, help text) and default + command behaviour. +- `src/main.rs` parses the CLI, configures logging, and dispatches to the + runner. +- `src/runner/mod.rs` and `src/runner/process/` implement subcommand behaviour + and user-visible status logs. +- `src/cli_policy.rs` derives network policy from CLI settings. +- Tests cover CLI parsing and runner behaviour in `tests/cli_tests.rs` and + `tests/runner_tests.rs`, plus behavioural steps in + `tests/bdd/steps/cli.rs` and `tests/bdd/steps/process.rs`. + +OrthoConfig is currently not wired in. The user guide for it is +`docs/ortho-config-users-guide.md`, which explains configuration layering, +localised help via Fluent, and error localisation helpers. Design expectations +for CLI behaviour are in `docs/netsuke-design.md` and the roadmap entry in +`docs/roadmap.md` (Phase 3 → “CLI and Feature Completeness”). + +Testing guidance for fixtures, DI, and BDD lives in: + +- `docs/rust-testing-with-rstest-fixtures.md` +- `docs/reliable-testing-in-rust-via-dependency-injection.md` +- `docs/behavioural-testing-in-rust-with-cucumber.md` (applies to Gherkin + structure, even though we use `rstest-bdd`) +- `docs/rust-doctest-dry-guide.md` (for any new public API docs) + +## Plan of Work + +1. Audit current CLI output and help. Capture current `--help` output for the + root command and each subcommand, plus error messages from invalid flags and + missing values. Review `tracing` info logs emitted by + `src/runner/process/mod.rs` and `src/runner/mod.rs` to identify copy that + needs to be clarified. Document gaps in `Surprises & Discoveries` and + update `Decision Log` if scope changes. + +2. Introduce an OrthoConfig-backed configuration layer for CLI data. Add a + new module (for example `src/cli_config.rs`) that defines `CliConfig` and + subcommand argument structs using `#[derive(OrthoConfig, Deserialize, + Serialize, Parser)]` (or the equivalent pattern in the guide). Configure a + prefix such as `NETSUKE` and ensure fields map to orthographic CLI flags, + env vars, and config file keys. Use OrthoConfig helpers (`ConfigDiscovery`, + `compose_layers`, `merge_from_layers`, or `SubcmdConfigMerge`) so + configuration files and environment variables are layered beneath explicit + CLI overrides. Add explicit handling for missing required values and make + sure clap defaults do not incorrectly override config layers (use + `cli_default_as_absent` where needed). + +3. Localise CLI help and clap errors. Create Fluent resources (for example + `locales/en-US/messages.ftl` and a CLI-specific bundle) and wire a + `FluentLocalizer` into CLI parsing. Use `command().localize(&localizer)` + before parsing and `localize_clap_error_with_command` on errors. Ensure the + fallback path preserves stock clap output when localisation fails. + +4. Refine CLI output messages. Update docstrings and help text in + `src/cli.rs` (or the new config module) to be plain language and + action-oriented. Review `tracing::info!` messages for build/manifest/graph + flows and update wording to align with the user guide. Ensure stderr/stdout + separation remains correct and messages are consistent across subcommands. + If necessary, introduce a small output helper module to centralise user + message formatting. + +5. Add tests. Extend unit tests in `tests/cli_tests.rs` with `rstest` fixtures + that validate: + + - OrthoConfig precedence (defaults < file < env < CLI), using + `MergeComposer` or `compose_layers` to avoid touching process state. + - Localised help contains expected copy, and clap errors are localised when + possible. + - Unhappy paths (invalid schemes, invalid jobs count, missing required + values) return the correct error kinds and messages. + + Add behavioural coverage with `rstest-bdd` in `tests/features` and step + definitions in `tests/bdd/steps/cli.rs` to exercise real CLI invocations. + Use distinct step wording to avoid the `rstest-bdd` pattern ambiguity + pitfalls noted in prior gotchas. Cover at least one happy path and one + unhappy path for help output and configuration layering. + +6. Documentation updates. Update `docs/users-guide.md` to explain the new + configuration layering, localised help, and any changes to CLI output. + Record design decisions in `docs/netsuke-design.md`. Mark the “Refine all + CLI output…” item as done in `docs/roadmap.md` once tests pass. + +## Concrete Steps + +All commands are run from the repository root (`/root/repo`). Use `tee` with +`set -o pipefail` to preserve exit codes, as required by `AGENTS.md`. + +1. Capture current CLI output for reference: + + set -o pipefail + cargo run -- --help 2>&1 | tee /tmp/netsuke-help.log + cargo run -- build --help 2>&1 | tee /tmp/netsuke-build-help.log + cargo run -- clean --help 2>&1 | tee /tmp/netsuke-clean-help.log + cargo run -- graph --help 2>&1 | tee /tmp/netsuke-graph-help.log + cargo run -- manifest --help 2>&1 | tee /tmp/netsuke-manifest-help.log + +2. Implement OrthoConfig integration and localisation. Update or add the + relevant files and ensure new Fluent resources exist. + +3. Update tests and docs. + +4. Format and lint documentation if modified: + + set -o pipefail + make fmt 2>&1 | tee /tmp/netsuke-fmt.log + make markdownlint 2>&1 | tee /tmp/netsuke-markdownlint.log + make nixie 2>&1 | tee /tmp/netsuke-nixie.log + +5. Run core quality gates: + + set -o pipefail + make check-fmt 2>&1 | tee /tmp/netsuke-check-fmt.log + make lint 2>&1 | tee /tmp/netsuke-lint.log + make test 2>&1 | tee /tmp/netsuke-test.log + +## Validation and Acceptance + +- Running `netsuke --help` (via `cargo run -- --help`) prints localised, + plain-language descriptions for every flag and subcommand. +- Subcommand help (`build`, `clean`, `graph`, `manifest`) is descriptive and + matches the user guide. Error output for invalid CLI inputs is localised + when a translation exists. +- Configuration precedence is verified by unit tests: defaults < config file + < environment variables < CLI overrides. +- Behavioural tests exercise at least one happy path and one unhappy path that + assert CLI output clarity and config layering. +- `make check-fmt`, `make lint`, and `make test` pass. +- `docs/users-guide.md` reflects the updated CLI behaviour and configuration + model, and `docs/roadmap.md` marks the CLI output item as done. + +## Idempotence and Recovery + +- OrthoConfig and localisation changes are safe to re-run; regenerate + configuration layers and help output as often as needed. +- If localisation setup fails, fall back to default clap strings and record + the failure in `Surprises & Discoveries` with the error output. +- If tests fail mid-run, fix the underlying issue and re-run the same command + with the same log path, overwriting the log file to keep evidence current. + +## Artifacts and Notes + +Keep the following short transcripts for evidence: + +- `netsuke --help` output with localised strings (from + `/tmp/netsuke-help.log`). +- A failing CLI invocation showing localised error output (record the command + and a short excerpt of stderr). +- Test summaries from `/tmp/netsuke-test.log` showing the new unit and BDD + tests passing. + +## Interfaces and Dependencies + +- Add `ortho_config` as a dependency using a caret version (for example, + `ortho_config = "0.7.0"`), enabling `yaml`/`json5` features only if required + and documenting the decision in `docs/netsuke-design.md`. +- Define `CliConfig` in `src/cli_config.rs` (or equivalent) with fields that + map to existing CLI flags: `file`, `directory`, `jobs`, `verbose`, + `fetch_allow_scheme`, `fetch_allow_host`, `fetch_block_host`, + `fetch_default_deny`, plus subcommand args. Ensure `#[ortho_config(prefix = + "NETSUKE")]` is used for orthographic naming. +- Use `FluentLocalizer` from `ortho_config` and wire it into clap command + creation. Provide Fluent keys for usage, about, flag help, and + `clap-error-` messages. +- Ensure CLI parsing in `src/main.rs` uses the localiser and merges config + layers before calling `runner::run`. +- If a helper module is introduced (for example `src/cli_output.rs`), keep it + small, with a single responsibility for formatting user-visible messages. + +## Revision note (required when editing an ExecPlan) + +- 2026-01-02: Initial ExecPlan created to cover OrthoConfig integration, + localised help, CLI output clarity, and test/documentation updates. From 250e2aa023619e6a142522c7fbd0db814a27fd91 Mon Sep 17 00:00:00 2001 From: Leynos Date: Fri, 2 Jan 2026 04:06:22 +0000 Subject: [PATCH 2/9] feat(cli): add OrthoConfig configuration layering and localisation - Introduce OrthoConfig-backed layering for CLI configuration with priorities: defaults, config files, env vars, CLI flags - Make CLI defaults absent so they do not mask config file or environment values - Support configuration discovery honoring NETSUKE_CONFIG_PATH and standard OS config locations - Add Fluent-based localisation of CLI help and error messages with English and Spanish support - Parse and apply --locale and NETSUKE_LOCALE environment variable for localisation - Enhance CLI parsing to produce localized error messages - Update CLI documentation and user guide with localisation and config layering details - Add unit tests and behavioural tests for configuration merging and localisation - Modify main executable to perform config merging and setup localized CLI parsing This improves user experience by supporting layered, overrideable configurations and localized CLI output, improving clarity and internationalisation. Co-authored-by: terragon-labs[bot] --- Cargo.lock | 405 ++++++++++++++++++++++++++- Cargo.toml | 8 +- build.rs | 17 +- docs/execplans/cli-output-clarity.md | 62 ++-- docs/netsuke-design.md | 57 ++-- docs/roadmap.md | 2 +- docs/users-guide.md | 40 +++ locales/en-US/messages.ftl | 21 ++ locales/es-ES/messages.ftl | 21 ++ src/cli.rs | 232 ++++++++++++++- src/cli_localization.rs | 82 ++++++ src/cli_policy.rs | 2 + src/host_pattern.rs | 34 +++ src/lib.rs | 1 + src/main.rs | 48 +++- src/runner/mod.rs | 9 +- src/runner/process/file_io.rs | 4 +- src/runner/process/mod.rs | 2 +- tests/bdd/steps/cli.rs | 22 +- tests/cli_tests.rs | 106 ++++++- tests/features/cli.feature | 7 +- 21 files changed, 1072 insertions(+), 110 deletions(-) create mode 100644 locales/en-US/messages.ftl create mode 100644 locales/es-ES/messages.ftl create mode 100644 src/cli_localization.rs diff --git a/Cargo.lock b/Cargo.lock index ed5047ae..c7e5ec08 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -121,6 +121,15 @@ dependencies = [ "wait-timeout", ] +[[package]] +name = "atomic" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a89cbf775b137e9b968e67227ef7f775587cde3fd31b0d8599dbd0f598a48340" +dependencies = [ + "bytemuck", +] + [[package]] name = "autocfg" version = "1.5.0" @@ -139,7 +148,7 @@ dependencies = [ "miniz_oxide", "object", "rustc-demangle", - "windows-targets", + "windows-targets 0.52.6", ] [[package]] @@ -192,6 +201,12 @@ dependencies = [ "serde", ] +[[package]] +name = "bytemuck" +version = "1.24.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1fbdf580320f38b612e485521afda1ee26d10cc9884efaaa750d383e13e3c5f4" + [[package]] name = "camino" version = "1.2.0" @@ -255,6 +270,18 @@ dependencies = [ "clap_derive", ] +[[package]] +name = "clap-dispatch" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0a558b9547b590c876e46e301da15d3b0e93b0384fd50d2c7870f7ea86760df5" +dependencies = [ + "heck 0.5.0", + "proc-macro2", + "quote", + "syn 2.0.104", +] + [[package]] name = "clap_builder" version = "4.5.41" @@ -405,6 +432,48 @@ dependencies = [ "crypto-common", ] +[[package]] +name = "directories" +version = "6.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "16f5094c54661b38d03bd7e50df373292118db60b585c08a411c6d840017fe7d" +dependencies = [ + "dirs-sys 0.5.0", +] + +[[package]] +name = "dirs" +version = "5.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "44c45a9d03d6676652bcb5e724c7e988de1acad23a711b5217ab9cbecbec2225" +dependencies = [ + "dirs-sys 0.4.1", +] + +[[package]] +name = "dirs-sys" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "520f05a5cbd335fae5a99ff7a6ab8627577660ee5cfd6a94a6a929b52ff0321c" +dependencies = [ + "libc", + "option-ext", + "redox_users 0.4.6", + "windows-sys 0.48.0", +] + +[[package]] +name = "dirs-sys" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e01a3366d27ee9890022452ee61b2b63a67e6f13f58900b651ff5665f0bb1fab" +dependencies = [ + "libc", + "option-ext", + "redox_users 0.5.2", + "windows-sys 0.59.0", +] + [[package]] name = "displaydoc" version = "0.2.5" @@ -422,6 +491,12 @@ version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1435fa1053d8b2fbbe9be7e97eca7f33d37b28409959813daefc1446a14247f1" +[[package]] +name = "dunce" +version = "1.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "92773504d58c093f6de2459af4af33faa518c13451eb8f2b5698ed3d36e7c813" + [[package]] name = "either" version = "1.15.0" @@ -456,13 +531,29 @@ version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "37909eebbb50d72f9059c3b6d82c0463f2ff062c9e95845c43a6c9c0355411be" +[[package]] +name = "figment" +version = "0.10.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8cb01cd46b0cf372153850f4c6c272d9cbea2da513e07538405148f95bd789f3" +dependencies = [ + "atomic", + "parking_lot", + "pear", + "serde", + "tempfile", + "toml 0.8.23", + "uncased", + "version_check", +] + [[package]] name = "find-crate" version = "0.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "59a98bbaacea1c0eb6a0876280051b892eb73594fd90cf3b20e9c817029c57d2" dependencies = [ - "toml", + "toml 0.5.11", ] [[package]] @@ -946,6 +1037,12 @@ dependencies = [ "serde_core", ] +[[package]] +name = "inlinable_string" +version = "0.1.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c8fae54786f62fb2918dcfae3d568594e50eb9b5c25bf04371af6fe7516452fb" + [[package]] name = "insta" version = "1.43.1" @@ -1067,6 +1164,16 @@ version = "0.2.174" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1171693293099992e19cddea4e8b849964e9846f4acee11b3948bcc337be8776" +[[package]] +name = "libredox" +version = "0.1.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3d0b95e02c851351f877147b7deea7b1afb1df71b63aa5f8270716e0c5720616" +dependencies = [ + "bitflags", + "libc", +] + [[package]] name = "linux-raw-sys" version = "0.9.4" @@ -1254,6 +1361,7 @@ dependencies = [ "mockable", "mockall", "ninja_env", + "ortho_config", "predicates 3.1.3", "rstest", "rstest-bdd", @@ -1351,6 +1459,50 @@ version = "1.70.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a4895175b425cb1f87721b59f0f286c2092bd4af812243672510e1ac53e2e0ad" +[[package]] +name = "option-ext" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" + +[[package]] +name = "ortho_config" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2ee9d60c6be312d7c6ce47b7146e3c9b8464c54fc304ef8f7656e29c268faeb1" +dependencies = [ + "camino", + "clap", + "clap-dispatch", + "directories", + "dirs", + "dunce", + "figment", + "fluent-bundle", + "fluent-syntax", + "ortho_config_macros", + "serde", + "serde_json", + "thiserror 2.0.17", + "toml 0.9.10+spec-1.1.0", + "tracing", + "uncased", + "unic-langid", + "xdg", +] + +[[package]] +name = "ortho_config_macros" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "992d8fc0b33823adb993fe8968614ac0df3964d9e3564f7a147fd253e27a3acd" +dependencies = [ + "heck 0.5.0", + "proc-macro2", + "quote", + "syn 2.0.104", +] + [[package]] name = "overload" version = "0.1.1" @@ -1383,7 +1535,30 @@ dependencies = [ "libc", "redox_syscall", "smallvec 1.15.1", - "windows-targets", + "windows-targets 0.52.6", +] + +[[package]] +name = "pear" +version = "0.2.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bdeeaa00ce488657faba8ebf44ab9361f9365a97bd39ffb8a60663f57ff4b467" +dependencies = [ + "inlinable_string", + "pear_codegen", + "yansi", +] + +[[package]] +name = "pear_codegen" +version = "0.2.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4bab5b985dc082b345f812b7df84e1bef27e7207b39e448439ba8bd69c93f147" +dependencies = [ + "proc-macro2", + "proc-macro2-diagnostics", + "quote", + "syn 2.0.104", ] [[package]] @@ -1496,7 +1671,7 @@ version = "3.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "219cb19e96be00ab2e37d6e299658a0cfa83e52429179969b0f0121b4ac46983" dependencies = [ - "toml_edit", + "toml_edit 0.23.10+spec-1.0.0", ] [[package]] @@ -1538,6 +1713,19 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "proc-macro2-diagnostics" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "af066a9c399a26e020ada66a034357a868728e72cd426f3adcd35f80d88d88c8" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.104", + "version_check", + "yansi", +] + [[package]] name = "quote" version = "1.0.40" @@ -1562,6 +1750,28 @@ dependencies = [ "bitflags", ] +[[package]] +name = "redox_users" +version = "0.4.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba009ff324d1fc1b900bd1fdb31564febe58a8ccc8a6fdbb93b543d33b13ca43" +dependencies = [ + "getrandom 0.2.16", + "libredox", + "thiserror 1.0.69", +] + +[[package]] +name = "redox_users" +version = "0.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a4e608c6638b9c18977b00b475ac1f28d14e84b27d8d42f70e0bf1e3dec127ac" +dependencies = [ + "getrandom 0.2.16", + "libredox", + "thiserror 2.0.17", +] + [[package]] name = "regex" version = "1.12.2" @@ -1961,6 +2171,24 @@ dependencies = [ "serde_json", ] +[[package]] +name = "serde_spanned" +version = "0.6.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bf41e0cfaf7226dca15e8197172c295a782857fcb97fad1808a166870dee75a3" +dependencies = [ + "serde", +] + +[[package]] +name = "serde_spanned" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f8bbf91e5a4d6315eee45e704372590b30e260ee83af6639d64557f51b067776" +dependencies = [ + "serde_core", +] + [[package]] name = "serial_test" version = "3.2.0" @@ -2335,6 +2563,42 @@ dependencies = [ "serde", ] +[[package]] +name = "toml" +version = "0.8.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc1beb996b9d83529a9e75c17a1686767d148d70663143c7854d8b4a09ced362" +dependencies = [ + "serde", + "serde_spanned 0.6.9", + "toml_datetime 0.6.11", + "toml_edit 0.22.27", +] + +[[package]] +name = "toml" +version = "0.9.10+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0825052159284a1a8b4d6c0c86cbc801f2da5afd2b225fa548c72f2e74002f48" +dependencies = [ + "indexmap", + "serde_core", + "serde_spanned 1.0.4", + "toml_datetime 0.7.5+spec-1.1.0", + "toml_parser", + "toml_writer", + "winnow", +] + +[[package]] +name = "toml_datetime" +version = "0.6.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "22cddaf88f4fbc13c51aebbf5f8eceb5c7c5a9da2ac40a13519eb5b0a0e8f11c" +dependencies = [ + "serde", +] + [[package]] name = "toml_datetime" version = "0.7.5+spec-1.1.0" @@ -2344,6 +2608,20 @@ dependencies = [ "serde_core", ] +[[package]] +name = "toml_edit" +version = "0.22.27" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "41fe8c660ae4257887cf66394862d21dbca4a6ddd26f04a3560410406a2f819a" +dependencies = [ + "indexmap", + "serde", + "serde_spanned 0.6.9", + "toml_datetime 0.6.11", + "toml_write", + "winnow", +] + [[package]] name = "toml_edit" version = "0.23.10+spec-1.0.0" @@ -2351,7 +2629,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "84c8b9f757e028cee9fa244aea147aab2a9ec09d5325a9b01e0a49730c2b5269" dependencies = [ "indexmap", - "toml_datetime", + "toml_datetime 0.7.5+spec-1.1.0", "toml_parser", "winnow", ] @@ -2365,6 +2643,18 @@ dependencies = [ "winnow", ] +[[package]] +name = "toml_write" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5d99f8c9a7727884afe522e9bd5edbfc91a3312b36a77b5fb8926e4c31a41801" + +[[package]] +name = "toml_writer" +version = "1.0.6+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ab16f14aed21ee8bfd8ec22513f7287cd4a91aa92e44edfe2c17ddd004e92607" + [[package]] name = "tracing" version = "0.1.41" @@ -2457,6 +2747,15 @@ version = "1.18.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1dccffe3ce07af9386bfd29e80c0ab1a8205a2fc34e4bcd40364df902cfa8f3f" +[[package]] +name = "uncased" +version = "0.9.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e1b88fcfe09e89d3866a5c11019378088af2d24c3fbd4f0543f96b479ec90697" +dependencies = [ + "version_check", +] + [[package]] name = "unic-langid" version = "0.9.6" @@ -2681,13 +2980,22 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" +[[package]] +name = "windows-sys" +version = "0.48.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "677d2418bec65e3338edb076e806bc1ec15693c5d0104683f2efe857f61056a9" +dependencies = [ + "windows-targets 0.48.5", +] + [[package]] name = "windows-sys" version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "282be5f36a8ce781fad8c8ae18fa3f9beff57ec1b52cb3de0789201425d9a33d" dependencies = [ - "windows-targets", + "windows-targets 0.52.6", ] [[package]] @@ -2696,7 +3004,22 @@ version = "0.59.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1e38bc4d79ed67fd075bcc251a1c39b32a1776bbe92e5bef1f0bf1f8c531853b" dependencies = [ - "windows-targets", + "windows-targets 0.52.6", +] + +[[package]] +name = "windows-targets" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9a2fa6e2155d7247be68c096456083145c183cbbbc2764150dda45a87197940c" +dependencies = [ + "windows_aarch64_gnullvm 0.48.5", + "windows_aarch64_msvc 0.48.5", + "windows_i686_gnu 0.48.5", + "windows_i686_msvc 0.48.5", + "windows_x86_64_gnu 0.48.5", + "windows_x86_64_gnullvm 0.48.5", + "windows_x86_64_msvc 0.48.5", ] [[package]] @@ -2705,28 +3028,46 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9b724f72796e036ab90c1021d4780d4d3d648aca59e491e6b98e725b84e99973" dependencies = [ - "windows_aarch64_gnullvm", - "windows_aarch64_msvc", - "windows_i686_gnu", + "windows_aarch64_gnullvm 0.52.6", + "windows_aarch64_msvc 0.52.6", + "windows_i686_gnu 0.52.6", "windows_i686_gnullvm", - "windows_i686_msvc", - "windows_x86_64_gnu", - "windows_x86_64_gnullvm", - "windows_x86_64_msvc", + "windows_i686_msvc 0.52.6", + "windows_x86_64_gnu 0.52.6", + "windows_x86_64_gnullvm 0.52.6", + "windows_x86_64_msvc 0.52.6", ] +[[package]] +name = "windows_aarch64_gnullvm" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2b38e32f0abccf9987a4e3079dfb67dcd799fb61361e53e2882c3cbaf0d905d8" + [[package]] name = "windows_aarch64_gnullvm" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32a4622180e7a0ec044bb555404c800bc9fd9ec262ec147edd5989ccd0c02cd3" +[[package]] +name = "windows_aarch64_msvc" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc35310971f3b2dbbf3f0690a219f40e2d9afcf64f9ab7cc1be722937c26b4bc" + [[package]] name = "windows_aarch64_msvc" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "09ec2a7bb152e2252b53fa7803150007879548bc709c039df7627cabbd05d469" +[[package]] +name = "windows_i686_gnu" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a75915e7def60c94dcef72200b9a8e58e5091744960da64ec734a6c6e9b3743e" + [[package]] name = "windows_i686_gnu" version = "0.52.6" @@ -2739,24 +3080,48 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0eee52d38c090b3caa76c563b86c3a4bd71ef1a819287c19d586d7334ae8ed66" +[[package]] +name = "windows_i686_msvc" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8f55c233f70c4b27f66c523580f78f1004e8b5a8b659e05a4eb49d4166cca406" + [[package]] name = "windows_i686_msvc" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "240948bc05c5e7c6dabba28bf89d89ffce3e303022809e73deaefe4f6ec56c66" +[[package]] +name = "windows_x86_64_gnu" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "53d40abd2583d23e4718fddf1ebec84dbff8381c07cae67ff7768bbf19c6718e" + [[package]] name = "windows_x86_64_gnu" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "147a5c80aabfbf0c7d901cb5895d1de30ef2907eb21fbbab29ca94c5b08b1a78" +[[package]] +name = "windows_x86_64_gnullvm" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0b7b52767868a23d5bab768e390dc5f5c55825b6d30b86c844ff2dc7414044cc" + [[package]] name = "windows_x86_64_gnullvm" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "24d5b23dc417412679681396f2b49f3de8c1473deb516bd34410872eff51ed0d" +[[package]] +name = "windows_x86_64_msvc" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ed94fce61571a4006852b7389a063ab983c02eb1bb37b47f8272ce92d06d9538" + [[package]] name = "windows_x86_64_msvc" version = "0.52.6" @@ -2797,6 +3162,18 @@ version = "0.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ea2f10b9bb0928dfb1b42b65e1f9e36f7f54dbdf08457afefb38afcdec4fa2bb" +[[package]] +name = "xdg" +version = "3.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2fb433233f2df9344722454bc7e96465c9d03bff9d77c248f9e7523fe79585b5" + +[[package]] +name = "yansi" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cfe53a6657fd280eaa890a3bc59152892ffa3e30101319d168b781ed6529b049" + [[package]] name = "yoke" version = "0.8.0" diff --git a/Cargo.toml b/Cargo.toml index 651a1217..40beb454 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,6 +5,7 @@ edition = "2024" rust-version = "1.89.0" include = [ "src/**", + "locales/**", "Cargo.toml", "README.md", "LICENSE", @@ -14,8 +15,9 @@ license = "ISC" description = "A YAML-powered Ninja/Jinja hybrid build system." [features] -default = [] +default = ["serde_json"] legacy-digests = ["sha1", "md5"] +serde_json = ["ortho_config/serde_json"] [dependencies] clap = { version = "4.5.0", features = ["derive"] } @@ -50,10 +52,14 @@ 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 = "0.7.0" [build-dependencies] clap = { version = "4.5.0", features = ["derive"] } clap_mangen = "0.2.29" +ortho_config = "0.7.0" +serde = { version = "1", features = ["derive"] } +serde_json = { version = "1", features = ["preserve_order"] } thiserror = "1" time = { version = "0.3.44", features = ["formatting"] } diff --git a/build.rs b/build.rs index f6730140..06fb6a59 100644 --- a/build.rs +++ b/build.rs @@ -1,9 +1,11 @@ //! Build script: generate the CLI manual page into target/generated-man// for //! release packaging. -use clap::CommandFactory; +use clap::{ArgMatches, CommandFactory}; use clap_mangen::Man; use std::{ - env, fs, + env, + ffi::OsString, + fs, path::{Path, PathBuf}, }; use time::{OffsetDateTime, format_description::well_known::Iso8601}; @@ -18,6 +20,9 @@ mod host_pattern; use host_pattern::{HostPattern, HostPatternError}; +type LocalizedParseFn = + fn(Vec, &dyn ortho_config::Localizer) -> Result<(cli::Cli, ArgMatches), clap::Error>; + fn manual_date() -> String { let Ok(raw) = env::var("SOURCE_DATE_EPOCH") else { return FALLBACK_DATE.into(); @@ -67,8 +72,12 @@ fn main() -> Result<(), Box> { // Exercise the host pattern symbols so the shared module remains linked // when the build script is compiled without tests. const _: usize = std::mem::size_of::(); - let _: fn(&str) -> Result = HostPattern::parse; - let _: fn(&HostPattern, &str) -> bool = HostPattern::matches; + const _: fn(&[OsString]) -> Option = cli::locale_hint_from_args; + const _: fn(&cli::Cli, &ArgMatches) -> ortho_config::OrthoResult = + cli::merge_with_config; + const _: LocalizedParseFn = cli::parse_with_localizer_from; + const _: fn(&str) -> Result = HostPattern::parse; + const _: fn(&HostPattern, &str) -> bool = HostPattern::matches; // Regenerate the manual page when the CLI or metadata changes. println!("cargo:rerun-if-changed=src/cli.rs"); diff --git a/docs/execplans/cli-output-clarity.md b/docs/execplans/cli-output-clarity.md index 1453b7d9..039b94a2 100644 --- a/docs/execplans/cli-output-clarity.md +++ b/docs/execplans/cli-output-clarity.md @@ -23,11 +23,12 @@ outputs and configuration precedence. context. - [ ] Inventory current CLI output and help messages (help text, errors, subcommand feedback) and record gaps. -- [ ] Implement OrthoConfig-backed CLI configuration and localised help +- [x] Implement OrthoConfig-backed CLI configuration and localised help plumbing. -- [ ] Refine user-facing CLI output and update docs. -- [ ] Add unit tests and rstest-bdd behavioural tests for happy/unhappy paths. -- [ ] Run formatting, lint, and test gates; update roadmap entry to done. +- [x] Refine user-facing CLI output and update docs. +- [x] Add unit tests and rstest-bdd behavioural tests for happy/unhappy paths. +- [x] (2026-01-02 00:00Z) Run formatting, lint, and test gates; update roadmap + entry to done. ## Surprises & Discoveries @@ -36,14 +37,22 @@ outputs and configuration precedence. ## Decision Log -- Decision: (pending) - Rationale: (pending) - Date/Author: (pending) +- Decision: Use OrthoConfig merge composition to layer defaults, config files, + environment variables, and CLI overrides while treating clap defaults as + absent. Rationale: Preserves deterministic precedence and avoids masking + file/env values with clap defaults. Date/Author: 2026-01-02 (Codex) + +- Decision: Provide Fluent-backed localisation with English defaults and a + Spanish catalogue layered as a consumer resource. Rationale: Validates + localised help/error support while keeping fallback behaviour intact. + Date/Author: 2026-01-02 (Codex) ## Outcomes & Retrospective -- Outcome: Not started. - Notes: Initial plan captured; implementation pending. +- Outcome: Complete. + Notes: OrthoConfig configuration layering, localisation resources (including + Spanish), updated CLI output, and tests are in place; `make check-fmt`, + `make lint`, and `make test` now pass. ## Context and Orientation @@ -57,8 +66,8 @@ Key runtime entry points and CLI definitions live in these files: and user-visible status logs. - `src/cli_policy.rs` derives network policy from CLI settings. - Tests cover CLI parsing and runner behaviour in `tests/cli_tests.rs` and - `tests/runner_tests.rs`, plus behavioural steps in - `tests/bdd/steps/cli.rs` and `tests/bdd/steps/process.rs`. + `tests/runner_tests.rs`, plus behavioural steps in `tests/bdd/steps/cli.rs` + and `tests/bdd/steps/process.rs`. OrthoConfig is currently not wired in. The user guide for it is `docs/ortho-config-users-guide.md`, which explains configuration layering, @@ -80,20 +89,21 @@ Testing guidance for fixtures, DI, and BDD lives in: root command and each subcommand, plus error messages from invalid flags and missing values. Review `tracing` info logs emitted by `src/runner/process/mod.rs` and `src/runner/mod.rs` to identify copy that - needs to be clarified. Document gaps in `Surprises & Discoveries` and - update `Decision Log` if scope changes. + needs to be clarified. Document gaps in `Surprises & Discoveries` and update + `Decision Log` if scope changes. 2. Introduce an OrthoConfig-backed configuration layer for CLI data. Add a new module (for example `src/cli_config.rs`) that defines `CliConfig` and - subcommand argument structs using `#[derive(OrthoConfig, Deserialize, - Serialize, Parser)]` (or the equivalent pattern in the guide). Configure a - prefix such as `NETSUKE` and ensure fields map to orthographic CLI flags, - env vars, and config file keys. Use OrthoConfig helpers (`ConfigDiscovery`, - `compose_layers`, `merge_from_layers`, or `SubcmdConfigMerge`) so - configuration files and environment variables are layered beneath explicit - CLI overrides. Add explicit handling for missing required values and make - sure clap defaults do not incorrectly override config layers (use - `cli_default_as_absent` where needed). + subcommand argument structs using + `#[derive(OrthoConfig, Deserialize, Serialize, Parser)]` (or the equivalent + pattern in the guide). Configure a prefix such as `NETSUKE` and ensure + fields map to orthographic CLI flags, env vars, and config file keys. Use + OrthoConfig helpers (`ConfigDiscovery`, `compose_layers`, + `merge_from_layers`, or `SubcmdConfigMerge`) so configuration files and + environment variables are layered beneath explicit CLI overrides. Add + explicit handling for missing required values and make sure clap defaults do + not incorrectly override config layers (use `cli_default_as_absent` where + needed). 3. Localise CLI help and clap errors. Create Fluent resources (for example `locales/en-US/messages.ftl` and a CLI-specific bundle) and wire a @@ -168,8 +178,8 @@ All commands are run from the repository root (`/root/repo`). Use `tee` with - Running `netsuke --help` (via `cargo run -- --help`) prints localised, plain-language descriptions for every flag and subcommand. - Subcommand help (`build`, `clean`, `graph`, `manifest`) is descriptive and - matches the user guide. Error output for invalid CLI inputs is localised - when a translation exists. + matches the user guide. Error output for invalid CLI inputs is localised when + a translation exists. - Configuration precedence is verified by unit tests: defaults < config file < environment variables < CLI overrides. - Behavioural tests exercise at least one happy path and one unhappy path that @@ -206,8 +216,8 @@ Keep the following short transcripts for evidence: - Define `CliConfig` in `src/cli_config.rs` (or equivalent) with fields that map to existing CLI flags: `file`, `directory`, `jobs`, `verbose`, `fetch_allow_scheme`, `fetch_allow_host`, `fetch_block_host`, - `fetch_default_deny`, plus subcommand args. Ensure `#[ortho_config(prefix = - "NETSUKE")]` is used for orthographic naming. + `fetch_default_deny`, plus subcommand args. Ensure + `#[ortho_config(prefix = "NETSUKE")]` is used for orthographic naming. - Use `FluentLocalizer` from `ortho_config` and wire it into clap command creation. Provide Fluent keys for usage, about, flag help, and `clap-error-` messages. diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index 5a3fe449..bb081831 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -1049,8 +1049,8 @@ Structural view of the which module and configuration wiring: ```mermaid classDiagram class StdlibConfig { - +workspace_root_path() -> Option<&Utf8Path> - +workspace_skip_dirs() -> &[String] + +workspace_root_path() -> OptionalPath + +workspace_skip_dirs() -> StringList +which_cache_capacity() -> NonZeroUsize } @@ -1059,23 +1059,21 @@ classDiagram } class WhichModule { - +register(env: &mut Environment, config: WhichConfig) + +register(env: Environment, config: WhichConfig) } class WhichResolver { - -cache: Arc>> - -cwd_override: Option> + -cache: LruCache + -cwd_override: OptionalPath -workspace_skips: WorkspaceSkipList - +new(cwd_override: Option>, - skips: WorkspaceSkipList, - cache_capacity: NonZeroUsize) -> Result - +resolve(command: &str, options: &WhichOptions) -> Result, Error> + +new(cwd_override: OptionalPath, skips: WorkspaceSkipList, cache_capacity: NonZeroUsize) -> Result + +resolve(command: String, options: WhichOptions) -> Result } class EnvSnapshot { +cwd: Utf8PathBuf - +raw_path: Option - +capture(cwd_override: Option<&Utf8Path>) -> Result + +raw_path: OptionalString + +capture(cwd_override: OptionalPath) -> Result } class WhichOptions { @@ -1086,14 +1084,12 @@ classDiagram } class WhichConfig { - +new(cwd_override: Option>, - skips: WorkspaceSkipList, - cache_capacity: NonZeroUsize) -> WhichConfig + +new(cwd_override: OptionalPath, skips: WorkspaceSkipList, cache_capacity: NonZeroUsize) -> WhichConfig } class WorkspaceSkipList { +default() -> WorkspaceSkipList - +from_names(names: IntoIterator>) -> WorkspaceSkipList + +from_names(names: StringList) -> WorkspaceSkipList } class CwdMode { @@ -2014,18 +2010,25 @@ 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.rs`. Clap's -`default_value_t` attribute marks `Build` as the default subcommand, so -invoking `netsuke` with no explicit command still triggers a build. 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 CLI is implemented using clap's derive API in `src/cli.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 localised +via Fluent resources; `--locale` or `NETSUKE_LOCALE` selects the locale, and +English plus Spanish catalogues ship in `locales/`. 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 Ninja executable may be overridden via the `NINJA_ENV` environment variable. For example, `NINJA_ENV=/opt/ninja/bin/ninja netsuke build` forces diff --git a/docs/roadmap.md b/docs/roadmap.md index 023bbf05..2cb979f4 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -184,7 +184,7 @@ library, and CLI ergonomics. - [x] Implement the graph subcommand by invoking `ninja -t graph` to output a DOT representation of the dependency graph. - - [ ] Refine all CLI output for clarity, ensuring help messages are + - [x] Refine all CLI output for clarity, ensuring help messages are descriptive and command feedback is intuitive. - [x] Implement the `manifest` subcommand to persist the generated Ninja file diff --git a/docs/users-guide.md b/docs/users-guide.md index 88793a98..1fc4dee0 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -485,6 +485,26 @@ netsuke [OPTIONS] [COMMAND] [TARGETS...] - `-v, --verbose`: Enable verbose logging output. +- `--locale `: Localise CLI help and error messages (for example + `en-US` or `es-ES`). + +### Network Policy Options + +Netsuke's `fetch` helper is guarded by a configurable network policy. The +policy is configured by these global options: + +- `--fetch-allow-scheme `: Allow additional URL schemes beyond the + defaults. + +- `--fetch-allow-host `: Allow the provided hostnames when default deny + is enabled (wildcards such as `*.example.com` are supported). + +- `--fetch-block-host `: Always block the provided hostnames (wildcards + supported), even if they are allowlisted. + +- `--fetch-default-deny`: Deny all hosts by default and only permit the + allowlist. + ### Commands - `build` (default): Compiles the manifest and runs Ninja to build the @@ -507,6 +527,26 @@ netsuke [OPTIONS] [COMMAND] [TARGETS...] the generated `build.ninja`, outputting DOT to stdout (suitable for Graphviz). Future versions may support other formats like `--html`. +### Configuration and Localisation + +Netsuke layers configuration in this order, with later entries overriding +earlier ones: defaults, configuration files, environment variables, and +command-line flags. + +Configuration files are discovered using OrthoConfig. Netsuke honours +`NETSUKE_CONFIG_PATH` first, then searches `$XDG_CONFIG_HOME/netsuke`, each +entry in `$XDG_CONFIG_DIRS` (falling back to `/etc/xdg` on Unix-like targets), +Windows application data directories, `$HOME/.config/netsuke`, +`$HOME/.netsuke.toml`, and finally the project root. + +Environment variables use the `NETSUKE_` prefix (for example, +`NETSUKE_JOBS=8`). Use `__` to separate nested keys if you need to match +structured configuration. + +Use `--locale ` or `NETSUKE_LOCALE` to select localised CLI copy and +error messages. Spanish (`es-ES`) is included as a reference translation; +unsupported locales fall back to English. + ### Exit Codes - `0`: Success. diff --git a/locales/en-US/messages.ftl b/locales/en-US/messages.ftl new file mode 100644 index 00000000..403ab663 --- /dev/null +++ b/locales/en-US/messages.ftl @@ -0,0 +1,21 @@ +# Netsuke CLI localisation resources. + +cli.about = Netsuke compiles YAML + Jinja manifests into Ninja build plans. +cli.long_about = Netsuke transforms YAML + Jinja manifests into reproducible Ninja graphs and runs Ninja with safe defaults. +cli.usage = { $usage } + +cli.subcommand.build.about = Build targets defined in the manifest (default). +cli.subcommand.build.long_about = Build the requested targets; when none are provided, use the manifest defaults. +cli.subcommand.clean.about = Remove build artefacts via Ninja. +cli.subcommand.clean.long_about = Generate a temporary Ninja file, then run `ninja -t clean`. +cli.subcommand.graph.about = Emit the dependency graph in DOT format. +cli.subcommand.graph.long_about = Generate a temporary Ninja file, then run `ninja -t graph` to emit DOT output. +cli.subcommand.manifest.about = Write the generated Ninja manifest without running Ninja. +cli.subcommand.manifest.long_about = Generate the Ninja file and write it to the specified path or '-' for stdout. + +clap-error-missing-argument = Missing required argument: { $argument } +clap-error-missing-subcommand = Missing subcommand. Available options: { $valid_subcommands } +clap-error-unknown-argument = Unknown argument: { $argument } +clap-error-invalid-value = Invalid value for { $argument }: { $value } +clap-error-invalid-subcommand = Unknown subcommand: { $subcommand } +clap-error-value-validation = Invalid value for { $argument }: { $value } diff --git a/locales/es-ES/messages.ftl b/locales/es-ES/messages.ftl new file mode 100644 index 00000000..035f82bf --- /dev/null +++ b/locales/es-ES/messages.ftl @@ -0,0 +1,21 @@ +# Recursos de localización para la CLI de Netsuke. + +cli.about = Netsuke compila manifiestos YAML + Jinja en planes de compilación Ninja. +cli.long_about = Netsuke transforma manifiestos YAML + Jinja en grafos Ninja reproducibles y ejecuta Ninja con valores seguros. +cli.usage = { $usage } + +cli.subcommand.build.about = Compila objetivos definidos en el manifiesto (predeterminado). +cli.subcommand.build.long_about = Compila los objetivos solicitados; si no se indican, usa los predeterminados del manifiesto. +cli.subcommand.clean.about = Elimina artefactos de compilación mediante Ninja. +cli.subcommand.clean.long_about = Genera un archivo Ninja temporal y ejecuta `ninja -t clean`. +cli.subcommand.graph.about = Emite el grafo de dependencias en formato DOT. +cli.subcommand.graph.long_about = Genera un archivo Ninja temporal y ejecuta `ninja -t graph` para emitir DOT. +cli.subcommand.manifest.about = Escribe el manifiesto Ninja sin ejecutar Ninja. +cli.subcommand.manifest.long_about = Genera el archivo Ninja y lo escribe en la ruta indicada o '-' para stdout. + +clap-error-missing-argument = Falta el argumento requerido: { $argument } +clap-error-missing-subcommand = Falta el subcomando. Opciones disponibles: { $valid_subcommands } +clap-error-unknown-argument = Argumento desconocido: { $argument } +clap-error-invalid-value = Valor no válido para { $argument }: { $value } +clap-error-invalid-subcommand = Subcomando desconocido: { $subcommand } +clap-error-value-validation = Valor no válido para { $argument }: { $value } diff --git a/src/cli.rs b/src/cli.rs index 85343549..0a4f62ed 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -3,13 +3,27 @@ //! This module defines the [`Cli`] structure and its subcommands. //! It mirrors the design described in `docs/netsuke-design.md`. -use clap::{Args, Parser, Subcommand}; +use clap::{ArgMatches, Args, CommandFactory, FromArgMatches, Parser, Subcommand}; +use ortho_config::LanguageIdentifier; +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::{ + CliValueExtractor, ConfigDiscovery, LocalizationArgs, Localizer, MergeComposer, OrthoConfig, + OrthoMergeExt, OrthoResult, sanitize_value, +}; +use serde::{Deserialize, Serialize}; +use std::ffi::OsString; use std::path::PathBuf; +use std::str::FromStr; use crate::host_pattern::HostPattern; /// 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_"; fn parse_jobs(s: &str) -> Result { let value: usize = s @@ -41,6 +55,16 @@ fn parse_scheme(s: &str) -> Result { Ok(trimmed.to_ascii_lowercase()) } +fn parse_locale(s: &str) -> Result { + let trimmed = s.trim(); + if trimmed.is_empty() { + return Err(String::from("locale must not be empty")); + } + LanguageIdentifier::from_str(trimmed) + .map(|_| trimmed.to_owned()) + .map_err(|_| format!("invalid locale '{s}'")) +} + /// Parse a host pattern supplied via CLI flags. /// /// The returned [`HostPattern`] retains both the wildcard flag and the @@ -51,55 +75,75 @@ fn parse_host_pattern(s: &str) -> Result { } /// A modern, friendly build system that uses YAML and Jinja, powered by Ninja. -#[derive(Debug, Parser)] +#[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(), cli_default_as_absent)] pub file: PathBuf, - /// Change to this directory before doing anything. + /// 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", value_parser = parse_jobs)] pub jobs: Option, - /// Enable verbose logging output. + /// Enable verbose diagnostic logging. #[arg(short, long)] + #[ortho_config(default = false, cli_default_as_absent)] pub verbose: bool, + /// Locale tag for CLI copy (for example: en-US, es-ES). + #[arg(long, value_name = "LOCALE", value_parser = parse_locale)] + pub locale: Option, + /// Additional URL schemes allowed for the `fetch` helper. #[arg( long = "fetch-allow-scheme", value_name = "SCHEME", value_parser = parse_scheme )] + #[ortho_config(merge_strategy = "append")] pub fetch_allow_scheme: Vec, - /// Hostnames that must be explicitly allowed for network access. + /// Hostnames that are permitted when default deny is enabled. + /// + /// Supports wildcards such as `*.example.com`. #[arg( long = "fetch-allow-host", value_name = "HOST", value_parser = parse_host_pattern )] + #[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", value_parser = parse_host_pattern )] + #[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, cli_default_as_absent)] pub fetch_default_deny: bool, /// Optional subcommand to execute; defaults to `build` when omitted. #[command(subcommand)] + #[ortho_config(skip_cli)] pub command: Option, } @@ -120,10 +164,11 @@ impl Cli { impl Default for Cli { fn default() -> Self { Self { - file: PathBuf::from("Netsukefile"), + 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(), @@ -135,26 +180,28 @@ impl Default for Cli { } /// Arguments accepted by the `build` command. -#[derive(Debug, Args, PartialEq, Eq, Clone)] +#[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)] +#[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) `default`. + /// 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 visualization. + /// Display the build dependency graph in DOT format for visualisation. Graph, /// Write the Ninja manifest to the specified file without invoking Ninja. @@ -166,3 +213,168 @@ pub enum Commands { file: PathBuf, }, } + +fn default_manifest_path() -> PathBuf { + PathBuf::from("Netsukefile") +} + +fn usage_body(usage: &str) -> &str { + usage.strip_prefix("Usage: ").unwrap_or(usage) +} + +fn localize_command(mut command: clap::Command, localizer: &dyn Localizer) -> clap::Command { + let rendered_usage = command.render_usage().to_string(); + let fallback_usage = usage_body(&rendered_usage).to_owned(); + let mut args = LocalizationArgs::default(); + args.insert("binary", command.get_name().to_owned().into()); + args.insert("usage", fallback_usage.clone().into()); + let usage = localizer.message("cli.usage", Some(&args), &fallback_usage); + command = command.override_usage(usage); + + if let Some(about) = command.get_about().map(ToString::to_string) { + let localized_text = localizer.message("cli.about", None, &about); + command = command.about(localized_text); + } else if let Some(message) = localizer.lookup("cli.about", None) { + command = command.about(message); + } + + if let Some(long_about) = command.get_long_about().map(ToString::to_string) { + let localized_text = localizer.message("cli.long_about", None, &long_about); + command = command.long_about(localized_text); + } else if let Some(message) = localizer.lookup("cli.long_about", None) { + command = command.long_about(message); + } + + localize_subcommands(&mut command, localizer); + + command +} + +fn localize_subcommands(command: &mut clap::Command, localizer: &dyn Localizer) { + for subcommand in command.get_subcommands_mut() { + let name = subcommand.get_name().to_owned(); + let about_key = format!("cli.subcommand.{name}.about"); + if let Some(about) = subcommand.get_about().map(ToString::to_string) { + let message = localizer.message(&about_key, None, &about); + *subcommand = subcommand.clone().about(message); + } else if let Some(message) = localizer.lookup(&about_key, None) { + *subcommand = subcommand.clone().about(message); + } + + let long_key = format!("cli.subcommand.{name}.long_about"); + if let Some(long_about) = subcommand.get_long_about().map(ToString::to_string) { + let message = localizer.message(&long_key, None, &long_about); + *subcommand = subcommand.clone().long_about(message); + } else if let Some(message) = localizer.lookup(&long_key, None) { + *subcommand = subcommand.clone().long_about(message); + } + } +} + +/// Inspect raw arguments and extract the `--locale` value when present. +#[must_use] +pub fn locale_hint_from_args(args: &[OsString]) -> Option { + let mut iter = args.iter(); + while let Some(arg) = iter.next() { + let text = arg.to_string_lossy(); + if text == "--locale" { + return iter.next().map(|next| next.to_string_lossy().into_owned()); + } + if let Some(value) = text.strip_prefix("--locale=") { + return Some(value.to_owned()); + } + } + None +} + +/// Parse CLI arguments with localised clap output. +/// +/// Returns both the parsed CLI struct and the `ArgMatches` required for +/// configuration merging. +/// +/// # Errors +/// +/// Returns a `clap::Error` with localisation applied when parsing fails. +pub fn parse_with_localizer_from( + iter: I, + localizer: &dyn Localizer, +) -> Result<(Cli, ArgMatches), clap::Error> +where + I: IntoIterator, + T: Into + Clone, +{ + let mut command = localize_command(Cli::command(), localizer); + let mut matches = command + .try_get_matches_from_mut(iter) + .map_err(|err| localize_clap_error_with_command(err, localizer, Some(&command)))?; + let cli = Cli::from_arg_matches_mut(&mut matches).map_err(|clap_err| { + let with_cmd = clap_err.with_cmd(&command); + localize_clap_error_with_command(with_cmd, localizer, Some(&command)) + })?; + Ok((cli, matches)) +} + +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: &serde_json::Value) -> bool { + matches!(value, serde_json::Value::Object(map) if map.is_empty()) +} + +/// 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.extract_user_provided(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 mut merged = composition.into_merge_result(Cli::merge_from_layers)?; + merged.command = command; + Ok(merged) +} diff --git a/src/cli_localization.rs b/src/cli_localization.rs new file mode 100644 index 00000000..8db8b218 --- /dev/null +++ b/src/cli_localization.rs @@ -0,0 +1,82 @@ +//! Locale-aware helpers for CLI messaging. +//! +//! Provides Fluent-backed localizers with an English fallback and +//! consumer-provided Spanish translations to validate localisation support. + +use ortho_config::LanguageIdentifier; +use ortho_config::{FluentLocalizer, FluentLocalizerBuilder, Localizer, NoOpLocalizer}; +use std::str::FromStr; + +const NETSUKE_EN_US: &str = include_str!("../locales/en-US/messages.ftl"); +const NETSUKE_ES_ES: &str = include_str!("../locales/es-ES/messages.ftl"); + +struct LayeredLocalizer { + primary: Box, + fallback: Box, +} + +impl LayeredLocalizer { + fn new(primary: Box, fallback: Box) -> Self { + Self { primary, fallback } + } +} + +impl Localizer for LayeredLocalizer { + fn lookup( + &self, + id: &str, + args: Option<&ortho_config::LocalizationArgs<'_>>, + ) -> Option { + self.primary + .lookup(id, args) + .or_else(|| self.fallback.lookup(id, args)) + } +} + +fn parse_locale(locale: &str) -> Option { + LanguageIdentifier::from_str(locale).ok() +} + +fn build_en_localizer() -> Box { + FluentLocalizer::with_en_us_defaults([NETSUKE_EN_US]).map_or_else( + |_| Box::new(NoOpLocalizer::new()) as Box, + |localizer| Box::new(localizer) as Box, + ) +} + +fn build_consumer_localizer( + builder: FluentLocalizerBuilder, + resource: &'static str, +) -> Option> { + builder + .with_consumer_resources([resource]) + .disable_defaults() + .try_build() + .ok() + .map(|localizer| Box::new(localizer) as Box) +} + +fn locale_language(locale: &LanguageIdentifier) -> &str { + locale.language.as_str() +} + +/// Build a CLI localizer with an English fallback. +#[must_use] +pub fn build_localizer(preferred_locale: Option<&str>) -> Box { + let fallback = build_en_localizer(); + let Some(preferred) = preferred_locale else { + return fallback; + }; + let Some(locale) = parse_locale(preferred) else { + return fallback; + }; + + if locale_language(&locale) == "es" { + let builder = FluentLocalizer::builder(locale); + if let Some(primary) = build_consumer_localizer(builder, NETSUKE_ES_ES) { + return Box::new(LayeredLocalizer::new(primary, fallback)); + } + } + + fallback +} diff --git a/src/cli_policy.rs b/src/cli_policy.rs index 8d741d06..0bbf2117 100644 --- a/src/cli_policy.rs +++ b/src/cli_policy.rs @@ -19,6 +19,7 @@ impl Cli { /// /// let cli = Cli { /// fetch_allow_scheme: vec!["http".into()], + /// locale: None, /// ..Cli::default() /// }; /// let policy = cli.network_policy().expect("policy"); @@ -33,6 +34,7 @@ impl Cli { /// let cli = Cli { /// fetch_allow_scheme: vec![String::from("http?")], /// fetch_allow_host: vec![HostPattern::parse("example.com").expect("parse host")], + /// locale: None, /// ..Cli::default() /// }; /// let err = cli diff --git a/src/host_pattern.rs b/src/host_pattern.rs index 4e531e7a..e0d37625 100644 --- a/src/host_pattern.rs +++ b/src/host_pattern.rs @@ -3,6 +3,8 @@ //! The module centralises normalisation and matching logic so CLI parsing and //! runtime policy evaluation agree on allowable host syntax. +use serde::{Deserialize, Serialize}; +use std::str::FromStr; use thiserror::Error; fn validate_label(label: &str, original: &str) -> Result<(), HostPatternError> { @@ -180,6 +182,38 @@ impl TryFrom for HostPattern { } } +impl FromStr for HostPattern { + type Err = HostPatternError; + + fn from_str(value: &str) -> Result { + Self::parse(value) + } +} + +impl Serialize for HostPattern { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + let text = if self.wildcard { + format!("*.{}", self.pattern) + } else { + self.pattern.clone() + }; + serializer.serialize_str(&text) + } +} + +impl<'de> Deserialize<'de> for HostPattern { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let text = String::deserialize(deserializer)?; + Self::parse(&text).map_err(serde::de::Error::custom) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/lib.rs b/src/lib.rs index a660ce82..434b32de 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -5,6 +5,7 @@ pub mod ast; pub mod cli; +pub mod cli_localization; mod cli_policy; pub(crate) mod diagnostics; pub mod hasher; diff --git a/src/main.rs b/src/main.rs index 5fa20da4..c0a49b16 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2,25 +2,48 @@ //! //! Parses command-line arguments and delegates execution to [`runner::run`]. -use clap::Parser; -use netsuke::{cli::Cli, runner}; +use netsuke::{cli, cli_localization, runner}; +use ortho_config::is_display_request; +use std::ffi::OsString; use std::io; use std::process::ExitCode; use tracing::Level; use tracing_subscriber::fmt; fn main() -> ExitCode { - let cli = Cli::parse().with_default_command(); - let max_level = if cli.verbose { + let args: Vec = std::env::args_os().collect(); + let locale_hint = cli::locale_hint_from_args(&args); + let env_locale = std::env::var("NETSUKE_LOCALE").ok(); + let locale = locale_hint.as_deref().or(env_locale.as_deref()); + let localizer = cli_localization::build_localizer(locale); + + let (parsed_cli, matches) = match cli::parse_with_localizer_from(args, localizer.as_ref()) { + Ok(parsed) => parsed, + Err(err) => { + if is_display_request(&err) { + err.exit(); + } + err.exit(); + } + }; + + let merged_cli = match cli::merge_with_config(&parsed_cli, &matches) { + Ok(merged) => merged.with_default_command(), + Err(err) => { + init_tracing(Level::ERROR); + tracing::error!(error = %err, "configuration load failed"); + return ExitCode::FAILURE; + } + }; + + let max_level = if merged_cli.verbose { Level::DEBUG } else { Level::ERROR }; - fmt() - .with_max_level(max_level) - .with_writer(io::stderr) - .init(); - match runner::run(&cli) { + init_tracing(max_level); + + match runner::run(&merged_cli) { Ok(()) => ExitCode::SUCCESS, Err(err) => { tracing::error!(error = %err, "runner failed"); @@ -28,3 +51,10 @@ fn main() -> ExitCode { } } } + +fn init_tracing(max_level: Level) { + fmt() + .with_max_level(max_level) + .with_writer(io::stderr) + .init(); +} diff --git a/src/runner/mod.rs b/src/runner/mod.rs index 061ba2a2..21abb296 100644 --- a/src/runner/mod.rs +++ b/src/runner/mod.rs @@ -119,6 +119,7 @@ pub fn run(cli: &Cli) -> Result<()> { /// directory: None, /// jobs: None, /// verbose: false, +/// locale: None, /// fetch_allow_scheme: Vec::new(), /// fetch_allow_host: Vec::new(), /// fetch_block_host: Vec::new(), @@ -169,7 +170,11 @@ fn handle_build(cli: &Cli, args: &BuildArgs) -> Result<()> { /// /// Returns an error if manifest generation or Ninja execution fails. fn handle_ninja_tool(cli: &Cli, tool: &str) -> Result<()> { - info!(target: "netsuke::subcommand", subcommand = tool, "Subcommand requested"); + info!( + target: "netsuke::subcommand", + subcommand = tool, + "Preparing Ninja tool invocation" + ); let ninja = generate_ninja(cli)?; let tmp = process::create_temp_ninja_file(&ninja)?; @@ -226,6 +231,7 @@ fn handle_graph(cli: &Cli) -> Result<()> { /// directory: None, /// jobs: None, /// verbose: false, +/// locale: None, /// fetch_allow_scheme: Vec::new(), /// fetch_allow_host: Vec::new(), /// fetch_block_host: Vec::new(), @@ -265,6 +271,7 @@ fn generate_ninja(cli: &Cli) -> Result { /// directory: None, /// jobs: None, /// verbose: false, +/// locale: None, /// fetch_allow_scheme: Vec::new(), /// fetch_allow_host: Vec::new(), /// fetch_block_host: Vec::new(), diff --git a/src/runner/process/file_io.rs b/src/runner/process/file_io.rs index b4bf9c40..d155e0bb 100644 --- a/src/runner/process/file_io.rs +++ b/src/runner/process/file_io.rs @@ -31,7 +31,7 @@ pub fn create_temp_ninja_file(content: &NinjaContent) -> AnyResult AnyResult<()> { Utf8Path::from_path(path).ok_or_else(|| anyhow!("non-UTF-8 path is not supported"))?; let (dir, relative) = derive_dir_and_relative(utf8_path)?; write_ninja_file_utf8(&dir, &relative, content)?; - info!("Generated Ninja file at {utf8_path}"); + info!("Wrote Ninja file to {utf8_path}"); Ok(()) } diff --git a/src/runner/process/mod.rs b/src/runner/process/mod.rs index 4d0f8166..932c525f 100644 --- a/src/runner/process/mod.rs +++ b/src/runner/process/mod.rs @@ -123,7 +123,7 @@ fn log_command_execution(cmd: &Command) { let redacted_args = redact_sensitive_args(&args); let arg_strings: Vec<&str> = redacted_args.iter().map(CommandArg::as_str).collect(); info!( - "Running command: {} {}", + "Executing command: {} {}", program_display, arg_strings.join(" ") ); diff --git a/tests/bdd/steps/cli.rs b/tests/bdd/steps/cli.rs index cd37caec..3cc24738 100644 --- a/tests/bdd/steps/cli.rs +++ b/tests/bdd/steps/cli.rs @@ -8,9 +8,10 @@ use crate::bdd::fixtures::{RefCellOptionExt, TestWorld}; use crate::bdd::helpers::parse_store::store_parse_outcome; use crate::bdd::types::{CliArgs, ErrorFragment, JobCount, PathString, TargetName, UrlString}; use anyhow::{Context, Result, bail, ensure}; -use clap::Parser; -use netsuke::cli::{BuildArgs, Cli, Commands}; +use netsuke::cli::{Cli, Commands}; +use netsuke::cli_localization; use rstest_bdd_macros::{given, then, when}; +use std::ffi::OsString; use std::path::PathBuf; // --------------------------------------------------------------------------- @@ -20,8 +21,11 @@ use std::path::PathBuf; /// Apply CLI parsing, storing result or error in world state. fn apply_cli(world: &TestWorld, args: &CliArgs) { let tokens = build_token_list(args); - let outcome = Cli::try_parse_from(tokens) - .map(normalize_cli) + let os_tokens: Vec = tokens.iter().map(OsString::from).collect(); + let locale_hint = netsuke::cli::locale_hint_from_args(&os_tokens); + let localizer = cli_localization::build_localizer(locale_hint.as_deref()); + let outcome = netsuke::cli::parse_with_localizer_from(os_tokens, localizer.as_ref()) + .map(|(cli, _matches)| normalize_cli(cli)) .map_err(|e| e.to_string()); store_parse_outcome(&world.cli, &world.cli_error, outcome); } @@ -78,14 +82,8 @@ fn build_token_list(args: &CliArgs) -> Vec { } /// Normalise a parsed CLI by setting default command if missing. -fn normalize_cli(mut cli: Cli) -> Cli { - if cli.command.is_none() { - cli.command = Some(Commands::Build(BuildArgs { - emit: None, - targets: Vec::new(), - })); - } - cli +fn normalize_cli(cli: Cli) -> Cli { + cli.with_default_command() } // --------------------------------------------------------------------------- diff --git a/tests/cli_tests.rs b/tests/cli_tests.rs index c79333e7..a284c63b 100644 --- a/tests/cli_tests.rs +++ b/tests/cli_tests.rs @@ -4,12 +4,15 @@ //! using `rstest` for parameterised coverage of success and error scenarios. use anyhow::{Context, Result, bail, ensure}; -use clap::Parser; use clap::error::ErrorKind; +use clap::{CommandFactory, FromArgMatches, Parser}; use netsuke::cli::{BuildArgs, Cli, Commands}; +use netsuke::cli_localization; use netsuke::host_pattern::HostPattern; use netsuke::stdlib::NetworkPolicyViolation; +use ortho_config::{CliValueExtractor, MergeComposer, sanitize_value}; use rstest::rstest; +use serde_json::json; use std::path::PathBuf; use url::Url; @@ -19,6 +22,7 @@ struct CliCase { directory: Option, jobs: Option, verbose: bool, + locale: Option<&'static str>, allow_scheme: Vec, allow_host: Vec<&'static str>, block_host: Vec<&'static str>, @@ -34,6 +38,7 @@ impl Default for CliCase { directory: None, jobs: None, verbose: false, + locale: None, allow_scheme: Vec::new(), allow_host: Vec::new(), block_host: Vec::new(), @@ -64,6 +69,11 @@ impl Default for CliCase { verbose: true, ..CliCase::default() })] +#[case(CliCase { + argv: vec!["netsuke", "--locale", "es-ES"], + locale: Some("es-ES"), + ..CliCase::default() +})] #[case(CliCase { argv: vec!["netsuke", "build", "--emit", "out.ninja", "a"], expected_cmd: Commands::Build(BuildArgs { @@ -117,6 +127,10 @@ fn parse_cli(#[case] case: CliCase) -> Result<()> { cli.verbose == case.verbose, "verbose flag should match input", ); + ensure!( + cli.locale.as_deref() == case.locale, + "locale should match input", + ); ensure!( cli.fetch_allow_scheme == case.allow_scheme, "allow-scheme flags should match input", @@ -256,6 +270,7 @@ fn cli_network_policy_rejects_invalid_scheme() { #[case(vec!["netsuke", "-j", "notanumber"], ErrorKind::ValueValidation)] #[case(vec!["netsuke", "--file", "alt.yml", "-C"], ErrorKind::InvalidValue)] #[case(vec!["netsuke", "manifest"], ErrorKind::MissingRequiredArgument)] +#[case(vec!["netsuke", "--locale", "nope"], ErrorKind::ValueValidation)] fn parse_cli_errors(#[case] argv: Vec<&str>, #[case] expected_error: ErrorKind) -> Result<()> { let err = Cli::try_parse_from(argv) .err() @@ -268,3 +283,92 @@ fn parse_cli_errors(#[case] argv: Vec<&str>, #[case] expected_error: ErrorKind) ); Ok(()) } + +#[rstest] +fn cli_extract_user_provided_omits_defaults() -> Result<()> { + let mut matches = Cli::command() + .try_get_matches_from(["netsuke"]) + .context("parse matches for default CLI")?; + let cli = Cli::from_arg_matches_mut(&mut matches).context("build CLI from matches")?; + let value = cli + .extract_user_provided(&matches) + .context("extract CLI overrides")?; + let object = value + .as_object() + .context("expected extracted CLI value to be an object")?; + ensure!( + !object.contains_key("file"), + "default file should not be treated as a CLI override", + ); + ensure!( + !object.contains_key("verbose"), + "default verbose flag should not be treated as a CLI override", + ); + ensure!( + !object.contains_key("fetch_default_deny"), + "default deny flag should not be treated as a CLI override", + ); + Ok(()) +} + +#[rstest] +fn cli_merge_layers_respects_precedence_and_appends_lists() -> Result<()> { + let mut composer = MergeComposer::new(); + let mut defaults = sanitize_value(&Cli::default())?; + let defaults_object = defaults + .as_object_mut() + .context("defaults should be an object")?; + defaults_object.insert("jobs".to_owned(), json!(1)); + defaults_object.insert("fetch_allow_scheme".to_owned(), json!(["https"])); + composer.push_defaults(defaults); + composer.push_file( + json!({ + "file": "Configfile", + "jobs": 2, + "fetch_allow_scheme": ["http"], + "locale": "en-US" + }), + None, + ); + composer.push_environment(json!({ + "jobs": 3, + "fetch_allow_scheme": ["ftp"] + })); + composer.push_cli(json!({ + "jobs": 4, + "fetch_allow_scheme": ["git"], + "verbose": true + })); + let merged = Cli::merge_from_layers(composer.layers())?; + ensure!( + merged.file == PathBuf::from("Configfile"), + "file layer should override defaults", + ); + ensure!(merged.jobs == Some(4), "CLI layer should override jobs",); + ensure!( + merged.fetch_allow_scheme == vec!["https", "http", "ftp", "git"], + "list values should append in layer order", + ); + ensure!( + merged.locale.as_deref() == Some("en-US"), + "file layer should populate locale when CLI does not override", + ); + ensure!(merged.verbose, "CLI layer should set verbose"); + Ok(()) +} + +#[rstest] +fn cli_localises_invalid_subcommand_in_spanish() -> Result<()> { + let localizer = cli_localization::build_localizer(Some("es-ES")); + let err = netsuke::cli::parse_with_localizer_from( + ["netsuke", "--locale", "es-ES", "unknown"], + localizer.as_ref(), + ) + .err() + .context("parser should reject invalid subcommand")?; + ensure!( + err.to_string().contains("Subcomando desconocido"), + "expected Spanish localisation, got: {err}", + ); + Ok(()) +} diff --git a/tests/features/cli.feature b/tests/features/cli.feature index 401ec238..e1750032 100644 --- a/tests/features/cli.feature +++ b/tests/features/cli.feature @@ -50,7 +50,12 @@ Feature: CLI parsing Scenario: Unknown command fails When the CLI is parsed with invalid arguments "unknown" Then an error should be returned - And the error message should contain "unknown" + And the error message should contain "Unknown subcommand" + + Scenario: Unknown command is localised in Spanish + When the CLI is parsed with invalid arguments "--locale es-ES unknown" + Then an error should be returned + And the error message should contain "Subcomando desconocido" Scenario: Missing file argument value When the CLI is parsed with invalid arguments "--file" From 215c0bda33879844be074e6c644b956825465cbb Mon Sep 17 00:00:00 2001 From: Leynos Date: Fri, 2 Jan 2026 04:49:06 +0000 Subject: [PATCH 3/9] refactor(tests): use Path::new instead of PathBuf::from for comparison Changed test assertion to use `merged.file.as_path() == Path::new("Configfile")` instead of `PathBuf::from("Configfile")` for more idiomatic path comparison in tests. Co-authored-by: terragon-labs[bot] --- tests/cli_tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/cli_tests.rs b/tests/cli_tests.rs index a284c63b..71b95a37 100644 --- a/tests/cli_tests.rs +++ b/tests/cli_tests.rs @@ -13,7 +13,7 @@ use netsuke::stdlib::NetworkPolicyViolation; use ortho_config::{CliValueExtractor, MergeComposer, sanitize_value}; use rstest::rstest; use serde_json::json; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use url::Url; struct CliCase { @@ -341,7 +341,7 @@ fn cli_merge_layers_respects_precedence_and_appends_lists() -> Result<()> { })); let merged = Cli::merge_from_layers(composer.layers())?; ensure!( - merged.file == PathBuf::from("Configfile"), + merged.file.as_path() == Path::new("Configfile"), "file layer should override defaults", ); ensure!(merged.jobs == Some(4), "CLI layer should override jobs",); From 187011afa618253cf75de937989fda65b5b90939 Mon Sep 17 00:00:00 2001 From: Leynos Date: Fri, 2 Jan 2026 05:34:42 +0000 Subject: [PATCH 4/9] test(cli): add comprehensive CLI tests for locale and config merging - Added tests for locale_hint_from_args covering various argument forms and conditions. - Added tests for cli_merge_with_config to verify precedence and skipping empty CLI layers. - Introduced EnvGuard for managing environment variables in tests safely. - Improved testing coverage and robustness for CLI parsing and merging behavior. Co-authored-by: terragon-labs[bot] --- docs/execplans/cli-output-clarity.md | 13 +-- docs/users-guide.md | 4 +- src/cli.rs | 37 +++++--- src/cli_localization.rs | 4 +- src/host_pattern.rs | 53 ++++++----- src/lib.rs | 3 + src/main.rs | 8 +- tests/cli_tests.rs | 130 ++++++++++++++++++++++++++- 8 files changed, 203 insertions(+), 49 deletions(-) diff --git a/docs/execplans/cli-output-clarity.md b/docs/execplans/cli-output-clarity.md index 039b94a2..b1363970 100644 --- a/docs/execplans/cli-output-clarity.md +++ b/docs/execplans/cli-output-clarity.md @@ -27,8 +27,8 @@ outputs and configuration precedence. plumbing. - [x] Refine user-facing CLI output and update docs. - [x] Add unit tests and rstest-bdd behavioural tests for happy/unhappy paths. -- [x] (2026-01-02 00:00Z) Run formatting, lint, and test gates; update roadmap - entry to done. +- [x] (2026-01-02 00:00Z) Run formatting, lint, and test gates; mark the + roadmap entry as done. ## Surprises & Discoveries @@ -69,18 +69,19 @@ Key runtime entry points and CLI definitions live in these files: `tests/runner_tests.rs`, plus behavioural steps in `tests/bdd/steps/cli.rs` and `tests/bdd/steps/process.rs`. -OrthoConfig is currently not wired in. The user guide for it is +OrthoConfig is wired in. The user guide for it is `docs/ortho-config-users-guide.md`, which explains configuration layering, localised help via Fluent, and error localisation helpers. Design expectations for CLI behaviour are in `docs/netsuke-design.md` and the roadmap entry in `docs/roadmap.md` (Phase 3 → “CLI and Feature Completeness”). -Testing guidance for fixtures, DI, and BDD lives in: +Testing guidance for fixtures, dependency injection (DI), and behaviour-driven +development (BDD) lives in: - `docs/rust-testing-with-rstest-fixtures.md` - `docs/reliable-testing-in-rust-via-dependency-injection.md` - `docs/behavioural-testing-in-rust-with-cucumber.md` (applies to Gherkin - structure, even though we use `rstest-bdd`) + structure, even though `rstest-bdd` is used instead) - `docs/rust-doctest-dry-guide.md` (for any new public API docs) ## Plan of Work @@ -116,7 +117,7 @@ Testing guidance for fixtures, DI, and BDD lives in: action-oriented. Review `tracing::info!` messages for build/manifest/graph flows and update wording to align with the user guide. Ensure stderr/stdout separation remains correct and messages are consistent across subcommands. - If necessary, introduce a small output helper module to centralise user + If necessary, introduce a small output helper module to centralize user message formatting. 5. Add tests. Extend unit tests in `tests/cli_tests.rs` with `rstest` fixtures diff --git a/docs/users-guide.md b/docs/users-guide.md index 1fc4dee0..a4854767 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -540,8 +540,8 @@ Windows application data directories, `$HOME/.config/netsuke`, `$HOME/.netsuke.toml`, and finally the project root. Environment variables use the `NETSUKE_` prefix (for example, -`NETSUKE_JOBS=8`). Use `__` to separate nested keys if you need to match -structured configuration. +`NETSUKE_JOBS=8`). Use `__` to separate nested keys when matching structured +configuration. Use `--locale ` or `NETSUKE_LOCALE` to select localised CLI copy and error messages. Spanish (`es-ES`) is included as a reference translation; diff --git a/src/cli.rs b/src/cli.rs index 0a4f62ed..283544b0 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -142,6 +142,7 @@ pub struct Cli { pub fetch_default_deny: bool, /// Optional subcommand to execute; defaults to `build` when omitted. + #[serde(skip)] #[command(subcommand)] #[ortho_config(skip_cli)] pub command: Option, @@ -253,38 +254,54 @@ fn localize_command(mut command: clap::Command, localizer: &dyn Localizer) -> cl fn localize_subcommands(command: &mut clap::Command, localizer: &dyn Localizer) { for subcommand in command.get_subcommands_mut() { let name = subcommand.get_name().to_owned(); + let mut updated = std::mem::take(subcommand); let about_key = format!("cli.subcommand.{name}.about"); - if let Some(about) = subcommand.get_about().map(ToString::to_string) { + if let Some(about) = updated.get_about().map(ToString::to_string) { let message = localizer.message(&about_key, None, &about); - *subcommand = subcommand.clone().about(message); + updated = updated.about(message); } else if let Some(message) = localizer.lookup(&about_key, None) { - *subcommand = subcommand.clone().about(message); + updated = updated.about(message); } let long_key = format!("cli.subcommand.{name}.long_about"); - if let Some(long_about) = subcommand.get_long_about().map(ToString::to_string) { + if let Some(long_about) = updated.get_long_about().map(ToString::to_string) { let message = localizer.message(&long_key, None, &long_about); - *subcommand = subcommand.clone().long_about(message); + updated = updated.long_about(message); } else if let Some(message) = localizer.lookup(&long_key, None) { - *subcommand = subcommand.clone().long_about(message); + updated = updated.long_about(message); } + + *subcommand = updated; } } /// Inspect raw arguments and extract the `--locale` value when present. #[must_use] pub fn locale_hint_from_args(args: &[OsString]) -> Option { - let mut iter = args.iter(); + let mut hint = None; + let mut iter = args.iter().peekable(); while let Some(arg) = iter.next() { let text = arg.to_string_lossy(); + if text == "--" { + break; + } if text == "--locale" { - return iter.next().map(|next| next.to_string_lossy().into_owned()); + let Some(next) = iter.peek() else { + break; + }; + let next_text = next.to_string_lossy(); + if next_text == "--" { + break; + } + hint = Some(next_text.into_owned()); + iter.next(); + continue; } if let Some(value) = text.strip_prefix("--locale=") { - return Some(value.to_owned()); + hint = Some(value.to_owned()); } } - None + hint } /// Parse CLI arguments with localised clap output. diff --git a/src/cli_localization.rs b/src/cli_localization.rs index 8db8b218..fae6f58a 100644 --- a/src/cli_localization.rs +++ b/src/cli_localization.rs @@ -33,7 +33,7 @@ impl Localizer for LayeredLocalizer { } } -fn parse_locale(locale: &str) -> Option { +fn parse_locale_identifier(locale: &str) -> Option { LanguageIdentifier::from_str(locale).ok() } @@ -67,7 +67,7 @@ pub fn build_localizer(preferred_locale: Option<&str>) -> Box { let Some(preferred) = preferred_locale else { return fallback; }; - let Some(locale) = parse_locale(preferred) else { + let Some(locale) = parse_locale_identifier(preferred) else { return fallback; }; diff --git a/src/host_pattern.rs b/src/host_pattern.rs index e0d37625..1e5c1fbb 100644 --- a/src/host_pattern.rs +++ b/src/host_pattern.rs @@ -7,28 +7,38 @@ use serde::{Deserialize, Serialize}; use std::str::FromStr; use thiserror::Error; -fn validate_label(label: &str, original: &str) -> Result<(), HostPatternError> { - if label.is_empty() { - return Err(HostPatternError::EmptyLabel { - pattern: original.to_owned(), - }); - } - if !label.chars().all(|c| c.is_ascii_alphanumeric() || c == '-') { - return Err(HostPatternError::InvalidCharacters { - pattern: original.to_owned(), - }); - } - if label.starts_with('-') || label.ends_with('-') { - return Err(HostPatternError::InvalidLabelEdge { - pattern: original.to_owned(), - }); +struct ValidationContext<'a> { + original: &'a str, +} + +impl<'a> ValidationContext<'a> { + const fn new(original: &'a str) -> Self { + Self { original } } - if label.len() > 63 { - return Err(HostPatternError::LabelTooLong { - pattern: original.to_owned(), - }); + + fn validate_label(&self, label: &str) -> Result<(), HostPatternError> { + if label.is_empty() { + return Err(HostPatternError::EmptyLabel { + pattern: self.original.to_owned(), + }); + } + if !label.chars().all(|c| c.is_ascii_alphanumeric() || c == '-') { + return Err(HostPatternError::InvalidCharacters { + pattern: self.original.to_owned(), + }); + } + if label.starts_with('-') || label.ends_with('-') { + return Err(HostPatternError::InvalidLabelEdge { + pattern: self.original.to_owned(), + }); + } + if label.len() > 63 { + return Err(HostPatternError::LabelTooLong { + pattern: self.original.to_owned(), + }); + } + Ok(()) } - Ok(()) } /// Errors emitted when parsing host allowlist/blocklist patterns. @@ -116,8 +126,9 @@ pub(crate) fn normalise_host_pattern(pattern: &str) -> Result<(String, bool), Ho let normalised = host_body.to_ascii_lowercase(); let mut total_len = 0usize; + let ctx = ValidationContext::new(trimmed); for (index, label) in normalised.split('.').enumerate() { - validate_label(label, trimmed)?; + ctx.validate_label(label)?; total_len += label.len() + usize::from(index > 0); } if total_len > 255 { diff --git a/src/lib.rs b/src/lib.rs index 434b32de..f9297c9c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3,6 +3,9 @@ //! This library provides the command line interface definitions and //! helper functions for parsing `Netsukefile` manifests. +#[cfg(not(feature = "serde_json"))] +compile_error!("The `serde_json` feature is required to build Netsuke."); + pub mod ast; pub mod cli; pub mod cli_localization; diff --git a/src/main.rs b/src/main.rs index c0a49b16..f95561ad 100644 --- a/src/main.rs +++ b/src/main.rs @@ -3,7 +3,6 @@ //! Parses command-line arguments and delegates execution to [`runner::run`]. use netsuke::{cli, cli_localization, runner}; -use ortho_config::is_display_request; use std::ffi::OsString; use std::io; use std::process::ExitCode; @@ -19,12 +18,7 @@ fn main() -> ExitCode { let (parsed_cli, matches) = match cli::parse_with_localizer_from(args, localizer.as_ref()) { Ok(parsed) => parsed, - Err(err) => { - if is_display_request(&err) { - err.exit(); - } - err.exit(); - } + Err(err) => err.exit(), }; let merged_cli = match cli::merge_with_config(&parsed_cli, &matches) { diff --git a/tests/cli_tests.rs b/tests/cli_tests.rs index 71b95a37..ef3232ee 100644 --- a/tests/cli_tests.rs +++ b/tests/cli_tests.rs @@ -6,16 +6,52 @@ use anyhow::{Context, Result, bail, ensure}; use clap::error::ErrorKind; use clap::{CommandFactory, FromArgMatches, Parser}; -use netsuke::cli::{BuildArgs, Cli, Commands}; +use netsuke::cli::{BuildArgs, Cli, Commands, locale_hint_from_args}; use netsuke::cli_localization; use netsuke::host_pattern::HostPattern; use netsuke::stdlib::NetworkPolicyViolation; use ortho_config::{CliValueExtractor, MergeComposer, sanitize_value}; use rstest::rstest; use serde_json::json; +use std::ffi::{OsStr, OsString}; +use std::fs; use std::path::{Path, PathBuf}; +use tempfile::tempdir; use url::Url; +struct EnvGuard { + key: &'static str, + previous: Option, +} + +impl EnvGuard { + fn set(key: &'static str, value: impl AsRef) -> Self { + let previous = std::env::var_os(key); + // SAFETY: Tests run in a single process and the guard restores values. + unsafe { std::env::set_var(key, value.as_ref()) }; + Self { key, previous } + } + + fn unset(key: &'static str) -> Self { + let previous = std::env::var_os(key); + // SAFETY: Tests run in a single process and the guard restores values. + unsafe { std::env::remove_var(key) }; + Self { key, previous } + } +} + +impl Drop for EnvGuard { + fn drop(&mut self) { + if let Some(previous) = self.previous.take() { + // SAFETY: Restores the previous environment value for the test. + unsafe { std::env::set_var(self.key, previous) }; + } else { + // SAFETY: Restores the previous environment state for the test. + unsafe { std::env::remove_var(self.key) }; + } + } +} + struct CliCase { argv: Vec<&'static str>, file: PathBuf, @@ -284,6 +320,65 @@ fn parse_cli_errors(#[case] argv: Vec<&str>, #[case] expected_error: ErrorKind) Ok(()) } +fn os_args(args: &[&str]) -> Vec { + args.iter().map(|arg| OsString::from(*arg)).collect() +} + +#[rstest] +fn locale_hint_from_args_accepts_space_form() -> Result<()> { + let args = os_args(&["netsuke", "--locale", "es-ES"]); + let hint = locale_hint_from_args(&args); + ensure!( + hint.as_deref() == Some("es-ES"), + "expected Some(\"es-ES\"), got: {hint:?}" + ); + Ok(()) +} + +#[rstest] +fn locale_hint_from_args_accepts_equals_form() -> Result<()> { + let args = os_args(&["netsuke", "--locale=es-ES"]); + let hint = locale_hint_from_args(&args); + ensure!( + hint.as_deref() == Some("es-ES"), + "expected Some(\"es-ES\"), got: {hint:?}" + ); + Ok(()) +} + +#[rstest] +fn locale_hint_from_args_trailing_locale_flag_yields_none() -> Result<()> { + let args = os_args(&["netsuke", "--locale"]); + let hint = locale_hint_from_args(&args); + ensure!( + hint.is_none(), + "expected None for trailing --locale without value, got: {hint:?}" + ); + Ok(()) +} + +#[rstest] +fn locale_hint_from_args_ignores_args_after_double_dash() -> Result<()> { + let args = os_args(&["netsuke", "--verbose", "--", "--locale", "es-ES"]); + let hint = locale_hint_from_args(&args); + ensure!( + hint.is_none(), + "expected None when --locale appears after \"--\", got: {hint:?}" + ); + Ok(()) +} + +#[rstest] +fn locale_hint_from_args_uses_last_locale_flag() -> Result<()> { + let args = os_args(&["netsuke", "--locale", "es-ES", "--locale", "en-US"]); + let hint = locale_hint_from_args(&args); + ensure!( + hint.as_deref() == Some("en-US"), + "expected last --locale to win (\"en-US\"), got: {hint:?}" + ); + Ok(()) +} + #[rstest] fn cli_extract_user_provided_omits_defaults() -> Result<()> { let mut matches = Cli::command() @@ -357,6 +452,39 @@ fn cli_merge_layers_respects_precedence_and_appends_lists() -> Result<()> { Ok(()) } +#[rstest] +fn cli_merge_with_config_respects_precedence_and_skips_empty_cli_layer() -> Result<()> { + let temp_dir = tempdir().context("create temporary config directory")?; + let config_path = temp_dir.path().join("netsuke.toml"); + let config = r#" +jobs = 2 +fetch_allow_scheme = ["https"] +"#; + fs::write(&config_path, config).context("write netsuke.toml")?; + + let _config_guard = EnvGuard::set("NETSUKE_CONFIG_PATH", config_path.as_os_str()); + let _jobs_guard = EnvGuard::set("NETSUKE_JOBS", "4"); + let _scheme_guard = EnvGuard::unset("NETSUKE_FETCH_ALLOW_SCHEME"); + + let localizer = cli_localization::build_localizer(None); + let (cli, matches) = netsuke::cli::parse_with_localizer_from(["netsuke"], localizer.as_ref()) + .context("parse CLI args for merge")?; + let merged = netsuke::cli::merge_with_config(&cli, &matches) + .context("merge CLI and configuration layers")? + .with_default_command(); + + ensure!( + merged.jobs == Some(4), + "environment variables should override config when CLI has no value", + ); + ensure!( + merged.fetch_allow_scheme == vec!["https".to_owned()], + "config values should apply when CLI overrides are empty", + ); + + Ok(()) +} + #[rstest] fn cli_localises_invalid_subcommand_in_spanish() -> Result<()> { let localizer = cli_localization::build_localizer(Some("es-ES")); From 13910b86cb62120864c52cbf56484b07da4add71 Mon Sep 17 00:00:00 2001 From: Leynos Date: Fri, 2 Jan 2026 06:29:11 +0000 Subject: [PATCH 5/9] feat(cli): add OrthoConfig-backed localized CLI help and error support This change integrates OrthoConfig for layered configuration and Fluent for localized CLI help and error messages. English and Spanish locales are included, with support for the --locale flag and NETSUKE_LOCALE environment variable. It updates CLI parsing to use localized clap errors, refines user-facing CLI output for clarity, and implements configuration merging respecting OrthoConfig precedence. Comprehensive unit and behavioral tests verify parsing, localization fallback, and network policy enforcement derived from CLI flags. The extensive test suite replaces the older monolithic CLI tests, introducing modular test files for parsing, locale, merge, and policy coverage. Documentation is updated to reflect localization support, configuration layering, and refined CLI behaviors. Co-authored-by: terragon-labs[bot] --- Cargo.toml | 1 + build.rs | 2 +- docs/execplans/cli-output-clarity.md | 40 +-- docs/netsuke-design.md | 2 +- docs/users-guide.md | 6 +- src/cli.rs | 9 +- src/cli_localization.rs | 13 +- src/host_pattern.rs | 55 ++- src/lib.rs | 3 - src/stdlib/network/policy/mod.rs | 14 +- tests/cli_tests.rs | 502 --------------------------- tests/cli_tests/helpers.rs | 7 + tests/cli_tests/locale.rs | 79 +++++ tests/cli_tests/merge.rs | 121 +++++++ tests/cli_tests/mod.rs | 9 + tests/cli_tests/parsing.rs | 189 ++++++++++ tests/cli_tests/policy.rs | 96 +++++ 17 files changed, 586 insertions(+), 562 deletions(-) delete mode 100644 tests/cli_tests.rs create mode 100644 tests/cli_tests/helpers.rs create mode 100644 tests/cli_tests/locale.rs create mode 100644 tests/cli_tests/merge.rs create mode 100644 tests/cli_tests/mod.rs create mode 100644 tests/cli_tests/parsing.rs create mode 100644 tests/cli_tests/policy.rs diff --git a/Cargo.toml b/Cargo.toml index 40beb454..a114d009 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,6 +17,7 @@ description = "A YAML-powered Ninja/Jinja hybrid build system." [features] default = ["serde_json"] legacy-digests = ["sha1", "md5"] +# Required: OrthoConfig derive and core manifest types depend on serde_json. serde_json = ["ortho_config/serde_json"] [dependencies] diff --git a/build.rs b/build.rs index 06fb6a59..76bd5569 100644 --- a/build.rs +++ b/build.rs @@ -77,7 +77,7 @@ fn main() -> Result<(), Box> { cli::merge_with_config; const _: LocalizedParseFn = cli::parse_with_localizer_from; const _: fn(&str) -> Result = HostPattern::parse; - const _: fn(&HostPattern, &str) -> bool = HostPattern::matches; + const _: fn(&HostPattern, host_pattern::HostCandidate<'_>) -> bool = HostPattern::matches; // Regenerate the manual page when the CLI or metadata changes. println!("cargo:rerun-if-changed=src/cli.rs"); diff --git a/docs/execplans/cli-output-clarity.md b/docs/execplans/cli-output-clarity.md index b1363970..ff3eedd4 100644 --- a/docs/execplans/cli-output-clarity.md +++ b/docs/execplans/cli-output-clarity.md @@ -1,4 +1,4 @@ -# Refine CLI Output With OrthoConfig Localised Help +# Refine CLI Output With OrthoConfig Localized Help This ExecPlan is a living document. The sections `Progress`, `Surprises & Discoveries`, `Decision Log`, and `Outcomes & Retrospective` must @@ -11,7 +11,7 @@ No `PLANS.md` exists in the repository root at the time of writing. Users should see clear, descriptive CLI help and intuitive command feedback when running Netsuke. Configuration should be layered ergonomically via `ortho_config` so defaults, config files, environment variables, and CLI flags -behave consistently. Localised help strings should be used for CLI usage and +behave consistently. Localized help strings should be used for CLI usage and error output. Success is observable by running `netsuke --help`, subcommand help, and core commands (build, clean, graph, manifest) and seeing clear messages, plus passing the new unit and behavioural tests that assert these @@ -23,7 +23,7 @@ outputs and configuration precedence. context. - [ ] Inventory current CLI output and help messages (help text, errors, subcommand feedback) and record gaps. -- [x] Implement OrthoConfig-backed CLI configuration and localised help +- [x] Implement OrthoConfig-backed CLI configuration and localized help plumbing. - [x] Refine user-facing CLI output and update docs. - [x] Add unit tests and rstest-bdd behavioural tests for happy/unhappy paths. @@ -42,15 +42,15 @@ outputs and configuration precedence. absent. Rationale: Preserves deterministic precedence and avoids masking file/env values with clap defaults. Date/Author: 2026-01-02 (Codex) -- Decision: Provide Fluent-backed localisation with English defaults and a +- Decision: Provide Fluent-backed localization with English defaults and a Spanish catalogue layered as a consumer resource. Rationale: Validates - localised help/error support while keeping fallback behaviour intact. + localized help/error support while keeping fallback behaviour intact. Date/Author: 2026-01-02 (Codex) ## Outcomes & Retrospective - Outcome: Complete. - Notes: OrthoConfig configuration layering, localisation resources (including + Notes: OrthoConfig configuration layering, localization resources (including Spanish), updated CLI output, and tests are in place; `make check-fmt`, `make lint`, and `make test` now pass. @@ -71,7 +71,7 @@ Key runtime entry points and CLI definitions live in these files: OrthoConfig is wired in. The user guide for it is `docs/ortho-config-users-guide.md`, which explains configuration layering, -localised help via Fluent, and error localisation helpers. Design expectations +localized help via Fluent, and error localization helpers. Design expectations for CLI behaviour are in `docs/netsuke-design.md` and the roadmap entry in `docs/roadmap.md` (Phase 3 → “CLI and Feature Completeness”). @@ -106,11 +106,11 @@ development (BDD) lives in: not incorrectly override config layers (use `cli_default_as_absent` where needed). -3. Localise CLI help and clap errors. Create Fluent resources (for example +3. Localize CLI help and clap errors. Create Fluent resources (for example `locales/en-US/messages.ftl` and a CLI-specific bundle) and wire a `FluentLocalizer` into CLI parsing. Use `command().localize(&localizer)` before parsing and `localize_clap_error_with_command` on errors. Ensure the - fallback path preserves stock clap output when localisation fails. + fallback path preserves stock clap output when localization fails. 4. Refine CLI output messages. Update docstrings and help text in `src/cli.rs` (or the new config module) to be plain language and @@ -125,7 +125,7 @@ development (BDD) lives in: - OrthoConfig precedence (defaults < file < env < CLI), using `MergeComposer` or `compose_layers` to avoid touching process state. - - Localised help contains expected copy, and clap errors are localised when + - Localized help contains expected copy, and clap errors are localized when possible. - Unhappy paths (invalid schemes, invalid jobs count, missing required values) return the correct error kinds and messages. @@ -137,7 +137,7 @@ development (BDD) lives in: unhappy path for help output and configuration layering. 6. Documentation updates. Update `docs/users-guide.md` to explain the new - configuration layering, localised help, and any changes to CLI output. + configuration layering, localized help, and any changes to CLI output. Record design decisions in `docs/netsuke-design.md`. Mark the “Refine all CLI output…” item as done in `docs/roadmap.md` once tests pass. @@ -155,7 +155,7 @@ All commands are run from the repository root (`/root/repo`). Use `tee` with cargo run -- graph --help 2>&1 | tee /tmp/netsuke-graph-help.log cargo run -- manifest --help 2>&1 | tee /tmp/netsuke-manifest-help.log -2. Implement OrthoConfig integration and localisation. Update or add the +2. Implement OrthoConfig integration and localization. Update or add the relevant files and ensure new Fluent resources exist. 3. Update tests and docs. @@ -176,10 +176,10 @@ All commands are run from the repository root (`/root/repo`). Use `tee` with ## Validation and Acceptance -- Running `netsuke --help` (via `cargo run -- --help`) prints localised, +- Running `netsuke --help` (via `cargo run -- --help`) prints localized, plain-language descriptions for every flag and subcommand. - Subcommand help (`build`, `clean`, `graph`, `manifest`) is descriptive and - matches the user guide. Error output for invalid CLI inputs is localised when + matches the user guide. Error output for invalid CLI inputs is localized when a translation exists. - Configuration precedence is verified by unit tests: defaults < config file < environment variables < CLI overrides. @@ -191,9 +191,9 @@ All commands are run from the repository root (`/root/repo`). Use `tee` with ## Idempotence and Recovery -- OrthoConfig and localisation changes are safe to re-run; regenerate +- OrthoConfig and localization changes are safe to re-run; regenerate configuration layers and help output as often as needed. -- If localisation setup fails, fall back to default clap strings and record +- If localization setup fails, fall back to default clap strings and record the failure in `Surprises & Discoveries` with the error output. - If tests fail mid-run, fix the underlying issue and re-run the same command with the same log path, overwriting the log file to keep evidence current. @@ -202,9 +202,9 @@ All commands are run from the repository root (`/root/repo`). Use `tee` with Keep the following short transcripts for evidence: -- `netsuke --help` output with localised strings (from +- `netsuke --help` output with localized strings (from `/tmp/netsuke-help.log`). -- A failing CLI invocation showing localised error output (record the command +- A failing CLI invocation showing localized error output (record the command and a short excerpt of stderr). - Test summaries from `/tmp/netsuke-test.log` showing the new unit and BDD tests passing. @@ -222,7 +222,7 @@ Keep the following short transcripts for evidence: - Use `FluentLocalizer` from `ortho_config` and wire it into clap command creation. Provide Fluent keys for usage, about, flag help, and `clap-error-` messages. -- Ensure CLI parsing in `src/main.rs` uses the localiser and merges config +- Ensure CLI parsing in `src/main.rs` uses the localizer and merges config layers before calling `runner::run`. - If a helper module is introduced (for example `src/cli_output.rs`), keep it small, with a single responsibility for formatting user-visible messages. @@ -230,4 +230,4 @@ Keep the following short transcripts for evidence: ## Revision note (required when editing an ExecPlan) - 2026-01-02: Initial ExecPlan created to cover OrthoConfig integration, - localised help, CLI output clarity, and test/documentation updates. + localized help, CLI output clarity, and test/documentation updates. diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index bb081831..0a7106da 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -2017,7 +2017,7 @@ 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 localised +prefix with `__` as a nesting separator. CLI help and clap errors are localized via Fluent resources; `--locale` or `NETSUKE_LOCALE` selects the locale, and English plus Spanish catalogues ship in `locales/`. CLI execution and dispatch live in `src/runner.rs`, keeping `main.rs` focused on parsing. Process diff --git a/docs/users-guide.md b/docs/users-guide.md index a4854767..d015c0a6 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -485,7 +485,7 @@ netsuke [OPTIONS] [COMMAND] [TARGETS...] - `-v, --verbose`: Enable verbose logging output. -- `--locale `: Localise CLI help and error messages (for example +- `--locale `: Localize CLI help and error messages (for example `en-US` or `es-ES`). ### Network Policy Options @@ -527,7 +527,7 @@ policy is configured by these global options: the generated `build.ninja`, outputting DOT to stdout (suitable for Graphviz). Future versions may support other formats like `--html`. -### Configuration and Localisation +### Configuration and Localization Netsuke layers configuration in this order, with later entries overriding earlier ones: defaults, configuration files, environment variables, and @@ -543,7 +543,7 @@ Environment variables use the `NETSUKE_` prefix (for example, `NETSUKE_JOBS=8`). Use `__` to separate nested keys when matching structured configuration. -Use `--locale ` or `NETSUKE_LOCALE` to select localised CLI copy and +Use `--locale ` or `NETSUKE_LOCALE` to select localized CLI copy and error messages. Spanish (`es-ES`) is included as a reference translation; unsupported locales fall back to English. diff --git a/src/cli.rs b/src/cli.rs index 283544b0..020764a6 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -142,6 +142,8 @@ pub struct Cli { pub fetch_default_deny: bool, /// 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)] @@ -304,14 +306,14 @@ pub fn locale_hint_from_args(args: &[OsString]) -> Option { hint } -/// Parse CLI arguments with localised clap output. +/// 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 localisation applied when parsing fails. +/// Returns a `clap::Error` with localization applied when parsing fails. pub fn parse_with_localizer_from( iter: I, localizer: &dyn Localizer, @@ -331,10 +333,12 @@ where Ok((cli, matches)) } +/// 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 { @@ -343,6 +347,7 @@ fn config_discovery(directory: Option<&PathBuf>) -> ConfigDiscovery { builder.build() } +/// Return `true` when no CLI overrides were supplied. fn is_empty_value(value: &serde_json::Value) -> bool { matches!(value, serde_json::Value::Object(map) if map.is_empty()) } diff --git a/src/cli_localization.rs b/src/cli_localization.rs index fae6f58a..608d3591 100644 --- a/src/cli_localization.rs +++ b/src/cli_localization.rs @@ -1,7 +1,7 @@ //! Locale-aware helpers for CLI messaging. //! //! Provides Fluent-backed localizers with an English fallback and -//! consumer-provided Spanish translations to validate localisation support. +//! consumer-provided Spanish translations to validate localization support. use ortho_config::LanguageIdentifier; use ortho_config::{FluentLocalizer, FluentLocalizerBuilder, Localizer, NoOpLocalizer}; @@ -38,10 +38,13 @@ fn parse_locale_identifier(locale: &str) -> Option { } fn build_en_localizer() -> Box { - FluentLocalizer::with_en_us_defaults([NETSUKE_EN_US]).map_or_else( - |_| Box::new(NoOpLocalizer::new()) as Box, - |localizer| Box::new(localizer) as Box, - ) + match FluentLocalizer::with_en_us_defaults([NETSUKE_EN_US]) { + Ok(localizer) => Box::new(localizer) as Box, + Err(err) => { + tracing::warn!(error = %err, "failed to load default localization resources"); + Box::new(NoOpLocalizer::new()) as Box + } + } } fn build_consumer_localizer( diff --git a/src/host_pattern.rs b/src/host_pattern.rs index 1e5c1fbb..ac4820d8 100644 --- a/src/host_pattern.rs +++ b/src/host_pattern.rs @@ -7,34 +7,53 @@ use serde::{Deserialize, Serialize}; use std::str::FromStr; use thiserror::Error; +#[derive(Copy, Clone)] +struct HostPatternInput<'a>(&'a str); + +impl<'a> HostPatternInput<'a> { + const fn as_str(self) -> &'a str { + self.0 + } +} + +#[derive(Copy, Clone)] +pub(crate) struct HostCandidate<'a>(pub(crate) &'a str); + +impl<'a> HostCandidate<'a> { + const fn as_str(self) -> &'a str { + self.0 + } +} + struct ValidationContext<'a> { - original: &'a str, + original: HostPatternInput<'a>, } impl<'a> ValidationContext<'a> { - const fn new(original: &'a str) -> Self { + const fn new(original: HostPatternInput<'a>) -> Self { Self { original } } fn validate_label(&self, label: &str) -> Result<(), HostPatternError> { + let original = self.original.as_str(); if label.is_empty() { return Err(HostPatternError::EmptyLabel { - pattern: self.original.to_owned(), + pattern: original.to_owned(), }); } if !label.chars().all(|c| c.is_ascii_alphanumeric() || c == '-') { return Err(HostPatternError::InvalidCharacters { - pattern: self.original.to_owned(), + pattern: original.to_owned(), }); } if label.starts_with('-') || label.ends_with('-') { return Err(HostPatternError::InvalidLabelEdge { - pattern: self.original.to_owned(), + pattern: original.to_owned(), }); } if label.len() > 63 { return Err(HostPatternError::LabelTooLong { - pattern: self.original.to_owned(), + pattern: original.to_owned(), }); } Ok(()) @@ -97,8 +116,8 @@ pub enum HostPatternError { }, } -pub(crate) fn normalise_host_pattern(pattern: &str) -> Result<(String, bool), HostPatternError> { - let trimmed = pattern.trim(); +fn normalise_host_pattern(input: HostPatternInput<'_>) -> Result<(String, bool), HostPatternError> { + let trimmed = input.as_str().trim(); if trimmed.is_empty() { return Err(HostPatternError::Empty); } @@ -126,7 +145,7 @@ pub(crate) fn normalise_host_pattern(pattern: &str) -> Result<(String, bool), Ho let normalised = host_body.to_ascii_lowercase(); let mut total_len = 0usize; - let ctx = ValidationContext::new(trimmed); + let ctx = ValidationContext::new(HostPatternInput(trimmed)); for (index, label) in normalised.split('.').enumerate() { ctx.validate_label(label)?; total_len += label.len() + usize::from(index > 0); @@ -155,15 +174,15 @@ impl HostPattern { /// Returns an error when the pattern is empty, includes invalid /// characters, or uses a wildcard without a suffix. pub fn parse(pattern: &str) -> Result { - let (normalised, wildcard) = normalise_host_pattern(pattern)?; + let (normalised, wildcard) = normalise_host_pattern(HostPatternInput(pattern))?; Ok(Self { pattern: normalised, wildcard, }) } - pub(crate) fn matches(&self, candidate: &str) -> bool { - let host = candidate.to_ascii_lowercase(); + pub(crate) fn matches(&self, candidate: HostCandidate<'_>) -> bool { + let host = candidate.as_str().to_ascii_lowercase(); if self.wildcard { // Wildcard patterns match only subdomains, not the apex domain. // Example: "*.example.com" matches "sub.example.com" but not @@ -206,12 +225,12 @@ impl Serialize for HostPattern { where S: serde::Serializer, { - let text = if self.wildcard { - format!("*.{}", self.pattern) + if self.wildcard { + let text = format!("*.{}", self.pattern); + serializer.serialize_str(&text) } else { - self.pattern.clone() - }; - serializer.serialize_str(&text) + serializer.serialize_str(&self.pattern) + } } } @@ -261,7 +280,7 @@ mod tests { ) -> Result<()> { let parsed = HostPattern::parse(pattern)?; ensure!( - parsed.matches(host) == expected, + parsed.matches(HostCandidate(host)) == expected, "expected match={expected} for {host} against {pattern}", ); Ok(()) diff --git a/src/lib.rs b/src/lib.rs index f9297c9c..434b32de 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3,9 +3,6 @@ //! This library provides the command line interface definitions and //! helper functions for parsing `Netsukefile` manifests. -#[cfg(not(feature = "serde_json"))] -compile_error!("The `serde_json` feature is required to build Netsuke."); - pub mod ast; pub mod cli; pub mod cli_localization; diff --git a/src/stdlib/network/policy/mod.rs b/src/stdlib/network/policy/mod.rs index fb8fb3ec..bdb5727c 100644 --- a/src/stdlib/network/policy/mod.rs +++ b/src/stdlib/network/policy/mod.rs @@ -8,7 +8,7 @@ use std::collections::BTreeSet; use thiserror::Error; use url::Url; -use crate::host_pattern::{HostPattern, HostPatternError}; +use crate::host_pattern::{HostCandidate, HostPattern, HostPatternError}; /// Declarative allow- and deny-list policy for outbound network requests. /// @@ -298,18 +298,18 @@ impl NetworkPolicy { if self .blocked_hosts .iter() - .any(|pattern| pattern.matches(host)) + .any(|pattern| pattern.matches(HostCandidate(host))) { return Err(NetworkPolicyViolation::HostBlocked { host: host.to_owned(), }); } - if self - .allowed_hosts - .as_ref() - .is_some_and(|allowlist| !allowlist.iter().any(|pattern| pattern.matches(host))) - { + if self.allowed_hosts.as_ref().is_some_and(|allowlist| { + !allowlist + .iter() + .any(|pattern| pattern.matches(HostCandidate(host))) + }) { return Err(NetworkPolicyViolation::HostNotAllowlisted { host: host.to_owned(), }); diff --git a/tests/cli_tests.rs b/tests/cli_tests.rs deleted file mode 100644 index ef3232ee..00000000 --- a/tests/cli_tests.rs +++ /dev/null @@ -1,502 +0,0 @@ -//! Unit tests for CLI argument parsing and validation. -//! -//! This module exercises the command-line interface defined in [`netsuke::cli`] -//! using `rstest` for parameterised coverage of success and error scenarios. - -use anyhow::{Context, Result, bail, ensure}; -use clap::error::ErrorKind; -use clap::{CommandFactory, FromArgMatches, Parser}; -use netsuke::cli::{BuildArgs, Cli, Commands, locale_hint_from_args}; -use netsuke::cli_localization; -use netsuke::host_pattern::HostPattern; -use netsuke::stdlib::NetworkPolicyViolation; -use ortho_config::{CliValueExtractor, MergeComposer, sanitize_value}; -use rstest::rstest; -use serde_json::json; -use std::ffi::{OsStr, OsString}; -use std::fs; -use std::path::{Path, PathBuf}; -use tempfile::tempdir; -use url::Url; - -struct EnvGuard { - key: &'static str, - previous: Option, -} - -impl EnvGuard { - fn set(key: &'static str, value: impl AsRef) -> Self { - let previous = std::env::var_os(key); - // SAFETY: Tests run in a single process and the guard restores values. - unsafe { std::env::set_var(key, value.as_ref()) }; - Self { key, previous } - } - - fn unset(key: &'static str) -> Self { - let previous = std::env::var_os(key); - // SAFETY: Tests run in a single process and the guard restores values. - unsafe { std::env::remove_var(key) }; - Self { key, previous } - } -} - -impl Drop for EnvGuard { - fn drop(&mut self) { - if let Some(previous) = self.previous.take() { - // SAFETY: Restores the previous environment value for the test. - unsafe { std::env::set_var(self.key, previous) }; - } else { - // SAFETY: Restores the previous environment state for the test. - unsafe { std::env::remove_var(self.key) }; - } - } -} - -struct CliCase { - argv: Vec<&'static str>, - file: PathBuf, - directory: Option, - jobs: Option, - verbose: bool, - locale: Option<&'static str>, - allow_scheme: Vec, - allow_host: Vec<&'static str>, - block_host: Vec<&'static str>, - default_deny: bool, - expected_cmd: Commands, -} - -impl Default for CliCase { - fn default() -> Self { - Self { - argv: Vec::new(), - file: PathBuf::from("Netsukefile"), - directory: None, - jobs: None, - verbose: false, - locale: None, - allow_scheme: Vec::new(), - allow_host: Vec::new(), - block_host: Vec::new(), - default_deny: false, - expected_cmd: Commands::Build(BuildArgs { - emit: None, - targets: Vec::new(), - }), - } - } -} - -#[rstest] -#[case(CliCase { argv: vec!["netsuke"], ..CliCase::default() })] -#[case(CliCase { - argv: vec!["netsuke", "--file", "alt.yml", "-C", "work", "-j", "4", "build", "a", "b"], - file: PathBuf::from("alt.yml"), - directory: Some(PathBuf::from("work")), - jobs: Some(4), - expected_cmd: Commands::Build(BuildArgs { - emit: None, - targets: vec!["a".into(), "b".into()], - }), - ..CliCase::default() -})] -#[case(CliCase { - argv: vec!["netsuke", "--verbose"], - verbose: true, - ..CliCase::default() -})] -#[case(CliCase { - argv: vec!["netsuke", "--locale", "es-ES"], - locale: Some("es-ES"), - ..CliCase::default() -})] -#[case(CliCase { - argv: vec!["netsuke", "build", "--emit", "out.ninja", "a"], - expected_cmd: Commands::Build(BuildArgs { - emit: Some(PathBuf::from("out.ninja")), - targets: vec!["a".into()], - }), - ..CliCase::default() -})] -#[case(CliCase { - argv: vec!["netsuke", "manifest", "out.ninja"], - expected_cmd: Commands::Manifest { - file: PathBuf::from("out.ninja"), - }, - ..CliCase::default() -})] -#[case(CliCase { - argv: vec!["netsuke", "manifest", "-"], - expected_cmd: Commands::Manifest { - file: PathBuf::from("-"), - }, - ..CliCase::default() -})] -#[case(CliCase { - argv: vec![ - "netsuke", - "--fetch-allow-scheme", - "http", - "--fetch-default-deny", - "--fetch-allow-host", - "example.com", - "--fetch-block-host", - "deny.test", - ], - allow_scheme: vec![String::from("http")], - allow_host: vec!["example.com"], - block_host: vec!["deny.test"], - default_deny: true, - ..CliCase::default() -})] -fn parse_cli(#[case] case: CliCase) -> Result<()> { - let cli = Cli::try_parse_from(case.argv.clone()) - .context("parse CLI arguments")? - .with_default_command(); - ensure!(cli.file == case.file, "parsed file should match input"); - ensure!( - cli.directory == case.directory, - "parsed directory should match input", - ); - ensure!(cli.jobs == case.jobs, "parsed jobs should match input"); - ensure!( - cli.verbose == case.verbose, - "verbose flag should match input", - ); - ensure!( - cli.locale.as_deref() == case.locale, - "locale should match input", - ); - ensure!( - cli.fetch_allow_scheme == case.allow_scheme, - "allow-scheme flags should match input", - ); - let expected_allow_host = case - .allow_host - .iter() - .map(|pattern| { - HostPattern::parse(pattern) - .with_context(|| format!("parse expected allow host '{pattern}'")) - }) - .collect::>>()?; - ensure!( - cli.fetch_allow_host == expected_allow_host, - "allow-host flags should match input", - ); - let expected_block_host = case - .block_host - .iter() - .map(|pattern| { - HostPattern::parse(pattern) - .with_context(|| format!("parse expected block host '{pattern}'")) - }) - .collect::>>()?; - ensure!( - cli.fetch_block_host == expected_block_host, - "block-host flags should match input", - ); - ensure!( - cli.fetch_default_deny == case.default_deny, - "default-deny flag should match input", - ); - let command = cli.command.context("command should be set")?; - ensure!( - command == case.expected_cmd, - "parsed command should match expected {:?}", - case.expected_cmd - ); - Ok(()) -} - -#[rstest] -fn cli_network_policy_defaults_to_https() -> Result<()> { - let cli = Cli::default(); - let policy = cli.network_policy()?; - let https = Url::parse("https://example.com").expect("parse https URL"); - let http = Url::parse("http://example.com").expect("parse http URL"); - ensure!( - policy.evaluate(&https).is_ok(), - "HTTPS should be permitted by default", - ); - let err = policy - .evaluate(&http) - .expect_err("HTTP should be rejected by default"); - match err { - NetworkPolicyViolation::SchemeNotAllowed { scheme } => { - ensure!(scheme == "http", "unexpected scheme {scheme}"); - } - other => bail!("expected scheme violation, got {other:?}"), - } - Ok(()) -} - -#[rstest] -fn cli_network_policy_default_deny_blocks_unknown_hosts() -> Result<()> { - let mut cli = Cli { - fetch_default_deny: true, - ..Cli::default() - }; - cli.fetch_allow_host - .push(HostPattern::parse("example.com").context("parse allow host pattern")?); - let policy = cli.network_policy()?; - let allowed = Url::parse("https://example.com").expect("parse allowed URL"); - let denied = Url::parse("https://unauthorised.test").expect("parse denied URL"); - ensure!( - policy.evaluate(&allowed).is_ok(), - "explicit allowlist should permit matching host", - ); - let err = policy - .evaluate(&denied) - .expect_err("default deny should block other hosts"); - match err { - NetworkPolicyViolation::HostNotAllowlisted { host } => { - ensure!(host == "unauthorised.test", "unexpected host {host}"); - } - other => bail!("expected allowlist violation, got {other:?}"), - } - Ok(()) -} - -#[rstest] -fn cli_network_policy_blocklist_overrides_allowlist() -> Result<()> { - let mut cli = Cli::default(); - cli.fetch_allow_host - .push(HostPattern::parse("example.com").context("parse allow host pattern")?); - cli.fetch_block_host - .push(HostPattern::parse("example.com").context("parse block host pattern")?); - let policy = cli.network_policy()?; - let url = Url::parse("https://example.com").expect("parse conflicting URL"); - let err = policy - .evaluate(&url) - .expect_err("blocklist should override allowlist"); - let err_text = err.to_string(); - match err { - NetworkPolicyViolation::HostBlocked { host } => { - ensure!(host == "example.com", "unexpected host {host}"); - ensure!( - err_text == "host 'example.com' is blocked", - "unexpected error text: {err_text}", - ); - } - other => bail!("expected blocklist violation, got {other:?}"), - } - Ok(()) -} - -#[rstest] -fn cli_network_policy_rejects_invalid_scheme() { - let mut cli = Cli::default(); - cli.fetch_allow_scheme.push(String::from("1http")); - let err = cli - .network_policy() - .expect_err("invalid scheme should be rejected"); - assert!( - err.to_string().contains("invalid characters"), - "unexpected error text: {err}", - ); -} - -#[rstest] -#[case(vec!["netsuke", "unknowncmd"], ErrorKind::InvalidSubcommand)] -#[case(vec!["netsuke", "--file"], ErrorKind::InvalidValue)] -#[case( - vec!["netsuke", "--fetch-allow-host", "bad host"], - ErrorKind::ValueValidation -)] -#[case(vec!["netsuke", "-j", "notanumber"], ErrorKind::ValueValidation)] -#[case(vec!["netsuke", "--file", "alt.yml", "-C"], ErrorKind::InvalidValue)] -#[case(vec!["netsuke", "manifest"], ErrorKind::MissingRequiredArgument)] -#[case(vec!["netsuke", "--locale", "nope"], ErrorKind::ValueValidation)] -fn parse_cli_errors(#[case] argv: Vec<&str>, #[case] expected_error: ErrorKind) -> Result<()> { - let err = Cli::try_parse_from(argv) - .err() - .context("parser should reject invalid arguments")?; - ensure!( - err.kind() == expected_error, - "expected error kind {:?}, got {:?}", - expected_error, - err.kind() - ); - Ok(()) -} - -fn os_args(args: &[&str]) -> Vec { - args.iter().map(|arg| OsString::from(*arg)).collect() -} - -#[rstest] -fn locale_hint_from_args_accepts_space_form() -> Result<()> { - let args = os_args(&["netsuke", "--locale", "es-ES"]); - let hint = locale_hint_from_args(&args); - ensure!( - hint.as_deref() == Some("es-ES"), - "expected Some(\"es-ES\"), got: {hint:?}" - ); - Ok(()) -} - -#[rstest] -fn locale_hint_from_args_accepts_equals_form() -> Result<()> { - let args = os_args(&["netsuke", "--locale=es-ES"]); - let hint = locale_hint_from_args(&args); - ensure!( - hint.as_deref() == Some("es-ES"), - "expected Some(\"es-ES\"), got: {hint:?}" - ); - Ok(()) -} - -#[rstest] -fn locale_hint_from_args_trailing_locale_flag_yields_none() -> Result<()> { - let args = os_args(&["netsuke", "--locale"]); - let hint = locale_hint_from_args(&args); - ensure!( - hint.is_none(), - "expected None for trailing --locale without value, got: {hint:?}" - ); - Ok(()) -} - -#[rstest] -fn locale_hint_from_args_ignores_args_after_double_dash() -> Result<()> { - let args = os_args(&["netsuke", "--verbose", "--", "--locale", "es-ES"]); - let hint = locale_hint_from_args(&args); - ensure!( - hint.is_none(), - "expected None when --locale appears after \"--\", got: {hint:?}" - ); - Ok(()) -} - -#[rstest] -fn locale_hint_from_args_uses_last_locale_flag() -> Result<()> { - let args = os_args(&["netsuke", "--locale", "es-ES", "--locale", "en-US"]); - let hint = locale_hint_from_args(&args); - ensure!( - hint.as_deref() == Some("en-US"), - "expected last --locale to win (\"en-US\"), got: {hint:?}" - ); - Ok(()) -} - -#[rstest] -fn cli_extract_user_provided_omits_defaults() -> Result<()> { - let mut matches = Cli::command() - .try_get_matches_from(["netsuke"]) - .context("parse matches for default CLI")?; - let cli = Cli::from_arg_matches_mut(&mut matches).context("build CLI from matches")?; - let value = cli - .extract_user_provided(&matches) - .context("extract CLI overrides")?; - let object = value - .as_object() - .context("expected extracted CLI value to be an object")?; - ensure!( - !object.contains_key("file"), - "default file should not be treated as a CLI override", - ); - ensure!( - !object.contains_key("verbose"), - "default verbose flag should not be treated as a CLI override", - ); - ensure!( - !object.contains_key("fetch_default_deny"), - "default deny flag should not be treated as a CLI override", - ); - Ok(()) -} - -#[rstest] -fn cli_merge_layers_respects_precedence_and_appends_lists() -> Result<()> { - let mut composer = MergeComposer::new(); - let mut defaults = sanitize_value(&Cli::default())?; - let defaults_object = defaults - .as_object_mut() - .context("defaults should be an object")?; - defaults_object.insert("jobs".to_owned(), json!(1)); - defaults_object.insert("fetch_allow_scheme".to_owned(), json!(["https"])); - composer.push_defaults(defaults); - composer.push_file( - json!({ - "file": "Configfile", - "jobs": 2, - "fetch_allow_scheme": ["http"], - "locale": "en-US" - }), - None, - ); - composer.push_environment(json!({ - "jobs": 3, - "fetch_allow_scheme": ["ftp"] - })); - composer.push_cli(json!({ - "jobs": 4, - "fetch_allow_scheme": ["git"], - "verbose": true - })); - let merged = Cli::merge_from_layers(composer.layers())?; - ensure!( - merged.file.as_path() == Path::new("Configfile"), - "file layer should override defaults", - ); - ensure!(merged.jobs == Some(4), "CLI layer should override jobs",); - ensure!( - merged.fetch_allow_scheme == vec!["https", "http", "ftp", "git"], - "list values should append in layer order", - ); - ensure!( - merged.locale.as_deref() == Some("en-US"), - "file layer should populate locale when CLI does not override", - ); - ensure!(merged.verbose, "CLI layer should set verbose"); - Ok(()) -} - -#[rstest] -fn cli_merge_with_config_respects_precedence_and_skips_empty_cli_layer() -> Result<()> { - let temp_dir = tempdir().context("create temporary config directory")?; - let config_path = temp_dir.path().join("netsuke.toml"); - let config = r#" -jobs = 2 -fetch_allow_scheme = ["https"] -"#; - fs::write(&config_path, config).context("write netsuke.toml")?; - - let _config_guard = EnvGuard::set("NETSUKE_CONFIG_PATH", config_path.as_os_str()); - let _jobs_guard = EnvGuard::set("NETSUKE_JOBS", "4"); - let _scheme_guard = EnvGuard::unset("NETSUKE_FETCH_ALLOW_SCHEME"); - - let localizer = cli_localization::build_localizer(None); - let (cli, matches) = netsuke::cli::parse_with_localizer_from(["netsuke"], localizer.as_ref()) - .context("parse CLI args for merge")?; - let merged = netsuke::cli::merge_with_config(&cli, &matches) - .context("merge CLI and configuration layers")? - .with_default_command(); - - ensure!( - merged.jobs == Some(4), - "environment variables should override config when CLI has no value", - ); - ensure!( - merged.fetch_allow_scheme == vec!["https".to_owned()], - "config values should apply when CLI overrides are empty", - ); - - Ok(()) -} - -#[rstest] -fn cli_localises_invalid_subcommand_in_spanish() -> Result<()> { - let localizer = cli_localization::build_localizer(Some("es-ES")); - let err = netsuke::cli::parse_with_localizer_from( - ["netsuke", "--locale", "es-ES", "unknown"], - localizer.as_ref(), - ) - .err() - .context("parser should reject invalid subcommand")?; - ensure!( - err.to_string().contains("Subcomando desconocido"), - "expected Spanish localisation, got: {err}", - ); - Ok(()) -} diff --git a/tests/cli_tests/helpers.rs b/tests/cli_tests/helpers.rs new file mode 100644 index 00000000..7c5381b4 --- /dev/null +++ b/tests/cli_tests/helpers.rs @@ -0,0 +1,7 @@ +//! Shared helpers for CLI tests. + +use std::ffi::OsString; + +pub(super) fn os_args(args: &[&str]) -> Vec { + args.iter().map(|arg| OsString::from(*arg)).collect() +} diff --git a/tests/cli_tests/locale.rs b/tests/cli_tests/locale.rs new file mode 100644 index 00000000..98b2ced8 --- /dev/null +++ b/tests/cli_tests/locale.rs @@ -0,0 +1,79 @@ +//! Locale-specific CLI tests. + +use anyhow::{Context, Result, ensure}; +use rstest::rstest; + +use crate::helpers::os_args; +use netsuke::cli::locale_hint_from_args; +use netsuke::cli_localization; + +#[rstest] +fn locale_hint_from_args_accepts_space_form() -> Result<()> { + let args = os_args(&["netsuke", "--locale", "es-ES"]); + let hint = locale_hint_from_args(&args); + ensure!( + hint.as_deref() == Some("es-ES"), + "expected Some(\"es-ES\"), got: {hint:?}" + ); + Ok(()) +} + +#[rstest] +fn locale_hint_from_args_accepts_equals_form() -> Result<()> { + let args = os_args(&["netsuke", "--locale=es-ES"]); + let hint = locale_hint_from_args(&args); + ensure!( + hint.as_deref() == Some("es-ES"), + "expected Some(\"es-ES\"), got: {hint:?}" + ); + Ok(()) +} + +#[rstest] +fn locale_hint_from_args_trailing_locale_flag_yields_none() -> Result<()> { + let args = os_args(&["netsuke", "--locale"]); + let hint = locale_hint_from_args(&args); + ensure!( + hint.is_none(), + "expected None for trailing --locale without value, got: {hint:?}" + ); + Ok(()) +} + +#[rstest] +fn locale_hint_from_args_ignores_args_after_double_dash() -> Result<()> { + let args = os_args(&["netsuke", "--verbose", "--", "--locale", "es-ES"]); + let hint = locale_hint_from_args(&args); + ensure!( + hint.is_none(), + "expected None when --locale appears after \"--\", got: {hint:?}" + ); + Ok(()) +} + +#[rstest] +fn locale_hint_from_args_uses_last_locale_flag() -> Result<()> { + let args = os_args(&["netsuke", "--locale", "es-ES", "--locale", "en-US"]); + let hint = locale_hint_from_args(&args); + ensure!( + hint.as_deref() == Some("en-US"), + "expected last --locale to win (\"en-US\"), got: {hint:?}" + ); + Ok(()) +} + +#[rstest] +fn cli_localises_invalid_subcommand_in_spanish() -> Result<()> { + let localizer = cli_localization::build_localizer(Some("es-ES")); + let err = netsuke::cli::parse_with_localizer_from( + ["netsuke", "--locale", "es-ES", "unknown"], + localizer.as_ref(), + ) + .err() + .context("parser should reject invalid subcommand")?; + ensure!( + err.to_string().contains("Subcomando desconocido"), + "expected Spanish localization, got: {err}", + ); + Ok(()) +} diff --git a/tests/cli_tests/merge.rs b/tests/cli_tests/merge.rs new file mode 100644 index 00000000..c2b29352 --- /dev/null +++ b/tests/cli_tests/merge.rs @@ -0,0 +1,121 @@ +//! Configuration merge tests. + +use anyhow::{Context, Result, ensure}; +use clap::{CommandFactory, FromArgMatches}; +use netsuke::cli::Cli; +use netsuke::cli_localization; +use ortho_config::{CliValueExtractor, MergeComposer, sanitize_value}; +use rstest::rstest; +use serde_json::json; +use std::ffi::OsStr; +use std::fs; +use std::path::Path; +use tempfile::tempdir; +use test_support::{EnvVarGuard, env_lock::EnvLock}; + +#[rstest] +fn cli_extract_user_provided_omits_defaults() -> Result<()> { + let mut matches = Cli::command() + .try_get_matches_from(["netsuke"]) + .context("parse matches for default CLI")?; + let cli = Cli::from_arg_matches_mut(&mut matches).context("build CLI from matches")?; + let value = cli + .extract_user_provided(&matches) + .context("extract CLI overrides")?; + let object = value + .as_object() + .context("expected extracted CLI value to be an object")?; + ensure!( + !object.contains_key("file"), + "default file should not be treated as a CLI override", + ); + ensure!( + !object.contains_key("verbose"), + "default verbose flag should not be treated as a CLI override", + ); + ensure!( + !object.contains_key("fetch_default_deny"), + "default deny flag should not be treated as a CLI override", + ); + Ok(()) +} + +#[rstest] +fn cli_merge_layers_respects_precedence_and_appends_lists() -> Result<()> { + let mut composer = MergeComposer::new(); + let mut defaults = sanitize_value(&Cli::default())?; + let defaults_object = defaults + .as_object_mut() + .context("defaults should be an object")?; + defaults_object.insert("jobs".to_owned(), json!(1)); + defaults_object.insert("fetch_allow_scheme".to_owned(), json!(["https"])); + composer.push_defaults(defaults); + composer.push_file( + json!({ + "file": "Configfile", + "jobs": 2, + "fetch_allow_scheme": ["http"], + "locale": "en-US" + }), + None, + ); + composer.push_environment(json!({ + "jobs": 3, + "fetch_allow_scheme": ["ftp"] + })); + composer.push_cli(json!({ + "jobs": 4, + "fetch_allow_scheme": ["git"], + "verbose": true + })); + let merged = Cli::merge_from_layers(composer.layers())?; + ensure!( + merged.file.as_path() == Path::new("Configfile"), + "file layer should override defaults", + ); + ensure!(merged.jobs == Some(4), "CLI layer should override jobs"); + ensure!( + merged.fetch_allow_scheme == vec!["https", "http", "ftp", "git"], + "list values should append in layer order", + ); + ensure!( + merged.locale.as_deref() == Some("en-US"), + "file layer should populate locale when CLI does not override", + ); + ensure!(merged.verbose, "CLI layer should set verbose"); + Ok(()) +} + +#[rstest] +fn cli_merge_with_config_respects_precedence_and_skips_empty_cli_layer() -> Result<()> { + let _env_lock = EnvLock::acquire(); + let temp_dir = tempdir().context("create temporary config directory")?; + let config_path = temp_dir.path().join("netsuke.toml"); + let config = r#" +jobs = 2 +fetch_allow_scheme = ["https"] +"#; + fs::write(&config_path, config).context("write netsuke.toml")?; + + let _config_guard = EnvVarGuard::set("NETSUKE_CONFIG_PATH", config_path.as_os_str()); + let _jobs_guard = EnvVarGuard::set("NETSUKE_JOBS", OsStr::new("4")); + let _scheme_guard = EnvVarGuard::remove("NETSUKE_FETCH_ALLOW_SCHEME"); + + let localizer = cli_localization::build_localizer(None); + let (cli, matches) = netsuke::cli::parse_with_localizer_from(["netsuke"], localizer.as_ref()) + .context("parse CLI args for merge")?; + let merged = netsuke::cli::merge_with_config(&cli, &matches) + .context("merge CLI and configuration layers")? + .with_default_command(); + + ensure!( + merged.jobs == Some(4), + "environment variables should override config when CLI has no value", + ); + ensure!( + merged.fetch_allow_scheme == vec!["https".to_owned()], + "config values should apply when CLI overrides are empty", + ); + + Ok(()) +} diff --git a/tests/cli_tests/mod.rs b/tests/cli_tests/mod.rs new file mode 100644 index 00000000..2a6205a2 --- /dev/null +++ b/tests/cli_tests/mod.rs @@ -0,0 +1,9 @@ +//! Unit tests for CLI argument parsing and validation. +//! +//! This crate exercises the command-line interface defined in `netsuke::cli`. + +mod helpers; +mod locale; +mod merge; +mod parsing; +mod policy; diff --git a/tests/cli_tests/parsing.rs b/tests/cli_tests/parsing.rs new file mode 100644 index 00000000..a58c0205 --- /dev/null +++ b/tests/cli_tests/parsing.rs @@ -0,0 +1,189 @@ +//! CLI parsing coverage. + +use anyhow::{Context, Result, ensure}; +use clap::Parser; +use clap::error::ErrorKind; +use netsuke::cli::{BuildArgs, Cli, Commands}; +use netsuke::host_pattern::HostPattern; +use rstest::rstest; +use std::path::PathBuf; + +struct CliCase { + argv: Vec<&'static str>, + file: PathBuf, + directory: Option, + jobs: Option, + verbose: bool, + locale: Option<&'static str>, + allow_scheme: Vec, + allow_host: Vec<&'static str>, + block_host: Vec<&'static str>, + default_deny: bool, + expected_cmd: Commands, +} + +impl Default for CliCase { + fn default() -> Self { + Self { + argv: Vec::new(), + file: PathBuf::from("Netsukefile"), + directory: None, + jobs: None, + verbose: false, + locale: None, + allow_scheme: Vec::new(), + allow_host: Vec::new(), + block_host: Vec::new(), + default_deny: false, + expected_cmd: Commands::Build(BuildArgs { + emit: None, + targets: Vec::new(), + }), + } + } +} + +#[rstest] +#[case(CliCase { argv: vec!["netsuke"], ..CliCase::default() })] +#[case(CliCase { + argv: vec!["netsuke", "--file", "alt.yml", "-C", "work", "-j", "4", "build", "a", "b"], + file: PathBuf::from("alt.yml"), + directory: Some(PathBuf::from("work")), + jobs: Some(4), + expected_cmd: Commands::Build(BuildArgs { + emit: None, + targets: vec!["a".into(), "b".into()], + }), + ..CliCase::default() +})] +#[case(CliCase { + argv: vec!["netsuke", "--verbose"], + verbose: true, + ..CliCase::default() +})] +#[case(CliCase { + argv: vec!["netsuke", "--locale", "es-ES"], + locale: Some("es-ES"), + ..CliCase::default() +})] +#[case(CliCase { + argv: vec!["netsuke", "build", "--emit", "out.ninja", "a"], + expected_cmd: Commands::Build(BuildArgs { + emit: Some(PathBuf::from("out.ninja")), + targets: vec!["a".into()], + }), + ..CliCase::default() +})] +#[case(CliCase { + argv: vec!["netsuke", "manifest", "out.ninja"], + expected_cmd: Commands::Manifest { + file: PathBuf::from("out.ninja"), + }, + ..CliCase::default() +})] +#[case(CliCase { + argv: vec!["netsuke", "manifest", "-"], + expected_cmd: Commands::Manifest { + file: PathBuf::from("-"), + }, + ..CliCase::default() +})] +#[case(CliCase { + argv: vec![ + "netsuke", + "--fetch-allow-scheme", + "http", + "--fetch-default-deny", + "--fetch-allow-host", + "example.com", + "--fetch-block-host", + "deny.test", + ], + allow_scheme: vec![String::from("http")], + allow_host: vec!["example.com"], + block_host: vec!["deny.test"], + default_deny: true, + ..CliCase::default() +})] +fn parse_cli(#[case] case: CliCase) -> Result<()> { + let cli = Cli::try_parse_from(case.argv.clone()) + .context("parse CLI arguments")? + .with_default_command(); + ensure!(cli.file == case.file, "parsed file should match input"); + ensure!( + cli.directory == case.directory, + "parsed directory should match input", + ); + ensure!(cli.jobs == case.jobs, "parsed jobs should match input"); + ensure!( + cli.verbose == case.verbose, + "verbose flag should match input", + ); + ensure!( + cli.locale.as_deref() == case.locale, + "locale should match input", + ); + ensure!( + cli.fetch_allow_scheme == case.allow_scheme, + "allow-scheme flags should match input", + ); + let expected_allow_host = case + .allow_host + .iter() + .map(|pattern| { + HostPattern::parse(pattern) + .with_context(|| format!("parse expected allow host '{pattern}'")) + }) + .collect::>>()?; + ensure!( + cli.fetch_allow_host == expected_allow_host, + "allow-host flags should match input", + ); + let expected_block_host = case + .block_host + .iter() + .map(|pattern| { + HostPattern::parse(pattern) + .with_context(|| format!("parse expected block host '{pattern}'")) + }) + .collect::>>()?; + ensure!( + cli.fetch_block_host == expected_block_host, + "block-host flags should match input", + ); + ensure!( + cli.fetch_default_deny == case.default_deny, + "default-deny flag should match input", + ); + let command = cli.command.context("command should be set")?; + ensure!( + command == case.expected_cmd, + "parsed command should match expected {:?}", + case.expected_cmd + ); + Ok(()) +} + +#[rstest] +#[case(vec!["netsuke", "unknowncmd"], ErrorKind::InvalidSubcommand)] +#[case(vec!["netsuke", "--file"], ErrorKind::InvalidValue)] +#[case( + vec!["netsuke", "--fetch-allow-host", "bad host"], + ErrorKind::ValueValidation +)] +#[case(vec!["netsuke", "-j", "notanumber"], ErrorKind::ValueValidation)] +#[case(vec!["netsuke", "--file", "alt.yml", "-C"], ErrorKind::InvalidValue)] +#[case(vec!["netsuke", "manifest"], ErrorKind::MissingRequiredArgument)] +#[case(vec!["netsuke", "--locale", "nope"], ErrorKind::ValueValidation)] +fn parse_cli_errors(#[case] argv: Vec<&str>, #[case] expected_error: ErrorKind) -> Result<()> { + let err = Cli::try_parse_from(argv) + .err() + .context("parser should reject invalid arguments")?; + ensure!( + err.kind() == expected_error, + "expected error kind {:?}, got {:?}", + expected_error, + err.kind() + ); + Ok(()) +} diff --git a/tests/cli_tests/policy.rs b/tests/cli_tests/policy.rs new file mode 100644 index 00000000..e81d6f81 --- /dev/null +++ b/tests/cli_tests/policy.rs @@ -0,0 +1,96 @@ +//! Network policy tests derived from CLI input. + +use anyhow::{Context, Result, bail, ensure}; +use netsuke::cli::Cli; +use netsuke::host_pattern::HostPattern; +use netsuke::stdlib::NetworkPolicyViolation; +use rstest::rstest; +use url::Url; + +#[rstest] +fn cli_network_policy_defaults_to_https() -> Result<()> { + let cli = Cli::default(); + let policy = cli.network_policy()?; + let https = Url::parse("https://example.com").expect("parse https URL"); + let http = Url::parse("http://example.com").expect("parse http URL"); + ensure!( + policy.evaluate(&https).is_ok(), + "HTTPS should be permitted by default", + ); + let err = policy + .evaluate(&http) + .expect_err("HTTP should be rejected by default"); + match err { + NetworkPolicyViolation::SchemeNotAllowed { scheme } => { + ensure!(scheme == "http", "unexpected scheme {scheme}"); + } + other => bail!("expected scheme violation, got {other:?}"), + } + Ok(()) +} + +#[rstest] +fn cli_network_policy_default_deny_blocks_unknown_hosts() -> Result<()> { + let mut cli = Cli { + fetch_default_deny: true, + ..Cli::default() + }; + cli.fetch_allow_host + .push(HostPattern::parse("example.com").context("parse allow host pattern")?); + let policy = cli.network_policy()?; + let allowed = Url::parse("https://example.com").expect("parse allowed URL"); + let denied = Url::parse("https://unauthorised.test").expect("parse denied URL"); + ensure!( + policy.evaluate(&allowed).is_ok(), + "explicit allowlist should permit matching host", + ); + let err = policy + .evaluate(&denied) + .expect_err("default deny should block other hosts"); + match err { + NetworkPolicyViolation::HostNotAllowlisted { host } => { + ensure!(host == "unauthorised.test", "unexpected host {host}"); + } + other => bail!("expected allowlist violation, got {other:?}"), + } + Ok(()) +} + +#[rstest] +fn cli_network_policy_blocklist_overrides_allowlist() -> Result<()> { + let mut cli = Cli::default(); + cli.fetch_allow_host + .push(HostPattern::parse("example.com").context("parse allow host pattern")?); + cli.fetch_block_host + .push(HostPattern::parse("example.com").context("parse block host pattern")?); + let policy = cli.network_policy()?; + let url = Url::parse("https://example.com").expect("parse conflicting URL"); + let err = policy + .evaluate(&url) + .expect_err("blocklist should override allowlist"); + let err_text = err.to_string(); + match err { + NetworkPolicyViolation::HostBlocked { host } => { + ensure!(host == "example.com", "unexpected host {host}"); + ensure!( + err_text == "host 'example.com' is blocked", + "unexpected error text: {err_text}", + ); + } + other => bail!("expected blocklist violation, got {other:?}"), + } + Ok(()) +} + +#[rstest] +fn cli_network_policy_rejects_invalid_scheme() { + let mut cli = Cli::default(); + cli.fetch_allow_scheme.push(String::from("1http")); + let err = cli + .network_policy() + .expect_err("invalid scheme should be rejected"); + assert!( + err.to_string().contains("invalid characters"), + "unexpected error text: {err}", + ); +} From 5a92741096d194e116999ed73218dc8b46f40b4c Mon Sep 17 00:00:00 2001 From: Leynos Date: Fri, 2 Jan 2026 22:47:53 +0000 Subject: [PATCH 6/9] refactor(cli): extract CLI localization logic into separate module - Moved localization helpers and related functions from `src/cli.rs` to new module `src/cli_l10n.rs`. - Updated `build.rs` and `src/lib.rs` to include the new module. - Simplified `cli.rs` by re-exporting and using functions from `cli_l10n`. This improves code organization by separating localization concerns from CLI definitions. Co-authored-by: terragon-labs[bot] --- build.rs | 3 ++ src/cli.rs | 91 +++-------------------------------------------- src/cli_l10n.rs | 93 +++++++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 1 + 4 files changed, 101 insertions(+), 87 deletions(-) create mode 100644 src/cli_l10n.rs diff --git a/build.rs b/build.rs index 76bd5569..284c3dac 100644 --- a/build.rs +++ b/build.rs @@ -15,6 +15,9 @@ const FALLBACK_DATE: &str = "1970-01-01"; #[path = "src/cli.rs"] mod cli; +#[path = "src/cli_l10n.rs"] +mod cli_l10n; + #[path = "src/host_pattern.rs"] mod host_pattern; diff --git a/src/cli.rs b/src/cli.rs index 020764a6..3b40f34b 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -10,14 +10,16 @@ use ortho_config::figment::{Figment, providers::Env}; use ortho_config::localize_clap_error_with_command; use ortho_config::uncased::Uncased; use ortho_config::{ - CliValueExtractor, ConfigDiscovery, LocalizationArgs, Localizer, MergeComposer, OrthoConfig, - OrthoMergeExt, OrthoResult, sanitize_value, + CliValueExtractor, ConfigDiscovery, Localizer, MergeComposer, OrthoConfig, OrthoMergeExt, + OrthoResult, sanitize_value, }; use serde::{Deserialize, Serialize}; use std::ffi::OsString; use std::path::PathBuf; use std::str::FromStr; +pub use crate::cli_l10n::locale_hint_from_args; +use crate::cli_l10n::localize_command; use crate::host_pattern::HostPattern; /// Maximum number of jobs accepted by the CLI. @@ -221,91 +223,6 @@ fn default_manifest_path() -> PathBuf { PathBuf::from("Netsukefile") } -fn usage_body(usage: &str) -> &str { - usage.strip_prefix("Usage: ").unwrap_or(usage) -} - -fn localize_command(mut command: clap::Command, localizer: &dyn Localizer) -> clap::Command { - let rendered_usage = command.render_usage().to_string(); - let fallback_usage = usage_body(&rendered_usage).to_owned(); - let mut args = LocalizationArgs::default(); - args.insert("binary", command.get_name().to_owned().into()); - args.insert("usage", fallback_usage.clone().into()); - let usage = localizer.message("cli.usage", Some(&args), &fallback_usage); - command = command.override_usage(usage); - - if let Some(about) = command.get_about().map(ToString::to_string) { - let localized_text = localizer.message("cli.about", None, &about); - command = command.about(localized_text); - } else if let Some(message) = localizer.lookup("cli.about", None) { - command = command.about(message); - } - - if let Some(long_about) = command.get_long_about().map(ToString::to_string) { - let localized_text = localizer.message("cli.long_about", None, &long_about); - command = command.long_about(localized_text); - } else if let Some(message) = localizer.lookup("cli.long_about", None) { - command = command.long_about(message); - } - - localize_subcommands(&mut command, localizer); - - command -} - -fn localize_subcommands(command: &mut clap::Command, localizer: &dyn Localizer) { - for subcommand in command.get_subcommands_mut() { - let name = subcommand.get_name().to_owned(); - let mut updated = std::mem::take(subcommand); - let about_key = format!("cli.subcommand.{name}.about"); - if let Some(about) = updated.get_about().map(ToString::to_string) { - let message = localizer.message(&about_key, None, &about); - updated = updated.about(message); - } else if let Some(message) = localizer.lookup(&about_key, None) { - updated = updated.about(message); - } - - let long_key = format!("cli.subcommand.{name}.long_about"); - if let Some(long_about) = updated.get_long_about().map(ToString::to_string) { - let message = localizer.message(&long_key, None, &long_about); - updated = updated.long_about(message); - } else if let Some(message) = localizer.lookup(&long_key, None) { - updated = updated.long_about(message); - } - - *subcommand = updated; - } -} - -/// Inspect raw arguments and extract the `--locale` value when present. -#[must_use] -pub fn locale_hint_from_args(args: &[OsString]) -> Option { - let mut hint = None; - let mut iter = args.iter().peekable(); - while let Some(arg) = iter.next() { - let text = arg.to_string_lossy(); - if text == "--" { - break; - } - if text == "--locale" { - let Some(next) = iter.peek() else { - break; - }; - let next_text = next.to_string_lossy(); - if next_text == "--" { - break; - } - hint = Some(next_text.into_owned()); - iter.next(); - continue; - } - if let Some(value) = text.strip_prefix("--locale=") { - hint = Some(value.to_owned()); - } - } - hint -} - /// Parse CLI arguments with localized clap output. /// /// Returns both the parsed CLI struct and the `ArgMatches` required for diff --git a/src/cli_l10n.rs b/src/cli_l10n.rs new file mode 100644 index 00000000..d1ae7f11 --- /dev/null +++ b/src/cli_l10n.rs @@ -0,0 +1,93 @@ +//! CLI localization helpers. +//! +//! This module keeps clap localization logic separate from the core CLI +//! definitions. + +use clap::Command; +use ortho_config::{LocalizationArgs, Localizer}; +use std::ffi::OsString; + +fn usage_body(usage: &str) -> &str { + usage.strip_prefix("Usage: ").unwrap_or(usage) +} + +pub(crate) fn localize_command(mut command: Command, localizer: &dyn Localizer) -> Command { + let rendered_usage = command.render_usage().to_string(); + let fallback_usage = usage_body(&rendered_usage).to_owned(); + let mut args = LocalizationArgs::default(); + args.insert("binary", command.get_name().to_owned().into()); + args.insert("usage", fallback_usage.clone().into()); + let usage = localizer.message("cli.usage", Some(&args), &fallback_usage); + command = command.override_usage(usage); + + if let Some(about) = command.get_about().map(ToString::to_string) { + let localized_text = localizer.message("cli.about", None, &about); + command = command.about(localized_text); + } else if let Some(message) = localizer.lookup("cli.about", None) { + command = command.about(message); + } + + if let Some(long_about) = command.get_long_about().map(ToString::to_string) { + let localized_text = localizer.message("cli.long_about", None, &long_about); + command = command.long_about(localized_text); + } else if let Some(message) = localizer.lookup("cli.long_about", None) { + command = command.long_about(message); + } + + localize_subcommands(&mut command, localizer); + + command +} + +fn localize_subcommands(command: &mut Command, localizer: &dyn Localizer) { + for subcommand in command.get_subcommands_mut() { + let name = subcommand.get_name().to_owned(); + let mut updated = std::mem::take(subcommand); + let about_key = format!("cli.subcommand.{name}.about"); + if let Some(about) = updated.get_about().map(ToString::to_string) { + let message = localizer.message(&about_key, None, &about); + updated = updated.about(message); + } else if let Some(message) = localizer.lookup(&about_key, None) { + updated = updated.about(message); + } + + let long_key = format!("cli.subcommand.{name}.long_about"); + if let Some(long_about) = updated.get_long_about().map(ToString::to_string) { + let message = localizer.message(&long_key, None, &long_about); + updated = updated.long_about(message); + } else if let Some(message) = localizer.lookup(&long_key, None) { + updated = updated.long_about(message); + } + + *subcommand = updated; + } +} + +/// Inspect raw arguments and extract the `--locale` value when present. +#[must_use] +pub fn locale_hint_from_args(args: &[OsString]) -> Option { + let mut hint = None; + let mut iter = args.iter().peekable(); + while let Some(arg) = iter.next() { + let text = arg.to_string_lossy(); + if text == "--" { + break; + } + if text == "--locale" { + let Some(next) = iter.peek() else { + break; + }; + let next_text = next.to_string_lossy(); + if next_text == "--" { + break; + } + hint = Some(next_text.into_owned()); + iter.next(); + continue; + } + if let Some(value) = text.strip_prefix("--locale=") { + hint = Some(value.to_owned()); + } + } + hint +} diff --git a/src/lib.rs b/src/lib.rs index 434b32de..ebe8da90 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -5,6 +5,7 @@ pub mod ast; pub mod cli; +mod cli_l10n; pub mod cli_localization; mod cli_policy; pub(crate) mod diagnostics; From 0033515b29660b7421b5a29c37f298c4f2b4a956 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sat, 3 Jan 2026 02:36:46 +0000 Subject: [PATCH 7/9] docs(tests): add detailed comments for CLI config merge tests These tests validate OrthoConfig layer precedence including defaults, file, env, CLI, list-value appending, and user-provided override extraction. Added additional documentation comments to test modules for clarity and maintenance. Co-authored-by: terragon-labs[bot] --- build.rs | 5 +++-- src/cli.rs | 2 ++ src/lib.rs | 3 +++ tests/cli_tests/merge.rs | 3 +++ tests/cli_tests/mod.rs | 2 +- 5 files changed, 12 insertions(+), 3 deletions(-) diff --git a/build.rs b/build.rs index 284c3dac..f8dc3289 100644 --- a/build.rs +++ b/build.rs @@ -72,8 +72,9 @@ fn write_man_page(data: &[u8], dir: &Path, page_name: &str) -> std::io::Result

Result<(), Box> { - // Exercise the host pattern symbols so the shared module remains linked - // when the build script is compiled without tests. + // Exercise CLI localisation, 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 _: fn(&[OsString]) -> Option = cli::locale_hint_from_args; const _: fn(&cli::Cli, &ArgMatches) -> ortho_config::OrthoResult = diff --git a/src/cli.rs b/src/cli.rs index 3b40f34b..5771054d 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -265,6 +265,8 @@ fn config_discovery(directory: Option<&PathBuf>) -> ConfigDiscovery { } /// 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()) } diff --git a/src/lib.rs b/src/lib.rs index ebe8da90..08dab032 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3,6 +3,9 @@ //! This library provides the command line interface definitions and //! helper functions for parsing `Netsukefile` manifests. +#[cfg(not(feature = "serde_json"))] +compile_error!("The `serde_json` feature is required to build Netsuke."); + pub mod ast; pub mod cli; mod cli_l10n; diff --git a/tests/cli_tests/merge.rs b/tests/cli_tests/merge.rs index c2b29352..07552265 100644 --- a/tests/cli_tests/merge.rs +++ b/tests/cli_tests/merge.rs @@ -1,4 +1,7 @@ //! Configuration merge tests. +//! +//! These tests validate OrthoConfig layer precedence (defaults, file, env, +//! CLI), list-value appending, and user-provided override extraction. use anyhow::{Context, Result, ensure}; use clap::{CommandFactory, FromArgMatches}; diff --git a/tests/cli_tests/mod.rs b/tests/cli_tests/mod.rs index 2a6205a2..63a54c87 100644 --- a/tests/cli_tests/mod.rs +++ b/tests/cli_tests/mod.rs @@ -1,6 +1,6 @@ //! Unit tests for CLI argument parsing and validation. //! -//! This crate exercises the command-line interface defined in `netsuke::cli`. +//! This module exercises the command-line interface defined in `netsuke::cli`. mod helpers; mod locale; From 63743d5dce1f494b0c945f773f31385c0f87665d Mon Sep 17 00:00:00 2001 From: Leynos Date: Sat, 3 Jan 2026 13:05:30 +0000 Subject: [PATCH 8/9] docs(cli-output): clarify CLI localization approach and dependency documentation - Improve instructions on setting up Fluent localization, emphasizing following the existing locales/ layout. - Refine instructions for wiring FluentLocalizer before parsing CLI and handling errors. - Update dependency addition instructions to refer to the version in Cargo.toml rather than specifying a caret version explicitly. - Ensure documentation reflects current best practices for CLI output clarity and dependencies. Co-authored-by: terragon-labs[bot] --- docs/execplans/cli-output-clarity.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/execplans/cli-output-clarity.md b/docs/execplans/cli-output-clarity.md index ff3eedd4..0b0051b0 100644 --- a/docs/execplans/cli-output-clarity.md +++ b/docs/execplans/cli-output-clarity.md @@ -108,8 +108,9 @@ development (BDD) lives in: 3. Localize CLI help and clap errors. Create Fluent resources (for example `locales/en-US/messages.ftl` and a CLI-specific bundle) and wire a - `FluentLocalizer` into CLI parsing. Use `command().localize(&localizer)` - before parsing and `localize_clap_error_with_command` on errors. Ensure the + `FluentLocalizer` into CLI parsing. Follow the existing `locales/` layout + for project translations. Use `command().localize(&localizer)` before + parsing and `localize_clap_error_with_command` on errors. Ensure the fallback path preserves stock clap output when localization fails. 4. Refine CLI output messages. Update docstrings and help text in @@ -211,9 +212,9 @@ Keep the following short transcripts for evidence: ## Interfaces and Dependencies -- Add `ortho_config` as a dependency using a caret version (for example, - `ortho_config = "0.7.0"`), enabling `yaml`/`json5` features only if required - and documenting the decision in `docs/netsuke-design.md`. +- Add `ortho_config` as a dependency at the version specified in + `Cargo.toml`, enabling `yaml`/`json5` features only if required and + documenting the decision in `docs/netsuke-design.md`. - Define `CliConfig` in `src/cli_config.rs` (or equivalent) with fields that map to existing CLI flags: `file`, `directory`, `jobs`, `verbose`, `fetch_allow_scheme`, `fetch_allow_host`, `fetch_block_host`, From 2872937adaff233a6707a46d2f63849538064ad1 Mon Sep 17 00:00:00 2001 From: Leynos Date: Sat, 3 Jan 2026 14:53:57 +0000 Subject: [PATCH 9/9] refactor(cli): filter CLI overrides to include only user-provided flags - Remove default values from CLI override extraction to prevent false positives. - Implement `cli_overrides_from_matches` to keep only flags explicitly set via command line. - Adjust tests accordingly to reflect updated override behavior. - Remove serde_json from default Cargo features and enable it explicitly in ortho_config deps. Co-authored-by: terragon-labs[bot] --- Cargo.toml | 8 +++---- build.rs | 2 +- src/cli.rs | 48 ++++++++++++++++++++++++++++++++++------ src/cli_l10n.rs | 2 ++ src/lib.rs | 3 --- tests/cli_tests/merge.rs | 47 ++++++++++++++------------------------- 6 files changed, 64 insertions(+), 46 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index a114d009..afc43853 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,10 +15,8 @@ license = "ISC" description = "A YAML-powered Ninja/Jinja hybrid build system." [features] -default = ["serde_json"] +default = [] legacy-digests = ["sha1", "md5"] -# Required: OrthoConfig derive and core manifest types depend on serde_json. -serde_json = ["ortho_config/serde_json"] [dependencies] clap = { version = "4.5.0", features = ["derive"] } @@ -53,12 +51,12 @@ 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 = "0.7.0" +ortho_config = { version = "0.7.0", features = ["serde_json"] } [build-dependencies] clap = { version = "4.5.0", features = ["derive"] } clap_mangen = "0.2.29" -ortho_config = "0.7.0" +ortho_config = { version = "0.7.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 f8dc3289..7d81e6c8 100644 --- a/build.rs +++ b/build.rs @@ -72,7 +72,7 @@ fn write_man_page(data: &[u8], dir: &Path, page_name: &str) -> std::io::Result

Result<(), Box> { - // Exercise CLI localisation, config merge, and host pattern symbols so the + // 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::(); diff --git a/src/cli.rs b/src/cli.rs index 5771054d..019c9b86 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -3,6 +3,7 @@ //! This module defines the [`Cli`] structure and its subcommands. //! It mirrors the design described in `docs/netsuke-design.md`. +use clap::parser::ValueSource; use clap::{ArgMatches, Args, CommandFactory, FromArgMatches, Parser, Subcommand}; use ortho_config::LanguageIdentifier; use ortho_config::declarative::LayerComposition; @@ -10,13 +11,14 @@ use ortho_config::figment::{Figment, providers::Env}; use ortho_config::localize_clap_error_with_command; use ortho_config::uncased::Uncased; use ortho_config::{ - CliValueExtractor, ConfigDiscovery, Localizer, MergeComposer, OrthoConfig, OrthoMergeExt, - OrthoResult, sanitize_value, + ConfigDiscovery, Localizer, MergeComposer, OrthoConfig, OrthoMergeExt, OrthoResult, + sanitize_value, }; use serde::{Deserialize, Serialize}; use std::ffi::OsString; use std::path::PathBuf; use std::str::FromStr; +use std::sync::Arc; pub use crate::cli_l10n::locale_hint_from_args; use crate::cli_l10n::localize_command; @@ -64,7 +66,7 @@ fn parse_locale(s: &str) -> Result { } LanguageIdentifier::from_str(trimmed) .map(|_| trimmed.to_owned()) - .map_err(|_| format!("invalid locale '{s}'")) + .map_err(|_| format!("invalid locale '{trimmed}'")) } /// Parse a host pattern supplied via CLI flags. @@ -83,7 +85,7 @@ fn parse_host_pattern(s: &str) -> Result { 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(), cli_default_as_absent)] + #[ortho_config(default = default_manifest_path())] pub file: PathBuf, /// Run as if started in this directory. @@ -100,7 +102,7 @@ pub struct Cli { /// Enable verbose diagnostic logging. #[arg(short, long)] - #[ortho_config(default = false, cli_default_as_absent)] + #[ortho_config(default = false)] pub verbose: bool, /// Locale tag for CLI copy (for example: en-US, es-ES). @@ -140,7 +142,7 @@ pub struct Cli { /// Deny all hosts by default; only allow the declared allowlist. #[arg(long = "fetch-default-deny")] - #[ortho_config(default = false, cli_default_as_absent)] + #[ortho_config(default = false)] pub fetch_default_deny: bool, /// Optional subcommand to execute; defaults to `build` when omitted. @@ -219,6 +221,7 @@ pub enum Commands { }, } +/// Return the default manifest filename when none is provided. fn default_manifest_path() -> PathBuf { PathBuf::from("Netsukefile") } @@ -271,6 +274,37 @@ fn is_empty_value(value: &serde_json::Value) -> bool { matches!(value, serde_json::Value::Object(map) if map.is_empty()) } +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 => { + return Err(Arc::new(ortho_config::OrthoError::Validation { + key: String::from("cli"), + message: 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", + ] { + if matches.value_source(field) != Some(ValueSource::CommandLine) { + map.remove(field); + } + } + + Ok(serde_json::Value::Object(map)) +} + /// Merge configuration layers over the parsed CLI values. /// /// # Errors @@ -308,7 +342,7 @@ pub fn merge_with_config(cli: &Cli, matches: &ArgMatches) -> OrthoResult { Err(err) => errors.push(err), } - match cli.extract_user_provided(matches) { + match cli_overrides_from_matches(cli, matches) { Ok(value) if !is_empty_value(&value) => composer.push_cli(value), Ok(_) => {} Err(err) => errors.push(err), diff --git a/src/cli_l10n.rs b/src/cli_l10n.rs index d1ae7f11..12883b20 100644 --- a/src/cli_l10n.rs +++ b/src/cli_l10n.rs @@ -64,6 +64,8 @@ fn localize_subcommands(command: &mut Command, localizer: &dyn Localizer) { } /// Inspect raw arguments and extract the `--locale` value when present. +/// +/// When multiple `--locale` flags are provided, the last one is used. #[must_use] pub fn locale_hint_from_args(args: &[OsString]) -> Option { let mut hint = None; diff --git a/src/lib.rs b/src/lib.rs index 08dab032..ebe8da90 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3,9 +3,6 @@ //! This library provides the command line interface definitions and //! helper functions for parsing `Netsukefile` manifests. -#[cfg(not(feature = "serde_json"))] -compile_error!("The `serde_json` feature is required to build Netsuke."); - pub mod ast; pub mod cli; mod cli_l10n; diff --git a/tests/cli_tests/merge.rs b/tests/cli_tests/merge.rs index 07552265..6a037cd8 100644 --- a/tests/cli_tests/merge.rs +++ b/tests/cli_tests/merge.rs @@ -1,13 +1,12 @@ //! Configuration merge tests. //! //! These tests validate OrthoConfig layer precedence (defaults, file, env, -//! CLI), list-value appending, and user-provided override extraction. +//! CLI) and list-value appending. use anyhow::{Context, Result, ensure}; -use clap::{CommandFactory, FromArgMatches}; use netsuke::cli::Cli; use netsuke::cli_localization; -use ortho_config::{CliValueExtractor, MergeComposer, sanitize_value}; +use ortho_config::{MergeComposer, sanitize_value}; use rstest::rstest; use serde_json::json; use std::ffi::OsStr; @@ -16,33 +15,6 @@ use std::path::Path; use tempfile::tempdir; use test_support::{EnvVarGuard, env_lock::EnvLock}; -#[rstest] -fn cli_extract_user_provided_omits_defaults() -> Result<()> { - let mut matches = Cli::command() - .try_get_matches_from(["netsuke"]) - .context("parse matches for default CLI")?; - let cli = Cli::from_arg_matches_mut(&mut matches).context("build CLI from matches")?; - let value = cli - .extract_user_provided(&matches) - .context("extract CLI overrides")?; - let object = value - .as_object() - .context("expected extracted CLI value to be an object")?; - ensure!( - !object.contains_key("file"), - "default file should not be treated as a CLI override", - ); - ensure!( - !object.contains_key("verbose"), - "default verbose flag should not be treated as a CLI override", - ); - ensure!( - !object.contains_key("fetch_default_deny"), - "default deny flag should not be treated as a CLI override", - ); - Ok(()) -} - #[rstest] fn cli_merge_layers_respects_precedence_and_appends_lists() -> Result<()> { let mut composer = MergeComposer::new(); @@ -95,8 +67,11 @@ fn cli_merge_with_config_respects_precedence_and_skips_empty_cli_layer() -> Resu let temp_dir = tempdir().context("create temporary config directory")?; let config_path = temp_dir.path().join("netsuke.toml"); let config = r#" +file = "Configfile" jobs = 2 fetch_allow_scheme = ["https"] +verbose = true +fetch_default_deny = true "#; fs::write(&config_path, config).context("write netsuke.toml")?; @@ -111,6 +86,18 @@ fetch_allow_scheme = ["https"] .context("merge CLI and configuration layers")? .with_default_command(); + ensure!( + merged.file.as_path() == Path::new("Configfile"), + "config file should override the default manifest path", + ); + ensure!( + merged.verbose, + "config file should override the default verbose flag", + ); + ensure!( + merged.fetch_default_deny, + "config file should override the default deny flag", + ); ensure!( merged.jobs == Some(4), "environment variables should override config when CLI has no value",