diff --git a/docs/execplans/3-12-1-define-design-tokens.md b/docs/execplans/3-12-1-define-design-tokens.md index ed56e10b..a9a65337 100644 --- a/docs/execplans/3-12-1-define-design-tokens.md +++ b/docs/execplans/3-12-1-define-design-tokens.md @@ -5,7 +5,7 @@ This ExecPlan (execution plan) is a living document. The sections `Decision Log`, and `Outcomes & Retrospective` must be kept up to date as work proceeds. -Status: DRAFT +Status: COMPLETE ## Purpose / big picture @@ -129,13 +129,19 @@ Observable success means: resolved tokens. Added `resolve_from_theme()` and `resolve_from_theme_with()` functions to `output_prefs` module that delegate to theme system. Updated `main.rs` to resolve `OutputMode` and - pass it to theme resolution. Theme infrastructure is complete and - functional. Full reporter integration (using spacing tokens in status.rs - and status_timing.rs) deferred to follow-up work. -- [ ] Stage D: Add behavioural coverage for theme selection and consistency. - Deferred pending full reporter integration. -- [ ] Stage E: Update documentation, mark the roadmap item done, and run the - full quality gates. + pass it to theme resolution. Completed reporter integration by routing + semantic prefixes through resolved theme symbols and replacing hard-coded + task/timing indentation with spacing token accessors in `status.rs` and + `status_timing.rs`. +- [x] (2026-03-21 00:00Z) Stage D: Add behavioural coverage for theme + selection and consistency. Activated the dormant `tests/cli_tests/` + harness, added theme parsing and merge-precedence tests, and added BDD + scenarios covering explicit ASCII and Unicode themes plus invalid + `--theme` input. +- [x] (2026-03-21 00:00Z) Stage E: Update documentation, mark the roadmap item + done, and run the full quality gates. Updated the user guide and design + record, corrected the execplan command-list Markdown structure, and + completed the roadmap entry after the full gates passed. ## Surprises & discoveries @@ -146,11 +152,12 @@ Observable success means: use OrthoConfig immediately, but the plan must avoid baking reporter code directly into the current `Cli` type. -- Observation: there is no current theme abstraction; `OutputPrefs` only tracks - whether emoji are allowed, while `TASK_INDENT` and several prefixes are still - hard-coded in the reporters. Evidence: `src/output_prefs.rs`, - `src/status.rs`, `src/status_timing.rs`, and `src/main.rs`. Impact: 3.12.1 - must create the abstraction before 3.12.2 can snapshot it. +- Observation: theme-token rollout was simplest when `OutputPrefs` became the + compatibility wrapper around the resolved theme rather than introducing a new + reporter-facing type. Evidence: `src/output_prefs.rs`, `src/status.rs`, and + `src/status_timing.rs`. Impact: existing reporter constructors and most call + sites stayed stable while tokens became authoritative for prefixes and + spacing. - Observation: strict clippy/lint behaviour has already required `build.rs` anchors for shared CLI helpers. Evidence: `build.rs` contains `const _` @@ -158,14 +165,11 @@ Observable success means: `cli_l10n::parse_bool_hint`, and related helpers. Impact: new theme parsing helpers must follow the same pattern. -- (2026-03-17) Observation: Implementing full reporter token integration - (replacing TASK_INDENT literals with spacing tokens in status.rs, - status_timing.rs) requires touching multiple reporter files and extensive - testing. Evidence: Current implementation successfully adds CLI theme - preference, theme resolution pipeline, and OutputPrefs façade, but full - reporter refactoring needs dedicated focus. Impact: Stages C and D are - partially complete with infrastructure in place. Follow-up work needed to - complete reporter integration and add comprehensive BDD coverage. +- Observation: the repository already contained a dormant `tests/cli_tests/` + module tree that was not an active Cargo integration target. Evidence: + `tests/cli_tests/mod.rs` existed without a matching `tests/cli_tests.rs` + harness. Impact: the roadmap item needed a small harness file plus stale-test + fixes before the new theme merge tests could participate in `cargo test`. ## Decision log @@ -187,9 +191,15 @@ Observable success means: language. Preset token sets are enough to unblock 3.12.2 snapshots and 3.12.3 terminal validation. Date/Author: 2026-03-13 / Codex. +- Decision: render semantic prefixes from theme symbols plus localized text + labels instead of keeping symbol selection inside Fluent select expressions. + Rationale: the roadmap item requires design tokens to become the single + source of truth for glyph choice, while translation should still own the text + labels (`Error:`, `Advertencia:`, and so on). Date/Author: 2026-03-21 / Codex. + ## Outcomes & retrospective -Status: Partially complete (2026-03-17) +Status: Complete (2026-03-21) Implementation achieved: @@ -198,24 +208,22 @@ Implementation achieved: resolution pipeline - CLI integration: `--theme` flag with OrthoConfig merging, localized validation, and precedence handling -- OutputPrefs compatibility façade delegates to theme system -- 12 passing unit tests for theme resolution precedence +- OutputPrefs compatibility façade now carries resolved theme tokens and + renders semantic prefixes from token glyphs plus localized labels +- Reporter integration: `src/status.rs` and `src/status_timing.rs` now use + theme-backed spacing accessors instead of hard-coded indentation literals +- Active unit/integration coverage for theme parsing, precedence, and rendered + output, including a reactivated `tests/cli_tests.rs` harness +- BDD coverage for explicit ASCII and Unicode theme rendering, verbose ASCII + timing output, and invalid `--theme` validation - Backward compatibility preserved: existing `no_emoji` preference continues to work -Remaining work (deferred to follow-up): - -- Reporter integration: Update `src/status.rs` and `src/status_timing.rs` to - use spacing tokens instead of hard-coded `TASK_INDENT` literals -- BDD coverage: Add `rstest-bdd` scenarios for end-to-end theme selection and - ASCII/Unicode consistency -- Documentation: Update `docs/users-guide.md` with theme selection guidance -- Mark roadmap 3.12.1 done after completion - -The implementation successfully adds the user-visible theme selection story -through OrthoConfig and centralizes token definitions. The infrastructure is -complete and functional. Full reporter integration requires focused work on -status rendering modules and comprehensive BDD test coverage. +The implementation now satisfies the roadmap item end to end: theme selection +flows through OrthoConfig, semantic prefixes and spacing come from one resolved +token set, ASCII and Unicode output stay structurally aligned, and the docs and +tests reflect the resulting behaviour. Roadmap item 3.12.1 is complete and its +full quality gates have passed. ## Context and orientation @@ -226,13 +234,13 @@ The current CLI output flow is spread across several modules: - `src/cli_l10n.rs` maps clap arguments and subcommands to Fluent help keys. - `src/output_mode.rs` resolves whether output is `Accessible` or `Standard` from `accessible`, `NO_COLOR`, and `TERM`. -- `src/output_prefs.rs` resolves whether emoji are allowed from `no_emoji`, - `NO_COLOR`, and `NETSUKE_NO_EMOJI`, then renders semantic prefixes such as - `Error:`, `Info:`, `Success:`, and `Timing:`. +- `src/output_prefs.rs` resolves the theme-backed output preferences and now + exposes semantic prefixes plus spacing accessors through a compatibility + façade. - `src/status.rs` renders pipeline stages, task progress, and completion - messages. It still embeds spacing decisions locally via `TASK_INDENT`. -- `src/status_timing.rs` renders verbose timing summaries and currently applies - its own prefix and indentation rules. + messages using `OutputPrefs` for semantic prefixes and task indentation. +- `src/status_timing.rs` renders verbose timing summaries using the same + `OutputPrefs` façade for timing prefixes and indentation. - `src/main.rs` renders top-level errors using `OutputPrefs`. - `src/runner/mod.rs` resolves `OutputMode` and `OutputPrefs`, then constructs the reporter stack. @@ -474,57 +482,57 @@ cd /home/user/project 1. Establish the current baseline and locate the relevant code. -```sh -rg -n "theme|OutputPrefs|no_emoji|accessible|progress" src tests docs -``` + ```sh + rg -n "theme|OutputPrefs|no_emoji|accessible|progress" src tests docs + ``` -1. Implement the theme module and CLI/config wiring. +2. Implement the theme module and CLI/config wiring. -```sh -cargo test --workspace theme -- --nocapture -``` + ```sh + cargo test --workspace theme -- --nocapture + ``` -Expected shape after Stage B: + Expected shape after Stage B: -```plaintext -running N tests -test ...theme... ok -test ...cli... ok -``` + ```plaintext + running N tests + test ...theme... ok + test ...cli... ok + ``` -1. Refresh BDD-generated scenario code if feature text changes. +3. Refresh BDD-generated scenario code if feature text changes. -```sh -touch tests/bdd_tests.rs -``` + ```sh + touch tests/bdd_tests.rs + ``` -1. Run the full validation gates with logged output. +4. Run the full validation gates with logged output. -```sh -set -o pipefail && make check-fmt 2>&1 | tee /tmp/netsuke-3-12-1-check-fmt.log -set -o pipefail && make lint 2>&1 | tee /tmp/netsuke-3-12-1-lint.log -set -o pipefail && make test 2>&1 | tee /tmp/netsuke-3-12-1-test.log -set -o pipefail && make fmt 2>&1 | tee /tmp/netsuke-3-12-1-fmt.log -set -o pipefail && PATH="/root/.bun/bin:$PATH" make markdownlint 2>&1 | tee /tmp/netsuke-3-12-1-markdownlint.log -set -o pipefail && make nixie 2>&1 | tee /tmp/netsuke-3-12-1-nixie.log -``` + ```sh + set -o pipefail && make check-fmt 2>&1 | tee /tmp/netsuke-3-12-1-check-fmt.log + set -o pipefail && make lint 2>&1 | tee /tmp/netsuke-3-12-1-lint.log + set -o pipefail && make test 2>&1 | tee /tmp/netsuke-3-12-1-test.log + set -o pipefail && make fmt 2>&1 | tee /tmp/netsuke-3-12-1-fmt.log + set -o pipefail && PATH="/root/.bun/bin:$PATH" make markdownlint 2>&1 | tee /tmp/netsuke-3-12-1-markdownlint.log + set -o pipefail && make nixie 2>&1 | tee /tmp/netsuke-3-12-1-nixie.log + ``` -Expected final signal: + Expected final signal: -```plaintext -make check-fmt # exits 0 -make lint # exits 0 -make test # exits 0 -make markdownlint # exits 0 -make nixie # exits 0 -``` + ```plaintext + make check-fmt # exits 0 + make lint # exits 0 + make test # exits 0 + make markdownlint # exits 0 + make nixie # exits 0 + ``` -1. Inspect scope before finalizing. +5. Inspect scope before finalizing. -```sh -git status --short -git diff --stat -``` + ```sh + git status --short + git diff --stat + ``` Only the intended source, test, and documentation files for 3.12.1 should remain modified. diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index d5c9ff23..a744ec9f 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -2083,6 +2083,15 @@ Timing summaries are completion diagnostics. They are suppressed when verbose mode is off and also suppressed on failed runs so failures do not imply a successful pipeline completion. +Theme resolution for CLI output is centralized in `src/theme.rs`. Netsuke +resolves one theme through OrthoConfig layers (`--theme`, `NETSUKE_THEME`, +config file, then mode defaults) and hands the resulting symbol and spacing +tokens to reporters through the `OutputPrefs` compatibility façade. This keeps +reporter code focused on status semantics rather than glyph choice, preserves +`no_emoji` as a legacy ASCII-forcing alias when no explicit theme is supplied, +and gives later roadmap items a stable snapshot surface for validating ASCII +and Unicode renderings without duplicating formatting rules. + For screen readers: The following flowchart shows how the build script audits localization keys against English and Spanish Fluent bundles. diff --git a/docs/roadmap.md b/docs/roadmap.md index d76d6e04..ded6be8c 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -301,9 +301,10 @@ library, and CLI ergonomics. ### 3.12. Visual design validation -- [ ] 3.12.1. Define design tokens for colours, symbols, and spacing. - - [ ] Wire tokens through CLI theme system. - - [ ] Ensure ASCII and Unicode modes remain consistent. +- [x] 3.12.1. Define design tokens for colours, symbols, and spacing. + - [x] Route reporter prefixes and spacing through resolved theme tokens. + - [x] Add end-to-end theme coverage and documentation for ASCII/Unicode + consistency. - [ ] 3.12.2. Snapshot progress and status output for themes. - [ ] Cover unicode and ascii themes. - [ ] Guard alignment and wrapping against regressions. diff --git a/docs/users-guide.md b/docs/users-guide.md index caa36718..63b5d346 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -763,17 +763,29 @@ When progress is disabled, Netsuke suppresses stage and task progress output in both standard and accessible modes. If verbose mode is enabled at the same time, only the completion timing summary remains visible on successful runs. -### Emoji and accessibility preferences +### Theme and accessibility preferences -Netsuke supports suppressing emoji glyphs in output for users who prefer -ASCII-only output or use environments where emoji are not rendered correctly. +Netsuke resolves a CLI theme through the same layered configuration model as +its other user-facing preferences: -Emoji are automatically suppressed when: +- CLI flag: `--theme auto|unicode|ascii` +- Environment variable: `NETSUKE_THEME=auto|unicode|ascii` +- Configuration file: `theme = "auto" | "unicode" | "ascii"` -- `NO_COLOR` is set (any value) -- `NETSUKE_NO_EMOJI` is set (any value) +Theme precedence is: + +1. Explicit `theme` +2. Legacy `no_emoji = true` +3. `NETSUKE_NO_EMOJI` present +4. `NO_COLOR` present +5. Output mode default (`unicode` for standard output, `ascii` for + accessible output) -Emoji suppression can be forced on explicitly: +`auto` keeps the mode-sensitive default. `unicode` forces Unicode symbols even +in accessible mode, while `ascii` forces ASCII-safe symbols everywhere. + +Netsuke still supports the legacy no-emoji compatibility flag for users who +already rely on it: - CLI flag: `--no-emoji true` - Environment variable: `NETSUKE_NO_EMOJI` (any value, including empty) @@ -781,13 +793,16 @@ Emoji suppression can be forced on explicitly: Only `--no-emoji true` acts as a hard override; `--no-emoji false` and omitting the flag both defer to environment variable detection. `NETSUKE_NO_EMOJI` uses -presence-based semantics — setting it to any value (including `"false"` or -`"0"`) suppresses emoji. +presence-based semantics, so setting it to any value (including `"false"` or +`"0"`) still selects the ASCII theme unless an explicit `theme` overrides it. + +In all output modes, Netsuke uses semantic text prefixes, so meaning is never +conveyed solely by colour. The active theme swaps only the glyph set: -In all output modes, Netsuke uses semantic text prefixes (`Error:`, `Warning:`, -and `Success:`) so that meaning is never conveyed solely by colour or symbol. -When emoji is permitted, these prefixes include a leading glyph for quick -visual scanning. +- Unicode theme: `✖ Error:`, `⚠ Warning:`, `✔ Success:`, `ℹ Info:`, + `⏱ Timing:` +- ASCII theme: `X Error:`, `! Warning:`, `+ Success:`, `i Info:`, + `T Timing:` ### Exit Codes diff --git a/locales/en-US/messages.ftl b/locales/en-US/messages.ftl index ad262a0b..a8315035 100644 --- a/locales/en-US/messages.ftl +++ b/locales/en-US/messages.ftl @@ -351,26 +351,12 @@ status.tool.manifest = Manifest cli.flag.no_emoji.help = Suppress emoji glyphs in output. # Semantic prefixes for accessible output. -semantic.prefix.error = { $emoji -> - [yes] ✖ Error: - *[no] Error: -} -semantic.prefix.warning = { $emoji -> - [yes] ⚠ Warning: - *[no] Warning: -} -semantic.prefix.success = { $emoji -> - [yes] ✔ Success: - *[no] Success: -} -semantic.prefix.info = { $emoji -> - [yes] ℹ Info: - *[no] Info: -} -semantic.prefix.timing = { $emoji -> - [yes] ⏱ Timing: - *[no] Timing: -} +semantic.prefix.error = Error: +semantic.prefix.warning = Warning: +semantic.prefix.success = Success: +semantic.prefix.info = Info: +semantic.prefix.timing = Timing: +semantic.prefix.rendered = {"{"}symbol{"}"} {"{"}label{"}"} # Plural form examples for translators. # These messages demonstrate Fluent's select expression syntax using CLDR diff --git a/locales/es-ES/messages.ftl b/locales/es-ES/messages.ftl index f764adb4..dfca49ac 100644 --- a/locales/es-ES/messages.ftl +++ b/locales/es-ES/messages.ftl @@ -351,26 +351,12 @@ status.tool.manifest = Manifiesto cli.flag.no_emoji.help = Suprimir glifos emoji en la salida. # Prefijos semánticos para salida accesible. -semantic.prefix.error = { $emoji -> - [yes] ✖ Error: - *[no] Error: -} -semantic.prefix.warning = { $emoji -> - [yes] ⚠ Advertencia: - *[no] Advertencia: -} -semantic.prefix.success = { $emoji -> - [yes] ✔ Éxito: - *[no] Éxito: -} -semantic.prefix.info = { $emoji -> - [yes] ℹ Info: - *[no] Info: -} -semantic.prefix.timing = { $emoji -> - [yes] ⏱ Tiempos: - *[no] Tiempos: -} +semantic.prefix.error = Error: +semantic.prefix.warning = Advertencia: +semantic.prefix.success = Éxito: +semantic.prefix.info = Info: +semantic.prefix.timing = Tiempos: +semantic.prefix.rendered = {"{"}symbol{"}"} {"{"}label{"}"} # Ejemplos de formas plurales para traductores. # Estos mensajes demuestran la sintaxis de expresiones select de Fluent diff --git a/src/localization/keys.rs b/src/localization/keys.rs index fc5aebf8..9c9e1998 100644 --- a/src/localization/keys.rs +++ b/src/localization/keys.rs @@ -311,6 +311,7 @@ define_keys! { SEMANTIC_PREFIX_SUCCESS => "semantic.prefix.success", SEMANTIC_PREFIX_INFO => "semantic.prefix.info", SEMANTIC_PREFIX_TIMING => "semantic.prefix.timing", + SEMANTIC_PREFIX_RENDERED => "semantic.prefix.rendered", EXAMPLE_FILES_PROCESSED => "example.files_processed", EXAMPLE_ERRORS_FOUND => "example.errors_found", } diff --git a/src/output_prefs.rs b/src/output_prefs.rs index c0b96256..54d4e510 100644 --- a/src/output_prefs.rs +++ b/src/output_prefs.rs @@ -1,16 +1,16 @@ -//! Output preference resolution for emoji and semantic prefix formatting. +//! Output preference resolution for theme-backed CLI formatting. //! -//! This module determines whether Netsuke should include emoji glyphs in its -//! output and provides localized semantic prefix helpers (`Error:`, -//! `Warning:`, `Success:`) that adapt to the resolved preference. Preferences -//! are auto-detected from the `NO_COLOR` and `NETSUKE_NO_EMOJI` environment -//! variables, or forced via explicit configuration. +//! This module resolves a full CLI theme, including `NETSUKE_THEME`, and +//! exposes localized semantic prefix helpers plus spacing-token helpers such +//! as [`OutputPrefs::task_indent`] and [`OutputPrefs::timing_indent`]. +//! Preferences are auto-detected from `NO_COLOR`, `NETSUKE_NO_EMOJI`, and +//! `NETSUKE_THEME`, or forced through explicit CLI/configuration values. use std::env; -use crate::localization::{self, LocalizedMessage, keys}; +use crate::localization::{self, keys}; use crate::output_mode::OutputMode; -use crate::theme::{self, ThemePreference}; +use crate::theme::{self, ResolvedTheme, ThemePreference}; /// Resolved output formatting preferences. /// @@ -19,8 +19,8 @@ use crate::theme::{self, ThemePreference}; /// messages. /// /// This is now a compatibility facade over the theme system introduced in -/// roadmap 3.12.1. The `emoji` field is preserved for backward compatibility, -/// but prefix rendering delegates to the resolved theme tokens. +/// roadmap 3.12.1. Callers still ask for output preferences, while the +/// implementation delegates prefix and spacing decisions to the resolved theme. /// /// # Examples /// @@ -32,25 +32,47 @@ use crate::theme::{self, ThemePreference}; /// ``` #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct OutputPrefs { - /// Whether emoji glyphs are permitted in output. - emoji: bool, + resolved_theme: ResolvedTheme, } impl OutputPrefs { + #[must_use] + const fn from_theme(resolved_theme: ResolvedTheme) -> Self { + Self { resolved_theme } + } + /// Return `true` when emoji glyphs are permitted. #[must_use] pub const fn emoji_allowed(self) -> bool { - self.emoji + self.resolved_theme.tokens.emoji_allowed + } + + /// Return the task-progress indentation string for the active theme. + #[must_use] + pub const fn task_indent(self) -> &'static str { + self.resolved_theme.tokens.spacing.task_indent + } + + /// Return the timing-summary indentation string for the active theme. + #[must_use] + pub const fn timing_indent(self) -> &'static str { + self.resolved_theme.tokens.spacing.timing_indent } - /// Fluent argument value for the `$emoji` select expression. - const fn emoji_arg(self) -> &'static str { - if self.emoji { "yes" } else { "no" } + fn render_prefix(symbol: &'static str, label_key: &'static str) -> String { + let label = localization::message(label_key).to_string(); + localization::message(keys::SEMANTIC_PREFIX_RENDERED) + .to_string() + .replace("{symbol}", symbol) + .replace("{label}", &label) } /// Render the localized error prefix for the current preferences. /// - /// Returns `"✖ Error:"` when emoji is allowed, `"Error:"` otherwise. + /// The output depends on the active locale installed via + /// [`crate::localization::set_localizer`]. For the default en-US localizer, + /// this returns `"✖ Error:"` for the Unicode theme and `"X Error:"` for + /// the ASCII theme. /// /// # Examples /// @@ -58,25 +80,37 @@ impl OutputPrefs { /// use netsuke::output_prefs::resolve_with; /// /// let prefs = resolve_with(Some(true), |_| None); - /// let prefix = prefs.error_prefix().to_string(); + /// let prefix = prefs.error_prefix(); /// assert!(prefix.contains("Error:")); /// ``` #[must_use] - pub fn error_prefix(self) -> LocalizedMessage { - localization::message(keys::SEMANTIC_PREFIX_ERROR).with_arg("emoji", self.emoji_arg()) + pub fn error_prefix(self) -> String { + Self::render_prefix( + self.resolved_theme.tokens.symbols.error, + keys::SEMANTIC_PREFIX_ERROR, + ) } /// Render the localized warning prefix for the current preferences. /// - /// Returns `"⚠ Warning:"` when emoji is allowed, `"Warning:"` otherwise. + /// The output depends on the active locale installed via + /// [`crate::localization::set_localizer`]. For the default en-US localizer, + /// this returns `"⚠ Warning:"` for the Unicode theme and `"! Warning:"` + /// for the ASCII theme. #[must_use] - pub fn warning_prefix(self) -> LocalizedMessage { - localization::message(keys::SEMANTIC_PREFIX_WARNING).with_arg("emoji", self.emoji_arg()) + pub fn warning_prefix(self) -> String { + Self::render_prefix( + self.resolved_theme.tokens.symbols.warning, + keys::SEMANTIC_PREFIX_WARNING, + ) } /// Render the localized success prefix for the current preferences. /// - /// Returns `"✔ Success:"` when emoji is allowed, `"Success:"` otherwise. + /// The output depends on the active locale installed via + /// [`crate::localization::set_localizer`]. For the default en-US localizer, + /// this returns `"✔ Success:"` for the Unicode theme and `"+ Success:"` + /// for the ASCII theme. /// /// # Examples /// @@ -84,28 +118,43 @@ impl OutputPrefs { /// use netsuke::output_prefs::resolve_with; /// /// let prefs = resolve_with(Some(true), |_| None); - /// let prefix = prefs.success_prefix().to_string(); + /// let prefix = prefs.success_prefix(); /// assert!(prefix.contains("Success:")); /// ``` #[must_use] - pub fn success_prefix(self) -> LocalizedMessage { - localization::message(keys::SEMANTIC_PREFIX_SUCCESS).with_arg("emoji", self.emoji_arg()) + pub fn success_prefix(self) -> String { + Self::render_prefix( + self.resolved_theme.tokens.symbols.success, + keys::SEMANTIC_PREFIX_SUCCESS, + ) } /// Render the localized informational prefix for the current preferences. /// - /// Returns `"ℹ Info:"` when emoji is allowed, `"Info:"` otherwise. + /// The output depends on the active locale installed via + /// [`crate::localization::set_localizer`]. For the default en-US localizer, + /// this returns `"ℹ Info:"` for the Unicode theme and `"i Info:"` for the + /// ASCII theme. #[must_use] - pub fn info_prefix(self) -> LocalizedMessage { - localization::message(keys::SEMANTIC_PREFIX_INFO).with_arg("emoji", self.emoji_arg()) + pub fn info_prefix(self) -> String { + Self::render_prefix( + self.resolved_theme.tokens.symbols.info, + keys::SEMANTIC_PREFIX_INFO, + ) } /// Render the localized timing prefix for the current preferences. /// - /// Returns `"⏱ Timing:"` when emoji is allowed, `"Timing:"` otherwise. + /// The output depends on the active locale installed via + /// [`crate::localization::set_localizer`]. For the default en-US localizer, + /// this returns `"⏱ Timing:"` for the Unicode theme and `"T Timing:"` for + /// the ASCII theme. #[must_use] - pub fn timing_prefix(self) -> LocalizedMessage { - localization::message(keys::SEMANTIC_PREFIX_TIMING).with_arg("emoji", self.emoji_arg()) + pub fn timing_prefix(self) -> String { + Self::render_prefix( + self.resolved_theme.tokens.symbols.timing, + keys::SEMANTIC_PREFIX_TIMING, + ) } } @@ -159,9 +208,7 @@ where F: Fn(&str) -> Option, { let resolved_theme = theme::resolve_theme(theme, no_emoji, mode, read_env); - OutputPrefs { - emoji: resolved_theme.tokens.emoji_allowed, - } + OutputPrefs::from_theme(resolved_theme) } /// Resolve output preferences from explicit configuration and environment. @@ -227,162 +274,10 @@ pub fn resolve_with(no_emoji: Option, read_env: F) -> OutputPrefs where F: Fn(&str) -> Option, { - // Explicit CLI override: only `Some(true)` forces emoji off. - // `Some(false)` deliberately falls through — it does not re-enable - // emoji when an environment variable is present. - if let Some(true) = no_emoji { - return OutputPrefs { emoji: false }; - } - - if read_env("NO_COLOR").is_some() { - return OutputPrefs { emoji: false }; - } - - if read_env("NETSUKE_NO_EMOJI").is_some() { - return OutputPrefs { emoji: false }; - } - - OutputPrefs { emoji: true } + let resolved_theme = theme::resolve_theme(None, no_emoji, OutputMode::Standard, read_env); + OutputPrefs::from_theme(resolved_theme) } #[cfg(test)] -mod tests { - use super::*; - use rstest::rstest; - - #[derive(Debug)] - struct ThemeResolutionCase<'a> { - theme: Option, - no_emoji: Option, - mode: OutputMode, - no_color: Option<&'a str>, - no_emoji_env: Option<&'a str>, - expected_emoji: bool, - } - - /// Build an environment lookup from optional `NO_COLOR` and - /// `NETSUKE_NO_EMOJI` values. - fn fake_env<'a>( - no_color: Option<&'a str>, - no_emoji_env: Option<&'a str>, - ) -> impl Fn(&str) -> Option + 'a { - move |key| match key { - "NO_COLOR" => no_color.map(String::from), - "NETSUKE_NO_EMOJI" => no_emoji_env.map(String::from), - _ => None, - } - } - - #[rstest] - #[case::explicit_no_emoji_forces_off(Some(true), None, None, false)] - #[case::false_defers_to_no_color(Some(false), Some("1"), None, false)] - #[case::false_defers_to_netsuke_no_emoji(Some(false), None, Some("1"), false)] - #[case::no_color_disables_emoji(None, Some("1"), None, false)] - #[case::no_color_empty_disables_emoji(None, Some(""), None, false)] - #[case::netsuke_no_emoji_disables(None, None, Some("1"), false)] - #[case::netsuke_no_emoji_empty_disables(None, None, Some(""), false)] - #[case::netsuke_no_emoji_false_string_disables(None, None, Some("false"), false)] - #[case::netsuke_no_emoji_zero_string_disables(None, None, Some("0"), false)] - #[case::false_defers_to_netsuke_no_emoji_false_string(Some(false), None, Some("false"), false)] - #[case::default_allows_emoji(None, None, None, true)] - #[case::no_color_takes_precedence_over_missing_netsuke(None, Some("1"), None, false)] - #[case::both_env_vars_disable(None, Some("1"), Some("1"), false)] - fn resolve_output_prefs( - #[case] no_emoji: Option, - #[case] no_color: Option<&str>, - #[case] no_emoji_env: Option<&str>, - #[case] expected_emoji: bool, - ) { - let env = fake_env(no_color, no_emoji_env); - assert_eq!(resolve_with(no_emoji, env).emoji_allowed(), expected_emoji); - } - - #[test] - fn emoji_allowed_returns_true_when_permitted() { - let prefs = resolve_with(Some(false), |_| None); - assert!(prefs.emoji_allowed()); - } - - #[test] - fn emoji_allowed_returns_false_when_suppressed() { - let prefs = resolve_with(Some(true), |_| None); - assert!(!prefs.emoji_allowed()); - } - - #[rstest] - #[case::unicode_theme_overrides_no_emoji_env(ThemeResolutionCase { - theme: Some(ThemePreference::Unicode), - no_emoji: None, - mode: OutputMode::Standard, - no_color: None, - no_emoji_env: Some("1"), - expected_emoji: true, - })] - #[case::ascii_theme_stays_ascii_without_env(ThemeResolutionCase { - theme: Some(ThemePreference::Ascii), - no_emoji: None, - mode: OutputMode::Standard, - no_color: None, - no_emoji_env: None, - expected_emoji: false, - })] - #[case::auto_theme_no_color_forces_ascii(ThemeResolutionCase { - theme: Some(ThemePreference::Auto), - no_emoji: None, - mode: OutputMode::Standard, - no_color: Some("1"), - no_emoji_env: None, - expected_emoji: false, - })] - #[case::auto_theme_standard_without_env_uses_unicode(ThemeResolutionCase { - theme: Some(ThemePreference::Auto), - no_emoji: None, - mode: OutputMode::Standard, - no_color: None, - no_emoji_env: None, - expected_emoji: true, - })] - #[case::auto_theme_legacy_no_emoji_stays_ascii(ThemeResolutionCase { - theme: Some(ThemePreference::Auto), - no_emoji: Some(true), - mode: OutputMode::Standard, - no_color: None, - no_emoji_env: None, - expected_emoji: false, - })] - fn resolve_from_theme_with_uses_theme_resolution(#[case] case: ThemeResolutionCase<'_>) { - let env = fake_env(case.no_color, case.no_emoji_env); - let prefs = resolve_from_theme_with(case.theme, case.no_emoji, case.mode, env); - assert_eq!(prefs.emoji_allowed(), case.expected_emoji); - } - - #[rstest] - #[case::error_with_emoji(true, OutputPrefs::error_prefix, "Error:")] - #[case::error_without_emoji(false, OutputPrefs::error_prefix, "Error:")] - #[case::success_with_emoji(true, OutputPrefs::success_prefix, "Success:")] - #[case::success_without_emoji(false, OutputPrefs::success_prefix, "Success:")] - #[case::warning_with_emoji(true, OutputPrefs::warning_prefix, "Warning:")] - #[case::warning_without_emoji(false, OutputPrefs::warning_prefix, "Warning:")] - #[case::info_with_emoji(true, OutputPrefs::info_prefix, "Info:")] - #[case::info_without_emoji(false, OutputPrefs::info_prefix, "Info:")] - #[case::timing_with_emoji(true, OutputPrefs::timing_prefix, "Timing:")] - #[case::timing_without_emoji(false, OutputPrefs::timing_prefix, "Timing:")] - fn prefix_rendering( - #[case] emoji: bool, - #[case] prefix_fn: fn(OutputPrefs) -> LocalizedMessage, - #[case] expected_text: &str, - ) { - let prefs = OutputPrefs { emoji }; - let rendered = prefix_fn(prefs).to_string(); - assert!( - rendered.contains(expected_text), - "expected '{expected_text}' in '{rendered}'" - ); - if !emoji { - assert!( - rendered.is_ascii(), - "expected ASCII-only prefix, got '{rendered}'" - ); - } - } -} +#[path = "output_prefs_tests.rs"] +mod tests; diff --git a/src/output_prefs_tests.rs b/src/output_prefs_tests.rs new file mode 100644 index 00000000..81fde3d2 --- /dev/null +++ b/src/output_prefs_tests.rs @@ -0,0 +1,192 @@ +//! Tests for output preference resolution and token-backed prefix rendering. + +use super::*; +use crate::cli_localization; +use crate::localization::{self, LocalizerGuard}; +use anyhow::{Result, ensure}; +use rstest::{fixture, rstest}; +use std::error::Error; +use std::fmt; +use std::sync::{Arc, MutexGuard}; +use test_support::localizer::localizer_test_lock; + +#[derive(Debug)] +struct ThemeResolutionCase<'a> { + theme: Option, + no_emoji: Option, + mode: OutputMode, + no_color: Option<&'a str>, + no_emoji_env: Option<&'a str>, + expected_emoji: bool, +} + +/// Build an environment lookup from optional `NO_COLOR` and +/// `NETSUKE_NO_EMOJI` values. +fn fake_env<'a>( + no_color: Option<&'a str>, + no_emoji_env: Option<&'a str>, +) -> impl Fn(&str) -> Option + 'a { + move |key| match key { + "NO_COLOR" => no_color.map(String::from), + "NETSUKE_NO_EMOJI" => no_emoji_env.map(String::from), + _ => None, + } +} + +#[rstest] +#[case::explicit_no_emoji_forces_ascii(Some(true), None, None, false)] +#[case::false_defers_to_no_color(Some(false), Some("1"), None, false)] +#[case::false_defers_to_netsuke_no_emoji(Some(false), None, Some("1"), false)] +#[case::explicit_false_allows_unicode(Some(false), None, None, true)] +#[case::no_color_disables_emoji(None, Some("1"), None, false)] +#[case::no_color_empty_disables_emoji(None, Some(""), None, false)] +#[case::netsuke_no_emoji_disables(None, None, Some("1"), false)] +#[case::netsuke_no_emoji_empty_disables(None, None, Some(""), false)] +#[case::netsuke_no_emoji_false_string_disables(None, None, Some("false"), false)] +#[case::netsuke_no_emoji_zero_string_disables(None, None, Some("0"), false)] +#[case::false_defers_to_netsuke_no_emoji_false_string(Some(false), None, Some("false"), false)] +#[case::default_allows_unicode(None, None, None, true)] +#[case::no_color_takes_precedence_over_missing_netsuke(None, Some("1"), None, false)] +#[case::both_env_vars_disable(None, Some("1"), Some("1"), false)] +fn resolve_output_prefs( + #[case] no_emoji: Option, + #[case] no_color: Option<&str>, + #[case] no_emoji_env: Option<&str>, + #[case] expected_emoji: bool, +) { + let env = fake_env(no_color, no_emoji_env); + assert_eq!(resolve_with(no_emoji, env).emoji_allowed(), expected_emoji); +} + +#[fixture] +fn en_us_localizer() -> Result { + let lock = localizer_test_lock()?; + let localizer = Arc::from(cli_localization::build_localizer(Some("en-US"))); + let guard = localization::set_localizer_for_tests(localizer); + Ok(EnUsLocalizerFixture { + _lock: lock, + _guard: guard, + }) +} + +#[rstest] +#[case::unicode_theme_overrides_no_emoji_env(ThemeResolutionCase { + theme: Some(ThemePreference::Unicode), + no_emoji: None, + mode: OutputMode::Standard, + no_color: None, + no_emoji_env: Some("1"), + expected_emoji: true, +})] +#[case::ascii_theme_stays_ascii_without_env(ThemeResolutionCase { + theme: Some(ThemePreference::Ascii), + no_emoji: None, + mode: OutputMode::Standard, + no_color: None, + no_emoji_env: None, + expected_emoji: false, +})] +#[case::auto_theme_no_color_forces_ascii(ThemeResolutionCase { + theme: Some(ThemePreference::Auto), + no_emoji: None, + mode: OutputMode::Standard, + no_color: Some("1"), + no_emoji_env: None, + expected_emoji: false, +})] +#[case::auto_theme_standard_without_env_uses_unicode(ThemeResolutionCase { + theme: Some(ThemePreference::Auto), + no_emoji: None, + mode: OutputMode::Standard, + no_color: None, + no_emoji_env: None, + expected_emoji: true, +})] +#[case::auto_theme_legacy_no_emoji_stays_ascii(ThemeResolutionCase { + theme: Some(ThemePreference::Auto), + no_emoji: Some(true), + mode: OutputMode::Standard, + no_color: None, + no_emoji_env: None, + expected_emoji: false, +})] +fn resolve_from_theme_with_uses_theme_resolution(#[case] case: ThemeResolutionCase<'_>) { + let env = fake_env(case.no_color, case.no_emoji_env); + let prefs = resolve_from_theme_with(case.theme, case.no_emoji, case.mode, env); + assert_eq!(prefs.emoji_allowed(), case.expected_emoji); +} + +#[rstest] +#[case::unicode_error(Some(ThemePreference::Unicode), OutputPrefs::error_prefix, "✖ Error:")] +#[case::ascii_error(Some(ThemePreference::Ascii), OutputPrefs::error_prefix, "X Error:")] +#[case::unicode_success( + Some(ThemePreference::Unicode), + OutputPrefs::success_prefix, + "✔ Success:" +)] +#[case::ascii_success( + Some(ThemePreference::Ascii), + OutputPrefs::success_prefix, + "+ Success:" +)] +#[case::unicode_warning( + Some(ThemePreference::Unicode), + OutputPrefs::warning_prefix, + "⚠ Warning:" +)] +#[case::ascii_warning( + Some(ThemePreference::Ascii), + OutputPrefs::warning_prefix, + "! Warning:" +)] +#[case::unicode_info(Some(ThemePreference::Unicode), OutputPrefs::info_prefix, "ℹ Info:")] +#[case::ascii_info(Some(ThemePreference::Ascii), OutputPrefs::info_prefix, "i Info:")] +#[case::unicode_timing( + Some(ThemePreference::Unicode), + OutputPrefs::timing_prefix, + "⏱ Timing:" +)] +#[case::ascii_timing(Some(ThemePreference::Ascii), OutputPrefs::timing_prefix, "T Timing:")] +fn prefix_rendering_uses_theme_symbols( + en_us_localizer: Result, + #[case] theme: Option, + #[case] prefix_fn: fn(OutputPrefs) -> String, + #[case] expected: &str, +) -> Result<()> { + let _localizer = en_us_localizer?; + let prefs = resolve_from_theme_with(theme, None, OutputMode::Standard, |_| None); + ensure!( + prefix_fn(prefs) == expected, + "prefix output should match the pinned en-US expectation" + ); + Ok(()) +} + +#[rstest] +#[case::accessible_auto(OutputMode::Accessible)] +#[case::standard_ascii(OutputMode::Standard)] +fn spacing_accessors_follow_resolved_theme(#[case] mode: OutputMode) { + let prefs = resolve_from_theme_with(Some(ThemePreference::Auto), Some(true), mode, |_| None); + assert_eq!(prefs.task_indent(), " "); + assert_eq!(prefs.timing_indent(), " "); +} +struct EnUsLocalizerFixture { + _lock: MutexGuard<'static, ()>, + _guard: LocalizerGuard, +} +#[derive(Debug)] +struct EnUsLocalizerFixtureError(String); + +impl fmt::Display for EnUsLocalizerFixtureError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(&self.0) + } +} + +impl Error for EnUsLocalizerFixtureError {} + +impl From>> for EnUsLocalizerFixtureError { + fn from(err: std::sync::PoisonError>) -> Self { + Self(err.to_string()) + } +} diff --git a/src/status.rs b/src/status.rs index d857f743..62ed2a7f 100644 --- a/src/status.rs +++ b/src/status.rs @@ -56,7 +56,6 @@ use pipeline::PIPELINE_STAGE_TOTAL; pub use pipeline::{PipelineStage, report_pipeline_stage}; pub use timing::VerboseTimingReporter; -const TASK_INDENT: &str = " "; fn stage_label(current: StageNumber, total: StageNumber, description: &str) -> String { localization::message(keys::STATUS_STAGE_LABEL) .with_arg("current", current.get().to_string()) @@ -167,7 +166,7 @@ impl StatusReporter for AccessibleReporter { .writer .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); - drop(writeln!(w, "{TASK_INDENT}{message}")); + drop(writeln!(w, "{}{message}", self.prefs.task_indent())); } } diff --git a/src/status_tests.rs b/src/status_tests.rs index 2c3ff175..2eb60985 100644 --- a/src/status_tests.rs +++ b/src/status_tests.rs @@ -1,20 +1,32 @@ //! Tests for status stage modelling and index conversions. +// FIXME: Remove this module-level allow once rstest supports #[expect] in macro +// expansions or we refactor tests to avoid >4 parameters. +// Tracking: https://github.com/leynos/netsuke/issues/TBD +// +// rstest macro generates code that triggers `too_many_arguments` lint, but the +// `#[expect]` attribute cannot suppress lints from macro expansions. Since this +// lint is expected and cannot be suppressed at the function level, we must use +// `#[allow]` at the module level as a last resort. +#![allow( + clippy::too_many_arguments, + reason = "rstest macro expansion generates >4 params; #[expect] doesn't work in macros" +)] + use super::*; +use crate::cli_localization; +use crate::localization::{self, LocalizerGuard}; use crate::output_prefs; +use anyhow::{Result, ensure}; use rstest::{fixture, rstest}; +use std::sync::{Arc, MutexGuard}; +use test_support::fluent::normalize_fluent_isolates; +use test_support::localizer::localizer_test_lock; fn test_prefs() -> crate::output_prefs::OutputPrefs { output_prefs::resolve_with(None, |_| None) } -fn strip_isolates(value: &str) -> String { - value - .chars() - .filter(|ch| !matches!(ch, '\u{2068}' | '\u{2069}')) - .collect() -} - fn stage6_message(reporter: &IndicatifReporter) -> String { let state = reporter .state @@ -27,6 +39,23 @@ fn stage6_message(reporter: &IndicatifReporter) -> String { .message() } +struct EnUsLocalizerFixture { + _lock: MutexGuard<'static, ()>, + _guard: LocalizerGuard, +} + +#[fixture] +fn en_us_localizer() -> Result { + let lock = localizer_test_lock() + .map_err(|err| anyhow::anyhow!("failed to acquire localizer test lock: {err}"))?; + let localizer = Arc::from(cli_localization::build_localizer(Some("en-US"))); + let guard = localization::set_localizer_for_tests(localizer); + Ok(EnUsLocalizerFixture { + _lock: lock, + _guard: guard, + }) +} + #[fixture] fn force_text_reporter() -> IndicatifReporter { IndicatifReporter::with_force_text_task_updates(test_prefs(), true) @@ -83,28 +112,42 @@ fn localization_key_from_static_str() { #[case(1, 2, "cc -c src/main.c", "Task 1/2: cc -c src/main.c")] #[case(2, 2, "", "Task 2/2")] fn task_progress_update_formats_expected_text( + en_us_localizer: Result, #[case] current: u32, #[case] total: u32, #[case] description: &str, #[case] expected: &str, -) { +) -> Result<()> { + let _localizer = en_us_localizer?; let rendered = task_progress_update(current, total, description); - assert_eq!(strip_isolates(&rendered), expected); + ensure!( + normalize_fluent_isolates(&rendered) == expected, + "task progress text should match the pinned en-US expectation" + ); + Ok(()) } #[rstest] fn indicatif_reporter_ignores_task_updates_when_stage6_is_not_running( + en_us_localizer: Result, force_text_reporter: IndicatifReporter, -) { +) -> Result<()> { + let _localizer = en_us_localizer?; force_text_reporter.report_task_progress(1, 2, "cc -c src/a.c"); let stage6_message = stage6_message(&force_text_reporter); - assert!(!strip_isolates(&stage6_message).contains("Task 1/2")); + ensure!( + !normalize_fluent_isolates(&stage6_message).contains("Task 1/2"), + "stage 6 should not include task progress before the stage is running" + ); + Ok(()) } #[rstest] fn indicatif_reporter_sets_stage6_bar_message_for_non_text_updates( + en_us_localizer: Result, running_stage6_reporter: IndicatifReporter, -) { +) -> Result<()> { + let _localizer = en_us_localizer?; running_stage6_reporter.report_task_progress(1, 2, "cc -c src/a.c"); let stage6_message = stage6_message(&running_stage6_reporter); let task = task_progress_update(1, 2, "cc -c src/a.c"); @@ -119,11 +162,18 @@ fn indicatif_reporter_sets_stage6_bar_message_for_non_text_updates( .with_arg("label", stage_line) .with_arg("task_progress", &task) .to_string(); - assert_eq!(strip_isolates(&stage6_message), strip_isolates(&expected)); + ensure!( + normalize_fluent_isolates(&stage6_message) == normalize_fluent_isolates(&expected), + "stage 6 message should match expected format" + ); + Ok(()) } #[rstest] -fn accessible_reporter_formats_stage_with_info_prefix() { +fn accessible_reporter_formats_stage_with_info_prefix( + en_us_localizer: Result, +) -> Result<()> { + let _localizer = en_us_localizer?; let prefs = test_prefs(); let reporter = AccessibleReporter::with_writer(prefs, Vec::new()); reporter.report_stage( @@ -136,20 +186,24 @@ fn accessible_reporter_formats_stage_with_info_prefix() { .writer .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); - let line = strip_isolates(&String::from_utf8_lossy(&output)); - let info_prefix = strip_isolates(&prefs.info_prefix().to_string()); - assert!( + let line = normalize_fluent_isolates(&String::from_utf8_lossy(&output)); + let info_prefix = normalize_fluent_isolates(&prefs.info_prefix()); + ensure!( line.starts_with(&info_prefix), "stage line should start with info prefix; line was: {line:?}, prefix was: {info_prefix:?}" ); - assert!( + ensure!( line.contains("Stage 1/6: Reading manifest file"), "stage line should contain the stage label; line was: {line:?}" ); + Ok(()) } #[rstest] -fn accessible_reporter_indents_task_progress() { +fn accessible_reporter_indents_task_progress( + en_us_localizer: Result, +) -> Result<()> { + let _localizer = en_us_localizer?; let prefs = test_prefs(); let reporter = AccessibleReporter::with_writer(prefs, Vec::new()); reporter.report_task_progress(1, 2, "cc -c src/main.c"); @@ -158,28 +212,33 @@ fn accessible_reporter_indents_task_progress() { .writer .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); - let line = strip_isolates(&String::from_utf8_lossy(&output)); - let info_prefix = strip_isolates(&prefs.info_prefix().to_string()); - assert!( - line.starts_with(TASK_INDENT), - "task line should be indented by two spaces; line was: {line:?}" + let line = normalize_fluent_isolates(&String::from_utf8_lossy(&output)); + let info_prefix = normalize_fluent_isolates(&prefs.info_prefix()); + ensure!( + line.starts_with(prefs.task_indent()), + "task line should be indented by the resolved task token; line was: {line:?}" ); - assert!( + ensure!( !line.trim_start().starts_with(&info_prefix), "task line should not include info prefix; line was: {line:?}, prefix was: {info_prefix:?}" ); + Ok(()) } #[rstest] -fn completion_line_includes_success_prefix() { +fn completion_line_includes_success_prefix( + en_us_localizer: Result, +) -> Result<()> { + let _localizer = en_us_localizer?; let prefs = test_prefs(); - let line = strip_isolates(&format_completion_line( + let line = normalize_fluent_isolates(&format_completion_line( prefs, LocalizationKey::new(keys::STATUS_TOOL_MANIFEST), )); - let success_prefix = strip_isolates(&prefs.success_prefix().to_string()); - assert!( + let success_prefix = normalize_fluent_isolates(&prefs.success_prefix()); + ensure!( line.starts_with(&success_prefix), "completion line should start with success prefix; line was: {line:?}, prefix was: {success_prefix:?}" ); + Ok(()) } diff --git a/src/status_timing.rs b/src/status_timing.rs index 78a1f5fb..52cde1d4 100644 --- a/src/status_timing.rs +++ b/src/status_timing.rs @@ -1,6 +1,6 @@ //! Verbose timing summary support for status reporting. -use super::{LocalizationKey, StageNumber, StatusReporter, TASK_INDENT}; +use super::{LocalizationKey, StageNumber, StatusReporter}; use crate::localization::{self, keys}; use crate::output_prefs::OutputPrefs; use std::io::{self, Write}; @@ -173,7 +173,7 @@ fn render_summary_lines(prefs: OutputPrefs, entries: &[CompletedStage]) -> Vec Vec OutputPrefs { output_prefs::resolve_with(None, |_| None) } -fn strip_isolates(value: &str) -> String { - value - .chars() - .filter(|ch| !matches!(ch, '\u{2068}' | '\u{2069}')) - .collect() -} - #[derive(Debug)] struct FakeClock { values: Mutex>, @@ -89,21 +83,24 @@ fn timing_recorder_renders_happy_path_summary(test_prefs: OutputPrefs) { let [header, stage1, stage2, stage3, total_line] = lines.as_slice() else { panic!("expected 5 timing summary lines"); }; - assert!(strip_isolates(header).contains("Timing:")); - assert!(strip_isolates(header).contains("Stage timing summary:")); + assert!(normalize_fluent_isolates(header).contains("Timing:")); + assert!(normalize_fluent_isolates(header).contains("Stage timing summary:")); assert_eq!( - strip_isolates(stage1), + normalize_fluent_isolates(stage1), " - Stage 1/6: Reading manifest file: 12ms" ); assert_eq!( - strip_isolates(stage2), + normalize_fluent_isolates(stage2), " - Stage 2/6: Parsing YAML document: 4ms" ); assert_eq!( - strip_isolates(stage3), + normalize_fluent_isolates(stage3), " - Stage 3/6: Expanding template directives: 7ms" ); - assert_eq!(strip_isolates(total_line), " Total pipeline time: 23ms"); + assert_eq!( + normalize_fluent_isolates(total_line), + " Total pipeline time: 23ms" + ); } #[rstest] @@ -186,11 +183,11 @@ fn verbose_timing_reporter_finalizes_current_stage_on_complete(test_prefs: Outpu let [header, stage_line, total_line] = lines.as_slice() else { panic!("expected 3 timing summary lines"); }; - assert!(strip_isolates(header).contains("Timing:")); - assert!(strip_isolates(header).contains("Stage timing summary:")); - assert!(strip_isolates(stage_line).contains("Stage 1/6: Reading manifest file")); - assert!(strip_isolates(stage_line).ends_with(": 15ms")); - assert!(strip_isolates(total_line).contains("Total pipeline time: 15ms")); + assert!(normalize_fluent_isolates(header).contains("Timing:")); + assert!(normalize_fluent_isolates(header).contains("Stage timing summary:")); + assert!(normalize_fluent_isolates(stage_line).contains("Stage 1/6: Reading manifest file")); + assert!(normalize_fluent_isolates(stage_line).ends_with(": 15ms")); + assert!(normalize_fluent_isolates(total_line).contains("Total pipeline time: 15ms")); } #[derive(Debug, Default)] diff --git a/src/theme.rs b/src/theme.rs index 5d1e561d..276e9118 100644 --- a/src/theme.rs +++ b/src/theme.rs @@ -111,7 +111,7 @@ pub enum SemanticColour { /// Symbol tokens for status indicators. /// /// These tokens control which glyphs appear in prefixes and progress output. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct SymbolTokens { /// Symbol for error messages (✖ or X). pub error: &'static str, @@ -128,7 +128,7 @@ pub struct SymbolTokens { /// Spacing tokens for indentation and layout. /// /// These tokens centralize spacing decisions so reporters stay consistent. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct SpacingTokens { /// Indentation for task progress under stage headers (currently 2 spaces). pub task_indent: &'static str, @@ -137,7 +137,7 @@ pub struct SpacingTokens { } /// Complete design token set for a resolved theme. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct DesignTokens { /// Semantic colour tokens. pub colours: ColourTokens, @@ -150,7 +150,7 @@ pub struct DesignTokens { } /// Resolved theme including all design tokens. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct ResolvedTheme { /// The complete token set for this theme. pub tokens: DesignTokens, diff --git a/tests/bdd/steps/accessibility_preferences.rs b/tests/bdd/steps/accessibility_preferences.rs index f78cea18..c6c3a279 100644 --- a/tests/bdd/steps/accessibility_preferences.rs +++ b/tests/bdd/steps/accessibility_preferences.rs @@ -6,6 +6,7 @@ //! stored in [`TestWorld`] rather than mutating the real process environment. use crate::bdd::fixtures::TestWorld; +use crate::bdd::helpers::assertions::normalize_fluent_isolates; use crate::bdd::types::EnvVarValue; use anyhow::{Result, ensure}; use netsuke::output_prefs; @@ -112,17 +113,17 @@ fn render_prefix_with( #[when("the error prefix is rendered")] fn render_error_prefix(world: &TestWorld) -> Result<()> { - render_prefix_with(world, |prefs| prefs.error_prefix().to_string()) + render_prefix_with(world, |prefs| prefs.error_prefix()) } #[when("the warning prefix is rendered")] fn render_warning_prefix(world: &TestWorld) -> Result<()> { - render_prefix_with(world, |prefs| prefs.warning_prefix().to_string()) + render_prefix_with(world, |prefs| prefs.warning_prefix()) } #[when("the success prefix is rendered")] fn render_success_prefix(world: &TestWorld) -> Result<()> { - render_prefix_with(world, |prefs| prefs.success_prefix().to_string()) + render_prefix_with(world, |prefs| prefs.success_prefix()) } // --------------------------------------------------------------------------- @@ -159,8 +160,10 @@ fn prefix_contains(world: &TestWorld, expected: EnvVarValue) -> Result<()> { .get() .ok_or_else(|| anyhow::anyhow!("prefix has not been rendered"))?; let expected_str = expected.as_str(); + let normalized_rendered = normalize_fluent_isolates(&rendered); + let normalized_expected = normalize_fluent_isolates(expected_str); ensure!( - rendered.contains(expected_str), + normalized_rendered.contains(&normalized_expected), "expected prefix to contain '{expected_str}', got '{rendered}'" ); Ok(()) @@ -172,8 +175,9 @@ fn prefix_is_ascii(world: &TestWorld) -> Result<()> { .rendered_prefix .get() .ok_or_else(|| anyhow::anyhow!("prefix has not been rendered"))?; + let normalized_rendered = normalize_fluent_isolates(&rendered); ensure!( - rendered.is_ascii(), + normalized_rendered.is_ascii(), "expected ASCII-only prefix, got '{rendered}'" ); Ok(()) @@ -185,8 +189,9 @@ fn prefix_has_non_ascii(world: &TestWorld) -> Result<()> { .rendered_prefix .get() .ok_or_else(|| anyhow::anyhow!("prefix has not been rendered"))?; + let normalized_rendered = normalize_fluent_isolates(&rendered); ensure!( - !rendered.is_ascii(), + !normalized_rendered.is_ascii(), "expected non-ASCII (emoji) characters in prefix, got '{rendered}'" ); Ok(()) diff --git a/tests/bdd/steps/cli.rs b/tests/bdd/steps/cli.rs index 2b2286a3..a39dccdc 100644 --- a/tests/bdd/steps/cli.rs +++ b/tests/bdd/steps/cli.rs @@ -5,6 +5,7 @@ //! Steps store results in [`TestWorld`] for downstream assertions. use crate::bdd::fixtures::{RefCellOptionExt, TestWorld}; +use crate::bdd::helpers::assertions::normalize_fluent_isolates; use crate::bdd::helpers::parse_store::store_parse_outcome; use crate::bdd::helpers::tokens::build_tokens; use crate::bdd::types::{CliArgs, ErrorFragment, JobCount, PathString, TargetName, UrlString}; @@ -228,8 +229,10 @@ fn verify_cli_policy_rejects( let Err(err) = policy.evaluate(&parsed) else { bail!("expected CLI policy to reject {}", url); }; + let normalized_error = normalize_fluent_isolates(&err.to_string()); + let normalized_message = normalize_fluent_isolates(message.as_str()); ensure!( - err.to_string().contains(message.as_str()), + normalized_error.contains(&normalized_message), "expected error to mention '{}', got '{err}'", message, ); @@ -257,8 +260,10 @@ fn verify_error_contains(world: &TestWorld, fragment: &ErrorFragment) -> Result< .cli_error .get() .context("no error was returned by CLI parsing")?; + let normalized_error = normalize_fluent_isolates(&error); + let normalized_fragment = normalize_fluent_isolates(fragment.as_str()); ensure!( - error.contains(fragment.as_str()), + normalized_error.contains(&normalized_fragment), "Error message '{error}' does not contain expected '{}'", fragment ); diff --git a/tests/cli_tests.rs b/tests/cli_tests.rs new file mode 100644 index 00000000..1adb9621 --- /dev/null +++ b/tests/cli_tests.rs @@ -0,0 +1,8 @@ +//! CLI integration test harness. +//! +//! This file activates the `tests/cli_tests/` module tree as a Cargo +//! integration test target so parsing, merge, locale, and policy tests run +//! under `cargo test` and `make test`. + +#[path = "cli_tests/mod.rs"] +mod cli_tests; diff --git a/tests/cli_tests/locale.rs b/tests/cli_tests/locale.rs index ae818d0d..1a68a3dd 100644 --- a/tests/cli_tests/locale.rs +++ b/tests/cli_tests/locale.rs @@ -3,7 +3,7 @@ use anyhow::{Context, Result, ensure}; use rstest::rstest; -use crate::helpers::os_args; +use crate::cli_tests::helpers::os_args; use netsuke::cli::{diag_json_hint_from_args, locale_hint_from_args}; use netsuke::cli_localization; use std::sync::Arc; diff --git a/tests/cli_tests/merge.rs b/tests/cli_tests/merge.rs index 32e65c5f..2f85e8b9 100644 --- a/tests/cli_tests/merge.rs +++ b/tests/cli_tests/merge.rs @@ -1,11 +1,12 @@ //! Configuration merge tests. //! -//! These tests validate OrthoConfig layer precedence (defaults, file, env, +//! These tests validate `OrthoConfig` layer precedence (defaults, file, env, //! CLI) and list-value appending. use anyhow::{Context, Result, ensure}; use netsuke::cli::Cli; use netsuke::cli_localization; +use netsuke::theme::ThemePreference; use ortho_config::{MergeComposer, sanitize_value}; use rstest::{fixture, rstest}; use serde_json::json; @@ -18,23 +19,21 @@ use test_support::{EnvVarGuard, env_lock::EnvLock}; #[fixture] fn default_cli_json() -> Result { - sanitize_value(&Cli::default()) + Ok(sanitize_value(&Cli::default())?) } -#[rstest] -fn cli_merge_layers_respects_precedence_and_appends_lists( - default_cli_json: Result, -) -> Result<()> { +fn build_precedence_and_append_composer(defaults: serde_json::Value) -> MergeComposer { let mut composer = MergeComposer::new(); - let mut defaults = default_cli_json?; - let defaults_object = defaults - .as_object_mut() - .context("defaults should be an object")?; + let mut seeded_defaults = defaults; + let Some(defaults_object) = seeded_defaults.as_object_mut() else { + panic!("defaults should be an object"); + }; defaults_object.insert("jobs".to_owned(), json!(1)); defaults_object.insert("fetch_allow_scheme".to_owned(), json!(["https"])); defaults_object.insert("progress".to_owned(), json!(true)); defaults_object.insert("diag_json".to_owned(), json!(false)); - composer.push_defaults(defaults); + defaults_object.insert("theme".to_owned(), json!("auto")); + composer.push_defaults(seeded_defaults); composer.push_file( json!({ "file": "Configfile", @@ -42,7 +41,8 @@ fn cli_merge_layers_respects_precedence_and_appends_lists( "fetch_allow_scheme": ["http"], "locale": "en-US", "progress": false, - "diag_json": true + "diag_json": true, + "theme": "ascii" }), None, ); @@ -50,16 +50,21 @@ fn cli_merge_layers_respects_precedence_and_appends_lists( "jobs": 3, "fetch_allow_scheme": ["ftp"], "progress": true, - "diag_json": false + "diag_json": false, + "theme": "unicode" })); composer.push_cli(json!({ "jobs": 4, "fetch_allow_scheme": ["git"], "progress": false, "diag_json": true, + "theme": "ascii", "verbose": true })); - let merged = Cli::merge_from_layers(composer.layers())?; + composer +} + +fn assert_precedence_and_append_invariants(merged: &Cli) -> Result<()> { ensure!( merged.file.as_path() == Path::new("Configfile"), "file layer should override defaults", @@ -81,10 +86,23 @@ fn cli_merge_layers_respects_precedence_and_appends_lists( merged.locale.as_deref() == Some("en-US"), "file layer should populate locale when CLI does not override", ); + ensure!( + merged.theme == Some(ThemePreference::Ascii), + "CLI layer should override theme selection", + ); ensure!(merged.verbose, "CLI layer should set verbose"); Ok(()) } +#[rstest] +fn cli_merge_layers_respects_precedence_and_appends_lists( + default_cli_json: Result, +) -> Result<()> { + let composer = build_precedence_and_append_composer(default_cli_json?); + let merged = Cli::merge_from_layers(composer.layers())?; + assert_precedence_and_append_invariants(&merged) +} + #[rstest] fn cli_merge_with_config_respects_precedence_and_skips_empty_cli_layer() -> Result<()> { let _env_lock = EnvLock::acquire(); @@ -99,11 +117,13 @@ fetch_default_deny = true locale = "es-ES" progress = false diag_json = true +theme = "ascii" "#; 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 _theme_guard = EnvVarGuard::set("NETSUKE_THEME", OsStr::new("unicode")); let _scheme_guard = EnvVarGuard::remove("NETSUKE_FETCH_ALLOW_SCHEME"); let localizer = Arc::from(cli_localization::build_localizer(None)); @@ -145,6 +165,10 @@ diag_json = true merged.progress == Some(false), "config progress should apply when CLI and env do not override", ); + ensure!( + merged.theme == Some(ThemePreference::Unicode), + "environment theme should override config when CLI has no value", + ); ensure!( merged.diag_json, "config diag_json should apply when CLI and env do not override", @@ -153,6 +177,31 @@ diag_json = true Ok(()) } +#[rstest] +fn cli_merge_with_config_prefers_cli_theme_over_env_and_file() -> Result<()> { + let _env_lock = EnvLock::acquire(); + let temp_dir = tempdir().context("create temporary config directory")?; + let config_path = temp_dir.path().join("netsuke.toml"); + fs::write(&config_path, "theme = \"ascii\"\n").context("write netsuke.toml")?; + + let _config_guard = EnvVarGuard::set("NETSUKE_CONFIG_PATH", config_path.as_os_str()); + let _theme_guard = EnvVarGuard::set("NETSUKE_THEME", OsStr::new("unicode")); + + let localizer = Arc::from(cli_localization::build_localizer(None)); + let (cli, matches) = + netsuke::cli::parse_with_localizer_from(["netsuke", "--theme", "ascii"], &localizer) + .context("parse CLI args for theme override merge")?; + let merged = netsuke::cli::merge_with_config(&cli, &matches) + .context("merge theme across CLI, env, and config layers")? + .with_default_command(); + + ensure!( + merged.theme == Some(ThemePreference::Ascii), + "CLI theme should override env and config layers", + ); + Ok(()) +} + #[rstest] fn cli_merge_layers_prefers_cli_then_env_then_file_for_locale( default_cli_json: Result, diff --git a/tests/cli_tests/parsing.rs b/tests/cli_tests/parsing.rs index b77980ac..b249c0aa 100644 --- a/tests/cli_tests/parsing.rs +++ b/tests/cli_tests/parsing.rs @@ -1,12 +1,14 @@ //! CLI parsing coverage. use anyhow::{Context, Result, ensure}; -use clap::Parser; use clap::error::ErrorKind; -use netsuke::cli::{BuildArgs, Cli, Commands}; +use netsuke::cli::{BuildArgs, Commands}; +use netsuke::cli_localization; use netsuke::host_pattern::HostPattern; +use netsuke::theme::ThemePreference; use rstest::rstest; use std::path::PathBuf; +use std::sync::Arc; struct CliCase { argv: Vec<&'static str>, @@ -21,6 +23,7 @@ struct CliCase { block_host: Vec<&'static str>, default_deny: bool, progress: Option, + theme: Option, expected_cmd: Commands, } @@ -39,6 +42,7 @@ impl Default for CliCase { block_host: Vec::new(), default_deny: false, progress: None, + theme: None, expected_cmd: Commands::Build(BuildArgs { emit: None, targets: Vec::new(), @@ -70,6 +74,21 @@ impl Default for CliCase { progress: Some(false), ..CliCase::default() })] +#[case(CliCase { + argv: vec!["netsuke", "--theme", "auto"], + theme: Some(ThemePreference::Auto), + ..CliCase::default() +})] +#[case(CliCase { + argv: vec!["netsuke", "--theme", "ascii"], + theme: Some(ThemePreference::Ascii), + ..CliCase::default() +})] +#[case(CliCase { + argv: vec!["netsuke", "--theme", "unicode"], + theme: Some(ThemePreference::Unicode), + ..CliCase::default() +})] #[case(CliCase { argv: vec!["netsuke", "--locale", "es-ES"], locale: Some("es-ES"), @@ -120,9 +139,10 @@ impl Default for CliCase { ..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(); + let localizer = Arc::from(cli_localization::build_localizer(None)); + let (parsed_cli, _) = netsuke::cli::parse_with_localizer_from(case.argv.clone(), &localizer) + .context("parse CLI arguments")?; + let cli = parsed_cli.with_default_command(); ensure!(cli.file == case.file, "parsed file should match input"); ensure!( cli.directory == case.directory, @@ -177,6 +197,7 @@ fn parse_cli(#[case] case: CliCase) -> Result<()> { cli.progress == case.progress, "progress flag should match input", ); + ensure!(cli.theme == case.theme, "theme flag should match input"); let command = cli.command.context("command should be set")?; ensure!( command == case.expected_cmd, @@ -197,8 +218,10 @@ fn parse_cli(#[case] case: CliCase) -> Result<()> { #[case(vec!["netsuke", "--file", "alt.yml", "-C"], ErrorKind::InvalidValue)] #[case(vec!["netsuke", "manifest"], ErrorKind::MissingRequiredArgument)] #[case(vec!["netsuke", "--locale", "nope"], ErrorKind::ValueValidation)] +#[case(vec!["netsuke", "--theme", "neon"], ErrorKind::ValueValidation)] fn parse_cli_errors(#[case] argv: Vec<&str>, #[case] expected_error: ErrorKind) -> Result<()> { - let err = Cli::try_parse_from(argv) + let localizer = Arc::from(cli_localization::build_localizer(None)); + let err = netsuke::cli::parse_with_localizer_from(argv, &localizer) .err() .context("parser should reject invalid arguments")?; ensure!( diff --git a/tests/cli_tests/policy.rs b/tests/cli_tests/policy.rs index e81d6f81..bd1ebde9 100644 --- a/tests/cli_tests/policy.rs +++ b/tests/cli_tests/policy.rs @@ -5,6 +5,7 @@ use netsuke::cli::Cli; use netsuke::host_pattern::HostPattern; use netsuke::stdlib::NetworkPolicyViolation; use rstest::rstest; +use test_support::fluent::normalize_fluent_isolates; use url::Url; #[rstest] @@ -21,7 +22,7 @@ fn cli_network_policy_defaults_to_https() -> Result<()> { .evaluate(&http) .expect_err("HTTP should be rejected by default"); match err { - NetworkPolicyViolation::SchemeNotAllowed { scheme } => { + NetworkPolicyViolation::SchemeNotAllowed { scheme, .. } => { ensure!(scheme == "http", "unexpected scheme {scheme}"); } other => bail!("expected scheme violation, got {other:?}"), @@ -48,7 +49,7 @@ fn cli_network_policy_default_deny_blocks_unknown_hosts() -> Result<()> { .evaluate(&denied) .expect_err("default deny should block other hosts"); match err { - NetworkPolicyViolation::HostNotAllowlisted { host } => { + NetworkPolicyViolation::HostNotAllowlisted { host, .. } => { ensure!(host == "unauthorised.test", "unexpected host {host}"); } other => bail!("expected allowlist violation, got {other:?}"), @@ -70,10 +71,11 @@ fn cli_network_policy_blocklist_overrides_allowlist() -> Result<()> { .expect_err("blocklist should override allowlist"); let err_text = err.to_string(); match err { - NetworkPolicyViolation::HostBlocked { host } => { + NetworkPolicyViolation::HostBlocked { host, .. } => { ensure!(host == "example.com", "unexpected host {host}"); ensure!( - err_text == "host 'example.com' is blocked", + normalize_fluent_isolates(&err_text) + .contains("Host 'example.com' is blocked by policy."), "unexpected error text: {err_text}", ); } diff --git a/tests/features/cli.feature b/tests/features/cli.feature index 243f7a60..ca938ddb 100644 --- a/tests/features/cli.feature +++ b/tests/features/cli.feature @@ -87,6 +87,16 @@ Feature: CLI parsing Then an error should be returned And the error message should contain "notanumber" + Scenario: Invalid theme value fails validation + When the CLI is parsed with invalid arguments "--theme neon" + Then an error should be returned + And the error message should contain "Invalid theme 'neon'" + + Scenario: Invalid theme value is localised in Spanish + When the CLI is parsed with invalid arguments "--locale es-ES --theme neon" + Then an error should be returned + And the error message should contain "Tema no válido 'neon'" + Scenario: Blocklist overrides allowlist for network policy flags When the CLI is parsed with "--fetch-allow-host example.com --fetch-block-host example.com" Then parsing succeeds diff --git a/tests/features/progress_output.feature b/tests/features/progress_output.feature index bf4652d2..fab9b69b 100644 --- a/tests/features/progress_output.feature +++ b/tests/features/progress_output.feature @@ -41,6 +41,27 @@ Feature: Progress output And stderr should contain "- Stage 1/6:" And stderr should contain "Total pipeline time:" + Scenario: Standard mode honours explicit ASCII theme prefixes + Given a minimal Netsuke workspace + When netsuke is run with arguments "--theme ascii --accessible false --progress true manifest -" + Then the command should succeed + And stderr should contain "+ Success:" + And stderr should not contain "✔ Success:" + + Scenario: Accessible mode honours explicit Unicode theme prefixes + Given a minimal Netsuke workspace + When netsuke is run with arguments "--theme unicode --accessible true --progress true manifest -" + Then the command should succeed + And stderr should contain "ℹ Info:" + And stderr should contain "✔ Success:" + + Scenario: Verbose mode honours explicit ASCII timing prefixes + Given a minimal Netsuke workspace + When netsuke is run with arguments "--theme ascii --accessible false --progress true --verbose manifest -" + Then the command should succeed + And stderr should contain "T Timing:" + And stderr should not contain "⏱ Timing:" + Scenario: Stage summaries localize to Spanish with success prefix Given a minimal Netsuke workspace When netsuke is run with arguments "--accessible false --locale es-ES --progress true manifest -"