Skip to content

Commit 7799b5f

Browse files
Define design tokens; refactor CLI modules and theme wiring (#275)
* docs(execplans): add detailed ExecPlan for defining CLI theme design tokens Introduce a comprehensive execution plan (ExecPlan 3.12.1) that outlines the approach to creating a tokenized theme layer for CLI output. The plan covers purpose, constraints, tolerances, risks, progress steps, decisions, and anticipated outcomes. It establishes goals for resolving themes via OrthoConfig, centralizing design tokens for colors, symbols, and spacing, and preserving accessibility and backward compatibility. This foundational documentation sets the stage for subsequent implementation stages, testing, and validation required to build a robust CLI theme system. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * feat(theme): add CLI theme support with token resolution system Introduce a new theme system controlling CLI output symbols, spacing, and semantic colors. Add ThemePreference enum with options auto, unicode, ascii and integrate CLI --theme flag. Implement theme resolution pipeline considering CLI input, no_emoji legacy flag, environment, and output mode. Update output preferences to delegate to theme tokens while preserving backward compatibility. Add localization keys and validation errors for theme parsing. Include 12 unit tests for theme resolution precedence and token consistency. This forms the foundation for future theme-driven CLI presentation and improved customization. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * docs(execplans): update execplan for design tokens and theme module progress - Clarify completion status of theme module and associated enums/types - Adjust formatting and line breaks for improved readability - Note deferred work on reporter integration and BDD coverage Also include minor build.rs and theme.rs adjustments for theme resolution fn type references and clippy test attribute. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * docs(theme): fix formatting in design tokens doc and add Default derive to ThemePreference - Corrected line breaks and spacing in docs/execplans/3-12-1-define-design-tokens.md for readability. - Added #[derive(Default)] and #[default] attribute to ThemePreference enum to consolidate default implementation and remove manual impl. - Removed redundant Default impl from src/theme.rs. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * feat(cli,theme): add localized CLI validation and enhance theme resolution - Introduce localized parsing with `LocalizedValueParser` to provide CLI argument validation errors with localized messages. - Move CLI validation parser configuration to a dedicated `validation` module for better organization. - Enhance theme parsing with robust error handling and add unit tests to cover valid and invalid inputs. - Refactor `ThemePreference` to centralize parsing logic with a `parse_raw` method and a canonical list of valid options. - Update theme resolution logic to consider the new `NO_COLOR` environment variable alongside `NETSUKE_NO_EMOJI` for ASCII symbol enforcement. - Adjust `OutputPrefs` to reflect whether emoji are allowed based on the resolved theme. - Revise and extend tests to cover new environment variable influences and updated theme resolution precedence. These changes improve CLI UX by providing localized validation feedback and precisely control CLI theme presentation based on configuration and environment. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * feat(cli): add layered configuration merging and diag_json resolution - Introduce `config_merge` module to handle CLI configuration discovery and merging - Support layering of default, file, environment, and CLI overrides - Preserve diagnostic JSON flag from merged layers for early error reporting - Refactor CLI module to use new config merging utilities - Add tests for theme resolution and output preferences This improves configuration handling flexibility by cleanly merging multiple sources, enabling consistent CLI behavior and localization support. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> * refactor(output_prefs): fix env var precedence and update resolution docs - Corrected environment variable precedence in theme resolution tests to properly handle NO_COLOR before NETSUKE_NO_EMOJI. - Updated output_prefs resolution documentation to include NO_COLOR in the correct precedence order. - Minor doc corrections in execplans markdown for clarity and punctuation. These changes improve accuracy of environment variable handling in theme resolution, ensuring backward compatibility and enhanced theming logic. Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com> --------- Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
1 parent 28e097d commit 7799b5f

File tree

16 files changed

+1476
-238
lines changed

16 files changed

+1476
-238
lines changed

build.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@ mod host_pattern;
3434
#[path = "src/localization/mod.rs"]
3535
mod localization;
3636

37+
#[path = "src/output_mode.rs"]
38+
mod output_mode;
39+
40+
#[path = "src/theme.rs"]
41+
mod theme;
42+
3743
mod build_l10n_audit;
3844

3945
use host_pattern::{HostPattern, HostPatternError};
@@ -43,6 +49,13 @@ type LocalizedParseFn = fn(
4349
&Arc<dyn ortho_config::Localizer>,
4450
) -> Result<(cli::Cli, ArgMatches), clap::Error>;
4551

52+
type ResolveThemeFn = fn(
53+
Option<theme::ThemePreference>,
54+
Option<bool>,
55+
output_mode::OutputMode,
56+
fn(&str) -> Option<String>,
57+
) -> theme::ResolvedTheme;
58+
4659
fn manual_date() -> String {
4760
let Ok(raw) = env::var("SOURCE_DATE_EPOCH") else {
4861
return FALLBACK_DATE.into();
@@ -102,6 +115,8 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
102115
const _: LocalizedParseFn = cli::parse_with_localizer_from;
103116
const _: fn(&str) -> Result<HostPattern, HostPatternError> = HostPattern::parse;
104117
const _: fn(&HostPattern, host_pattern::HostCandidate<'_>) -> bool = HostPattern::matches;
118+
const _: fn(Option<bool>) -> output_mode::OutputMode = output_mode::resolve;
119+
const _: ResolveThemeFn = theme::resolve_theme;
105120

106121
// Regenerate the manual page when the CLI or metadata changes.
107122
println!("cargo:rerun-if-changed=src/cli/mod.rs");

docs/execplans/3-10-1-guarantee-status-message-ordering.md

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -276,21 +276,18 @@ model provides the necessary ordering guarantees.
276276
`NINJA_STDERR_MARKER`) to avoid coupling to localized UI strings.
277277

278278
2. **Status messages do not contaminate stdout in standard mode**: Verifies
279-
stream
280-
routing in non-accessible mode using the same stable markers.
279+
stream routing in non-accessible mode using the same stable markers.
281280

282281
3. **Build artifacts can be captured via stdout redirection**: Verifies that
283282
`netsuke manifest -` output goes to stdout without status contamination.
284283

285284
Supporting infrastructure added:
286285

287286
- `FakeNinjaConfig` struct in `tests/bdd/steps/progress_output.rs` for
288-
configurable
289-
fixture generation with optional stderr markers.
287+
configurable fixture generation with optional stderr markers.
290288
- `install_fake_ninja_with_config()` function for flexible fixture setup.
291289
- Updated `fake_ninja_emits_stdout_output` fixture to emit both stdout and
292-
stderr
293-
markers for comprehensive stream routing verification.
290+
stderr markers for comprehensive stream routing verification.
294291

295292
### Stage E: Documentation (completed)
296293

docs/execplans/3-12-1-define-design-tokens.md

Lines changed: 582 additions & 0 deletions
Large diffs are not rendered by default.

locales/en-US/messages.ftl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ cli.flag.fetch_default_deny.help = Deny all hosts by default; only allow the dec
1717
cli.flag.accessible.help = Force accessible output mode on or off.
1818
cli.flag.progress.help = Force standard stage and task progress summaries on or off.
1919
cli.flag.diag_json.help = Emit machine-readable diagnostics as JSON on stderr.
20+
cli.flag.theme.help = CLI theme preset (auto, unicode, ascii).
2021

2122
# Subcommand descriptions.
2223
cli.subcommand.build.about = Build targets defined in the manifest (default).
@@ -43,6 +44,7 @@ cli.validation.scheme.invalid_start = Scheme '{ $scheme }' must start with an AS
4344
cli.validation.scheme.invalid = Invalid scheme '{ $scheme }'.
4445
cli.validation.locale.empty = Locale must not be empty.
4546
cli.validation.locale.invalid = Invalid locale '{ $locale }'.
47+
cli.validation.theme.invalid = Invalid theme '{ $theme }'. Valid options: auto, unicode, ascii.
4648
cli.validation.config.expected_object = Expected parsed CLI values to serialize to an object, got { $value }.
4749

4850
# Clap error messages.

locales/es-ES/messages.ftl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ cli.flag.fetch_default_deny.help = Denegar todos los hosts por defecto; solo per
1717
cli.flag.accessible.help = Forzar el modo de salida accesible (activado o desactivado).
1818
cli.flag.progress.help = Forzar los resúmenes de progreso estándar de etapas y tareas (activados o desactivados).
1919
cli.flag.diag_json.help = Emitir diagnósticos legibles por máquinas como JSON en stderr.
20+
cli.flag.theme.help = Tema predefinido de CLI (auto, unicode, ascii).
2021

2122
# Descripciones de subcomandos.
2223
cli.subcommand.build.about = Compila objetivos definidos en el manifiesto (predeterminado).
@@ -43,6 +44,7 @@ cli.validation.scheme.invalid_start = El esquema '{ $scheme }' debe comenzar con
4344
cli.validation.scheme.invalid = Esquema no válido '{ $scheme }'.
4445
cli.validation.locale.empty = La configuración regional no debe estar vacía.
4546
cli.validation.locale.invalid = Configuración regional no válida '{ $locale }'.
47+
cli.validation.theme.invalid = Tema no válido '{ $theme }'. Opciones válidas: auto, unicode, ascii.
4648
cli.validation.config.expected_object = Se esperaba que los valores de la CLI se serializaran como un objeto, se obtuvo { $value }.
4749

4850
# Mensajes de error de Clap.

src/cli/config_merge.rs

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
//! CLI configuration discovery and merge helpers.
2+
//!
3+
//! This module keeps config-layer plumbing out of `cli::mod` so the public CLI
4+
//! surface stays focused on argument definitions and parsing.
5+
6+
use clap::ArgMatches;
7+
use clap::parser::ValueSource;
8+
use ortho_config::declarative::LayerComposition;
9+
use ortho_config::figment::{Figment, providers::Env};
10+
use ortho_config::uncased::Uncased;
11+
use ortho_config::{
12+
ConfigDiscovery, LocalizationArgs, MergeComposer, OrthoMergeExt, OrthoResult, sanitize_value,
13+
};
14+
use std::path::{Path, PathBuf};
15+
use std::sync::Arc;
16+
17+
use crate::localization::{self, keys};
18+
19+
use super::{CONFIG_ENV_VAR, Cli, ENV_PREFIX, validation_message};
20+
21+
/// Return the default manifest filename when none is provided.
22+
pub(super) fn default_manifest_path() -> PathBuf {
23+
PathBuf::from("Netsukefile")
24+
}
25+
26+
/// Return the prefixed environment provider for CLI configuration.
27+
fn env_provider() -> Env {
28+
Env::prefixed(ENV_PREFIX)
29+
}
30+
31+
/// Build configuration discovery rooted in the optional working directory.
32+
fn config_discovery(directory: Option<&Path>) -> ConfigDiscovery {
33+
let mut builder = ConfigDiscovery::builder("netsuke").env_var(CONFIG_ENV_VAR);
34+
if let Some(dir) = directory {
35+
builder = builder.clear_project_roots().add_project_root(dir);
36+
}
37+
builder.build()
38+
}
39+
40+
/// Return `true` when no CLI overrides were supplied.
41+
///
42+
/// The merge pipeline treats an empty JSON object as "no overrides".
43+
fn is_empty_value(value: &serde_json::Value) -> bool {
44+
matches!(value, serde_json::Value::Object(map) if map.is_empty())
45+
}
46+
47+
fn diag_json_from_layer(value: &serde_json::Value) -> Option<bool> {
48+
value
49+
.as_object()
50+
.and_then(|map| map.get("diag_json"))
51+
.and_then(serde_json::Value::as_bool)
52+
}
53+
54+
/// Resolve the effective diagnostic JSON preference from the raw config layers.
55+
///
56+
/// This is used before full config merging so startup and merge-time failures
57+
/// can still honour `diag_json` values sourced from config files or the
58+
/// environment.
59+
#[must_use]
60+
pub fn resolve_merged_diag_json(cli: &Cli, matches: &ArgMatches) -> bool {
61+
let mut diag_json = Cli::default().diag_json;
62+
63+
let discovery = config_discovery(cli.directory.as_deref());
64+
let file_layers = discovery.compose_layers();
65+
for layer in file_layers.value {
66+
let layer_value = layer.into_value();
67+
if let Some(layer_diag_json) = diag_json_from_layer(&layer_value) {
68+
diag_json = layer_diag_json;
69+
}
70+
}
71+
72+
let env_provider = env_provider()
73+
.map(|key| Uncased::new(key.as_str().to_ascii_uppercase()))
74+
.split("__");
75+
if let Ok(value) = Figment::from(env_provider).extract::<serde_json::Value>()
76+
&& let Some(env_diag_json) = diag_json_from_layer(&value)
77+
{
78+
diag_json = env_diag_json;
79+
}
80+
81+
if matches.value_source("diag_json") == Some(ValueSource::CommandLine) {
82+
cli.diag_json
83+
} else {
84+
diag_json
85+
}
86+
}
87+
88+
fn cli_overrides_from_matches(cli: &Cli, matches: &ArgMatches) -> OrthoResult<serde_json::Value> {
89+
let value = sanitize_value(cli)?;
90+
let mut map = match value {
91+
serde_json::Value::Object(map) => map,
92+
other => {
93+
let mut args = LocalizationArgs::default();
94+
args.insert("value", format!("{other:?}").into());
95+
let localizer = localization::localizer();
96+
return Err(Arc::new(ortho_config::OrthoError::Validation {
97+
key: String::from("cli"),
98+
message: validation_message(
99+
localizer.as_ref(),
100+
keys::CLI_CONFIG_EXPECTED_OBJECT,
101+
Some(&args),
102+
&format!("expected parsed CLI values to serialize to an object, got {other:?}"),
103+
),
104+
}));
105+
}
106+
};
107+
108+
map.remove("command");
109+
for field in [
110+
"file",
111+
"verbose",
112+
"fetch_default_deny",
113+
"fetch_allow_scheme",
114+
"fetch_allow_host",
115+
"fetch_block_host",
116+
"accessible",
117+
"progress",
118+
"no_emoji",
119+
"theme",
120+
"diag_json",
121+
] {
122+
if matches.value_source(field) != Some(ValueSource::CommandLine) {
123+
map.remove(field);
124+
}
125+
}
126+
127+
Ok(serde_json::Value::Object(map))
128+
}
129+
130+
/// Merge configuration layers over the parsed CLI values.
131+
///
132+
/// # Errors
133+
///
134+
/// Returns an [`ortho_config::OrthoError`] if layer composition or merging
135+
/// fails.
136+
pub fn merge_with_config(cli: &Cli, matches: &ArgMatches) -> OrthoResult<Cli> {
137+
let command = cli.command.clone();
138+
let mut errors = Vec::new();
139+
let mut composer = MergeComposer::with_capacity(4);
140+
141+
match sanitize_value(&Cli::default()) {
142+
Ok(value) => composer.push_defaults(value),
143+
Err(err) => errors.push(err),
144+
}
145+
146+
let discovery = config_discovery(cli.directory.as_deref());
147+
let mut file_layers = discovery.compose_layers();
148+
errors.append(&mut file_layers.required_errors);
149+
if file_layers.value.is_empty() {
150+
errors.append(&mut file_layers.optional_errors);
151+
}
152+
for layer in file_layers.value {
153+
composer.push_layer(layer);
154+
}
155+
156+
let env_provider = env_provider()
157+
.map(|key| Uncased::new(key.as_str().to_ascii_uppercase()))
158+
.split("__");
159+
match Figment::from(env_provider)
160+
.extract::<serde_json::Value>()
161+
.into_ortho_merge()
162+
{
163+
Ok(value) => composer.push_environment(value),
164+
Err(err) => errors.push(err),
165+
}
166+
167+
match cli_overrides_from_matches(cli, matches) {
168+
Ok(value) if !is_empty_value(&value) => composer.push_cli(value),
169+
Ok(_) => {}
170+
Err(err) => errors.push(err),
171+
}
172+
173+
let composition = LayerComposition::new(composer.layers(), errors);
174+
let mut merged = composition.into_merge_result(Cli::merge_from_layers)?;
175+
merged.command = command;
176+
Ok(merged)
177+
}

0 commit comments

Comments
 (0)