From d829675dfb9a9b42d2257f9fbf547669547c3b4e Mon Sep 17 00:00:00 2001 From: default <216188+jdx@users.noreply.github.com> Date: Fri, 21 Aug 2026 13:17:49 +0000 Subject: [PATCH 1/8] refactor(parse): carry phase-1 bindings on the word MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `prefix_bindings` was a `VecDeque` popped in step with `input`, holding the flag Phase 1 had read each leading word as. Two queues staying aligned is an invariant nothing checks, and it was delicate enough to need explaining at three call sites: `collect_variadic_flag_values` popped it twice for no reason but alignment, and the short-bundle re-queue pushed a `None` to keep the count right. Move it onto the word. `input` becomes a `VecDeque`, Phase 1 writes `input[idx].binding` in place, and Phase 2 reads it off the word it popped. Behaviour-preserving by construction: `prefix_bindings.pop_front().flatten()` returned `None` both for "Phase 1 pushed `None`" and for "Phase 1 never reached this word", and the only consumer that distinguishes anything is the `binding.is_none()` guard on the short-flag arm — which wants exactly that collapsed answer. A per-word `Option` gives the same answer at both sites. Co-Authored-By: Claude Opus 5 --- lib/src/parse.rs | 123 +++++++++++++++++++++++++++-------------------- 1 file changed, 70 insertions(+), 53 deletions(-) diff --git a/lib/src/parse.rs b/lib/src/parse.rs index 24e2d6f64..9e6fb7699 100644 --- a/lib/src/parse.rs +++ b/lib/src/parse.rs @@ -44,7 +44,7 @@ fn merge_subcommand_flags( if crossing_mount { // A mounted command owns its flags outright, including names an inherited global also // uses: a word after the mounted command belongs to the mounted program. Words before - // it keep resolving to the global they were read as, via `prefix_bindings`. Aliases the + // it keep resolving to the global they were read as, via `Token::binding`. Aliases the // mounted command does not declare (e.g. a global's short) stay inherited. for (key, flag) in new_flags { available.insert(key, flag); @@ -793,6 +793,37 @@ enum MountTiming { WhenAWordIsUnknown, } +/// One word on its way through the parser, with what the parser has learned about it. +/// +/// This holds what a side queue used to: the flag Phase 1 read a word as, previously a +/// `VecDeque` popped in step with the words. Two queues staying aligned is an invariant +/// nothing checks, and it was delicate enough to need explaining at three call sites; on +/// the word itself there is nothing to keep aligned. +struct Token { + word: String, + /// The flag Phase 1 read this word as, and the command level it read it at. + /// + /// `Some((flag, command_level))` for a flag word, `None` for its value, for anything + /// unresolved, and for every word Phase 1 never reached. The words stay in the queue + /// for Phase 2 to re-parse — that is how they reach `out.flags` and `as_env()` — but by + /// then the recognized flags have changed, because each descent drops the parent's + /// non-global flags and a mounted command may declare the same name as a global seen + /// here. Recording the owner keeps a word bound to the flag it was read as. + /// + /// The level matters to strict parsing: clap permits an inherited global once on each + /// side of a subcommand boundary. + binding: Option<(Arc, usize)>, +} + +impl Token { + fn new(word: String) -> Self { + Self { + word, + binding: None, + } + } +} + fn parse_partial_with_env( spec: &Spec, input: &[String], @@ -805,12 +836,16 @@ fn parse_partial_with_env( return parse_partial_with_env(&viewed, input, custom_env, mount_outputs, mount_timing); } trace!("parse_partial: {input:?}"); - let mut input = input.iter().cloned().collect::>(); + let mut input = input + .iter() + .cloned() + .map(Token::new) + .collect::>(); let argv0 = input.pop_front(); if spec.multicall { if let Some(raw) = argv0 { - if let Some(applet) = multicall_applet(&raw, &spec.name, Some(spec.bin.as_str())) { - input.push_front(applet.to_string()); + if let Some(applet) = multicall_applet(&raw.word, &spec.name, Some(spec.bin.as_str())) { + input.push_front(Token::new(applet.to_string())); } } } @@ -854,16 +889,8 @@ fn parse_partial_with_env( // - Non-global flags are specific to the current command, not subcommands // - Global flags affect all commands and should be passed to mount points let mut prefix_flags: Vec<(Arc, Vec)> = vec![]; - // Which flag each word skipped here belongs to, aligned with the leading words left in - // `input`: `Some((flag, command_level))` for a flag word, `None` for its value (or - // anything unresolved). The level matters to strict parsing: clap permits an inherited - // global once on each side of a subcommand boundary. - // - // The words stay in `input` for Phase 2 to re-parse — that is how they reach `out.flags` - // and `as_env()` — but by then the recognized flags have changed, because each descent - // drops the parent's non-global flags and a mounted command may declare the same name as - // a global seen here. Recording the owner keeps a word bound to the flag it was read as. - let mut prefix_bindings: VecDeque, usize)>> = VecDeque::new(); + // Which flag each word skipped here belongs to is recorded on the word — see + // `Token::binding`. let mut command_arg_found = false; let mut variadic_flag_active = false; let mut idx = 0; @@ -908,15 +935,15 @@ fn parse_partial_with_env( // outrank it. Without this, a task runner would spawn its discovery process // once per task invocation. let default_catches_it = spec.default_subcommand.as_deref().is_some_and(|name| { - default_accepts_word(&out.cmd, name, &input[idx]) + default_accepts_word(&out.cmd, name, &input[idx].word) && !out.cmd.mounts.iter().any(|m| m.overrides_default) }); if !mounts_resolved && !out.cmd.mounts.is_empty() && !default_catches_it - && is_command_word(&input[idx]) - && !is_negative_number(&input[idx]) - && out.cmd.find_subcommand(&input[idx]).is_none() + && is_command_word(&input[idx].word) + && !is_negative_number(&input[idx].word) + && out.cmd.find_subcommand(&input[idx].word).is_none() { mounts_resolved = true; let mut mounted = out.cmd.clone(); @@ -928,16 +955,16 @@ fn parse_partial_with_env( out.cmd = mounted; } if variadic_flag_active - && out.cmd.find_subcommand(&input[idx]).is_some() + && out.cmd.find_subcommand(&input[idx].word).is_some() && !out.cmd.subcommand_precedence_over_arg { break; } - if let Some(subcommand) = out.cmd.find_subcommand(&input[idx]) { + if let Some(subcommand) = out.cmd.find_subcommand(&input[idx].word) { if out.cmd.args_conflicts_with_subcommands && command_arg_found { bail!( "subcommand '{}' cannot be used with arguments on its parent command", - input[idx] + input[idx].word ); } let mut subcommand = subcommand.clone(); @@ -963,11 +990,11 @@ fn parse_partial_with_env( variadic_flag_active = false; // Continue from current position (don't reset to 0) // After remove(), idx now points to the next element - } else if !is_command_word(&input[idx]) - || declared_numeric_short(&out.available_flags, &input[idx]) + } else if !is_command_word(&input[idx].word) + || declared_numeric_short(&out.available_flags, &input[idx].word) { // Check if this is a known flag - let word = input[idx].clone(); + let word = input[idx].word.clone(); let flag_key = get_flag_key(&word); // A short token keys on its first letter, so `-az` would be recorded as @@ -992,7 +1019,7 @@ fn parse_partial_with_env( // // Only globals are forwarded to mounts: a non-global flag belongs to the // command that declared it, not to what is mounted below it. - prefix_bindings.push_back(Some((Arc::clone(&f), out.cmds.len() - 1))); + input[idx].binding = Some((Arc::clone(&f), out.cmds.len() - 1)); let mut forwarded = f.global.then(|| vec![word.clone()]); idx += 1; @@ -1001,14 +1028,13 @@ fn parse_partial_with_env( if f.arg.is_some() && !word.contains('=') && idx < input.len() - && (!is_flag_like(&input[idx]) + && (!is_flag_like(&input[idx].word) || (f.arg.as_ref().is_some_and(|arg| arg.allow_negative_numbers) - && is_negative_number(&input[idx]))) + && is_negative_number(&input[idx].word))) { if let Some(words) = forwarded.as_mut() { - words.push(input[idx].clone()); + words.push(input[idx].word.clone()); } - prefix_bindings.push_back(None); idx += 1; } if let Some(words) = forwarded { @@ -1022,7 +1048,6 @@ fn parse_partial_with_env( } } else { if variadic_flag_active && out.cmd.subcommand_precedence_over_arg { - prefix_bindings.push_back(None); idx += 1; continue; } @@ -1039,7 +1064,7 @@ fn parse_partial_with_env( if let Some(subcommand) = out .cmd .find_subcommand(default_name) - .filter(|_| default_accepts_word(&out.cmd, default_name, &input[idx])) + .filter(|_| default_accepts_word(&out.cmd, default_name, &input[idx].word)) { if out.cmd.args_conflicts_with_subcommands && command_arg_found { bail!( @@ -1079,7 +1104,7 @@ fn parse_partial_with_env( // subcommands already won above, and a default_subcommand already caught. // clap's `allow_external_subcommands` is this, not `unknown_flags=value`. if out.cmd.external_subcommand { - let rest: Vec = input.drain(idx..).collect(); + let rest: Vec = input.drain(idx..).map(|t| t.word).collect(); out.external = Some(rest); break; } @@ -1114,10 +1139,10 @@ fn parse_partial_with_env( let mut scalar_occurrences: HashMap<(usize, usize), u8> = HashMap::new(); while !input.is_empty() { - let mut w = input.pop_front().unwrap(); - // The flag this word was read as in Phase 1, if it skipped it (see `prefix_bindings`). - // Words pushed back below get a `None` so the two queues stay aligned. - let binding = prefix_bindings.pop_front().flatten(); + let token = input.pop_front().unwrap(); + // The flag this word was read as in Phase 1, if it skipped it (see `Token::binding`). + let binding = token.binding; + let mut w = token.word; // A short's attached value is re-queued with `grouped_flag` set, and that // continuation is not a following word. `require_equals` refuses only the // following word; `-i9229` and `-i=9229` still bind. `default_missing` binds @@ -1171,7 +1196,6 @@ fn parse_partial_with_env( &mut out.flag_awaiting_value, &mut w, &mut input, - &mut prefix_bindings, custom_env, )?; if should_return { @@ -1314,7 +1338,6 @@ fn parse_partial_with_env( &mut out.flag_awaiting_value, &mut val, &mut input, - &mut prefix_bindings, custom_env, )?; if should_return { @@ -1438,8 +1461,7 @@ fn parse_partial_with_env( ); let rest = &w[1 + short.len_utf8()..]; if !rest.is_empty() { - input.push_front(format!("-{rest}")); - prefix_bindings.push_front(None); + input.push_front(Token::new(format!("-{rest}"))); } // A fully consumed short is no longer a grouped continuation. // Leaving this set after `-ai` made `-i` skip `require_equals` @@ -1555,7 +1577,6 @@ fn parse_partial_with_env( &mut out.flag_awaiting_value, &mut w, &mut input, - &mut prefix_bindings, custom_env, )?; if should_return { @@ -1579,7 +1600,7 @@ fn parse_partial_with_env( } let remaining_values = 1 + input .iter() - .filter(|word| !enable_flags || !is_flag_like(word)) + .filter(|token| !enable_flags || !is_flag_like(&token.word)) .count(); if remaining_values > required_after { break; @@ -2975,8 +2996,7 @@ fn bind_pending_flag_value( flags: &mut IndexMap, ParseValue>, flag_awaiting_value: &mut Vec>, word: &mut String, - input: &mut VecDeque, - prefix_bindings: &mut VecDeque, usize)>>, + input: &mut VecDeque, custom_env: Option<&HashMap>, ) -> miette::Result { // Held before the drain pops it, along with what the flag is already carrying: a @@ -3013,7 +3033,6 @@ fn bind_pending_flag_value( &flag, carried, input, - prefix_bindings, custom_env, ) } @@ -3039,8 +3058,7 @@ fn collect_variadic_flag_values( flag_awaiting_value: &mut Vec>, flag: &Arc, carried: usize, - input: &mut VecDeque, - prefix_bindings: &mut VecDeque, usize)>>, + input: &mut VecDeque, custom_env: Option<&HashMap>, ) -> miette::Result { let max = flag @@ -3055,15 +3073,16 @@ fn collect_variadic_flag_values( .saturating_sub(carried) < max { - let Some(next) = input.front() else { break }; + let Some(next) = input.front().map(|token| token.word.as_str()) else { + break; + }; if flag .arg .as_ref() .and_then(|arg| arg.value_terminator.as_deref()) - == Some(next.as_str()) + == Some(next) { input.pop_front(); - prefix_bindings.pop_front(); break; } // The separator is left where it is: stopping here hands it to the arm that @@ -3078,9 +3097,7 @@ fn collect_variadic_flag_values( { break; } - let mut word = input.pop_front().unwrap(); - // The two queues are read in step, so a word taken here takes its binding with it. - prefix_bindings.pop_front(); + let mut word = input.pop_front().unwrap().word; flag_awaiting_value.push(Arc::clone(flag)); if drain_pending_flag_values( spec, From e8cf6b1273a471fe6d89094ae0ea219e9bbce9d4 Mon Sep 17 00:00:00 2001 From: default <216188+jdx@users.noreply.github.com> Date: Fri, 21 Aug 2026 13:34:01 +0000 Subject: [PATCH 2/8] feat(parse)!: record where each value came from MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ParseOutput` said what a command line produced and nothing about how. A value that was typed, a value from `$MYCLI_TOKEN` and a value from `default=` were indistinguishable once parsed, so "why is this set" had no answer — the question behind jdx/mise discussion #8883, where a hand-written scanner silently ignored `mise --env=production` while `mise --env production` worked. Two halves, because neither alone is enough. `tokens` says what each word of argv became — a table keyed by token cannot show a value that came from nowhere in argv. `flag_origins` / `arg_origins` say where a value came from when no token supplied it — a table keyed by declaration cannot show a token that bound to nothing. `Token` now carries its argv position, so a word attributes back to what the caller wrote even after the queue has been popped, re-queued, split on `=` and had subcommand words removed from the middle. Words the parser makes up fold onto the token they came from: `-abj8` is one word that names three flags and a value. Also here, because they are the same question: `overridden_flags` names the flag that did the overriding, which is what "`--quiet` is unset despite its default" needs; and `Parser::explain` returns what the parse learned instead of bailing on the first error, which is the case a report is wanted for. Breaking: `ParseOutput` gains four fields and `#[non_exhaustive]`. Nothing outside this crate constructs one, and the semver gate is off below 6.x by design. Wall clock is not measured here: this machine was under load average 34 and the bench moved 40% on identical code. `usage-argv` is untouched, so the gated instruction counts in benches/gate cannot have moved. Co-Authored-By: Claude Opus 5 --- lib/src/parse.rs | 944 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 893 insertions(+), 51 deletions(-) diff --git a/lib/src/parse.rs b/lib/src/parse.rs index 9e6fb7699..93d5c8ec3 100644 --- a/lib/src/parse.rs +++ b/lib/src/parse.rs @@ -267,11 +267,124 @@ fn get_flag_key(word: &str) -> &str { } } +/// Where a value came from, when it did not come from the command line. +/// +/// About the *value*, not the flag. `--color` typed bare with `default_missing` has a +/// token for the flag and none for the value, and that distinction is the whole question +/// a spec author is asking when they ask why `--color` came out `always`. Values that were +/// typed are attributed to the token that carried them instead — see [`TokenRole::Value`]. +#[derive(Debug, Clone, PartialEq, Eq)] +#[non_exhaustive] +pub enum ValueOrigin { + /// A flag that takes a value was given without one, so the declaration supplied it: + /// `default_missing`, or the empty tri-state a bare `value_optional` flag records. + /// One variant for both, because from argv's side the same thing happened — the flag + /// was typed and the value was not. + DefaultMissing, + /// An environment variable, named. + /// + /// Named because a flag may list several — `env`, `env_fallback` and `deprecated_env`, + /// folded together by [`SpecFlag::env_names`] — and "it came from the environment" does + /// not say which declaration fired or which one to delete. + Env(String), + /// A declared `default`, on the flag or on the flag's argument. + /// + /// Not two variants: the precedence between them is a spec-authoring oddity rather than + /// a fact about the value, and `usage lint` is the place to complain about declaring + /// both. + Default, + /// A `default_if` whose condition matched, with the condition that decided it. The + /// selector alone is ambiguous — several conditions may name it with different `when` + /// values. + DefaultIf { + selector: String, + when: Option, + }, +} + +/// What one word of the command line became. +/// +/// Several because a single token can do more than one thing: `-abc` sets three flags, +/// `-j8` is a flag and its value. +#[derive(Debug, Clone)] +#[non_exhaustive] +pub enum TokenRole { + /// argv[0]. Also a `Command` when a multicall symlink makes the basename a word. + Program, + /// Selected a subcommand. + Command { name: String }, + /// Named a flag, in this spelling. `negated` for the `negate` form. + Flag { + flag: Arc, + spelling: String, + negated: bool, + }, + /// Supplied a flag's value. Several values when a `delimiter` split the word. + Value { + flag: Arc, + values: Vec, + /// Whether the value rode along on the flag's own token (`--env=prod`, `-j8`) + /// rather than following it as its own word. + attached: bool, + }, + /// Filled a positional argument. Several values when a `delimiter` split the word. + Arg { + arg: Arc, + values: Vec, + }, + /// An explicit `--`, consumed as a separator. + Separator, + /// A flag-like word no declaration matched. `bound_as` is the positional that took it + /// under `unknown_flags="value"`, and `None` when the word was refused. + UnknownFlag { bound_as: Option> }, + /// Forwarded to an external subcommand. + External, + /// The parser stopped before this word — a help request, a refused value. + Unread, +} + +/// One word of the command line, and what it became. +#[derive(Debug, Clone)] +#[non_exhaustive] +pub struct TokenBinding { + /// Position in the argv slice the parse was given, argv[0] included. + pub index: usize, + pub word: String, + /// Roles a word the parser made up contributed, folded onto the token it was derived + /// from: the tail of a short bundle onto the bundle, a multicall applet name onto + /// argv[0]. `word` is what the caller wrote, not what the parser read. + pub synthesized: bool, + pub roles: Vec, +} + +#[non_exhaustive] pub struct ParseOutput { pub cmd: SpecCommand, pub cmds: Vec, pub args: IndexMap, ParseValue>, pub flags: IndexMap, ParseValue>, + /// What each word of the command line became, in argv order, one entry per word. + /// + /// The token half of provenance; [`ParseOutput::flag_origins`] and + /// [`ParseOutput::arg_origins`] are the other half. A table keyed by token cannot show + /// a value that came from nowhere in argv, and a table keyed by declaration cannot show + /// a token that bound to nothing, so both exist. + pub tokens: Vec, + /// Where a flag's value came from when it did not come from argv, in the order the + /// fallbacks fired. Keyed as [`ParseOutput::flags`] is. + /// + /// A list rather than one origin: repeated bare occurrences of a `var` flag each take a + /// `default_missing` value, so one flag can have several. + pub flag_origins: IndexMap, Vec>, + /// Where an argument's value came from when it did not come from argv. Keyed as + /// [`ParseOutput::args`] is. + pub arg_origins: IndexMap, Vec>, + /// Flags a later occurrence removed, and the flag that removed them. + /// + /// The overriding name is the half a caller needs: the fallback phase silently declines + /// to fill an overridden flag, so "why is `--quiet` unset when its default says + /// otherwise" has no answer without it. + pub overridden_flags: BTreeMap, /// Every flag the parser recognizes at this point, keyed by each of its aliases /// (`--long`, `-s`, negations). /// @@ -497,6 +610,35 @@ impl<'a> Parser<'a> { /// /// Returns the parsed arguments and flags, with defaults and env vars applied. pub fn parse(self, input: &[String]) -> Result { + let out = self.parse_collecting(input)?; + if let Some(err) = out + .errors + .iter() + .find(|e| matches!(e, UsageErr::Help(_) | UsageErr::Version(_))) + { + bail!("{err}"); + } + if !out.errors.is_empty() { + bail!("{}", out.errors.iter().map(|e| e.to_string()).join("\n")); + } + Ok(out) + } + + /// Everything the parse learned, whether or not it succeeded. + /// + /// [`Parser::parse`] wants the first error and nothing else, which is right for a + /// caller about to act on a command line. A caller that wants to *explain* one wants + /// the opposite: the bindings that worked and every complaint about the rest, since a + /// report saying only "missing required " is the report you already had. + /// + /// Failures that stop the parse dead — a mount that will not run, a word no + /// declaration can take — still come back as `Err`. There is no output to describe in + /// those cases; see [`Parser::explain`] for what to do about it. + pub fn explain(self, input: &[String]) -> Result { + self.parse_collecting(input) + } + + fn parse_collecting(self, input: &[String]) -> Result { let custom_env = self.env.as_ref(); let (mut out, overridden_flags) = parse_partial_with_env( self.spec, @@ -512,7 +654,12 @@ impl<'a> Parser<'a> { // half-typed `--jobs ` is exactly what a completion is asked about — but a // full parse has nothing left to wait for, and dropping the flag silently // made a forgotten value look like a working command. - while try_bind_default_missing(&mut out.flags, &mut out.flag_awaiting_value, custom_env)? {} + while try_bind_default_missing( + &mut out.flags, + &mut out.flag_awaiting_value, + custom_env, + &mut out.flag_origins, + )? {} if let Some(flag) = out.flag_awaiting_value.first() { let token = flag .long @@ -574,6 +721,10 @@ impl<'a> Parser<'a> { ParseValue::String(values.into_iter().next().unwrap_or_default()) }; out.args.insert(Arc::new(arg.clone()), parsed); + out.arg_origins + .entry(Arc::new(arg.clone())) + .or_default() + .push(ValueOrigin::Env(env_name.to_string())); continue; } if !arg.default.is_empty() { @@ -590,6 +741,10 @@ impl<'a> Parser<'a> { // For var=true, always return a vec (MultiString) out.args .insert(Arc::new(arg.clone()), ParseValue::MultiString(values)); + out.arg_origins + .entry(Arc::new(arg.clone())) + .or_default() + .push(ValueOrigin::Default); } else { validate_choice_value( ChoiceTarget::arg(arg), @@ -602,6 +757,10 @@ impl<'a> Parser<'a> { Arc::new(arg.clone()), ParseValue::String(arg.default[0].clone()), ); + out.arg_origins + .entry(Arc::new(arg.clone())) + .or_default() + .push(ValueOrigin::Default); } } } @@ -657,6 +816,10 @@ impl<'a> Parser<'a> { out.flags .insert(Arc::clone(flag), ParseValue::Bool(is_true)); } + out.flag_origins + .entry(Arc::clone(flag)) + .or_default() + .push(ValueOrigin::Env(env_name.to_string())); } } // Decide every `default_if` against argv+env only. Binding as we go would put @@ -664,7 +827,7 @@ impl<'a> Parser<'a> { // explicit — Go's `Given()` and the derive's `__given_*` both ignore defaults // here, so an unconditional `default` on `--json` must not fire // `default_if "--json"`. - let mut from_default_if: Vec<(Arc, String)> = Vec::new(); + let mut from_default_if: Vec<(Arc, crate::SpecDefaultIf)> = Vec::new(); for flag in &flags { if out.flags.contains_key(flag) || overridden_flags.contains(&flag.name) { continue; @@ -672,23 +835,47 @@ impl<'a> Parser<'a> { if let Some(condition) = flag.default_if.iter().find(|condition| { default_if_condition_matches(condition, &out, &overridden_flags, custom_env) }) { - from_default_if.push((Arc::clone(flag), condition.value.clone())); + from_default_if.push((Arc::clone(flag), condition.clone())); } } - for (flag, value) in &from_default_if { - bind_flag_fallback(flag, std::slice::from_ref(value), &mut out, custom_env)?; + for (flag, condition) in &from_default_if { + // The whole condition, not just the value: several conditions may name the same + // selector with different `when` values, so the selector alone does not say + // which one fired. + bind_flag_fallback( + flag, + std::slice::from_ref(&condition.value), + &mut out, + custom_env, + ValueOrigin::DefaultIf { + selector: condition.selector.clone(), + when: condition.when.clone(), + }, + )?; } for flag in &flags { if out.flags.contains_key(flag) || overridden_flags.contains(&flag.name) { continue; } if !flag.default.is_empty() { - bind_flag_fallback(flag, &flag.default, &mut out, custom_env)?; + bind_flag_fallback( + flag, + &flag.default, + &mut out, + custom_env, + ValueOrigin::Default, + )?; continue; } if let Some(arg) = flag.arg.as_ref() { if !arg.default.is_empty() { - bind_flag_fallback(flag, &arg.default, &mut out, custom_env)?; + bind_flag_fallback( + flag, + &arg.default, + &mut out, + custom_env, + ValueOrigin::Default, + )?; } } } @@ -716,16 +903,6 @@ impl<'a> Parser<'a> { ); } } - if let Some(err) = out - .errors - .iter() - .find(|e| matches!(e, UsageErr::Help(_) | UsageErr::Version(_))) - { - bail!("{err}"); - } - if !out.errors.is_empty() { - bail!("{}", out.errors.iter().map(|e| e.to_string()).join("\n")); - } // Applied once, here, because this is where the CLI's own version is known: a // `deprecated_warn_at` the spec has not reached yet is an author saying *not yet*. crate::warn::retain_reached(&mut out.warnings, self.spec.version.as_deref()); @@ -798,9 +975,18 @@ enum MountTiming { /// This holds what a side queue used to: the flag Phase 1 read a word as, previously a /// `VecDeque` popped in step with the words. Two queues staying aligned is an invariant /// nothing checks, and it was delicate enough to need explaining at three call sites; on -/// the word itself there is nothing to keep aligned. +/// the word itself there is nothing to keep aligned. The argv position is here for the +/// same reason: the queue is popped, re-queued, split on `=`, and has subcommand words +/// removed from the middle, so position in the queue stops meaning position in argv on the +/// first descent. struct Token { word: String, + /// Where in the caller's argv this word came from. + /// + /// A word the parser made up points at the token it was derived from — the tail of a + /// short bundle at the bundle, a multicall applet name at argv[0] — because that is the + /// token a reader would point at, and there is nothing else to point at. + argv: usize, /// The flag Phase 1 read this word as, and the command level it read it at. /// /// `Some((flag, command_level))` for a flag word, `None` for its value, for anything @@ -816,14 +1002,61 @@ struct Token { } impl Token { - fn new(word: String) -> Self { + fn new(word: String, argv: usize) -> Self { Self { word, + argv, binding: None, } } } +/// The token trace, while it is being built. +/// +/// One row per word of the caller's argv, so a role can be recorded against a position +/// without the recorder having to know how many words came before it. Words the parser +/// made up have no row of their own and fold onto the row they were derived from. +struct Trace { + tokens: Vec, +} + +impl Trace { + fn new(input: &[String]) -> Self { + Self { + tokens: input + .iter() + .enumerate() + .map(|(index, word)| TokenBinding { + index, + word: word.clone(), + synthesized: false, + roles: vec![], + }) + .collect(), + } + } + + fn record(&mut self, argv: usize, role: TokenRole) { + if let Some(token) = self.tokens.get_mut(argv) { + token.roles.push(role); + } + } + + /// Note that what was read at this position is not what the caller wrote there. + fn note_synthesized(&mut self, argv: usize) { + if let Some(token) = self.tokens.get_mut(argv) { + token.synthesized = true; + } + } + + /// Every word the parse never reached, once it has stopped. + fn close(&mut self, unread: &VecDeque) { + for token in unread { + self.record(token.argv, TokenRole::Unread); + } + } +} + fn parse_partial_with_env( spec: &Spec, input: &[String], @@ -836,16 +1069,24 @@ fn parse_partial_with_env( return parse_partial_with_env(&viewed, input, custom_env, mount_outputs, mount_timing); } trace!("parse_partial: {input:?}"); + let mut trace = Trace::new(input); let mut input = input .iter() - .cloned() - .map(Token::new) + .enumerate() + .map(|(argv, word)| Token::new(word.clone(), argv)) .collect::>(); let argv0 = input.pop_front(); + if let Some(argv0) = argv0.as_ref() { + trace.record(argv0.argv, TokenRole::Program); + } if spec.multicall { if let Some(raw) = argv0 { if let Some(applet) = multicall_applet(&raw.word, &spec.name, Some(spec.bin.as_str())) { - input.push_front(Token::new(applet.to_string())); + // A symlink invocation reads a word the caller never typed — the basename of + // the program itself — so argv[0] is both the program and, below, whatever + // that word selects. + trace.note_synthesized(raw.argv); + input.push_front(Token::new(applet.to_string(), raw.argv)); } } } @@ -859,6 +1100,10 @@ fn parse_partial_with_env( cmds: vec![spec.cmd.clone()], args: IndexMap::new(), flags: IndexMap::new(), + tokens: vec![], + flag_origins: IndexMap::new(), + arg_origins: IndexMap::new(), + overridden_flags: BTreeMap::new(), available_flags: gather_flags(&spec.cmd), flag_awaiting_value: vec![], errors: vec![], @@ -979,7 +1224,15 @@ fn parse_partial_with_env( crossing_mount, ); // Remove subcommand from input - input.remove(idx); + let selected = input.remove(idx); + if let Some(selected) = selected { + trace.record( + selected.argv, + TokenRole::Command { + name: subcommand.name.clone(), + }, + ); + } command_has_argv = idx < input.len(); out.cmds.push(subcommand.clone()); out.cmd = subcommand.clone(); @@ -1104,8 +1357,11 @@ fn parse_partial_with_env( // subcommands already won above, and a default_subcommand already caught. // clap's `allow_external_subcommands` is this, not `unknown_flags=value`. if out.cmd.external_subcommand { - let rest: Vec = input.drain(idx..).map(|t| t.word).collect(); - out.external = Some(rest); + let rest: Vec = input.drain(idx..).collect(); + for token in &rest { + trace.record(token.argv, TokenRole::External); + } + out.external = Some(rest.into_iter().map(|t| t.word).collect()); break; } // This could be a positional argument, so stop subcommand search @@ -1142,6 +1398,7 @@ fn parse_partial_with_env( let token = input.pop_front().unwrap(); // The flag this word was read as in Phase 1, if it skipped it (see `Token::binding`). let binding = token.binding; + let argv = token.argv; let mut w = token.word; // A short's attached value is re-queued with `grouped_flag` set, and that // continuation is not a following word. `require_equals` refuses only the @@ -1158,6 +1415,11 @@ fn parse_partial_with_env( // is not cleared here either, so clearing it would let one arg report the same // violation once per invocation. out.args.clear(); + // With the values gone, so is where they came from — otherwise the second + // invocation of `run lint ::: test` reports the first one's provenance. The + // token trace is *not* cleared: those words were read, and a report that + // dropped them would show a command line with a hole in it. + out.arg_origins.clear(); next_arg_idx = 0; out.flag_awaiting_value.clear(); // Clear any pending flag values enable_flags = true; // Reset -- separator effect @@ -1197,9 +1459,13 @@ fn parse_partial_with_env( &mut w, &mut input, custom_env, + &mut trace, + argv, + // The token a hyphen-valued flag takes is the following word, never attached. + false, )?; if should_return { - record_cursor(&mut out, next_arg_idx, seen_double_dash); + record_stop(&mut out, next_arg_idx, seen_double_dash, trace, &input); return Ok((out, overridden_flags)); } continue; @@ -1225,7 +1491,12 @@ fn parse_partial_with_env( && is_negative_number(&w)))) }) { - try_bind_default_missing(&mut out.flags, &mut out.flag_awaiting_value, custom_env)?; + try_bind_default_missing( + &mut out.flags, + &mut out.flag_awaiting_value, + custom_env, + &mut out.flag_origins, + )?; } // The first explicit `--` is still a separator after an `automatic` argument has @@ -1250,6 +1521,7 @@ fn parse_partial_with_env( // neither counts as one nor unlocks a `double_dash="required"` arg. } else { seen_double_dash = true; + trace.record(argv, TokenRole::Separator); // Everything after an explicit `--` belongs to the arg that requires one, so // jump the cursor there — past any earlier arg, including a greedy variadic @@ -1293,9 +1565,19 @@ fn parse_partial_with_env( .entry(Arc::as_ptr(f) as usize) .or_default() .insert(word.to_string()); + // Recorded before the action check below: a token that named a flag named it + // whether or not the parse can carry on afterwards. + trace.record( + argv, + TokenRole::Flag { + flag: Arc::clone(f), + spelling: word.to_string(), + negated: f.negate.as_deref() == Some(word), + }, + ); if f.action != crate::SpecFlagAction::Set { out.errors.push(render_action_err(spec, &out.cmd, f, word)); - record_cursor(&mut out, next_arg_idx, seen_double_dash); + record_stop(&mut out, next_arg_idx, seen_double_dash, trace, &input); return Ok((out, overridden_flags)); } apply_flag_overrides( @@ -1304,6 +1586,7 @@ fn parse_partial_with_env( &mut out.flags, &mut out.flag_awaiting_value, &mut overridden_flags, + &mut out.overridden_flags, ); // An attached value only means something to a flag that takes one: // `--jobs=` is an empty string, while `--force=yes` has nothing to @@ -1339,9 +1622,14 @@ fn parse_partial_with_env( &mut val, &mut input, custom_env, + &mut trace, + argv, + // The `=` settled that this text is the value, so it rode in on + // the flag's own token. + true, )?; if should_return { - record_cursor(&mut out, next_arg_idx, seen_double_dash); + record_stop(&mut out, next_arg_idx, seen_double_dash, trace, &input); return Ok((out, overridden_flags)); } } @@ -1392,12 +1680,12 @@ fn parse_partial_with_env( if is_help_arg(spec, &out.cmd, &w) { out.errors .push(render_help_err(spec, &out.cmd, w.len() > 2)); - record_cursor(&mut out, next_arg_idx, seen_double_dash); + record_stop(&mut out, next_arg_idx, seen_double_dash, trace, &input); return Ok((out, overridden_flags)); } if is_version_arg(spec, &out.cmds, &w) { out.errors.push(render_version_err(spec, w.len() > 2)); - record_cursor(&mut out, next_arg_idx, seen_double_dash); + record_stop(&mut out, next_arg_idx, seen_double_dash, trace, &input); return Ok((out, overridden_flags)); } reject_unknown_flag_if_asked(spec, &out.cmds, &w)?; @@ -1445,23 +1733,35 @@ fn parse_partial_with_env( if f.action != crate::SpecFlagAction::Set { out.errors .push(render_action_err(spec, &out.cmd, f, &format!("-{short}"))); - record_cursor(&mut out, next_arg_idx, seen_double_dash); + record_stop(&mut out, next_arg_idx, seen_double_dash, trace, &input); return Ok((out, overridden_flags)); } parsed_flag_spellings .entry(Arc::as_ptr(f) as usize) .or_default() .insert(format!("-{short}")); + trace.record( + argv, + TokenRole::Flag { + flag: Arc::clone(f), + spelling: format!("-{short}"), + // A short spelling is never the negated form: `negate` is a long. + negated: false, + }, + ); apply_flag_overrides( f, &out.available_flags, &mut out.flags, &mut out.flag_awaiting_value, &mut overridden_flags, + &mut out.overridden_flags, ); let rest = &w[1 + short.len_utf8()..]; if !rest.is_empty() { - input.push_front(Token::new(format!("-{rest}"))); + // `-abc` is one token that names three flags, so the tail is read at the + // bundle's own position rather than at one of its own. + input.push_front(Token::new(format!("-{rest}"), argv)); } // A fully consumed short is no longer a grouped continuation. // Leaving this set after `-ai` made `-i` skip `require_equals` @@ -1505,18 +1805,18 @@ fn parse_partial_with_env( // neither reaches the whole-token spellings below. if let Some(err) = supplied_short(spec, &out.cmds, short) { out.errors.push(err); - record_cursor(&mut out, next_arg_idx, seen_double_dash); + record_stop(&mut out, next_arg_idx, seen_double_dash, trace, &input); return Ok((out, overridden_flags)); } if is_help_arg(spec, &out.cmd, &w) { out.errors .push(render_help_err(spec, &out.cmd, w.len() > 2)); - record_cursor(&mut out, next_arg_idx, seen_double_dash); + record_stop(&mut out, next_arg_idx, seen_double_dash, trace, &input); return Ok((out, overridden_flags)); } if is_version_arg(spec, &out.cmds, &w) { out.errors.push(render_version_err(spec, w.len() > 2)); - record_cursor(&mut out, next_arg_idx, seen_double_dash); + record_stop(&mut out, next_arg_idx, seen_double_dash, trace, &input); return Ok((out, overridden_flags)); } reject_unknown_flag_if_asked(spec, &out.cmds, &w)?; @@ -1563,7 +1863,7 @@ fn parse_partial_with_env( span: (0, 0).into(), input: format!("{token} {w}"), }); - record_cursor(&mut out, next_arg_idx, seen_double_dash); + record_stop(&mut out, next_arg_idx, seen_double_dash, trace, &input); return Ok((out, overridden_flags)); } if enable_flags && !out.flag_awaiting_value.is_empty() { @@ -1578,9 +1878,12 @@ fn parse_partial_with_env( &mut w, &mut input, custom_env, + &mut trace, + argv, + attached_continuation, )?; if should_return { - record_cursor(&mut out, next_arg_idx, seen_double_dash); + record_stop(&mut out, next_arg_idx, seen_double_dash, trace, &input); return Ok((out, overridden_flags)); } continue; @@ -1660,7 +1963,7 @@ fn parse_partial_with_env( } } if refused { - record_cursor(&mut out, next_arg_idx, seen_double_dash); + record_stop(&mut out, next_arg_idx, seen_double_dash, trace, &input); return Ok((out, overridden_flags)); } // `double_dash="automatic"` means the first value this arg takes is the last @@ -1670,6 +1973,27 @@ fn parse_partial_with_env( if arg.double_dash == SpecDoubleDashChoices::Automatic { enable_flags = false; } + // A flag-like word reaching a positional while flags are still being read was + // offered to every declaration and matched none: under the default + // `unknown_flags="value"` it becomes data, and saying so is the difference + // between "you have a typo" and "this argument took your typo". + let unknown_flag = enable_flags + && !positional_negative_number + && is_flag_like(&w) + && binding.is_none(); + trace.record( + argv, + if unknown_flag { + TokenRole::UnknownFlag { + bound_as: Some(Arc::new(arg.clone())), + } + } else { + TokenRole::Arg { + arg: Arc::new(arg.clone()), + values: parts.clone(), + } + }, + ); if arg.var { let arr = out .args @@ -1694,18 +2018,18 @@ fn parse_partial_with_env( if is_help_arg(spec, &out.cmd, &w) { out.errors .push(render_help_err(spec, &out.cmd, w.len() > 2)); - record_cursor(&mut out, next_arg_idx, seen_double_dash); + record_stop(&mut out, next_arg_idx, seen_double_dash, trace, &input); return Ok((out, overridden_flags)); } if is_version_arg(spec, &out.cmds, &w) { out.errors.push(render_version_err(spec, w.len() > 2)); - record_cursor(&mut out, next_arg_idx, seen_double_dash); + record_stop(&mut out, next_arg_idx, seen_double_dash, trace, &input); return Ok((out, overridden_flags)); } bail!("unexpected word: {w}"); } - record_cursor(&mut out, next_arg_idx, seen_double_dash); + record_stop(&mut out, next_arg_idx, seen_double_dash, trace, &input); // `out.flags` is keyed by `SpecFlag`, whose equality is intentionally name-only. Two // declarations with the same canonical name therefore share one public value entry even @@ -2440,6 +2764,7 @@ fn bind_flag_fallback( values: &[String], out: &mut ParseOutput, custom_env: Option<&HashMap>, + origin: ValueOrigin, ) -> Result<(), miette::Error> { if values.is_empty() { return Ok(()); @@ -2483,6 +2808,10 @@ fn bind_flag_fallback( ParseValue::Bool(fallback_is_true(&values[0])), ); } + out.flag_origins + .entry(Arc::clone(flag)) + .or_default() + .push(origin); Ok(()) } @@ -2688,6 +3017,10 @@ fn apply_flag_overrides( parsed_flags: &mut IndexMap, ParseValue>, pending_flags: &mut Vec>, overridden_flags: &mut HashSet, + // The reportable half of the same fact: which flag did the overriding. The set above + // only stops a default or an environment value restoring what was overridden, and + // "`--quiet` is unset despite its default" has no answer without the name. + attributed: &mut BTreeMap, ) { let overridden_names: HashSet = available_flags .values() @@ -2698,9 +3031,13 @@ fn apply_flag_overrides( parsed_flags.retain(|parsed, _| !overridden_names.contains(&parsed.name)); pending_flags.retain(|pending| !overridden_names.contains(&pending.name)); + for name in &overridden_names { + attributed.insert(name.clone(), flag.name.clone()); + } overridden_flags.extend(overridden_names); // An explicit occurrence always restores this flag, including self-overrides. overridden_flags.remove(&flag.name); + attributed.remove(&flag.name); } #[cfg(feature = "docs")] @@ -2998,6 +3335,12 @@ fn bind_pending_flag_value( word: &mut String, input: &mut VecDeque, custom_env: Option<&HashMap>, + trace: &mut Trace, + // Which token supplied `word`, and whether it rode along on the flag's own token + // (`--jobs=8`, `-j8`) rather than following it. A variadic run's later words carry + // their own positions and are recorded where they are read. + argv: usize, + attached: bool, ) -> miette::Result { // Held before the drain pops it, along with what the flag is already carrying: a // `var_max` bounds the values this occurrence takes, not the list they are appended @@ -3010,7 +3353,8 @@ fn bind_pending_flag_value( let carried = flags.get(&flag).map(value_count).unwrap_or(0); (flag, carried) }); - if drain_pending_flag_values( + let mut bound = vec![]; + let refused = drain_pending_flag_values( spec, cmd, errors, @@ -3018,7 +3362,19 @@ fn bind_pending_flag_value( flag_awaiting_value, word, custom_env, - )? { + &mut bound, + )?; + for (flag, values) in bound { + trace.record( + argv, + TokenRole::Value { + flag, + values, + attached, + }, + ); + } + if refused { return Ok(true); } let Some((flag, carried)) = collecting else { @@ -3034,6 +3390,7 @@ fn bind_pending_flag_value( carried, input, custom_env, + trace, ) } @@ -3060,6 +3417,7 @@ fn collect_variadic_flag_values( carried: usize, input: &mut VecDeque, custom_env: Option<&HashMap>, + trace: &mut Trace, ) -> miette::Result { let max = flag .arg @@ -3097,9 +3455,12 @@ fn collect_variadic_flag_values( { break; } - let mut word = input.pop_front().unwrap().word; + let taken = input.pop_front().unwrap(); + let argv = taken.argv; + let mut word = taken.word; flag_awaiting_value.push(Arc::clone(flag)); - if drain_pending_flag_values( + let mut bound = vec![]; + let refused = drain_pending_flag_values( spec, cmd, errors, @@ -3107,7 +3468,21 @@ fn collect_variadic_flag_values( flag_awaiting_value, &mut word, custom_env, - )? { + &mut bound, + )?; + for (flag, values) in bound { + // A later word of the same occurrence is its own token, and never attached: + // only the first value can ride along on the flag. + trace.record( + argv, + TokenRole::Value { + flag, + values, + attached: false, + }, + ); + } + if refused { return Ok(true); } } @@ -3166,6 +3541,7 @@ fn try_bind_default_missing( flags: &mut IndexMap, ParseValue>, flag_awaiting_value: &mut Vec>, custom_env: Option<&HashMap>, + origins: &mut IndexMap, Vec>, ) -> miette::Result { let Some(flag) = flag_awaiting_value.last() else { return Ok(false); @@ -3178,6 +3554,10 @@ fn try_bind_default_missing( // empty collection distinguishes it from an explicitly empty // `--flag=` string without inventing a sentinel value. let variadic_value = flag.arg.as_ref().is_some_and(|arg| arg.var); + origins + .entry(Arc::clone(&flag)) + .or_default() + .push(ValueOrigin::DefaultMissing); if flag.var { // A repeated bare occurrence is still an occurrence. The string collection // uses an empty value for it, just as the concrete `default_missing` path @@ -3214,6 +3594,10 @@ fn try_bind_default_missing( )?; } let flag = flag_awaiting_value.pop().unwrap(); + origins + .entry(Arc::clone(&flag)) + .or_default() + .push(ValueOrigin::DefaultMissing); let collecting = flag.var || flag.arg.as_ref().is_some_and(|arg| arg.var); if collecting { let arr = flags @@ -3228,6 +3612,11 @@ fn try_bind_default_missing( Ok(true) } +/// `bound` collects what each drained flag took, in the order it took it. The values are +/// the word after any `delimiter` split, which is the only place that split is known: by the +/// time they are in `flags` a scalar and a one-element list are indistinguishable, and a +/// second occurrence has appended to the same list. +#[allow(clippy::too_many_arguments)] fn drain_pending_flag_values( spec: &Spec, cmd: &SpecCommand, @@ -3236,6 +3625,7 @@ fn drain_pending_flag_values( flag_awaiting_value: &mut Vec>, word: &mut String, custom_env: Option<&HashMap>, + bound: &mut Vec<(Arc, Vec)>, ) -> miette::Result { while let Some(flag) = flag_awaiting_value.pop() { let arg = flag.arg.as_ref().unwrap(); @@ -3260,6 +3650,7 @@ fn drain_pending_flag_values( } } word.clear(); + bound.push((Arc::clone(&flag), parts.clone())); // Two ways to hold several values, and both record a list: a `var` flag // collects one per occurrence, a variadic argument collects several from one. if flag.var || arg.var { @@ -3358,12 +3749,23 @@ fn validate_choice_values( Ok(()) } -/// Publish where Phase 2 left its positional cursor, so callers that do not re-run the parse — -/// completions, above all — agree with it. Called on every exit from the loop, including the -/// early ones that render help, where the cursor is still the useful answer. -fn record_cursor(out: &mut ParseOutput, next_arg_idx: usize, seen_double_dash: bool) { +/// Everything a parse records about where it stopped: the positional cursor, so callers that +/// do not re-run the parse — completions, above all — agree with it, and the token trace. +/// +/// Every exit from the binding phase comes through here, which is what makes it the right +/// place to close the trace: whatever is still queued was never read, and saying so is more +/// useful than leaving those words out of the report entirely. +fn record_stop( + out: &mut ParseOutput, + next_arg_idx: usize, + seen_double_dash: bool, + mut trace: Trace, + unread: &VecDeque, +) { out.next_arg = out.cmd.args.get(next_arg_idx).cloned().map(Arc::new); out.double_dash_seen = seen_double_dash; + trace.close(unread); + out.tokens = trace.tokens; } /// Record that `arg` was handed a word before the `--` it requires. @@ -3441,6 +3843,44 @@ impl Display for ParseValue { } } +/// One `tokens` line for [`Debug`]: the position, the word, and what it became. +fn render_token(token: &TokenBinding) -> String { + let roles = token.roles.iter().map(render_role).join(", "); + let synthesized = if token.synthesized { " (read as)" } else { "" }; + format!("[{}] {}{synthesized}: {roles}", token.index, token.word) +} + +fn render_role(role: &TokenRole) -> String { + match role { + TokenRole::Program => "program".to_string(), + TokenRole::Command { name } => format!("subcommand {name}"), + TokenRole::Flag { + flag, + spelling, + negated, + } => { + let negated = if *negated { ", negated" } else { "" }; + format!("flag {} as {spelling}{negated}", flag.name) + } + TokenRole::Value { + flag, + values, + attached, + } => { + let attached = if *attached { ", attached" } else { "" }; + format!("value of {} = {values:?}{attached}", flag.name) + } + TokenRole::Arg { arg, values } => format!("arg {} = {values:?}", arg.name), + TokenRole::Separator => "separator".to_string(), + TokenRole::UnknownFlag { bound_as } => match bound_as { + Some(arg) => format!("unknown flag, bound as {}", arg.name), + None => "unknown flag".to_string(), + }, + TokenRole::External => "external".to_string(), + TokenRole::Unread => "unread".to_string(), + } +} + impl Debug for ParseOutput { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { f.debug_struct("ParseOutput") @@ -3472,6 +3912,27 @@ impl Debug for ParseOutput { .field("flag_awaiting_value", &self.flag_awaiting_value) .field("errors", &self.errors) .field("external", &self.external) + // Provenance, one line per token and one per fallback. This is the parser's + // debug channel under `USAGE_LOG=trace`, so it is where a spec author looks + // first — `usage explain` renders the same facts for a reader. + .field( + "tokens", + &self.tokens.iter().map(render_token).collect_vec(), + ) + .field( + "origins", + &self + .flag_origins + .iter() + .map(|(f, o)| format!("{}: {o:?}", f.name)) + .chain( + self.arg_origins + .iter() + .map(|(a, o)| format!("{}: {o:?}", a.name)), + ) + .collect_vec(), + ) + .field("overridden_flags", &self.overridden_flags) .finish() } } @@ -8794,4 +9255,385 @@ cmd "rm" { } } } + + // Provenance: which token bound what, and where a value came from when no token did. + + /// Every role a token was given, rendered the way `Debug` renders it, so a test can + /// assert on the whole picture rather than on one field at a time. + fn roles(parsed: &ParseOutput, index: usize) -> Vec { + parsed + .tokens + .iter() + .find(|token| token.index == index) + .unwrap_or_else(|| panic!("no token at {index}")) + .roles + .iter() + .map(render_role) + .collect() + } + + fn origins(parsed: &ParseOutput, flag: &str) -> Vec { + parsed + .flag_origins + .iter() + .find(|(f, _)| f.name == flag) + .map(|(_, origins)| origins.clone()) + .unwrap_or_default() + } + + fn explain_with_env(spec: &Spec, words: &[&str], env: &[(&str, &str)]) -> ParseOutput { + let env = env + .iter() + .map(|(k, v)| ((*k).to_string(), (*v).to_string())) + .collect(); + Parser::new(spec) + .with_env(env) + .explain(&input(words)) + .unwrap() + } + + fn explain(spec: &Spec, words: &[&str]) -> ParseOutput { + explain_with_env(spec, words, &[]) + } + + #[test] + fn an_attached_long_value_is_recorded_on_the_flag_token() { + let spec: Spec = "name \"ex\"\nbin \"ex\"\nflag \"--env \"\n" + .parse() + .unwrap(); + + let parsed = explain(&spec, &["ex", "--env=prod"]); + + assert_eq!(roles(&parsed, 0), ["program"]); + assert_eq!( + roles(&parsed, 1), + ["flag env as --env", "value of env = [\"prod\"], attached"] + ); + // This is jdx/mise discussion #8883: a hand-written scanner dropped the attached + // form while the detached one worked, and nothing could show the difference. + assert!(origins(&parsed, "env").is_empty(), "typed, so no fallback"); + } + + #[test] + fn a_detached_long_value_is_recorded_on_its_own_token() { + let spec: Spec = "name \"ex\"\nbin \"ex\"\nflag \"--env \"\n" + .parse() + .unwrap(); + + let parsed = explain(&spec, &["ex", "--env", "prod"]); + + assert_eq!(roles(&parsed, 1), ["flag env as --env"]); + assert_eq!(roles(&parsed, 2), ["value of env = [\"prod\"]"]); + } + + #[test] + fn a_short_bundle_is_attributed_to_the_bundle_token() { + let spec: Spec = "name \"ex\"\nbin \"ex\"\nflag \"-a\"\nflag \"-b\"\nflag \"-j \"\n" + .parse() + .unwrap(); + + let parsed = explain(&spec, &["ex", "-abj8"]); + + // One word the caller wrote, four things it did — and the re-queued tails are + // folded back onto it rather than appearing as tokens nobody typed. + assert_eq!( + roles(&parsed, 1), + [ + "flag a as -a", + "flag b as -b", + "flag j as -j", + "value of j = [\"8\"], attached", + ] + ); + assert_eq!(parsed.tokens.len(), 2); + } + + #[test] + fn a_delimiter_splits_one_token_into_several_values() { + let spec: Spec = "name \"ex\"\nbin \"ex\"\nflag \"--tags ...\" delimiter=\",\"\n" + .parse() + .unwrap(); + + let parsed = explain(&spec, &["ex", "--tags", "a,b,c"]); + + assert_eq!( + roles(&parsed, 2), + ["value of tags = [\"a\", \"b\", \"c\"]"], + "the values meant, not the word typed" + ); + } + + #[test] + fn a_separator_and_the_words_after_it_are_distinguished() { + let spec: Spec = "name \"ex\"\nbin \"ex\"\narg \"\"\narg \"[raw]...\"\n" + .parse() + .unwrap(); + + let parsed = explain(&spec, &["ex", "a", "--", "-x"]); + + assert_eq!(roles(&parsed, 1), ["arg src = [\"a\"]"]); + assert_eq!(roles(&parsed, 2), ["separator"]); + // Past the separator `-x` is data, not an unknown flag. + assert_eq!(roles(&parsed, 3), ["arg raw = [\"-x\"]"]); + } + + #[test] + fn a_second_separator_is_data() { + let spec: Spec = "name \"ex\"\nbin \"ex\"\narg \"[raw]...\"\n" + .parse() + .unwrap(); + + let parsed = explain(&spec, &["ex", "--", "a", "--", "b"]); + + assert_eq!(roles(&parsed, 1), ["separator"]); + assert_eq!(roles(&parsed, 3), ["arg raw = [\"--\"]"]); + } + + #[test] + fn an_unknown_flag_says_what_took_it() { + let spec: Spec = "name \"ex\"\nbin \"ex\"\narg \"[rest]...\"\n" + .parse() + .unwrap(); + + let parsed = explain(&spec, &["ex", "--wat"]); + + // The default is lax, so the word became data. Which is the useful thing to be + // told: the alternative reading is "you have a typo". + assert_eq!(roles(&parsed, 1), ["unknown flag, bound as rest"]); + } + + #[test] + fn a_subcommand_word_is_not_a_positional() { + let spec: Spec = "name \"ex\"\nbin \"ex\"\ncmd \"build\" {\n arg \"\"\n}\n" + .parse() + .unwrap(); + + let parsed = explain(&spec, &["ex", "build", "a"]); + + assert_eq!(roles(&parsed, 1), ["subcommand build"]); + assert_eq!(roles(&parsed, 2), ["arg target = [\"a\"]"]); + } + + #[test] + fn a_multicall_applet_is_read_at_argv0() { + let spec: Spec = + "name \"box\"\nbin \"box\"\nmulticall #true\ncmd \"ls\" {\n flag \"-l\"\n}\n" + .parse() + .unwrap(); + + let parsed = explain(&spec, &["/usr/bin/ls", "-l"]); + + // argv[0] is both the program and the word that selected the applet, and the word + // read there is not the word the caller wrote. + assert_eq!(roles(&parsed, 0), ["program", "subcommand ls"]); + assert!(parsed.tokens[0].synthesized); + assert_eq!(parsed.tokens[0].word, "/usr/bin/ls"); + } + + #[test] + fn words_the_parse_never_reached_say_so() { + let spec: Spec = "name \"ex\"\nbin \"ex\"\narg \"[rest]...\"\n" + .parse() + .unwrap(); + + let parsed = Parser::new(&spec) + .explain(&input(&["ex", "--help", "a"])) + .unwrap(); + + assert_eq!(roles(&parsed, 2), ["unread"]); + } + + #[test] + fn an_env_origin_names_the_variable_that_fired() { + let spec: Spec = + "name \"ex\"\nbin \"ex\"\nflag \"--token \" env=\"EX_TOKEN\" env_fallback=\"EX_TOKEN_OLD\"\n" + .parse() + .unwrap(); + + let primary = explain_with_env(&spec, &["ex"], &[("EX_TOKEN", "a")]); + assert_eq!( + origins(&primary, "token"), + [ValueOrigin::Env("EX_TOKEN".to_string())] + ); + + // The fallback firing is a different fact from the primary firing, and which one it + // was is what says which declaration to delete. + let fallback = explain_with_env(&spec, &["ex"], &[("EX_TOKEN_OLD", "b")]); + assert_eq!( + origins(&fallback, "token"), + [ValueOrigin::Env("EX_TOKEN_OLD".to_string())] + ); + } + + #[test] + fn a_default_origin_is_recorded_for_flags_and_args() { + let spec: Spec = + "name \"ex\"\nbin \"ex\"\nflag \"--color \" default=\"auto\"\narg \"[src]\" default=\".\"\n" + .parse() + .unwrap(); + + let parsed = explain(&spec, &["ex"]); + + assert_eq!(origins(&parsed, "color"), [ValueOrigin::Default]); + let (arg, origins) = parsed.arg_origins.iter().next().unwrap(); + assert_eq!(arg.name, "src"); + assert_eq!(origins, &[ValueOrigin::Default]); + } + + #[test] + fn a_default_if_origin_carries_the_condition_that_fired() { + let spec: Spec = r#" +name "ex" +bin "ex" +flag "--profile

" +flag "--strict" { + default_if "--profile" "prod" "true" +} + "# + .parse() + .unwrap(); + + let parsed = explain(&spec, &["ex", "--profile", "prod"]); + + // The selector alone is ambiguous: several conditions may name it with different + // `when` values, so the report has to say which one matched. + assert_eq!( + origins(&parsed, "strict"), + [ValueOrigin::DefaultIf { + selector: "--profile".to_string(), + when: Some("prod".to_string()), + }] + ); + } + + #[test] + fn a_bare_optional_value_flag_records_default_missing() { + let spec: Spec = + "name \"ex\"\nbin \"ex\"\nflag \"--color \" default_missing=\"always\"\nflag \"-v\"\n" + .parse() + .unwrap(); + + let parsed = explain(&spec, &["ex", "--color", "-v"]); + + // The flag was typed and the value was not, which is the distinction a spec author + // is asking about when they ask why `--color` came out `always`. + assert_eq!(roles(&parsed, 1), ["flag color as --color"]); + assert_eq!(origins(&parsed, "color"), [ValueOrigin::DefaultMissing]); + assert_eq!(roles(&parsed, 2), ["flag v as -v"]); + } + + #[test] + fn a_var_flag_can_take_one_value_from_argv_and_one_from_default_missing() { + let spec: Spec = + "name \"ex\"\nbin \"ex\"\nflag \"--color \" var=#true default_missing=\"always\"\n" + .parse() + .unwrap(); + + let parsed = explain(&spec, &["ex", "--color=red", "--color"]); + + // Why origins are a list: one declaration, two occurrences, two different answers. + assert_eq!( + roles(&parsed, 1), + [ + "flag color as --color", + "value of color = [\"red\"], attached" + ] + ); + assert_eq!(origins(&parsed, "color"), [ValueOrigin::DefaultMissing]); + } + + #[test] + fn an_override_names_the_flag_that_did_it() { + let spec: Spec = + "name \"ex\"\nbin \"ex\"\nflag \"--quiet\" default=\"true\"\nflag \"--loud\" overrides=\"--quiet\"\n" + .parse() + .unwrap(); + + let parsed = explain(&spec, &["ex", "--loud"]); + + // Without the overriding name, "`--quiet` is unset despite its default" has no + // answer: the fallback phase silently declines to fill an overridden flag. + assert_eq!(parsed.overridden_flags.get("quiet").unwrap(), "loud"); + assert!(origins(&parsed, "quiet").is_empty()); + } + + #[test] + fn a_restart_token_leaves_the_tokens_and_clears_the_arg_origins() { + let spec: Spec = r#" +name "ex" +bin "ex" +cmd "run" restart_token=":::" { + arg "" default="build" +} + "# + .parse() + .unwrap(); + + let parsed = explain(&spec, &["ex", "run", "lint", ":::", "test"]); + + // The values belong to the last invocation, so provenance must too — but the words + // of the first were still read, and a report that dropped them would show a command + // line with a hole in it. + assert_eq!(roles(&parsed, 2), ["arg task = [\"lint\"]"]); + assert_eq!(roles(&parsed, 4), ["arg task = [\"test\"]"]); + assert!(parsed.arg_origins.is_empty()); + } + + #[test] + fn explain_keeps_the_bindings_of_a_command_line_that_fails() { + let spec: Spec = "name \"ex\"\nbin \"ex\"\nflag \"--env \"\narg \"\"\n" + .parse() + .unwrap(); + + let parsed = Parser::new(&spec) + .explain(&input(&["ex", "--env=prod"])) + .unwrap(); + + // `parse` reports "missing required " and nothing else, which is the report the + // caller already had. This is the case the whole thing exists for. + assert!(Parser::new(&spec) + .parse(&input(&["ex", "--env=prod"])) + .is_err()); + assert_eq!( + roles(&parsed, 1), + ["flag env as --env", "value of env = [\"prod\"], attached"] + ); + assert!( + parsed.errors.iter().any(|e| e.to_string().contains("src")), + "{:?}", + parsed.errors + ); + } + + #[test] + fn an_external_subcommand_forwards_whole_tokens() { + let spec: Spec = "name \"ex\"\nbin \"ex\"\nexternal_subcommand #true\ncmd \"build\"\n" + .parse() + .unwrap(); + + let parsed = explain(&spec, &["ex", "deploy", "--now"]); + + assert_eq!(roles(&parsed, 1), ["external"]); + assert_eq!(roles(&parsed, 2), ["external"]); + } + + #[test] + fn a_view_keeps_the_callers_argv_positions() { + let spec: Spec = r#" +bin "ex" +view "runner" root="run" +cmd "run" { + flag "--token " +} + "# + .parse() + .unwrap(); + + let parsed = explain(&spec, &["runner", "--token", "secret"]); + + // A view re-enters the parse with the same argv, so the positions still mean what + // the caller wrote. + assert_eq!(roles(&parsed, 0), ["program"]); + assert_eq!(roles(&parsed, 2), ["value of token = [\"secret\"]"]); + } } From 6cc9154da63ef84b880d8ade10b3607fe1dc6580 Mon Sep 17 00:00:00 2001 From: default <216188+jdx@users.noreply.github.com> Date: Fri, 21 Aug 2026 13:47:46 +0000 Subject: [PATCH 3/8] feat(cli): add usage explain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `docs/spec/argv.md` opens by saying it exists to define "which token binds to which flag or argument", and nothing in the toolchain would show you that for a given command line. `usage explain` does: a row per argv token saying what it became, then the values that came from somewhere other than argv, then anything that went wrong. Two tables, because neither alone is enough. A table keyed by token cannot show a value that came from nowhere in argv; a table keyed by declaration cannot show a token that bound to nothing. jdx/mise discussion #8883 — `mise --env=production` silently ignored by a hand-written scanner while `mise --env production` worked — lives in the first, and "why is my default not applying" in the second, so `shadowed` names the default that lost and what beat it. Exits 0 even when the explained command line does not parse. The report succeeded; the thing being reported failed. Exiting nonzero would make the tool useless in the case it exists for. When the parse cannot continue at all — `--jobs` with no value — the binding phase is asked on its own, so the report is the tokens that got that far plus the refusal rather than the refusal alone. `--env KEY=VALUE` makes a report reproducible: pasted into a bug report it has to mean the same thing on the machine that reads it, and it is what lets the snapshot test not depend on whatever the machine exports. `OutputFormat` moves from `lint` up to `cli::mod`, since a third copy of the same four-line `FromStr` is how two spellings of `--format` drift apart. One thing found while writing the tests and documented rather than papered over: `double_dash="automatic"` on `argv` ends this command's own flag parsing at the program name, but a later `--` is still honoured as a separator (a78564c0). So an explained line carrying its own `--` needs the leading one, and a test pins that. Co-Authored-By: Claude Opus 5 --- AGENTS.md | 4 + cli/assets/fig.ts | 57 ++ cli/assets/usage.1 | 46 + cli/src/cli/explain.rs | 872 ++++++++++++++++++ cli/src/cli/lint.rs | 20 +- cli/src/cli/mod.rs | 25 + cli/tests/explain.rs | 213 +++++ .../explain__explains_the_worked_example.snap | 32 + cli/usage.usage.kdl | 29 + docs/cli/reference/commands.json | 142 +++ docs/cli/reference/explain.md | 57 ++ docs/cli/reference/index.md | 1 + docs/spec/argv.md | 32 + docs/spec/reference/flag.md | 4 + examples/explain.usage.kdl | 22 + lib/src/parse.rs | 10 + 16 files changed, 1547 insertions(+), 19 deletions(-) create mode 100644 cli/src/cli/explain.rs create mode 100644 cli/tests/explain.rs create mode 100644 cli/tests/snapshots/explain__explains_the_worked_example.snap create mode 100644 docs/cli/reference/explain.md create mode 100644 examples/explain.usage.kdl diff --git a/AGENTS.md b/AGENTS.md index 9a7ae8cf9..781a0e853 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -116,6 +116,10 @@ The `parse()` function parses command-line arguments against a spec, returning: - Matched command path - Parsed args and flags with values - Env var and default fallbacks applied +- Provenance: `tokens` says what each word of argv became, and + `flag_origins`/`arg_origins` say where a value came from when no token supplied + it. `Parser::explain` returns all of it with the errors kept rather than bailing + on the first, which is what `usage explain` renders. ### Documentation Generation (`lib/src/docs/`) diff --git a/cli/assets/fig.ts b/cli/assets/fig.ts index 21c4d254e..06851f8a6 100644 --- a/cli/assets/fig.ts +++ b/cli/assets/fig.ts @@ -194,6 +194,63 @@ const completionSpec: Fig.Spec = { }, ], }, + { + name: "explain", + description: "Explain what a command line binds to", + options: [ + { + name: ["-f", "--file"], + description: + 'A usage spec file or script with a usage shebang, use "-" to read from stdin', + isRepeatable: false, + args: { + name: "file", + template: "filepaths", + }, + }, + { + name: ["-s", "--spec"], + description: "Raw string spec input", + isRepeatable: false, + args: { + name: "spec", + }, + }, + { + name: "--format", + description: "Output format", + isRepeatable: false, + args: { + name: "format", + suggestions: ["text", "json"], + }, + }, + { + name: "--view", + description: "A spec-declared executable view to explain", + isRepeatable: false, + args: { + name: "view", + }, + }, + { + name: ["-e", "--env"], + description: + "Environment to explain against, as KEY=VALUE, repeatable", + isRepeatable: true, + args: { + name: "env", + }, + }, + ], + args: { + name: "argv", + description: + "The command line to explain, starting with the program name", + isOptional: true, + isVariadic: true, + }, + }, { name: "fish", description: "Execute a shell script using fish", diff --git a/cli/assets/usage.1 b/cli/assets/usage.1 index d14bd7d80..4d8a39797 100644 --- a/cli/assets/usage.1 +++ b/cli/assets/usage.1 @@ -32,6 +32,9 @@ Execute a script, parsing args and exposing them as environment variables \fIAliases: \fRx .RE .TP +\fBexplain\fR +Explain what a command line binds to +.TP \fBfish\fR Execute a shell script with the specified shell .TP @@ -176,6 +179,49 @@ path to script to execute .TP \fB\fR arguments to pass to script +.SH "USAGE EXPLAIN" +Explain what a command line binds to + +Prints a row per argv token saying what it became, then the values that came from +somewhere other than argv, then anything that went wrong. Exits 0 even when the explained +command line does not parse: the report succeeded, and that is the case worth a report. +.PP +\fBUsage:\fR usage explain [OPTIONS] [] ... +.PP +\fBOptions:\fR +.PP +.TP +\fB\-f, \-\-file\fR \fI\fR +A usage spec file or script with a usage shebang, use "\-" to read from stdin +.TP +\fB\-s, \-\-spec\fR \fI\fR +Raw string spec input +.TP +\fB\-\-format\fR \fI\fR +Output format +.RS +\fIDefault: \fRtext +.RE +.TP +\fB\-\-view\fR \fI\fR +A spec\-declared executable view to explain +.TP +\fB\-e, \-\-env\fR \fI\fR +Environment to explain against, as KEY=VALUE, repeatable + +Given at all, these are the *whole* environment: an explanation pasted into a bug +report has to mean the same thing on the machine that reads it. Omitted, the process +environment is used, which is what an execution would see. +\fBArguments:\fR +.PP +.TP +\fB\fR +The command line to explain, starting with the program name + +`usage`'s own flags come before it, and flag parsing ends at the program name, so +both `explain \-f f.kdl mycli \-\-env=prod` and `explain \-f f.kdl \-\- mycli \-\-env=prod` +work. Separate with `\-\-` when the explained line carries its own: the first `\-\-` is +still `usage`'s separator, so `explain \-f f.kdl mycli a \-\- b` loses one. .SH "USAGE FISH" Execute a shell script with the specified shell diff --git a/cli/src/cli/explain.rs b/cli/src/cli/explain.rs new file mode 100644 index 000000000..a579fc697 --- /dev/null +++ b/cli/src/cli/explain.rs @@ -0,0 +1,872 @@ +//! What a command line binds to, and where each value came from. +//! +//! `docs/spec/argv.md` exists to define "which token binds to which flag or argument", and +//! until now nothing would show you that for a given command line. The parser knew and threw +//! it away. So a spec author debugging a flag that will not take a value, and a reader +//! learning why `--color bar` leaves `bar` to the positionals, both had to reason about the +//! grammar rather than ask. +//! +//! Two tables, deliberately. A table keyed by token cannot show a value that came from +//! nowhere in argv; a table keyed by declaration cannot show a token that bound to nothing. +//! jdx/mise discussion #8883 — `mise --env=production` silently ignored while +//! `mise --env production` worked — lives in the first. "Why is my default not applying" +//! lives in the second. + +use std::collections::HashMap; +use std::path::PathBuf; + +use itertools::Itertools; +use miette::Result; +use usage::parse::{ParseOutput, Parser, TokenRole, ValueOrigin}; +use usage::{Spec, SpecArg, SpecFlag}; + +use crate::cli::generate::{file_or_spec, select_view}; +use crate::cli::OutputFormat; + +/// Explain what a command line binds to +/// +/// Prints a row per argv token saying what it became, then the values that came from +/// somewhere other than argv, then anything that went wrong. Exits 0 even when the explained +/// command line does not parse: the report succeeded, and that is the case worth a report. +#[derive(Debug, usage_rs::Args)] +// No `unknown_flags = "value"`, unlike `exec`: `double_dash="automatic"` on `argv` below +// already ends this command's own flag parsing at the program name, so a well-formed +// invocation never offers a foreign flag here. Keeping the root's `error` means +// `usage explain -f f.kdl --nope` says there is no such flag rather than silently explaining +// nothing. +#[usage(effect = "read", verbatim_doc_comment)] +pub struct Explain { + /// A usage spec file or script with a usage shebang, use "-" to read from stdin + #[usage(short, long, value_hint = usage_rs::ValueHint::FilePath)] + file: Option, + + /// Raw string spec input + #[usage(short, long, required_unless = "--file", overrides = "--file")] + spec: Option, + + /// Output format + #[usage(long, default = "text", value_enum)] + format: OutputFormat, + + /// A spec-declared executable view to explain + #[usage(long)] + view: Option, + + /// Environment to explain against, as KEY=VALUE, repeatable + /// + /// Given at all, these are the *whole* environment: an explanation pasted into a bug + /// report has to mean the same thing on the machine that reads it. Omitted, the process + /// environment is used, which is what an execution would see. + #[usage(short, long, var = true)] + env: Vec, + + /// The command line to explain, starting with the program name + /// + /// `usage`'s own flags come before it, and flag parsing ends at the program name, so + /// both `explain -f f.kdl mycli --env=prod` and `explain -f f.kdl -- mycli --env=prod` + /// work. Separate with `--` when the explained line carries its own: the first `--` is + /// still `usage`'s separator, so `explain -f f.kdl mycli a -- b` loses one. + #[usage(double_dash = "automatic", value_hint = usage_rs::ValueHint::CommandWithArguments)] + argv: Vec, +} + +impl usage_rs::Run for Explain { + type Output = Result<()>; + + fn run(self) -> Self::Output { + let spec = select_view(file_or_spec(&self.file, &self.spec)?, self.view.as_deref())?; + let env = self.env_map()?; + // A bare invocation is a legitimate question — it asks which defaults and + // environment values fire with no command line at all — so the program name is + // filled in rather than refused. + let argv = if self.argv.is_empty() { + vec![spec.bin.clone()] + } else { + self.argv.clone() + }; + + let explanation = explain(&spec, &argv, env); + match self.format { + OutputFormat::Text => print!("{}", explanation.render()), + OutputFormat::Json => println!( + "{}", + serde_json::to_string_pretty(&explanation) + .map_err(|e| miette::miette!("failed to serialize the explanation: {e}"))? + ), + } + Ok(()) + } +} + +impl Explain { + fn env_map(&self) -> Result>> { + if self.env.is_empty() { + return Ok(None); + } + let mut env = HashMap::new(); + for entry in &self.env { + let (key, value) = entry + .split_once('=') + .ok_or_else(|| miette::miette!("--env wants KEY=VALUE, got `{entry}`"))?; + env.insert(key.to_string(), value.to_string()); + } + Ok(Some(env)) + } +} + +/// The explanation as data rather than as printed lines. +/// +/// A seam for the same reason `lint_spec` is one: the layout is worth testing against a value +/// instead of against stdout. +pub fn explain(spec: &Spec, argv: &[String], env: Option>) -> Explanation { + let mut parser = Parser::new(spec); + if let Some(env) = env { + parser = parser.with_env(env); + } + match parser.explain(argv) { + Ok(out) => Explanation::from_parse(argv, &out, true, None), + // The parse could not carry on: a mount that will not run, a word no declaration can + // take, a value refused by `choices`. The binding phase still knows which tokens got + // that far, so ask it on its own rather than reporting the error with nothing around + // it. Mounts resolve eagerly on this path and the environment is not consulted, which + // is why the report says the fallbacks did not run. + Err(refused) => match usage::parse::parse_partial(spec, argv) { + Ok(out) => Explanation::from_parse(argv, &out, false, Some(refused.to_string())), + Err(_) => Explanation { + argv: argv.to_vec(), + refused: Some(refused.to_string()), + fallbacks_applied: false, + ..Explanation::default() + }, + }, + } +} + +#[derive(Debug, Default, serde::Serialize)] +pub struct Explanation { + /// The command line, as it was given. + pub argv: Vec, + /// The command path the parse resolved to. + pub command: Vec, + pub tokens: Vec, + pub values: Vec, + /// Declared defaults that did not win, and what won instead. + pub shadowed: Vec, + /// Flags a later occurrence removed, and the flag that removed them. + pub overridden: Vec, + pub errors: Vec, + /// The one failure the parse could not continue past, if there was one. + pub refused: Option, + /// Whether the environment-and-defaults phase ran. False when only the binding phase + /// could be reported. + pub fallbacks_applied: bool, +} + +#[derive(Debug, serde::Serialize)] +pub struct TokenRow { + pub index: usize, + pub text: String, + /// The parser read something else here — the basename of a multicall program, the tail + /// of a short bundle. `text` is what the caller wrote. + pub synthesized: bool, + pub roles: Vec, +} + +/// What one token did, named rather than pointing at a declaration. +#[derive(Debug, serde::Serialize)] +#[serde(tag = "kind", rename_all = "snake_case")] +pub enum RoleRow { + Program, + Subcommand { + name: String, + }, + Flag { + name: String, + spelling: String, + negated: bool, + }, + Value { + name: String, + values: Vec, + attached: bool, + }, + Arg { + name: String, + values: Vec, + }, + Separator, + UnknownFlag { + bound_as: Option, + }, + Refused { + reason: String, + }, + External, + Unread, +} + +#[derive(Debug, serde::Serialize)] +pub struct ValueRow { + /// `flag` or `arg`. + pub kind: String, + pub name: String, + /// How the declaration is spelled in help: `--jobs`, ``. + pub display: String, + pub value: String, + /// Where the value came from. Several when one declaration took values from more than + /// one place, which a `var` flag can. + pub origins: Vec, +} + +#[derive(Debug, Clone, serde::Serialize)] +#[serde(tag = "kind", rename_all = "snake_case")] +pub enum OriginRow { + /// Typed, at these argv positions. + Argv { + tokens: Vec, + }, + DefaultMissing, + Env { + name: String, + }, + Default, + DefaultIf { + selector: String, + when: Option, + }, +} + +#[derive(Debug, serde::Serialize)] +pub struct ShadowRow { + pub kind: String, + pub name: String, + pub display: String, + /// The value the declaration would have supplied. + pub value: String, + pub lost_to: Vec, +} + +#[derive(Debug, serde::Serialize)] +pub struct OverrideRow { + pub name: String, + pub by: String, +} + +impl Explanation { + fn from_parse( + argv: &[String], + out: &ParseOutput, + fallbacks_applied: bool, + refused: Option, + ) -> Self { + let tokens = out.tokens.iter().map(TokenRow::from_binding).collect_vec(); + let mut values = vec![]; + let mut shadowed = vec![]; + + for (flag, value) in &out.flags { + let origins = flag_origins(out, flag); + values.push(ValueRow { + kind: "flag".to_string(), + name: flag.name.clone(), + display: flag_display(flag), + value: value.to_string(), + origins: origins.clone(), + }); + // A declared default that did not supply the value lost to whatever did. This is + // the most common thing a spec author is confused about, and the parser records + // exactly enough to answer it without guessing. + if !flag.default.is_empty() && !origins.iter().any(|o| matches!(o, OriginRow::Default)) + { + shadowed.push(ShadowRow { + kind: "flag".to_string(), + name: flag.name.clone(), + display: flag_display(flag), + value: flag.default.join(" "), + lost_to: origins, + }); + } + } + for (arg, value) in &out.args { + let origins = arg_origins(out, arg); + values.push(ValueRow { + kind: "arg".to_string(), + name: arg.name.clone(), + display: arg.usage(), + value: value.to_string(), + origins: origins.clone(), + }); + if !arg.default.is_empty() && !origins.iter().any(|o| matches!(o, OriginRow::Default)) { + shadowed.push(ShadowRow { + kind: "arg".to_string(), + name: arg.name.clone(), + display: arg.usage(), + value: arg.default.join(" "), + lost_to: origins, + }); + } + } + + Self { + argv: argv.to_vec(), + command: out.cmds.iter().map(|cmd| cmd.name.clone()).collect(), + tokens, + values, + shadowed, + overridden: out + .overridden_flags + .iter() + .map(|(name, by)| OverrideRow { + name: name.clone(), + by: by.clone(), + }) + .collect(), + errors: out.errors.iter().map(|e| e.to_string()).collect(), + refused, + fallbacks_applied, + } + } + + /// The text report, as `usage explain` prints it. + pub fn render(&self) -> String { + let mut out = String::new(); + out.push_str(&format!("{}\n", self.argv.join(" "))); + if !self.command.is_empty() { + out.push_str(&format!("command {}\n", self.command.join(" "))); + } + if !self.fallbacks_applied { + out.push_str( + "\nthe parse stopped before the environment and defaults were applied,\n\ + so only what argv bound is shown\n", + ); + } + + out.push_str(&render_table("tokens", &self.tokens, |token| { + let roles = if token.roles.is_empty() { + // Not the same as "not read": the word was read and did nothing this parser + // records, which is worth seeing rather than quietly omitting. + "bound nothing".to_string() + } else { + token.roles.iter().map(render_role).join(", ") + }; + vec![format!("[{}]", token.index), token.text.clone(), roles] + })); + + out.push_str(&render_table("values", &self.values, |row| { + vec![ + row.kind.clone(), + row.display.clone(), + row.value.clone(), + render_origins(&row.origins), + ] + })); + out.push_str(&render_table("shadowed", &self.shadowed, |row| { + vec![ + row.kind.clone(), + row.display.clone(), + format!("default {}", row.value), + format!("lost to {}", render_origins(&row.lost_to)), + ] + })); + out.push_str(&render_table("overridden", &self.overridden, |row| { + vec![row.name.clone(), format!("by --{}", row.by)] + })); + + if !self.errors.is_empty() { + out.push_str("\nerrors\n"); + for error in &self.errors { + for line in error.lines() { + out.push_str(&format!(" {line}\n")); + } + } + } + if let Some(refused) = &self.refused { + out.push_str("\nrefused\n"); + for line in refused.lines() { + out.push_str(&format!(" {line}\n")); + } + } + out + } +} + +/// A titled block of aligned columns, or nothing at all when there are no rows. +/// +/// Every column but the last is padded to its widest cell, which is what makes the report +/// greppable: a reader scanning for "where did this come from" reads down one column. +fn render_table(title: &str, rows: &[T], columns: impl Fn(&T) -> Vec) -> String { + if rows.is_empty() { + return String::new(); + } + let rendered = rows.iter().map(columns).collect_vec(); + let widths = column_widths(&rendered); + let mut out = format!("\n{title}\n"); + for row in &rendered { + out.push_str(" "); + out.push_str(&pad_row(row, &widths)); + out.push('\n'); + } + out +} + +fn column_widths(rows: &[Vec]) -> Vec { + let count = rows.iter().map(Vec::len).max().unwrap_or(0); + (0..count) + .map(|column| { + rows.iter() + .filter_map(|row| row.get(column)) + .map(|cell| cell.chars().count()) + .max() + .unwrap_or(0) + }) + .collect() +} + +fn pad_row(row: &[String], widths: &[usize]) -> String { + row.iter() + .enumerate() + .map(|(column, cell)| { + // The last cell is never padded: trailing whitespace is invisible to a reader + // and annoying to everything else. + if column + 1 == row.len() { + cell.clone() + } else { + format!("{cell:width$}", width = widths[column]) + } + }) + .join(" ") + .trim_end() + .to_string() +} + +impl TokenRow { + fn from_binding(token: &usage::parse::TokenBinding) -> Self { + Self { + index: token.index, + text: token.word.clone(), + synthesized: token.synthesized, + roles: token.roles.iter().map(RoleRow::from_role).collect(), + } + } +} + +impl RoleRow { + fn from_role(role: &TokenRole) -> Self { + match role { + TokenRole::Program => Self::Program, + TokenRole::Command { name } => Self::Subcommand { name: name.clone() }, + TokenRole::Flag { + flag, + spelling, + negated, + } => Self::Flag { + name: flag.name.clone(), + spelling: spelling.clone(), + negated: *negated, + }, + TokenRole::Value { + flag, + values, + attached, + } => Self::Value { + name: flag.name.clone(), + values: values.clone(), + attached: *attached, + }, + TokenRole::Arg { arg, values } => Self::Arg { + name: arg.name.clone(), + values: values.clone(), + }, + TokenRole::Separator => Self::Separator, + TokenRole::UnknownFlag { bound_as } => Self::UnknownFlag { + bound_as: bound_as.as_ref().map(|arg| arg.name.clone()), + }, + TokenRole::Refused { reason } => Self::Refused { + reason: reason.clone(), + }, + TokenRole::External => Self::External, + TokenRole::Unread => Self::Unread, + // The lib's role list is `#[non_exhaustive]`, so a role added there shows up as + // an unexplained token rather than failing to compile a report of it. + _ => Self::Unread, + } + } +} + +fn render_role(role: &RoleRow) -> String { + match role { + RoleRow::Program => "program".to_string(), + RoleRow::Subcommand { name } => format!("subcommand {name}"), + RoleRow::Flag { + spelling, negated, .. + } => { + if *negated { + format!("flag {spelling} (negated)") + } else { + format!("flag {spelling}") + } + } + RoleRow::Value { + name, + values, + attached, + } => { + let attached = if *attached { ", attached" } else { "" }; + format!("value of {name} = {}{attached}", render_values(values)) + } + RoleRow::Arg { name, values } => format!("arg {name} = {}", render_values(values)), + RoleRow::Separator => "separator".to_string(), + RoleRow::UnknownFlag { bound_as } => match bound_as { + // Under the default `unknown_flags="value"` an unmatched flag-like word is data. + // Saying which argument took it is the difference between "you have a typo" and + // "this argument took your typo". + Some(arg) => format!("unknown flag, bound as {arg}"), + None => "unknown flag".to_string(), + }, + RoleRow::Refused { reason } => format!("refused, {reason}"), + RoleRow::External => "forwarded to an external command".to_string(), + RoleRow::Unread => "not read".to_string(), + } +} + +fn render_values(values: &[String]) -> String { + values.iter().map(|value| format!("{value:?}")).join(", ") +} + +fn render_origins(origins: &[OriginRow]) -> String { + if origins.is_empty() { + // Everything the parser filled has an origin; a bool flag that was simply named has + // no value to trace, and neither does a value from a source not yet modelled. + return "-".to_string(); + } + origins + .iter() + .map(|origin| match origin { + OriginRow::Argv { tokens } => { + format!("argv [{}]", tokens.iter().map(|i| i.to_string()).join(", ")) + } + OriginRow::DefaultMissing => "default_missing".to_string(), + OriginRow::Env { name } => format!("env {name}"), + OriginRow::Default => "default".to_string(), + OriginRow::DefaultIf { selector, when } => match when { + Some(when) => format!("default_if {selector} when={when:?}"), + None => format!("default_if {selector}"), + }, + }) + .join(", ") +} + +/// The argv positions that supplied a flag, then whatever the fallback phase recorded. +/// +/// The token trace is the record of what was typed, so it is scanned rather than duplicated +/// into a second map: a flag's own token counts when the flag holds no separate value, which +/// is how a bool or a counted flag gets an argv origin at all. +fn flag_origins(out: &ParseOutput, flag: &SpecFlag) -> Vec { + let mut origins = vec![]; + let mut tokens = vec![]; + for token in &out.tokens { + for role in &token.roles { + match role { + TokenRole::Value { flag: f, .. } if f.name == flag.name => tokens.push(token.index), + TokenRole::Flag { flag: f, .. } if f.name == flag.name && f.arg.is_none() => { + tokens.push(token.index) + } + _ => {} + } + } + } + tokens.dedup(); + if !tokens.is_empty() { + origins.push(OriginRow::Argv { tokens }); + } + origins.extend( + out.flag_origins + .iter() + .filter(|(f, _)| f.name == flag.name) + .flat_map(|(_, recorded)| recorded.iter().map(origin_row)), + ); + origins +} + +fn arg_origins(out: &ParseOutput, arg: &SpecArg) -> Vec { + let mut origins = vec![]; + let mut tokens = vec![]; + for token in &out.tokens { + for role in &token.roles { + match role { + TokenRole::Arg { arg: a, .. } if a.name == arg.name => tokens.push(token.index), + TokenRole::UnknownFlag { bound_as: Some(a) } if a.name == arg.name => { + tokens.push(token.index) + } + _ => {} + } + } + } + tokens.dedup(); + if !tokens.is_empty() { + origins.push(OriginRow::Argv { tokens }); + } + origins.extend( + out.arg_origins + .iter() + .filter(|(a, _)| a.name == arg.name) + .flat_map(|(_, recorded)| recorded.iter().map(origin_row)), + ); + origins +} + +fn origin_row(origin: &ValueOrigin) -> OriginRow { + match origin { + ValueOrigin::DefaultMissing => OriginRow::DefaultMissing, + ValueOrigin::Env(name) => OriginRow::Env { name: name.clone() }, + ValueOrigin::Default => OriginRow::Default, + ValueOrigin::DefaultIf { selector, when } => OriginRow::DefaultIf { + selector: selector.clone(), + when: when.clone(), + }, + // `ValueOrigin` is `#[non_exhaustive]`: a source added to the parser reads as an + // unnamed default here rather than stopping this from compiling. + _ => OriginRow::Default, + } +} + +/// How a flag is spelled in a report: the long form if it has one, else the short. +/// +/// Not [`SpecFlag::usage`], which renders the whole declaration (`-j --jobs `) and is +/// right for help output and too wide for a column. +fn flag_display(flag: &SpecFlag) -> String { + if let Some(long) = flag.long.first() { + return format!("--{long}"); + } + if let Some(short) = flag.short.first() { + return format!("-{short}"); + } + flag.name.clone() +} + +#[cfg(test)] +mod tests { + use super::*; + + fn argv(words: &[&str]) -> Vec { + words.iter().map(|w| (*w).to_string()).collect() + } + + fn env(pairs: &[(&str, &str)]) -> Option> { + Some( + pairs + .iter() + .map(|(k, v)| ((*k).to_string(), (*v).to_string())) + .collect(), + ) + } + + fn fixture() -> Spec { + std::fs::read_to_string(concat!( + env!("CARGO_MANIFEST_DIR"), + "/../examples/explain.usage.kdl" + )) + .unwrap() + .parse() + .unwrap() + } + + fn roles(explanation: &Explanation, index: usize) -> Vec { + explanation + .tokens + .iter() + .find(|token| token.index == index) + .unwrap_or_else(|| panic!("no token at {index}")) + .roles + .iter() + .map(render_role) + .collect() + } + + fn value(explanation: &Explanation, name: &str) -> String { + let row = explanation + .values + .iter() + .find(|row| row.name == name) + .unwrap_or_else(|| panic!("no value for {name}")); + format!("{} {}", row.value, render_origins(&row.origins)) + } + + #[test] + fn the_attached_form_binds_and_the_report_says_where() { + let spec = fixture(); + let explanation = explain( + &spec, + &argv(&["mycli", "--env=prod", "build", "a"]), + env(&[]), + ); + + // jdx/mise discussion #8883: mise's hand-written scanner ignored `--env=production` + // while `--env production` worked, and nothing would show the difference. + assert_eq!( + roles(&explanation, 1), + ["flag --env", "value of env = \"prod\", attached"] + ); + assert_eq!(value(&explanation, "env"), "prod argv [1]"); + } + + #[test] + fn a_short_bundle_reads_as_one_token() { + let spec = fixture(); + let explanation = explain(&spec, &argv(&["mycli", "-sj8", "build", "a"]), env(&[])); + + assert_eq!( + roles(&explanation, 1), + ["flag -s", "flag -j", "value of jobs = \"8\", attached"] + ); + // Not synthesized: `-sj8` is the word the caller wrote, and its tails are + // continuations of it. That flag is for a position where the parser read something + // else entirely, which is the multicall case. + assert!(!explanation.tokens[1].synthesized); + assert_eq!(explanation.tokens.len(), 4); + } + + #[test] + fn a_separator_is_not_a_value_and_what_follows_it_is() { + let spec = fixture(); + let explanation = explain( + &spec, + &argv(&["mycli", "build", "a", "--", "--raw"]), + env(&[]), + ); + + assert_eq!(roles(&explanation, 3), ["separator"]); + assert_eq!(roles(&explanation, 4), ["arg extra = \"--raw\""]); + } + + #[test] + fn an_env_value_names_the_variable() { + let spec = fixture(); + let explanation = explain( + &spec, + &argv(&["mycli", "build", "a"]), + env(&[("MYCLI_COLOR", "never")]), + ); + + assert_eq!(value(&explanation, "color"), "never env MYCLI_COLOR"); + } + + #[test] + fn a_shadowed_default_says_what_beat_it() { + let spec = fixture(); + let explanation = explain( + &spec, + &argv(&["mycli", "-j", "8", "build", "a"]), + env(&[("MYCLI_COLOR", "never")]), + ); + + let shadowed = explanation + .shadowed + .iter() + .map(|row| { + format!( + "{} {} {}", + row.display, + row.value, + render_origins(&row.lost_to) + ) + }) + .collect::>(); + assert!( + shadowed.contains(&"--jobs 1 argv [2]".to_string()), + "{shadowed:?}" + ); + assert!( + shadowed.contains(&"--color auto env MYCLI_COLOR".to_string()), + "{shadowed:?}" + ); + } + + #[test] + fn a_default_if_says_which_condition_fired() { + let spec = fixture(); + let explanation = explain( + &spec, + &argv(&["mycli", "--profile", "prod", "build", "a"]), + env(&[]), + ); + + assert_eq!( + value(&explanation, "strict"), + "true default_if --profile when=\"prod\"" + ); + } + + #[test] + fn an_unknown_flag_says_what_took_it() { + let spec = fixture(); + let explanation = explain(&spec, &argv(&["mycli", "build", "--wat"]), env(&[])); + + // Lax unknown flags are the default, so the word became data. Which is the useful + // thing to be told: the other reading is "you have a typo". + assert_eq!(roles(&explanation, 2), ["unknown flag, bound as target"]); + } + + #[test] + fn a_word_offered_to_an_argument_that_refused_it_says_so() { + let spec = fixture(); + let explanation = explain(&spec, &argv(&["mycli", "build", "a", "b"]), env(&[])); + + // `extra` only accepts words after `--`, so `b` was dropped. Reporting the token as + // having done nothing would hide the one thing that happened to it. + assert_eq!( + roles(&explanation, 3), + ["refused, extra only accepts words after `--`"] + ); + } + + #[test] + fn a_command_line_that_fails_still_gets_a_report() { + let spec = fixture(); + let explanation = explain(&spec, &argv(&["mycli", "--env=prod", "build"]), env(&[])); + + // The bindings that worked, then the complaint — rather than the complaint alone, + // which is the report the caller already had. + assert_eq!(value(&explanation, "env"), "prod argv [1]"); + assert!( + explanation.errors.iter().any(|e| e.contains("target")), + "{:?}", + explanation.errors + ); + assert!(explanation.refused.is_none()); + assert!(explanation.fallbacks_applied); + } + + #[test] + fn a_bare_invocation_explains_the_fallbacks_alone() { + let spec = fixture(); + let explanation = explain(&spec, &argv(&["mycli"]), env(&[("MYCLI_COLOR", "never")])); + + assert_eq!(roles(&explanation, 0), ["program"]); + assert_eq!(value(&explanation, "color"), "never env MYCLI_COLOR"); + assert_eq!(value(&explanation, "jobs"), "1 default"); + } + + #[test] + fn the_json_shape_carries_the_same_facts() { + let spec = fixture(); + let explanation = explain( + &spec, + &argv(&["mycli", "--env=prod", "build", "a"]), + env(&[]), + ); + let json = serde_json::to_value(&explanation).unwrap(); + + assert_eq!(json["tokens"][1]["roles"][0]["kind"], "flag"); + assert_eq!(json["tokens"][1]["roles"][1]["kind"], "value"); + assert_eq!(json["tokens"][1]["roles"][1]["attached"], true); + let env_row = json["values"] + .as_array() + .unwrap() + .iter() + .find(|row| row["name"] == "env") + .unwrap() + .clone(); + assert_eq!(env_row["origins"][0]["kind"], "argv"); + assert_eq!(env_row["origins"][0]["tokens"][0], 1); + } +} diff --git a/cli/src/cli/lint.rs b/cli/src/cli/lint.rs index d7aa62892..bc9489e4b 100644 --- a/cli/src/cli/lint.rs +++ b/cli/src/cli/lint.rs @@ -6,6 +6,7 @@ use usage::spec::cmd::SpecExample; use usage::{Parser, Spec, SpecArg, SpecCommand, SpecFlag, SpecFlagAction}; use crate::cli::generate::parse_file_or_stdin; +use crate::cli::OutputFormat; /// Lint a usage spec file for common issues #[derive(usage_rs::Args)] @@ -38,25 +39,6 @@ pub struct LintOptions { pub sorted: bool, } -#[derive(Clone, Copy, Default, usage_rs::ValueEnum)] -enum OutputFormat { - #[default] - Text, - Json, -} - -impl std::str::FromStr for OutputFormat { - type Err = String; - - fn from_str(value: &str) -> Result { - match value { - "text" => Ok(Self::Text), - "json" => Ok(Self::Json), - _ => Err(format!("`{value}` is not one of: text, json")), - } - } -} - #[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize)] #[serde(rename_all = "lowercase")] pub enum Severity { diff --git a/cli/src/cli/mod.rs b/cli/src/cli/mod.rs index 272b0e1f9..97428d51b 100644 --- a/cli/src/cli/mod.rs +++ b/cli/src/cli/mod.rs @@ -5,6 +5,7 @@ use usage_rs::{Cli as DeriveCli, Subcommands}; pub mod complete_word; mod exec; +mod explain; pub(crate) mod generate; mod lint; mod mcp; @@ -101,6 +102,7 @@ enum Command { Bash(shell::Bash), CompleteWord(complete_word::CompleteWord), Exec(exec::Exec), + Explain(explain::Explain), Fish(shell::Fish), Generate(generate::Generate), Lint(lint::Lint), @@ -155,3 +157,26 @@ impl Cli { usage_rs::Run::run(cli.command) } } + +/// How a command that can print either prose or JSON was asked to print. +/// +/// Shared rather than declared twice: `lint` and `explain` both offer it, and a third +/// copy of the same four-line `FromStr` is how the two spellings drift apart. +#[derive(Debug, Clone, Copy, Default, usage_rs::ValueEnum)] +pub(crate) enum OutputFormat { + #[default] + Text, + Json, +} + +impl std::str::FromStr for OutputFormat { + type Err = String; + + fn from_str(value: &str) -> Result { + match value { + "text" => Ok(Self::Text), + "json" => Ok(Self::Json), + _ => Err(format!("`{value}` is not one of: text, json")), + } + } +} diff --git a/cli/tests/explain.rs b/cli/tests/explain.rs new file mode 100644 index 000000000..610fc2acf --- /dev/null +++ b/cli/tests/explain.rs @@ -0,0 +1,213 @@ +use assert_cmd::Command; +use predicates::str::contains; + +fn usage_cmd() -> Command { + Command::new(assert_cmd::cargo::cargo_bin!("usage")) +} + +fn example_path(name: &str) -> String { + format!("{}/../examples/{}", env!("CARGO_MANIFEST_DIR"), name) +} + +/// The fixture, the whole environment, and whatever argv the test is about. +/// +/// The environment goes through `--env` rather than through the child process: `Parser` +/// reads the real environment when it is given no map, so a test that set `MYCLI_COLOR` on +/// the child would still be at the mercy of whatever else the machine exports. +fn explain(argv: &[&str]) -> String { + let mut cmd = usage_cmd(); + cmd.args(["explain", "-f", &example_path("explain.usage.kdl")]); + cmd.args(["-e", "MYCLI_COLOR=never", "-e", "MYCLI_PROFILE=prod"]); + cmd.arg("--"); + cmd.args(argv); + let output = cmd.output().unwrap(); + assert!(output.status.success(), "{output:?}"); + String::from_utf8(output.stdout).unwrap() +} + +#[test] +fn explains_the_worked_example() { + insta::assert_snapshot!(explain(&[ + "mycli", + "-j8", + "--env=prod", + "build", + "a", + "b", + "--", + "--raw" + ])); +} + +#[test] +fn binds_an_attached_long_flag() { + // jdx/mise discussion #8883: a hand-written scanner ignored `--env=production` while + // `--env production` worked. Both forms bind here, and the report says which token did. + let attached = explain(&["mycli", "--env=production", "build", "a"]); + let detached = explain(&["mycli", "--env", "production", "build", "a"]); + + assert!( + attached.contains("value of env = \"production\", attached"), + "{attached}" + ); + assert!( + detached.contains("value of env = \"production\""), + "{detached}" + ); + for report in [&attached, &detached] { + assert!(report.contains("--env"), "{report}"); + assert!(report.contains("production"), "{report}"); + } +} + +#[test] +fn the_separator_is_optional_before_the_explained_line() { + let mut cmd = usage_cmd(); + cmd.args(["explain", "-f", &example_path("explain.usage.kdl")]); + cmd.args(["-e", "MYCLI_COLOR=never", "-e", "MYCLI_PROFILE=prod"]); + // No `--`: `double_dash="automatic"` ends this command's own flag parsing at the + // program name, so a foreign `--env=prod` is data rather than a flag `usage` rejects. + cmd.args(["mycli", "-j8", "--env=prod", "build", "a"]); + let without = String::from_utf8(cmd.output().unwrap().stdout).unwrap(); + + assert_eq!( + without, + explain(&["mycli", "-j8", "--env=prod", "build", "a"]) + ); +} + +#[test] +fn a_line_of_its_own_needs_the_separator_to_keep_a_double_dash() { + let mut cmd = usage_cmd(); + cmd.args(["explain", "-f", &example_path("explain.usage.kdl")]); + cmd.args(["mycli", "build", "a", "--", "--raw"]); + let without = String::from_utf8(cmd.output().unwrap().stdout).unwrap(); + + // `usage`'s own parse takes the first `--` as its separator — `automatic` ends flag + // parsing but does not stop a later separator being honoured, which is what + // a78564c0 settled. So an explained line carrying its own `--` needs the leading one, + // and the report shows the difference rather than hiding it. + let with = explain(&["mycli", "build", "a", "--", "--raw"]); + assert!(without.starts_with("mycli build a --raw\n"), "{without}"); + assert!(with.starts_with("mycli build a -- --raw\n"), "{with}"); + assert!(with.contains("-- separator"), "{with}"); +} + +#[test] +fn a_second_separator_stays_data() { + let report = explain(&["mycli", "build", "a", "--", "--raw", "--", "more"]); + + // The first `--` separates; every later one is data, which is what every parser worth + // comparing against does and what jdx/usage#229 was about. + assert!(report.contains("[3] -- separator"), "{report}"); + assert!( + report.contains("[5] -- arg extra = \"--\""), + "{report}" + ); +} + +#[test] +fn reports_a_command_line_that_does_not_parse_and_exits_zero() { + let mut cmd = usage_cmd(); + cmd.args(["explain", "-f", &example_path("explain.usage.kdl")]); + cmd.args(["--", "mycli", "--env=prod", "build"]); + + // Exit 0: the report succeeded, and this is the case a report is wanted for. Anything + // else would make the tool useless under `set -e`. + cmd.assert() + .success() + .stdout(contains("value of env = \"prod\", attached")) + .stdout(contains("errors")) + .stdout(contains("target")); +} + +#[test] +fn reports_a_parse_that_could_not_continue() { + let mut cmd = usage_cmd(); + cmd.args(["explain", "-f", &example_path("explain.usage.kdl")]); + cmd.args(["--", "mycli", "-j"]); + + cmd.assert() + .success() + .stdout(contains( + "the parse stopped before the environment and defaults", + )) + .stdout(contains("flag -j")) + .stdout(contains("requires an argument")); +} + +#[test] +fn json_carries_the_same_facts() { + let mut cmd = usage_cmd(); + cmd.args(["explain", "-f", &example_path("explain.usage.kdl")]); + cmd.args([ + "--format", + "json", + "--", + "mycli", + "--env=prod", + "build", + "a", + ]); + let stdout = String::from_utf8(cmd.output().unwrap().stdout).unwrap(); + + let json: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + assert_eq!(json["command"], serde_json::json!(["mycli", "build"])); + assert_eq!(json["tokens"][1]["roles"][1]["kind"], "value"); + assert_eq!(json["tokens"][1]["roles"][1]["attached"], true); + assert_eq!(json["fallbacks_applied"], true); +} + +#[test] +fn reads_a_spec_from_stdin() { + let mut cmd = usage_cmd(); + cmd.args(["explain", "-f", "-", "--", "mycli", "--jobs", "8"]); + cmd.write_stdin("name \"mycli\"\nbin \"mycli\"\nflag \"--jobs \"\n"); + + cmd.assert().success().stdout(contains("value of jobs")); +} + +#[test] +fn reads_a_spec_from_an_argument() { + let mut cmd = usage_cmd(); + cmd.args([ + "explain", + "-s", + "name \"mycli\"\nbin \"mycli\"\nflag \"--jobs \" default=\"1\"\n", + "--", + "mycli", + ]); + + cmd.assert() + .success() + .stdout(contains("--jobs 1 default")); +} + +#[test] +fn a_bare_invocation_explains_the_fallbacks_alone() { + let mut cmd = usage_cmd(); + cmd.args(["explain", "-f", &example_path("explain.usage.kdl")]); + cmd.args(["-e", "MYCLI_COLOR=never"]); + + // No argv at all is a real question: which defaults and environment values fire when + // nothing is typed. + cmd.assert() + .success() + .stdout(contains("--color never env MYCLI_COLOR")) + .stdout(contains("--jobs 1 default")); +} + +#[test] +fn its_own_unknown_flags_are_still_refused() { + let mut cmd = usage_cmd(); + cmd.args([ + "explain", + "-f", + &example_path("explain.usage.kdl"), + "--nope", + ]); + + // Deliberately unlike `exec`: a typo in `usage explain`'s own flags is a mistake, not + // data, because the explained line starts at its program name. + cmd.assert().failure(); +} diff --git a/cli/tests/snapshots/explain__explains_the_worked_example.snap b/cli/tests/snapshots/explain__explains_the_worked_example.snap new file mode 100644 index 000000000..434540ba6 --- /dev/null +++ b/cli/tests/snapshots/explain__explains_the_worked_example.snap @@ -0,0 +1,32 @@ +--- +source: cli/tests/explain.rs +expression: "explain(&[\"mycli\", \"-j8\", \"--env=prod\", \"build\", \"a\", \"b\", \"--\", \"--raw\"])" +--- +mycli -j8 --env=prod build a b -- --raw +command mycli build + +tokens + [0] mycli program + [1] -j8 flag -j, value of jobs = "8", attached + [2] --env=prod flag --env, value of env = "prod", attached + [3] build subcommand build + [4] a arg target = "a" + [5] b refused, extra only accepts words after `--` + [6] -- separator + [7] --raw arg extra = "--raw" + +values + flag --jobs 8 argv [1] + flag --env prod argv [2] + flag --color never env MYCLI_COLOR + flag --profile prod env MYCLI_PROFILE + flag --strict true default_if --profile when="prod" + arg a argv [4] + arg [-- extra]… --raw argv [7] + +shadowed + flag --jobs default 1 lost to argv [1] + flag --color default auto lost to env MYCLI_COLOR + +errors + Argument can only be set after a `--` separator diff --git a/cli/usage.usage.kdl b/cli/usage.usage.kdl index 8522bcbb0..438bd0c68 100644 --- a/cli/usage.usage.kdl +++ b/cli/usage.usage.kdl @@ -49,6 +49,35 @@ cmd exec help="Execute a script, parsing args and exposing them as environment v arg help="path to script to execute" arg "[ARGS]..." help="arguments to pass to script" } +cmd explain help="Explain what a command line binds to" effect=read { + long_help "Explain what a command line binds to\n\nPrints a row per argv token saying what it became, then the values that came from\nsomewhere other than argv, then anything that went wrong. Exits 0 even when the explained\ncommand line does not parse: the report succeeded, and that is the case worth a report." + flag "-f --file" help="A usage spec file or script with a usage shebang, use \"-\" to read from stdin" { + arg + } + flag "-s --spec" help="Raw string spec input" overrides=--file required_unless=--file { + arg + } + flag --format help="Output format" default=text { + arg { + choices { + choice text + choice json + } + } + } + flag --view help="A spec-declared executable view to explain" { + arg + } + flag "-e --env" help="Environment to explain against, as KEY=VALUE, repeatable" var=#true { + long_help "Environment to explain against, as KEY=VALUE, repeatable\n\nGiven at all, these are the *whole* environment: an explanation pasted into a bug\nreport has to mean the same thing on the machine that reads it. Omitted, the process\nenvironment is used, which is what an execution would see." + arg + } + arg "[ARGV]..." help="The command line to explain, starting with the program name" double_dash=automatic { + long_help "The command line to explain, starting with the program name\n\n`usage`'s own flags come before it, and flag parsing ends at the program name, so\nboth `explain -f f.kdl mycli --env=prod` and `explain -f f.kdl -- mycli --env=prod`\nwork. Separate with `--` when the explained line carries its own: the first `--` is\nstill `usage`'s separator, so `explain -f f.kdl mycli a -- b` loses one." + } + complete argv type=command_args + complete file type=path +} cmd fish help="Execute a shell script using fish" unknown_flags=value { long_help "Execute a shell script with the specified shell\n\nTypically, this will be called by a script's shebang.\n\nIf using `var=#true` on args/flags, they will be joined with spaces using `shell_words::join()`\nto properly escape and quote values with spaces in them." flag -h help="Show help" diff --git a/docs/cli/reference/commands.json b/docs/cli/reference/commands.json index f937aa8fd..cfb2453d2 100644 --- a/docs/cli/reference/commands.json +++ b/docs/cli/reference/commands.json @@ -230,6 +230,148 @@ "hidden_aliases": [], "examples": [] }, + "explain": { + "full_cmd": ["explain"], + "usage": "explain [FLAGS] [ARGV]…", + "subcommands": {}, + "args": [ + { + "name": "ARGV", + "usage": "[ARGV]…", + "help": "The command line to explain, starting with the program name", + "help_long": "The command line to explain, starting with the program name\n\n`usage`'s own flags come before it, and flag parsing ends at the program name, so\nboth `explain -f f.kdl mycli --env=prod` and `explain -f f.kdl -- mycli --env=prod`\nwork. Separate with `--` when the explained line carries its own: the first `--` is\nstill `usage`'s separator, so `explain -f f.kdl mycli a -- b` loses one.", + "help_first_line": "The command line to explain, starting with the program name", + "required": false, + "double_dash": "Automatic", + "var": true, + "hide": false + } + ], + "flags": [ + { + "name": "file", + "usage": "-f --file ", + "help": "A usage spec file or script with a usage shebang, use \"-\" to read from stdin", + "help_first_line": "A usage spec file or script with a usage shebang, use \"-\" to read from stdin", + "short": ["f"], + "long": ["file"], + "hide": false, + "global": false, + "arg": { + "name": "FILE", + "usage": "", + "required": true, + "double_dash": "Optional", + "hide": false + } + }, + { + "name": "spec", + "usage": "-s --spec ", + "help": "Raw string spec input", + "help_first_line": "Raw string spec input", + "short": ["s"], + "long": ["spec"], + "required_unless": ["--file"], + "hide": false, + "global": false, + "arg": { + "name": "SPEC", + "usage": "", + "required": true, + "double_dash": "Optional", + "hide": false + }, + "overrides": ["--file"] + }, + { + "name": "format", + "usage": "--format ", + "help": "Output format", + "help_first_line": "Output format", + "short": [], + "long": ["format"], + "hide": false, + "global": false, + "arg": { + "name": "FORMAT", + "usage": "", + "required": true, + "double_dash": "Optional", + "hide": false, + "choices": { + "choices": ["text", "json"], + "details": [ + { + "value": "text" + }, + { + "value": "json" + } + ] + } + }, + "default": ["text"] + }, + { + "name": "view", + "usage": "--view ", + "help": "A spec-declared executable view to explain", + "help_first_line": "A spec-declared executable view to explain", + "short": [], + "long": ["view"], + "hide": false, + "global": false, + "arg": { + "name": "VIEW", + "usage": "", + "required": true, + "double_dash": "Optional", + "hide": false + } + }, + { + "name": "env", + "usage": "-e --env… ", + "help": "Environment to explain against, as KEY=VALUE, repeatable", + "help_long": "Environment to explain against, as KEY=VALUE, repeatable\n\nGiven at all, these are the *whole* environment: an explanation pasted into a bug\nreport has to mean the same thing on the machine that reads it. Omitted, the process\nenvironment is used, which is what an execution would see.", + "help_first_line": "Environment to explain against, as KEY=VALUE, repeatable", + "short": ["e"], + "long": ["env"], + "var": true, + "hide": false, + "global": false, + "arg": { + "name": "ENV", + "usage": "", + "required": true, + "double_dash": "Optional", + "hide": false + } + } + ], + "mounts": [], + "effect": "read", + "unknown_flags": null, + "hide": false, + "args_override_self": true, + "help": "Explain what a command line binds to", + "help_long": "Explain what a command line binds to\n\nPrints a row per argv token saying what it became, then the values that came from\nsomewhere other than argv, then anything that went wrong. Exits 0 even when the explained\ncommand line does not parse: the report succeeded, and that is the case worth a report.", + "name": "explain", + "aliases": [], + "hidden_aliases": [], + "examples": [], + "complete": { + "argv": { + "name": "argv", + "type_": "command_args" + }, + "file": { + "name": "file", + "type_": "path" + } + } + }, "fish": { "full_cmd": ["fish"], "usage": "fish [-h] [--help]