diff --git a/PLAN.md b/PLAN.md index 7abf42619..e78746ae6 100644 --- a/PLAN.md +++ b/PLAN.md @@ -417,13 +417,17 @@ Groups are the opposite case: `Command::get_groups`, `ArgGroup::get_args` and usage-argv, generated Go, and the clap bridge. It suppresses delimiter splitting only after `--` or once a `double_dash="automatic"` positional begins; the same argument still splits ordinary values. -- [ ] **Fixed arity and distinct value names** — clap can say - `num_args(2)` with ` `. `var_min` / `var_max` express the bound, +- [x] **Fixed arity and distinct value names** — clap can say + `num_args(2)` with ` `. Nested value `var_min` / `var_max` + express the per-occurrence bound without becoming limits on repeatable flag + occurrences, and the clap bridge now preserves it for positionals and non-repeatable value - flags. Optional-value ranges beginning at zero and repeatable `Append` ranges - remain bridge-lossy because their per-occurrence semantics cannot be represented - as one accumulated bound. The derive also has one display name for the collection, - so distinct ` ` labels are still absent. + flags. Repeatable `Append` ranges are enforced per occurrence. Optional-value ranges + beginning at zero remain bridge-lossy when clap exposes no `default_missing_value`. + Distinct names beside a non-fixed range are reported as lossy and reduced to the first + label so the emitted KDL remains valid. `#[arg(num_args = 2, value_names = ["START", "END"])]` + now preserves the exact bound and each display label through the derive, KDL, + clap bridge, Rust and Go parsers, help, and generated tables. - [x] **`allow_hyphen_values` on the derive path** — the spec said it and usage-lib honoured it (`lib/src/parse.rs`); usage-argv now has the same bit on `Flag`, so a detached value that looks like a flag binds when @@ -652,8 +656,10 @@ feature list is not an exhaustive audit. can remain on the command. Hidden `alias` / `aliases` retain their parse-only visibility through KDL, Rust and Go tables, help, completion, and the clap bridge. Non-default `rename_all` / `rename_all_env` casing and bare `env` - now migrate in place; `num_args` and `value_parser` produce targeted - migration diagnostics instead of a generic unknown-option error. + now migrate in place. `num_args` maps to portable bounds (including fixed + arity with distinct `value_names`); optional-value shapes that need + per-occurrence presence semantics and arbitrary `value_parser` callbacks + produce targeted migration diagnostics instead of a generic unknown-option error. - [x] **Command-with-arguments completion hints.** `ExecutablePath`, `CommandName`, `CommandString`, and `CommandWithArguments` lower to shell-native completion types. A forwarded argv vector offers commands for diff --git a/argv/src/help.rs b/argv/src/help.rs index 0c1cdcf96..2b2593687 100644 --- a/argv/src/help.rs +++ b/argv/src/help.rs @@ -253,7 +253,6 @@ fn flag_usage_masked(meta: &FlagMeta<'_>, show: &Shown) -> String { out.push('…'); } if flag.takes_value { - let name = meta.value_name.unwrap_or(flag.name); // Angled where the value must be given, squared where it need not — the same brackets // an argument uses, and for the same reason. pitchfork's `--bump` is the fleet's case. let (open, close) = if meta.value_optional { @@ -261,8 +260,31 @@ fn flag_usage_masked(meta: &FlagMeta<'_>, show: &Shown) -> String { } else { ('<', '>') }; - let _ = write!(out, " {open}{name}{close}"); - if flag.variadic { + let exact = exact_arity(meta.value_var_min, meta.value_var_max); + if meta.value_names.len() <= 1 && exact.is_some_and(|n| n > 1) { + let name = meta + .value_names + .first() + .copied() + .or(meta.value_name) + .unwrap_or(flag.name); + for _ in 0..exact.unwrap() { + let _ = write!(out, " {open}{name}{close}"); + } + } else if meta.value_names.len() <= 1 { + let name = meta + .value_names + .first() + .copied() + .or(meta.value_name) + .unwrap_or(flag.name); + let _ = write!(out, " {open}{name}{close}"); + } else { + for name in meta.value_names { + let _ = write!(out, " {open}{name}{close}"); + } + } + if flag.variadic && meta.value_names.len() <= 1 && exact.is_none() { out.push('…'); } } @@ -308,17 +330,44 @@ pub(crate) fn arg_usage(meta: &ArgMeta<'_>) -> String { // value without it does not reach this argument at all — and the brackets go *outside* // it, as usage-lib writes it: `[-- COMMAND]…`, one optional thing rather than a literal // `--` followed by an optional word. - if arg.double_dash == DoubleDash::Required { - let _ = write!(out, "{open}-- {}{close}", arg.name); + let exact = exact_arity(meta.var_min, meta.var_max); + if meta.value_names.len() <= 1 && exact.is_some_and(|n| n > 1) { + for index in 0..exact.unwrap() { + if index > 0 { + out.push(' '); + } + let _ = write!(out, "{open}{}{close}", arg.name); + } + } else if meta.value_names.len() <= 1 { + if arg.double_dash == DoubleDash::Required { + let _ = write!(out, "{open}-- {}{close}", arg.name); + } else { + let _ = write!(out, "{open}{}{close}", arg.name); + } } else { - let _ = write!(out, "{open}{}{close}", arg.name); + if arg.double_dash == DoubleDash::Required { + out.push_str("-- "); + } + for (index, name) in meta.value_names.iter().enumerate() { + if index > 0 { + out.push(' '); + } + let _ = write!(out, "{open}{name}{close}"); + } } - if arg.var { + if arg.var && meta.value_names.len() <= 1 && exact.is_none() { out.push('…'); } out } +fn exact_arity(min: Option, max: Option) -> Option { + match (min, max) { + (Some(min), Some(max)) if min == max => Some(min), + _ => None, + } +} + /// Everything `-h` prints. /// /// The short form: one line per entry, its help beside it. `--help` renders the same content diff --git a/argv/src/spec.rs b/argv/src/spec.rs index d150312a4..20ad8fdd1 100644 --- a/argv/src/spec.rs +++ b/argv/src/spec.rs @@ -769,6 +769,8 @@ pub struct FlagMeta<'a> { pub long_help: Option<&'a str>, /// The placeholder for the flag's value, such as `n` in `--jobs `. pub value_name: Option<&'a str>, + /// Ordered placeholders for one fixed-arity occurrence. + pub value_names: &'a [&'a str], pub env: Option<&'a str>, pub default: &'a [&'a str], /// Canonical choices plus aliases accepted by the value type. @@ -807,6 +809,10 @@ pub struct FlagMeta<'a> { pub repeatable: bool, pub var_min: Option, pub var_max: Option, + /// Bounds on values consumed by one occurrence, distinct from the + /// flag-level occurrence bounds above. + pub value_var_min: Option, + pub value_var_max: Option, /// Flags this one displaces when both are given. pub overrides: &'a [&'a str], /// Flags that cannot be given alongside this one. @@ -854,6 +860,7 @@ impl FlagMeta<'_> { help: None, long_help: None, value_name: None, + value_names: &[], env: None, default: &[], accepted_choices: &[], @@ -870,6 +877,8 @@ impl FlagMeta<'_> { repeatable: false, var_min: None, var_max: None, + value_var_min: None, + value_var_max: None, overrides: &[], conflicts: &[], delimiter: None, @@ -906,6 +915,8 @@ pub struct DefaultIf<'a> { #[derive(Debug, Clone, Copy)] pub struct ArgMeta<'a> { pub arg: &'a Arg<'a>, + /// Ordered placeholders for a fixed-arity positional. + pub value_names: &'a [&'a str], pub help: Option<&'a str>, pub long_help: Option<&'a str>, pub env: Option<&'a str>, @@ -947,6 +958,7 @@ impl ArgMeta<'_> { complete: None, complete_type: None, arg: &Arg::REQUIRED, + value_names: &[], help: None, long_help: None, env: None, @@ -1598,12 +1610,40 @@ fn write_flag(out: &mut String, meta: &FlagMeta<'_>, depth: usize) -> core::fmt: write_many_list(out, "required_unless", meta.required_unless, inner)?; if meta.flag.takes_value { indent(out, inner)?; - let name = meta.value_name.unwrap_or(meta.flag.name); - write!( - out, - "arg {}", - quoted(&placeholder(name, meta.flag.variadic, meta.value_optional)) - )?; + let exact = exact_arity(meta.value_var_min, meta.value_var_max); + let rendered = if meta.value_names.len() <= 1 && exact.is_some_and(|n| n > 1) { + let name = meta + .value_names + .first() + .copied() + .or(meta.value_name) + .unwrap_or(meta.flag.name); + (0..exact.unwrap()) + .map(|_| placeholder(name, false, meta.value_optional)) + .collect::>() + .join(" ") + } else if meta.value_names.len() <= 1 { + let name = meta + .value_names + .first() + .copied() + .or(meta.value_name) + .unwrap_or(meta.flag.name); + placeholder(name, meta.flag.variadic, meta.value_optional) + } else { + meta.value_names + .iter() + .map(|name| placeholder(name, false, meta.value_optional)) + .collect::>() + .join(" ") + }; + write!(out, "arg {}", quoted(&rendered))?; + if let Some(min) = meta.value_var_min { + write!(out, " var_min={min}")?; + } + if let Some(max) = meta.value_var_max { + write!(out, " var_max={max}")?; + } // Square brackets alone would round-trip as required, since usage-lib reads the // brackets *and* the attribute: `[BUMP]` without it comes back `required=#true`. if meta.value_optional { @@ -1990,8 +2030,42 @@ fn placeholder(name: &str, variadic: bool, optional: bool) -> String { format!("{open}{name}{close}{ellipsis}") } +fn exact_arity(min: Option, max: Option) -> Option { + match (min, max) { + (Some(min), Some(max)) if min == max => Some(min), + _ => None, + } +} + /// A positional's placeholder: angle brackets when required, square when not. fn arg_placeholder(name: &str, meta: &ArgMeta<'_>) -> String { + if let Some(arity) = + exact_arity(meta.var_min, meta.var_max).filter(|n| *n > 1 && meta.value_names.len() <= 1) + { + let values = (0..arity) + .map(|_| placeholder(name, false, !meta.required)) + .collect::>() + .join(" "); + return values; + } + if meta.value_names.len() > 1 { + let (open, close) = if meta.required { + ('<', '>') + } else { + ('[', ']') + }; + let values = meta + .value_names + .iter() + .map(|name| format!("{open}{name}{close}")) + .collect::>() + .join(" "); + return if meta.arg.double_dash == DoubleDash::Required { + format!("-- {values}") + } else { + values + }; + } let ellipsis = if meta.arg.var { "..." } else { "" }; if meta.required { format!("<{name}>{ellipsis}") diff --git a/clap_usage/src/report.rs b/clap_usage/src/report.rs index ae6a6b738..1b65863c0 100644 --- a/clap_usage/src/report.rs +++ b/clap_usage/src/report.rs @@ -202,18 +202,11 @@ fn argument_losses(arg: &Arg, path: &[String], losses: &mut BTreeSet 1) { - add( - FidelityFeature::DistinctValueNames, - format!("value_names={names:?}"), - ); - } if let Some(range) = arg.get_num_args() { let takes_values = matches!(arg.get_action(), ArgAction::Set | ArgAction::Append); let lost = takes_values && (arg.get_value_delimiter().is_some() - || !arg.is_positional() - && (range.min_values() == 0 || matches!(arg.get_action(), ArgAction::Append))); + || !arg.is_positional() && range.min_values() == 0); if lost { add( FidelityFeature::ValueArity, @@ -226,6 +219,21 @@ fn argument_losses(arg: &Arg, path: &[String], losses: &mut BTreeSet 1 + && (arg.get_value_delimiter().is_some() + || arg.get_num_args().is_some_and(|range| { + range.min_values() != value_names.len() || range.max_values() != value_names.len() + })) + { + add( + FidelityFeature::DistinctValueNames, + format!( + "{} value names with ranged or delimiter-split arity", + value_names.len() + ), + ); + } let mut hidden = Vec::new(); if arg.is_hide_default_value_set() { hidden.push("default_value"); diff --git a/clap_usage/tests/fidelity_report.rs b/clap_usage/tests/fidelity_report.rs index fa2f1958c..08fdab9d8 100644 --- a/clap_usage/tests/fidelity_report.rs +++ b/clap_usage/tests/fidelity_report.rs @@ -31,18 +31,81 @@ fn reports_detectable_losses_with_locations() { FidelityFeature::Environment, FidelityFeature::ValueHint, FidelityFeature::GranularHide, - FidelityFeature::ValueArity, - FidelityFeature::DistinctValueNames, ] { assert!( features.contains(&expected), "missing {expected:?}: {report:#?}" ); } + let pair = spec + .cmd + .flags + .iter() + .find(|flag| flag.name == "pair") + .unwrap(); + assert_eq!(pair.arg.as_ref().unwrap().value_names, ["START", "END"]); + assert_eq!( + ( + pair.arg.as_ref().unwrap().var_min, + pair.arg.as_ref().unwrap().var_max + ), + (Some(2), Some(2)) + ); + assert!(!features.contains(&FidelityFeature::DistinctValueNames)); assert!(report.losses().iter().all(|loss| loss.command == ["ex"])); assert!(!report.is_lossless()); } +#[test] +fn fixed_arity_distinct_value_names_are_lossless() { + let mut command = Command::new("ex").arg( + Arg::new("pair") + .long("pair") + .num_args(2) + .value_names(["START", "END"]), + ); + let (spec, report) = spec_with_report(&mut command, "ex"); + assert!(report.is_lossless(), "{report:#?}"); + let arg = spec.cmd.flags[0].arg.as_ref().unwrap(); + assert_eq!(arg.value_names, ["START", "END"]); + assert_eq!((arg.var_min, arg.var_max), (Some(2), Some(2))); +} + +#[test] +fn ranged_distinct_value_names_are_reported_as_lossy() { + let mut command = Command::new("ex").arg( + Arg::new("range") + .long("range") + .num_args(2..=4) + .value_names(["START", "END"]), + ); + let report = spec_with_report(&mut command, "ex").1; + assert!(report + .losses() + .iter() + .any(|loss| loss.feature == FidelityFeature::DistinctValueNames)); +} + +#[test] +fn delimited_distinct_value_names_are_reported_and_not_emitted_as_fixed_arity() { + let mut command = Command::new("ex").arg( + Arg::new("pair") + .long("pair") + .num_args(2) + .value_delimiter(',') + .value_names(["START", "END"]), + ); + let (spec, report) = spec_with_report(&mut command, "ex"); + let arg = spec.cmd.flags[0].arg.as_ref().unwrap(); + assert_eq!(arg.value_names, ["START"]); + assert_eq!((arg.var_min, arg.var_max), (None, None)); + assert!(report + .losses() + .iter() + .any(|loss| loss.feature == FidelityFeature::DistinctValueNames)); + spec.to_string().parse::().unwrap(); +} + #[test] fn reports_nested_paths_and_leaves_supported_commands_clean() { let mut clean = Command::new("ex") @@ -85,14 +148,31 @@ fn reports_delimited_arity_that_the_bridge_cannot_count() { } #[test] -fn builds_action_derived_arity_before_reporting() { - let mut command = - Command::new("ex").arg(Arg::new("values").long("values").action(ArgAction::Append)); +fn append_action_fixed_arity_is_lossless() { + let mut command = Command::new("ex").arg( + Arg::new("values") + .long("values") + .action(ArgAction::Append) + .num_args(2), + ); let (_, report) = spec_with_report(&mut command, "ex"); - assert!(report - .losses() - .iter() - .any(|loss| loss.feature == FidelityFeature::ValueArity)); + assert!(report.is_lossless(), "{report:#?}"); +} + +#[test] +fn value_names_infer_fixed_arity_without_num_args() { + let mut command = Command::new("ex") + .arg(Arg::new("pair").long("pair").value_names(["LEFT", "RIGHT"])) + .arg(Arg::new("coords").value_names(["X", "Y"])); + let (spec, report) = spec_with_report(&mut command, "ex"); + assert!(report.is_lossless(), "{report:#?}"); + let flag = spec.cmd.flags[0].arg.as_ref().unwrap(); + assert_eq!((flag.var_min, flag.var_max), (Some(2), Some(2))); + assert_eq!( + (spec.cmd.args[0].var_min, spec.cmd.args[0].var_max), + (Some(2), Some(2)) + ); + spec.to_string().parse::().unwrap(); } #[test] diff --git a/conformance/src/tables.rs b/conformance/src/tables.rs index a488e0b12..b2eb9a04d 100644 --- a/conformance/src/tables.rs +++ b/conformance/src/tables.rs @@ -321,6 +321,7 @@ fn flag_meta( help: opt(&f.help), long_help: opt(&f.help_long), value_name: arg.map(|a| leak(&a.name)), + value_names: arg.map_or(&[], |a| strs(&a.value_names)), // The value's own bracket bit, which is not the flag's — usage-lib renders a flag from // two independent `required` bits and a spec can write either without the other. Folded // with the value's own default the way usage-lib folds a positional's, so @@ -343,8 +344,10 @@ fn flag_meta( // The separator as declared, a `char`: the metadata is the cold model and says what // the spec said, where the binding table beside it holds the byte binding counts by. delimiter: arg.and_then(|a| a.delimiter), - var_min: f.var_min.or(arg.and_then(|a| a.var_min)), - var_max: f.var_max.or(arg.and_then(|a| a.var_max)), + var_min: f.var_min, + var_max: f.var_max, + value_var_min: arg.and_then(|a| a.var_min), + value_var_max: arg.and_then(|a| a.var_max), overrides: strs(&f.overrides), conflicts: strs(&f.conflicts), requires: strs(&f.requires), @@ -387,6 +390,7 @@ fn arg_meta( let choices = a.choices.as_ref(); ArgMeta { arg: table, + value_names: strs(&a.value_names), help: opt(&a.help), long_help: opt(&a.help_long), env: opt(&a.env), diff --git a/conformance/tests/placeholders.rs b/conformance/tests/placeholders.rs index a992cdc2e..b6205e6be 100644 --- a/conformance/tests/placeholders.rs +++ b/conformance/tests/placeholders.rs @@ -46,6 +46,38 @@ struct Ex { quiet: bool, } +#[derive(Debug, PartialEq, Cli)] +#[usage(bin = "fixed")] +struct Fixed { + /// A pair consumed by one flag occurrence. + #[arg(long, num_args = 2, value_names = ["START", "END"])] + range: Vec, + /// Two positional values with distinct labels. + #[arg(num_args = 2, value_names = ["LEFT", "RIGHT"])] + pair: Vec, +} + +#[test] +fn fixed_arity_values_keep_each_placeholder() { + use std::ffi::OsStr; + + let argv = ["--range", "1", "2", "a", "b"].map(OsStr::new); + let parsed = Fixed::parse_from(&argv).expect("two values per field should parse"); + assert_eq!(parsed.range, ["1", "2"]); + assert_eq!(parsed.pair, ["a", "b"]); + + let spec: LibSpec = Fixed::to_kdl().parse().expect("generated fixed-arity spec"); + let range = spec.cmd.flags[0].arg.as_ref().unwrap(); + assert_eq!(range.value_names, ["START", "END"]); + assert_eq!((range.var_min, range.var_max), (Some(2), Some(2))); + assert_eq!(spec.cmd.args[0].value_names, ["LEFT", "RIGHT"]); + + let page = usage_argv::help::render(Fixed::spec(), Fixed::spec().root.cmd, false) + .expect("fixed-arity help"); + assert!(page.contains("--range "), "{page}"); + assert!(page.contains("[LEFT] [RIGHT]"), "{page}"); +} + #[test] fn a_value_is_named_after_its_field_shouted() { let spec: LibSpec = Ex::to_kdl().parse().expect("valid spec"); diff --git a/derive/src/codegen.rs b/derive/src/codegen.rs index d3234d036..f64917f97 100644 --- a/derive/src/codegen.rs +++ b/derive/src/codegen.rs @@ -1035,7 +1035,7 @@ fn flag_table(i: usize, field: &Field) -> TokenStream { let takes_value = field.takes_value(); // The bound on one occurrence's values. A repeatable flag's bound counts occurrences // instead, which no single token can decide, so that one stays a post-binding check. - let var_max = match field.var_max.filter(|_| *variadic) { + let var_max = match field.value_var_max.filter(|_| *variadic) { Some(max) => { // Saturating, not `as`: on a 64-bit target a bound above `u32::MAX` would // narrow, and `4294967296` narrows to zero — a limit of "none" read as a limit @@ -1215,6 +1215,7 @@ fn flag_meta(cli: &Cli, i: usize, field: &Field, owner: &syn::Ident) -> TokenStr let env = option_str(field.env.as_deref()); let help_heading = option_str(field.help_heading.as_deref()); let value_name = option_str(field.value_name.as_deref()); + let value_names = &field.value_names; let complete_type = option_str(field.complete_type.as_deref()); let defaults = &field.default; let default = quote!(&[#(#defaults),*]); @@ -1240,6 +1241,7 @@ fn flag_meta(cli: &Cli, i: usize, field: &Field, owner: &syn::Ident) -> TokenStr let validate = option_str(field.validate.as_deref()); let validate_error = option_str(field.validate_error.as_deref()); let (var_min, var_max) = bounds_tokens(field); + let (value_var_min, value_var_max) = value_bounds_tokens(field); // Written as declared, in the spec's own spelling, so the emitted KDL says what // the struct says. let overrides = &field.overrides; @@ -1296,6 +1298,7 @@ fn flag_meta(cli: &Cli, i: usize, field: &Field, owner: &syn::Ident) -> TokenStr default: #default, help_heading: #help_heading, value_name: #value_name, + value_names: &[#(#value_names),*], hide: #hide, count: #count, repeatable: #repeatable, @@ -1312,6 +1315,8 @@ fn flag_meta(cli: &Cli, i: usize, field: &Field, owner: &syn::Ident) -> TokenStr validate_error: #validate_error, var_min: #var_min, var_max: #var_max, + value_var_min: #value_var_min, + value_var_max: #value_var_max, overrides: &[#(#overrides),*], conflicts: &[#(#conflicts),*], requires: &[#(#requires),*], @@ -1334,6 +1339,7 @@ fn arg_meta(cli: &Cli, i: usize, field: &Field, owner: &syn::Ident) -> TokenStre let env = option_str(field.env.as_deref()); let help_heading = option_str(field.help_heading.as_deref()); let complete_type = option_str(field.complete_type.as_deref()); + let value_names = &field.value_names; let defaults = &field.default; let default = quote!(&[#(#defaults),*]); let hide = field.hide; @@ -1355,7 +1361,11 @@ fn arg_meta(cli: &Cli, i: usize, field: &Field, owner: &syn::Ident) -> TokenStre choices_tokens(field); let validate = option_str(field.validate.as_deref()); let validate_error = option_str(field.validate_error.as_deref()); - let (var_min, var_max) = bounds_tokens(field); + let (var_min, var_max) = if matches!(field.kind, Kind::Flag { .. }) { + value_bounds_tokens(field) + } else { + bounds_tokens(field) + }; let delimiter = match field.delimiter { Some(c) => quote!(::std::option::Option::Some(#c)), None => quote!(::std::option::Option::None), @@ -1368,6 +1378,7 @@ fn arg_meta(cli: &Cli, i: usize, field: &Field, owner: &syn::Ident) -> TokenStre complete: #completer, complete_type: #complete_type, arg: &#table, + value_names: &[#(#value_names),*], help: #help, long_help: #long_help, env: #env, @@ -1432,6 +1443,15 @@ fn bounds_tokens(field: &Field) -> (TokenStream, TokenStream) { (render(field.var_min), render(field.var_max)) } +/// Bounds on the values consumed by one flag occurrence. +fn value_bounds_tokens(field: &Field) -> (TokenStream, TokenStream) { + let render = |bound: Option| match bound { + Some(n) => quote!(::std::option::Option::Some(#n)), + None => quote!(::std::option::Option::None), + }; + (render(field.value_var_min), render(field.value_var_max)) +} + /// Which kind of thing a key belongs to, in the bits above its index. /// /// A command, a flag, and an argument each get their own space, so no two things in @@ -4830,12 +4850,17 @@ fn post_binding(cli: &Cli) -> TokenStream { }); let bound_checks = cli.fields.iter().filter_map(|f| { - if f.var_min.is_none() && f.var_max.is_none() { + let (var_min, var_max) = if matches!(f.kind, Kind::Flag { variadic: true, .. }) { + (f.value_var_min, f.value_var_max) + } else { + (f.var_min, f.var_max) + }; + if (var_min.is_none() && var_max.is_none()) || f.shape != Shape::Many { return None; } let ident = &f.ident; let name = &f.name; - let min = match f.var_min { + let min = match var_min { Some(min) => quote! { if got < #min { return ::std::result::Result::Err( @@ -4860,7 +4885,7 @@ fn post_binding(cli: &Cli) -> TokenStream { .. } ) && f.repeatable; - let max = match f.var_max.filter(|_| counts_occurrences) { + let max = match var_max.filter(|_| counts_occurrences) { Some(max) => quote! { if got > #max { return ::std::result::Result::Err( diff --git a/derive/src/model.rs b/derive/src/model.rs index c8cd2e31f..27c37a6db 100644 --- a/derive/src/model.rs +++ b/derive/src/model.rs @@ -215,6 +215,8 @@ pub struct Field { /// which reads oddly when the two differ in case or shape — a spec saying /// `--tool ` came back as `--tool `, because the name was all there was. pub value_name: Option, + /// Ordered placeholders for one fixed-arity value occurrence. + pub value_names: Vec, /// The values this may take. Checked after the parse, since a choice list is /// about what a value *means* rather than which token it came from. pub choices: Vec, @@ -231,6 +233,14 @@ pub struct Field { pub complete_type: Option, pub var_min: Option, pub var_max: Option, + /// Bounds on the values consumed by one flag occurrence. + /// + /// Clap's `num_args` is per occurrence, while the spec's flag-level + /// `var_min`/`var_max` count occurrences. Keeping the two axes distinct is + /// what lets `--pair A B --pair C D` round-trip without turning the value + /// arity into an occurrence limit. + pub value_var_min: Option, + pub value_var_max: Option, /// Flags this one displaces. Applied while parsing rather than after it: the /// question is which of them came last, so the answer is decided by the token /// that arrives, not by the state it leaves behind. @@ -1345,6 +1355,7 @@ impl Field { default_value_t: None, help_heading: None, value_name: None, + value_names: Vec::new(), required_collection: false, choices: Vec::new(), validate: None, @@ -1352,6 +1363,8 @@ impl Field { value_enum: false, var_min: None, var_max: None, + value_var_min: None, + value_var_max: None, overrides: Vec::new(), conflicts: Vec::new(), requires: Vec::new(), @@ -1458,6 +1471,7 @@ impl Field { default_value_t: None, help_heading: None, value_name: None, + value_names: Vec::new(), required_collection: false, choices: Vec::new(), validate: None, @@ -1465,6 +1479,8 @@ impl Field { value_enum: false, var_min: None, var_max: None, + value_var_min: None, + value_var_max: None, overrides: Vec::new(), conflicts: Vec::new(), requires: Vec::new(), @@ -1565,6 +1581,7 @@ impl Field { default_value_t: None, help_heading: None, value_name: None, + value_names: Vec::new(), required_collection: false, choices: Vec::new(), validate: None, @@ -1572,6 +1589,8 @@ impl Field { value_enum: false, var_min: None, var_max: None, + value_var_min: None, + value_var_max: None, overrides: Vec::new(), conflicts: Vec::new(), requires: Vec::new(), @@ -1644,6 +1663,8 @@ impl Field { let mut help_heading = None; let mut effect = None; let mut value_name = None; + let mut value_names: Vec = Vec::new(); + let mut num_args: Option<(usize, Option)> = None; let mut required_collection = false; let mut help_attr: Option = None; let mut long_help_attr: Option = None; @@ -1845,13 +1866,13 @@ impl Field { "help_heading" => help_heading = Some(string_value(&meta)?), "effect" => effect = Some(effect_value(&meta)?), "value_name" => value_name = Some(string_value(&meta)?), + "value_names" => value_names = selectors(&meta)?, "num_args" => { - return Err(syn::Error::new_spanned( - &meta, - "clap's `num_args` maps to the Rust field shape plus \ - `var_min`/`var_max`; use `Option`, `Vec`, and those \ - bounds to declare the same arity", - )); + let (min, max) = num_args_value(&meta)?; + num_args = Some((min, max)); + if max.is_none_or(|max| max > 1) { + variadic = true; + } } "value_parser" => { return Err(syn::Error::new_spanned( @@ -1901,7 +1922,7 @@ impl Field { `value_terminator`, `require_equals`, \ `default_missing`, `default_if`, \ `required_if`, \ - `required_unless`, `help_heading`, `value_name`, \ + `required_unless`, `help_heading`, `value_name`, `value_names`, `num_args`, \ `verbatim_doc_comment`, \ `visible_alias`, `visible_aliases`, `required`, \ `double_dash`, and `skip`" @@ -2011,6 +2032,17 @@ impl Field { ty: value_ty, optional_collection, } = ValueKind::from_type(&field.ty, count, span)?; + let is_flag = !longs.is_empty() || !shorts.is_empty(); + let (mut value_var_min, mut value_var_max) = (None, None); + if let Some((min, max)) = num_args { + if is_flag { + value_var_min = Some(min); + value_var_max = max; + } else { + var_min = Some(min); + var_max = max; + } + } // The spec records a default and the generated code applies it; anything it // cannot apply would be documented and then ignored. // @@ -2121,12 +2153,36 @@ impl Field { )); } } - if (var_min.is_some() || var_max.is_some()) && shape != Shape::Many { + if let (Some(min), Some(max)) = (value_var_min, value_var_max) { + if min > max { + return Err(syn::Error::new( + span, + format!( + "`num_args` begins at {min} but ends at {max}, so nothing could satisfy it" + ), + )); + } + } + if (var_min.is_some_and(|min| min > 1) + || var_max.is_none() && var_min.is_some() + || var_max.is_some_and(|max| max > 1) + || value_var_min.is_some_and(|min| min > 1) + || value_var_max.is_none() && value_var_min.is_some() + || value_var_max.is_some_and(|max| max > 1)) + && shape != Shape::Many + { return Err(syn::Error::new( span, "`var_min` and `var_max` count values, so the field has to be a `Vec`", )); } + if num_args.is_some() && is_flag && value_var_min == Some(0) && default_missing.is_none() { + return Err(syn::Error::new( + span, + "a flag whose `num_args` begins at zero distinguishes an absent flag from a \ + present flag with no value; use `default_missing` for a portable optional value", + )); + } if !choices.is_empty() { // Each of them, not the first: a collection's second default is as unusable as its // first if the choices do not allow it. @@ -2173,7 +2229,6 @@ impl Field { `long` or `short` to declare the flag", )); } - let is_flag = !longs.is_empty() || !shorts.is_empty(); // Only a flag's value has a say in this. A positional's brackets come from its type // already — `Option` renders `[NAME]` and `T` renders `` — so the attribute // would be read by nothing, and a declaration nothing reads is worse than an error: the @@ -2210,7 +2265,7 @@ impl Field { that takes several values is a `Vec` field", )); } - if !is_flag && variadic { + if !is_flag && variadic && num_args.is_none() { return Err(syn::Error::new( span, "`variadic` describes a flag whose one occurrence keeps taking values; \ @@ -2341,6 +2396,17 @@ impl Field { // counting them is the whole point. Left uninferred, the emitted spec said `count` // without `var` where mise's says both, and help rendered `-v --verbose` for a flag // that can be given again. + if is_flag && value_names.len() > 1 { + variadic = true; + } + // usage-native bounds on a variadic flag describe the values consumed by + // its one occurrence. On a merely repeatable flag they instead count + // occurrences. Clap's `num_args` was already placed on the value axis + // above; move native bounds there once the final flag shape is known. + if is_flag && variadic && value_var_min.is_none() && value_var_max.is_none() { + value_var_min = var_min.take(); + value_var_max = var_max.take(); + } let repeatable = repeatable || (is_flag && !variadic && (shape == Shape::Many || shape == Shape::Count)); @@ -2583,6 +2649,45 @@ impl Field { )); } } + if !value_names.is_empty() { + if matches!(shape, Shape::Bool | Shape::Count) { + return Err(syn::Error::new( + span, + "`value_names` names value placeholders, and this field takes no value", + )); + } + if value_names.len() > 1 && shape != Shape::Many { + return Err(syn::Error::new( + span, + "more than one `value_names` entry requires a `Vec` field", + )); + } + if value_names.len() > 1 { + let arity = value_names.len(); + let bounds = if is_flag { + (&mut value_var_min, &mut value_var_max) + } else { + (&mut var_min, &mut var_max) + }; + match (*bounds.0, *bounds.1) { + (None, None) => { + *bounds.0 = Some(arity); + *bounds.1 = Some(arity); + } + (Some(min), Some(max)) if min == arity && max == arity => {} + _ => { + return Err(syn::Error::new( + span, + format!( + "{} value names describe a fixed arity of {arity}; set \ + `num_args = {arity}` or matching `var_min`/`var_max`", + value_names.len() + ), + )); + } + } + } + } // A positional is named by the same rule, and its name *is* its placeholder: clap // prints ` [PREV_TAG]` for `tag: String, prev_tag: Option`, so a spec @@ -2624,6 +2729,7 @@ impl Field { help_heading, effect, value_name, + value_names, required_collection, choices, validate, @@ -2633,6 +2739,8 @@ impl Field { value_enum, var_min, var_max, + value_var_min, + value_var_max, overrides, conflicts, requires, @@ -3147,6 +3255,57 @@ fn int_value(meta: &Meta) -> syn::Result { } } +fn num_args_value(meta: &Meta) -> syn::Result<(usize, Option)> { + fn bound(expr: &Expr) -> syn::Result { + match expr { + Expr::Lit(ExprLit { + lit: Lit::Int(value), + .. + }) => value.base10_parse(), + other => Err(syn::Error::new_spanned( + other, + "`num_args` bounds must be whole-number literals", + )), + } + } + + let value = &meta.require_name_value()?.value; + match value { + Expr::Lit(ExprLit { + lit: Lit::Int(value), + .. + }) => { + let count = value.base10_parse()?; + Ok((count, Some(count))) + } + Expr::Range(range) => { + let min = range.start.as_deref().map(bound).transpose()?.unwrap_or(0); + let mut max = range.end.as_deref().map(bound).transpose()?; + if matches!(range.limits, syn::RangeLimits::HalfOpen(_)) { + if let Some(end) = max.as_mut() { + *end = end.checked_sub(1).ok_or_else(|| { + syn::Error::new_spanned( + value, + "an exclusive `num_args` range cannot end at zero", + ) + })?; + } + } + if max.is_some_and(|max| min > max) { + return Err(syn::Error::new_spanned( + value, + "the `num_args` range has no possible value count", + )); + } + Ok((min, max)) + } + other => Err(syn::Error::new_spanned( + other, + "expected a count or range, as in `num_args = 2` or `num_args = 1..=3`", + )), + } +} + fn char_value(meta: &Meta) -> syn::Result { let value = &meta.require_name_value()?.value; match value { @@ -4218,16 +4377,79 @@ mod tests { } #[test] - fn lossy_clap_field_spellings_get_migration_diagnostics() { - let arity = rejection( - r#" + fn clap_field_spellings_preserve_supported_metadata() { + let parsed = cli(r#" struct Ex { - #[arg(long, num_args = 2)] + #[arg(long, num_args = 2, value_names = ["START", "END"])] pair: Vec, } + "#) + .expect("fixed arity and distinct labels are representable"); + assert_eq!(parsed.fields[0].var_min, None); + assert_eq!(parsed.fields[0].var_max, None); + assert_eq!(parsed.fields[0].value_var_min, Some(2)); + assert_eq!(parsed.fields[0].value_var_max, Some(2)); + assert_eq!(parsed.fields[0].value_names, ["START", "END"]); + + let inferred = cli(r#" + struct Ex { + #[arg(long, value_names = ["LEFT", "RIGHT"])] + pair: Vec, + } + "#) + .expect("value_names alone infer fixed arity"); + assert_eq!(inferred.fields[0].var_min, None); + assert_eq!(inferred.fields[0].var_max, None); + assert_eq!(inferred.fields[0].value_var_min, Some(2)); + assert_eq!(inferred.fields[0].value_var_max, Some(2)); + assert_eq!(inferred.fields[0].value_names, ["LEFT", "RIGHT"]); + + let optional_value = rejection( + r#" + struct Ex { + #[arg(long, num_args = 0..=2)] + values: Vec, + } "#, ); - assert!(arity.contains("var_min"), "{arity}"); + assert!( + optional_value.contains("default_missing"), + "{optional_value}" + ); + + let positional = cli(r#" + struct Ex { + #[arg(num_args = 0..=2)] + values: Vec, + } + "#) + .expect("a positional can consume no values without a present-empty flag state"); + assert_eq!(positional.fields[0].var_min, Some(0)); + assert_eq!(positional.fields[0].var_max, Some(2)); + + let usage_bounds = cli(r#" + struct Ex { + #[usage(long, variadic, var_min = 2, var_max = 3)] + values: Vec, + #[usage(long, var_min = 2, var_max = 4)] + tags: Vec, + } + "#) + .expect("usage bounds follow the declared collection axis"); + assert_eq!(usage_bounds.fields[0].var_min, None); + assert_eq!(usage_bounds.fields[0].value_var_min, Some(2)); + assert_eq!(usage_bounds.fields[0].value_var_max, Some(3)); + assert_eq!(usage_bounds.fields[1].var_min, Some(2)); + assert_eq!(usage_bounds.fields[1].var_max, Some(4)); + assert_eq!(usage_bounds.fields[1].value_var_min, None); + + cli(r#" + struct Ex { + #[arg(long, num_args = 0..=2, default_missing = "auto")] + values: Vec, + } + "#) + .expect("default_missing represents the present flag with no explicit value"); let parser = rejection( r#" diff --git a/docs/rust/args-and-flags.md b/docs/rust/args-and-flags.md index 94704d3b6..8073231af 100644 --- a/docs/rust/args-and-flags.md +++ b/docs/rust/args-and-flags.md @@ -71,6 +71,7 @@ jobs: Option, | `global` | Usable on any subcommand below this one | | `var` / `variadic` | Repeatable / greedy multi-value (see above) | | `var_min = n` / `var_max = n` | Bounds on how many values a `Vec` may hold | +| `num_args = n` / `num_args = a..=b` | clap-compatible spelling for exact or ranged `Vec` cardinality | | `choices("a", "b")` | Restrict values to a fixed set | | `value_enum` | Take choices from a `#[derive(ValueEnum)]` type | | `delimiter = ','` | Split one word into several values ([Validation](/rust/validation#delimiters)) | @@ -88,6 +89,7 @@ jobs: Option, | `complete = my_fn` | Custom completion function ([Completions](/rust/completions)) | | `value_hint = ValueHint::FilePath` | Ask the shell for path completion (see below) | | `value_name = "…"` | The placeholder shown in help (`--file `) | +| `value_names = ["A", "B"]` | Distinct placeholders for a fixed multi-value field | | `help = "…"` / `long_help = "…"` | Help text (doc comments are usually nicer) | | `help_heading = "…"` | Group the entry under a heading in help output | | `hide` | Omit from help, docs, and completions | @@ -120,6 +122,21 @@ needs the same thing already has `double_dash = "automatic"`. Emitted KDL: `#[usage(value_terminator = ";")]` ends a `Vec` without storing the terminator. It works on variadic flags and positionals and emits `value_terminator=";"` in KDL. +Fixed multi-value fields can retain clap's familiar attribute unchanged: + +```rust +#[arg(long, num_args = 2, value_names = ["START", "END"])] +range: Vec, +``` + +One `--range` occurrence consumes exactly two values and help prints +`--range `. The generated KDL uses the same two placeholders and +puts `var_min=2 var_max=2` on the flag's nested `arg`, not on the flag itself. +That distinction matters: bounds on the nested value apply to every occurrence, +while flag-level bounds count how many times a repeatable flag appears. A range +such as `num_args = 1..=3` sets the corresponding nested bounds; distinct +`value_names` require an exact bound matching their count. + `#[usage(require_equals)]` is clap's attribute of the same name: `--inspect=9229` binds and `--inspect 9229` is a missing value. The flag has to take a value. Emitted KDL: `flag "--inspect " require_equals=#true`. diff --git a/docs/rust/clap-compatibility.md b/docs/rust/clap-compatibility.md index df30a9260..a03fc6821 100644 --- a/docs/rust/clap-compatibility.md +++ b/docs/rust/clap-compatibility.md @@ -51,32 +51,32 @@ the Rust declaration, not only from generated KDL, wherever the bridge column sa ## Arguments and values -| clap surface | derive | argv | KDL | lib | output | bridge | Notes | -| ---------------------------------------------------------- | ---------- | ----- | ----- | ----- | ------ | ---------- | --------------------------------------------------------------------------------------------------------------------------------- | -| `long`, `short`, visible aliases | yes | yes | yes | yes | yes | yes | Multiple forms are accepted and advertised by parsing, help, completion, and generated tables. | -| hidden flag `alias` / `aliases` | yes | yes | yes | yes | yes | yes | Hidden aliases bind and round-trip through KDL and generated Rust/Go tables without appearing in help or completion. | -| explicit `id` | yes | yes | yes | yes | yes | yes | `#[arg(id = "…")]` supplies the stable field identity used by relationships and generated specs. | -| positional arguments | yes | yes | yes | yes | yes | yes | Required, optional, and variadic positionals are supported. | -| `Option`, `Vec`, `Option>` | yes | yes | yes | yes | yes | n/a | Values use `FromStr`; Unix `PathBuf` and `OsString` preserve non-UTF-8 bytes. | -| `ArgAction::Set`, `SetTrue`, `SetFalse`, `Append`, `Count` | yes | yes | yes | yes | yes | lossy | Common typed shapes are covered; arbitrary action/type combinations are not. | -| `default_value` | yes | yes | yes | yes | yes | yes | Defaults apply after argv and environment values and clear token-required metadata. | -| `default_missing_value` | yes | yes | yes | yes | yes | usage-only | `#[usage(default_missing = "…")]`; clap has no getter. | -| `default_value_if(s)` | yes | yes | yes | yes | yes | usage-only | Presence and equality predicates are portable; clap has no getter. | -| `env` | yes | yes | yes | yes | yes | lossy | Environment fallback works; the current bridge can lose the binding. | -| `value_delimiter` | yes | yes | yes | yes | yes | yes | ASCII delimiters round-trip and are applied before arity checks. | -| `num_args` ranges | yes | yes | yes | yes | yes | lossy | `var_min` / `var_max` cover accumulated ranges; optional and repeatable per-occurrence ranges can be bridge-lossy. | -| fixed `num_args` with distinct `value_names` | lossy | lossy | lossy | lossy | lossy | lossy | Bounds work; one collection display name is repeated instead of preserving ` `. | -| `allow_hyphen_values` | yes | yes | yes | yes | yes | yes | Supported on value-taking flags; forwarded positionals use `double_dash = "automatic"`. | -| `allow_negative_numbers` | yes | yes | yes | yes | yes | yes | Accepts negative numeric tokens without accepting arbitrary dash-prefixed values. | -| `require_equals` | yes | yes | yes | yes | yes | yes | Detached values are refused. | -| `value_terminator` | yes | yes | yes | yes | yes | yes | Ends a variadic value owner without binding the terminator token. | -| `trailing_var_arg` / `last` | yes | yes | yes | yes | yes | lossy | `double_dash` carries automatic/required/optional; clap shadow generation still drops automatic mode. | -| `dont_delimit_trailing_values` | yes | yes | yes | yes | yes | yes | Preserves delimiters after `--` and on automatic trailing positionals while ordinary values still split. | -| possible-values parser | yes | yes | yes | yes | yes | yes | Use `ValueEnum` or `choices`. | -| arbitrary `value_parser` callbacks | usage-only | yes | lossy | yes | yes | no | `FromStr` handles typed conversion and portable `validate` expressions handle declarative rules; Rust callbacks cannot enter KDL. | -| `ValueHint::{FilePath,DirPath}` | yes | yes | yes | yes | yes | lossy | Shell-native path completion is supported directly; `clap_usage` does not yet lower hints into completion nodes. | -| executable and command value hints | yes | yes | yes | yes | yes | lossy | Direct usage declarations work; the clap bridge currently reports and drops these hints. | -| identity and network `ValueHint`s | no | no | no | no | no | lossy | Username, hostname, URL, email, and related hints are not yet represented. | +| clap surface | derive | argv | KDL | lib | output | bridge | Notes | +| ---------------------------------------------------------- | ---------- | ---- | ----- | --- | ------ | ---------- | ------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `long`, `short`, visible aliases | yes | yes | yes | yes | yes | yes | Multiple forms are accepted and advertised by parsing, help, completion, and generated tables. | +| hidden flag `alias` / `aliases` | yes | yes | yes | yes | yes | yes | Hidden aliases bind and round-trip through KDL and generated Rust/Go tables without appearing in help or completion. | +| explicit `id` | yes | yes | yes | yes | yes | yes | `#[arg(id = "…")]` supplies the stable field identity used by relationships and generated specs. | +| positional arguments | yes | yes | yes | yes | yes | yes | Required, optional, and variadic positionals are supported. | +| `Option`, `Vec`, `Option>` | yes | yes | yes | yes | yes | n/a | Values use `FromStr`; Unix `PathBuf` and `OsString` preserve non-UTF-8 bytes. | +| `ArgAction::Set`, `SetTrue`, `SetFalse`, `Append`, `Count` | yes | yes | yes | yes | yes | lossy | Common typed shapes are covered; arbitrary action/type combinations are not. | +| `default_value` | yes | yes | yes | yes | yes | yes | Defaults apply after argv and environment values and clear token-required metadata. | +| `default_missing_value` | yes | yes | yes | yes | yes | usage-only | `#[usage(default_missing = "…")]`; clap has no getter. | +| `default_value_if(s)` | yes | yes | yes | yes | yes | usage-only | Presence and equality predicates are portable; clap has no getter. | +| `env` | yes | yes | yes | yes | yes | lossy | Environment fallback works; the current bridge can lose the binding. | +| `value_delimiter` | yes | yes | yes | yes | yes | yes | ASCII delimiters round-trip and are applied before arity checks. | +| `num_args` ranges | yes | yes | yes | yes | yes | lossy | Nested value `var_min` / `var_max` preserve per-occurrence ranges separately from flag occurrence bounds; zero-minimum flag ranges can be bridge-lossy. | +| fixed `num_args` with distinct `value_names` | yes | yes | yes | yes | yes | yes | `#[arg(num_args = 2, value_names = ["START", "END"])]` preserves the exact bound and both placeholders. | +| `allow_hyphen_values` | yes | yes | yes | yes | yes | yes | Supported on value-taking flags; forwarded positionals use `double_dash = "automatic"`. | +| `allow_negative_numbers` | yes | yes | yes | yes | yes | yes | Accepts negative numeric tokens without accepting arbitrary dash-prefixed values. | +| `require_equals` | yes | yes | yes | yes | yes | yes | Detached values are refused. | +| `value_terminator` | yes | yes | yes | yes | yes | yes | Ends a variadic value owner without binding the terminator token. | +| `trailing_var_arg` / `last` | yes | yes | yes | yes | yes | lossy | `double_dash` carries automatic/required/optional; clap shadow generation still drops automatic mode. | +| `dont_delimit_trailing_values` | yes | yes | yes | yes | yes | yes | Preserves delimiters after `--` and on automatic trailing positionals while ordinary values still split. | +| possible-values parser | yes | yes | yes | yes | yes | yes | Use `ValueEnum` or `choices`. | +| arbitrary `value_parser` callbacks | usage-only | yes | lossy | yes | yes | no | `FromStr` handles typed conversion and portable `validate` expressions handle declarative rules; Rust callbacks cannot enter KDL. | +| `ValueHint::{FilePath,DirPath}` | yes | yes | yes | yes | yes | lossy | Shell-native path completion is supported directly; `clap_usage` does not yet lower hints into completion nodes. | +| executable and command value hints | yes | yes | yes | yes | yes | lossy | Direct usage declarations work; the clap bridge currently reports and drops these hints. | +| identity and network `ValueHint`s | no | no | no | no | no | lossy | Username, hostname, URL, email, and related hints are not yet represented. | ## Relationships and command routing diff --git a/docs/spec/reference/arg.md b/docs/spec/reference/arg.md index 65866becc..2f5659489 100644 --- a/docs/spec/reference/arg.md +++ b/docs/spec/reference/arg.md @@ -14,6 +14,7 @@ arg "" var=#true // multiple args can be passed (e.g. mycli file1 file2 fi arg "..." // shorthand for var=#true (trailing ellipsis) arg "" var=#true var_min=3 // at least 3 args must be passed arg "" var=#true var_max=3 // up to 3 args can be passed +arg " " var_min=2 var_max=2 // exactly two values with distinct labels arg "" allow_negative_numbers=#true // -1 is a value, --force is still flag-like arg "..." value_terminator=";" // stop before ; without storing it ``` @@ -22,6 +23,10 @@ arg "..." value_terminator=";" // stop before ; without storing it dash-prefixed token would be treated as a flag. `value_terminator` is valid only on a variadic argument; it ends that argument without binding the terminator token. +Several placeholders in one argument declare fixed arity. Each label is retained in +help and generated Rust and Go tables; `var_min` and `var_max` must match the number +of placeholders. + `validate` is an [expr](https://expr-lang.org/) expression evaluated once for each value after defaults and environment fallbacks are applied. The only variable is `value`, always a string. The expression must return a boolean; `false` reports diff --git a/docs/spec/reference/flag.md b/docs/spec/reference/flag.md index c4d686ab3..30e04088e 100644 --- a/docs/spec/reference/flag.md +++ b/docs/spec/reference/flag.md @@ -18,6 +18,7 @@ flag "--include... " // same as above, ellipsis on fl flag "--include ..." // arg is variadic (--include a b c in one invocation) flag "--include " var=#true var_min=1 // at least 1 value required flag "--include " var=#true var_max=5 // up to 5 values allowed +flag "--range " // one occurrence takes exactly two values flag "--color" negate="--no-color" default=#true // $usage_color=#true by default // --no-color will set $usage_color=#false diff --git a/go/argv/help.go b/go/argv/help.go index e322cefcf..4fc0f438b 100644 --- a/go/argv/help.go +++ b/go/argv/help.go @@ -34,6 +34,12 @@ type Help struct { // ValueName is what a flag's value is called. Empty for a flag that takes // none. ValueName string + // ValueNames preserves a fixed-arity value's distinct placeholders. It is + // empty for the ordinary single-name case represented by ValueName. + ValueNames []string + // ValueArity is the exact number of values when a fixed-arity argument uses + // one placeholder for every slot. Zero means the arity is not exact. + ValueArity uint32 // ValueDemanded is the same required-and-undefaulted test as [Help.Demanded], // applied to the flag's *value* rather than to the flag. // @@ -228,8 +234,18 @@ func flagUsageShown(f *Flag, show shown, h *Help) string { if h != nil && h.ValueDemanded { open, close = "<", ">" } - out.WriteString(" " + open + name + close) - if f.Variadic { + if h != nil && len(h.ValueNames) > 1 { + for _, valueName := range h.ValueNames { + out.WriteString(" " + open + valueName + close) + } + } else if h != nil && h.ValueArity > 1 { + for i := uint32(0); i < h.ValueArity; i++ { + out.WriteString(" " + open + name + close) + } + } else { + out.WriteString(" " + open + name + close) + } + if f.Variadic && (h == nil || (len(h.ValueNames) <= 1 && h.ValueArity == 0)) { out.WriteString("…") } } @@ -247,12 +263,33 @@ func argUsage(a *Arg, h *Help) string { // typing the value without it does not reach this argument at all — and the // brackets go outside it, as usage-lib writes it: `[-- COMMAND]…`, one // optional thing rather than a literal `--` followed by an optional word. - if a.DoubleDash == DoubleDashRequired { + if h != nil && (len(h.ValueNames) > 1 || h.ValueArity > 1) { + if a.DoubleDash == DoubleDashRequired { + out.WriteString("-- ") + } + names := h.ValueNames + if len(names) <= 1 { + name := a.Name + if len(names) == 1 { + name = names[0] + } + names = make([]string, h.ValueArity) + for i := range names { + names[i] = name + } + } + for i, name := range names { + if i > 0 { + out.WriteByte(' ') + } + out.WriteString(open + name + close) + } + } else if a.DoubleDash == DoubleDashRequired { out.WriteString(open + "-- " + a.Name + close) } else { out.WriteString(open + a.Name + close) } - if a.Var { + if a.Var && (h == nil || (len(h.ValueNames) <= 1 && h.ValueArity == 0)) { out.WriteString("…") } return out.String() diff --git a/go/argv/page_test.go b/go/argv/page_test.go index 1e61198f8..346c86ee9 100644 --- a/go/argv/page_test.go +++ b/go/argv/page_test.go @@ -57,6 +57,43 @@ func TestHiddenFlagAliasesStayOutOfHelp(t *testing.T) { } } +func TestFixedArityHelpKeepsDistinctValueNames(t *testing.T) { + flag := &Flag{Key: 2, Name: "range", Longs: []string{"range"}, TakesValue: true, Variadic: true} + arg := &Arg{Key: 3, Name: "PAIR", Var: true} + root := &Command{Name: "ex", Key: 1, Flags: []*Flag{flag}, Args: []*Arg{arg}} + help := HelpTable{ + {Key: 1}, + {Key: 2, ValueDemanded: true, ValueNames: []string{"START", "END"}}, + {Key: 3, Demanded: true, ValueNames: []string{"LEFT", "RIGHT"}}, + } + page := ShortHelp(HelpSpec{Name: "ex", Bin: "ex"}, []string{"ex"}, []*Command{root}, help) + for _, want := range []string{"--range ", " "} { + if !strings.Contains(page, want) { + t.Errorf("missing %q in:\n%s", want, page) + } + } +} + +func TestFixedArityHelpRepeatsOneValueName(t *testing.T) { + flag := &Flag{Key: 2, Name: "pair", Longs: []string{"pair"}, TakesValue: true, Variadic: true} + arg := &Arg{Key: 3, Name: "ITEM", Var: true} + root := &Command{Name: "ex", Key: 1, Flags: []*Flag{flag}, Args: []*Arg{arg}} + help := HelpTable{ + {Key: 1}, + {Key: 2, ValueName: "ITEM", ValueArity: 2, ValueDemanded: true}, + {Key: 3, ValueArity: 2, Demanded: true}, + } + page := ShortHelp(HelpSpec{Name: "ex", Bin: "ex"}, []string{"ex"}, []*Command{root}, help) + for _, want := range []string{"--pair ", " "} { + if !strings.Contains(page, want) { + t.Errorf("missing %q in:\n%s", want, page) + } + } + if strings.Contains(page, "…") { + t.Errorf("exact arity must not render as variadic:\n%s", page) + } +} + // A description that ends in a break adds no blank line. // // clap's `long_about` often ends with one — a `///` block whose last line is diff --git a/go/internal/spec/spec.go b/go/internal/spec/spec.go index 825fcaec3..08e28cf55 100644 --- a/go/internal/spec/spec.go +++ b/go/internal/spec/spec.go @@ -296,6 +296,7 @@ func (f *Flag) defaults() []string { // Arg is one positional argument, or a flag's value, in the lowered spec. type Arg struct { Name string `json:"name"` + ValueNames []string `json:"value_names"` Required bool `json:"required"` Var bool `json:"var"` VarMax int `json:"var_max"` @@ -836,6 +837,8 @@ func (b *builder) flag(f *Flag) *argv.Flag { Demanded: f.Required && len(f.Default) == 0, Repeatable: f.Var, ValueName: valueName, + ValueNames: valueNames(f.Arg), + ValueArity: exactArity(f.Arg), ValueDemanded: valueDemanded, Short: first(f.Help, f.HelpFirstLine), Long: first(f.HelpLong, f.Help), @@ -857,7 +860,7 @@ func (b *builder) flag(f *Flag) *argv.Flag { IgnoreCase: f.Arg != nil && f.Arg.Choices != nil && f.Arg.Choices.IgnoreCase, Default: f.defaults(), Env: f.Env, - VarMin: clampVarMax(f.VarMin), + VarMin: clampVarMax(f.valueMinimum()), Validate: valueValidation(f.Arg), ValidateError: valueValidationError(f.Arg), // Occurrences. The per-occurrence value bound is a limit binding applies, @@ -879,6 +882,19 @@ func (b *builder) flag(f *Flag) *argv.Flag { return out } +// valueMinimum is the post-binding minimum for this flag. A variadic value has +// one occurrence, so its nested minimum is also its total minimum; an ordinary +// repeatable flag instead uses the flag-level occurrence minimum. +func (f *Flag) valueMinimum() int { + if f.Arg != nil && f.Arg.Var { + if f.Arg.VarMin != 0 { + return f.Arg.VarMin + } + return f.VarMin + } + return f.VarMin +} + func (b *builder) arg(a *Arg) *argv.Arg { out := &argv.Arg{ Key: b.next(), @@ -895,14 +911,16 @@ func (b *builder) arg(a *Arg) *argv.Arg { out.VarMax = clampVarMax(a.VarMax) } b.recordHelp(out.Key, argv.Help{ - Hide: a.Hide, - Demanded: a.Required && len(a.Default) == 0, - Short: first(a.Help, a.HelpFirstLine), - Long: first(a.HelpLong, a.Help), - Heading: a.HelpHeading, - Choices: a.Choices.visible(), - Env: a.Env, - Default: a.Default, + Hide: a.Hide, + Demanded: a.Required && len(a.Default) == 0, + ValueNames: a.ValueNames, + ValueArity: exactArity(a), + Short: first(a.Help, a.HelpFirstLine), + Long: first(a.HelpLong, a.Help), + Heading: a.HelpHeading, + Choices: a.Choices.visible(), + Env: a.Env, + Default: a.Default, }) b.record(out.Key, argv.Meta{ Name: a.Name, @@ -923,6 +941,20 @@ func (b *builder) arg(a *Arg) *argv.Arg { return out } +func valueNames(a *Arg) []string { + if a == nil || len(a.ValueNames) == 0 { + return nil + } + return a.ValueNames +} + +func exactArity(a *Arg) uint32 { + if a != nil && a.Var && a.VarMin > 1 && a.VarMin == a.VarMax { + return uint32(a.VarMin) + } + return 0 +} + func valueValidation(arg *Arg) string { if arg == nil { return "" diff --git a/go/internal/spec/spec_test.go b/go/internal/spec/spec_test.go index 4812779da..19600dba7 100644 --- a/go/internal/spec/spec_test.go +++ b/go/internal/spec/spec_test.go @@ -63,6 +63,32 @@ func TestAFlagsDefaultCanBeDeclaredOnItsValue(t *testing.T) { } } +func TestVariadicFlagMinimumComesFromItsValue(t *testing.T) { + root, meta := build(&Spec{ + Name: "ex", Bin: "ex", + Cmd: Cmd{Name: "ex", Flags: []Flag{ + {Name: "include", Long: []string{"include"}, VarMin: 7, VarMax: 8, + Arg: &Arg{Name: "pattern", Var: true, VarMin: 2, VarMax: 3}}, + {Name: "fallback", Long: []string{"fallback"}, VarMin: 4, + Arg: &Arg{Name: "value", Var: true}}, + }}, + }) + + include := metaFor(t, meta, root, "include") + if include.VarMin != 2 { + t.Errorf("nested value minimum should win for a variadic occurrence: got %d", include.VarMin) + } + if include.VarMax != 8 { + t.Errorf("flag-level maximum should remain an occurrence bound: got %d", include.VarMax) + } + if root.Flags[0].VarMax != 3 { + t.Errorf("nested value maximum should reach the parse table: got %d", root.Flags[0].VarMax) + } + if got := metaFor(t, meta, root, "fallback").VarMin; got != 4 { + t.Errorf("an unset nested minimum should retain the flag-level minimum: got %d", got) + } +} + // The other half of the same question, and the answer is the opposite one: // usage-lib does not read a nested `env`, so neither does this. Verified against // it rather than assumed — `arg "" env="EX_MODE"` inside a flag leaves the diff --git a/lib/src/go/mod.rs b/lib/src/go/mod.rs index d3f857655..7818896a9 100644 --- a/lib/src/go/mod.rs +++ b/lib/src/go/mod.rs @@ -524,7 +524,13 @@ impl Emitter<'_> { if let Some(env) = &flag.env { fields.push(format!("Env: {}", go_string(env))); } - if let Some(min) = flag.var_min { + let minimum = flag + .arg + .as_ref() + .filter(|arg| arg.var) + .and_then(|arg| arg.var_min) + .or(flag.var_min); + if let Some(min) = minimum { fields.push(format!("VarMin: {}", clamp_var_max(min))); } // Occurrences. The per-occurrence value bound is a limit binding applies @@ -971,6 +977,12 @@ fn flag_help(flag: &SpecFlag, named: &Named) -> String { if arg.required && arg.default.is_empty() { fields.push("ValueDemanded: true".to_string()); } + if !arg.value_names.is_empty() { + fields.push(format!("ValueNames: {}", string_slice(&arg.value_names))); + } + if arg.var && arg.var_min == arg.var_max && arg.var_min.is_some_and(|n| n > 1) { + fields.push(format!("ValueArity: {}", arg.var_min.unwrap())); + } } // The whole `help`, not its first line: usage-lib's short page prints the // text as declared, and mise has flags whose help is two lines. @@ -1015,6 +1027,12 @@ fn arg_help(arg: &SpecArg, named: &Named) -> String { if arg.required && arg.default.is_empty() { fields.push("Demanded: true".to_string()); } + if !arg.value_names.is_empty() { + fields.push(format!("ValueNames: {}", string_slice(&arg.value_names))); + } + if arg.var && arg.var_min == arg.var_max && arg.var_min.is_some_and(|n| n > 1) { + fields.push(format!("ValueArity: {}", arg.var_min.unwrap())); + } if let Some(help) = arg.help.as_deref().or(arg.help_first_line.as_deref()) { fields.push(format!("Short: {}", go_string(help))); } @@ -1822,7 +1840,7 @@ cmd "root" { name "ex" bin "ex" flag "--include ..." { - arg "..." var=#true var_max=2 + arg "..." var=#true var_min=2 var_max=2 } flag "--tag " var=#true var_max=1 "#); @@ -1830,10 +1848,36 @@ flag "--tag " var=#true var_max=1 out.contains("Name: \"include\", Longs: []string{\"include\"}, TakesValue: true, Variadic: true, VarMax: 2"), "{out}" ); + assert!( + out.contains("Name: \"include\", Flag: true") && out.contains("VarMin: 2"), + "the nested value minimum must reach post-binding metadata:\n{out}" + ); let tag = out.lines().find(|l| l.contains("\"tag\"")).unwrap(); assert!(!tag.contains("VarMax"), "occurrence bound leaked: {tag}"); } + #[test] + fn exact_arity_with_one_label_reaches_go_help() { + let out = go(r#" +name "ex" +bin "ex" +flag "--pair ..." { + arg "..." var=#true var_min=2 var_max=2 { + value_names "ITEM" + } +} +arg "..." var=#true var_min=2 var_max=2 { + value_names "ITEM" +} +"#); + assert_eq!(out.matches("ValueArity: 2").count(), 2, "{out}"); + assert_eq!( + out.matches("ValueNames: []string{\"ITEM\"}").count(), + 2, + "{out}" + ); + } + #[test] fn allow_hyphen_values_reaches_the_table() { let out = go(r#" diff --git a/lib/src/spec/arg.rs b/lib/src/spec/arg.rs index 8dd8959c3..86e9e99ad 100644 --- a/lib/src/spec/arg.rs +++ b/lib/src/spec/arg.rs @@ -10,7 +10,9 @@ use crate::spec::context::ParsingContext; use crate::spec::effect::{SpecCommandEffect, EFFECT_VALUES}; use crate::spec::helpers::{string_entry, NodeHelper}; use crate::spec::is_false; -use crate::{string, SpecChoice, SpecChoiceAlias, SpecChoices}; +use crate::{string, SpecChoices}; +#[cfg(feature = "clap")] +use crate::{SpecChoice, SpecChoiceAlias}; #[derive(Debug, Default, Clone, Serialize, PartialEq, Eq, strum::EnumString, strum::Display)] #[strum(serialize_all = "snake_case")] @@ -47,6 +49,10 @@ pub enum SpecDoubleDashChoices { pub struct SpecArg { /// Name of the argument (used in help text) pub name: String, + /// Ordered placeholders for a fixed-arity value, such as `START` and `END`. + /// Empty means the argument's `name` is the sole placeholder. + #[serde(skip_serializing_if = "Vec::is_empty")] + pub value_names: Vec, /// Generated usage string (e.g., "" or "[file]") pub usage: String, /// Short help text shown in command listings @@ -234,6 +240,13 @@ impl SpecArg { "var" => arg.var = child.arg(0)?.ensure_bool()?, "var_min" => arg.var_min = child.arg(0)?.ensure_usize().map(Some)?, "var_max" => arg.var_max = child.arg(0)?.ensure_usize().map(Some)?, + "value_names" => { + arg.value_names = child + .ensure_arg_len(1..)? + .args() + .map(|entry| entry.ensure_string()) + .collect::, _>>()?; + } "allow_negative_numbers" => { arg.allow_negative_numbers = child.arg(0)?.ensure_bool()?; } @@ -252,6 +265,25 @@ impl SpecArg { k => bail_parse!(ctx, child.node.name().span(), "unsupported arg child {k}"), } } + if let Some(first) = arg.value_names.first() { + arg.name.clone_from(first); + } + if arg.value_names.len() > 1 { + let arity = arg.value_names.len(); + match (arg.var_min, arg.var_max) { + (None, None) => { + arg.var_min = Some(arity); + arg.var_max = Some(arity); + } + (Some(min), Some(max)) if min == arity && max == arity => {} + _ => bail_parse!( + ctx, + node.node.name().span(), + "{arity} value names require var_min={arity} and var_max={arity}" + ), + } + arg.var = true; + } if arg.validate_error.is_some() && arg.validate.is_none() { bail_parse!( ctx, @@ -293,6 +325,41 @@ impl SpecArg { impl SpecArg { pub fn usage(&self) -> String { + let exact_arity = self.var.then_some(()).and_then(|()| { + self.var_min + .zip(self.var_max) + .filter(|(min, max)| min == max && *min > 1) + .map(|(arity, _)| arity) + }); + if self.value_names.len() > 1 || exact_arity.is_some() { + let labels = if self.value_names.len() > 1 { + self.value_names.clone() + } else { + vec![ + self.value_names + .first() + .cloned() + .unwrap_or_else(|| self.name.clone()); + exact_arity.expect("branch checked") + ] + }; + let placeholders = labels + .iter() + .map(|name| { + if self.required { + format!("<{name}>") + } else { + format!("[{name}]") + } + }) + .collect::>() + .join(" "); + return if self.double_dash == SpecDoubleDashChoices::Required { + format!("-- {placeholders}") + } else { + placeholders + }; + } let name = if self.double_dash == SpecDoubleDashChoices::Required { format!("-- {}", self.name) } else { @@ -412,9 +479,40 @@ impl From<&SpecArg> for KdlNode { impl From<&str> for SpecArg { fn from(input: &str) -> Self { + let (input, after_double_dash) = input + .strip_prefix("-- ") + .map_or((input, false), |rest| (rest, true)); + if let Some(placeholders) = fixed_placeholders(input) { + let required = placeholders + .iter() + .all(|placeholder| placeholder.starts_with('<')); + let value_names = placeholders + .iter() + .map(|placeholder| placeholder[1..placeholder.len() - 1].to_string()) + .collect::>(); + return SpecArg { + name: value_names[0].clone(), + value_names, + required, + var: true, + var_min: Some(placeholders.len()), + var_max: Some(placeholders.len()), + double_dash: if after_double_dash { + SpecDoubleDashChoices::Required + } else { + SpecDoubleDashChoices::Optional + }, + ..Default::default() + }; + } let mut arg = SpecArg { name: input.to_string(), required: true, + double_dash: if after_double_dash { + SpecDoubleDashChoices::Required + } else { + SpecDoubleDashChoices::Optional + }, ..Default::default() }; // Handle trailing ellipsis: "foo..." or "foo…" or "..." or "[foo]..." @@ -438,6 +536,13 @@ impl From<&str> for SpecArg { } _ => {} } + // The single-placeholder shorthand encloses the separator with the value: + // `[-- target]`. Multi-placeholder canonical output puts it before the + // placeholders (`-- [START] [END]`) and was handled above. + if let Some(name) = arg.name.strip_prefix("-- ") { + arg.double_dash = SpecDoubleDashChoices::Required; + arg.name = name.to_string(); + } // Also handle ellipsis inside brackets: "[args...]" or "" if !arg.var { if let Some(name) = arg @@ -449,20 +554,48 @@ impl From<&str> for SpecArg { arg.name = name.to_string(); } } - if let Some(name) = arg.name.strip_prefix("-- ") { - arg.double_dash = SpecDoubleDashChoices::Required; - arg.name = name.to_string(); - } arg } } impl FromStr for SpecArg { type Err = UsageErr; fn from_str(input: &str) -> std::result::Result { + if fixed_placeholders(input.strip_prefix("-- ").unwrap_or(input)).is_some_and( + |placeholders| { + placeholders + .windows(2) + .any(|pair| pair[0].starts_with('<') != pair[1].starts_with('<')) + }, + ) { + let message = + "fixed-arity placeholders must be either all required or all optional".to_string(); + return Err(UsageErr::InvalidInput( + message, + (0, input.len()).into(), + miette::NamedSource::new("argument", input.to_string()), + )); + } Ok(input.into()) } } +/// Return a multi-placeholder declaration without allocating for the overwhelmingly common +/// single-placeholder case. +fn fixed_placeholders(input: &str) -> Option> { + if !input.bytes().any(|byte| byte.is_ascii_whitespace()) { + return None; + } + let placeholders: Vec<_> = input.split_whitespace().collect(); + (placeholders.len() > 1 + && placeholders.iter().all(|placeholder| { + matches!( + (placeholder.chars().next(), placeholder.chars().last()), + (Some('<'), Some('>')) | (Some('['), Some(']')) + ) + })) + .then_some(placeholders) +} + /// A clap argument's defaults, as the spec has to record them. /// /// clap splits a value by the argument's `value_delimiter` before anyone sees it, defaults @@ -506,6 +639,12 @@ pub(crate) fn value_bounds(source: &clap::Arg, target: &mut SpecArg, zero_values } let Some(range) = source.get_num_args() else { + if target.value_names.len() > 1 { + let arity = target.value_names.len(); + target.var = true; + target.var_min = Some(arity); + target.var_max = Some(arity); + } return; }; let min = range.min_values(); @@ -519,6 +658,32 @@ pub(crate) fn value_bounds(source: &clap::Arg, target: &mut SpecArg, zero_values target.var_max = (max != usize::MAX).then_some(max); } +/// Value labels that can survive the spec's fixed-arity representation. +/// +/// Clap permits several labels beside a ranged `num_args`; usage gives distinct labels only to +/// an exact number of slots. Keep the first display label for a range and let the fidelity report +/// name the loss instead of emitting KDL that cannot be parsed back. +#[cfg(feature = "clap")] +pub(crate) fn value_names_from_clap(source: &clap::Arg) -> Vec { + let names: Vec = source + .get_value_names() + .unwrap_or_default() + .iter() + .map(ToString::to_string) + .collect(); + if names.len() <= 1 { + return names; + } + let mismatched_range = source.get_num_args().is_some_and(|range| { + range.min_values() != names.len() || range.max_values() != names.len() + }); + if source.get_value_delimiter().is_some() || mismatched_range { + names.into_iter().take(1).collect() + } else { + names + } +} + #[cfg(feature = "clap")] pub(crate) fn choices_from_clap(arg: &clap::Arg) -> Option { let possible = arg.get_possible_values(); @@ -578,14 +743,13 @@ impl From<&clap::Arg> for SpecArg { clap::ArgAction::Count | clap::ArgAction::Append ) || delimiter.is_some(); let choices = choices_from_clap(arg); + let value_names = value_names_from_clap(arg); let mut arg = Self { - name: arg - .get_value_names() - .unwrap_or_default() + name: value_names .first() .cloned() - .map(|name| name.to_string()) .unwrap_or_else(|| source.get_id().to_string()), + value_names, usage: "".into(), required, double_dash: if arg.is_last_set() { @@ -949,7 +1113,7 @@ mod possible_value_tests { #[cfg(test)] mod tests { - use crate::Spec; + use crate::{Spec, SpecArg}; use insta::assert_snapshot; #[test] @@ -1046,6 +1210,101 @@ arg "" { assert!(!arg.required); } + #[test] + fn fixed_arity_placeholders_round_trip() { + let spec: Spec = "arg \" \"\n".parse().unwrap(); + let arg = &spec.cmd.args[0]; + assert_eq!(arg.value_names, ["START", "END"]); + assert_eq!((arg.var_min, arg.var_max), (Some(2), Some(2))); + assert_eq!(arg.usage, " "); + + let reparsed: Spec = spec.to_string().parse().unwrap(); + assert_eq!(reparsed.cmd.args[0].value_names, ["START", "END"]); + } + + #[test] + fn fixed_arity_placeholders_reject_mismatched_bounds() { + let error = "arg \" \" var_min=1 var_max=2\n" + .parse::() + .unwrap_err(); + assert!( + format!("{error:?}").contains("require var_min=2 and var_max=2"), + "{error:?}" + ); + } + + #[test] + fn a_single_value_name_replaces_the_display_name() { + let spec: Spec = "arg \"\" { value_names \"INPUT\" }\n" + .parse() + .unwrap(); + let arg = &spec.cmd.args[0]; + assert_eq!(arg.name, "INPUT"); + assert_eq!(arg.usage, ""); + + let built = SpecArg::builder() + .name("input") + .required(true) + .value_names(["INPUT"]) + .build(); + assert_eq!(built.name, "INPUT"); + assert_eq!(built.usage, ""); + } + + #[test] + fn builder_fixed_arity_survives_later_bound_setters() { + let after = SpecArg::builder() + .value_names(["START", "END"]) + .var(false) + .var_min(1) + .var_max(4) + .build(); + let before = SpecArg::builder() + .var(false) + .var_min(1) + .var_max(4) + .value_names(["START", "END"]) + .build(); + for arg in [after, before] { + assert!(arg.var); + assert_eq!((arg.var_min, arg.var_max), (Some(2), Some(2))); + assert_eq!(arg.usage, "[START] [END]"); + } + } + + #[test] + fn one_label_with_exact_bounds_renders_each_value_slot() { + let spec: Spec = "arg \"…\" var_min=2 var_max=2 { value_names \"ITEM\" }\n" + .parse() + .unwrap(); + assert_eq!(spec.cmd.args[0].usage, " "); + let reparsed: Spec = spec.to_string().parse().unwrap(); + assert_eq!(reparsed.cmd.args[0].value_names, ["ITEM", "ITEM"]); + assert_eq!( + (reparsed.cmd.args[0].var_min, reparsed.cmd.args[0].var_max), + (Some(2), Some(2)) + ); + + let built = SpecArg::builder() + .value_names(["ITEM"]) + .required(true) + .var(true) + .var_min(2) + .var_max(2) + .build(); + assert_eq!(built.usage, " "); + } + + #[test] + fn fixed_arity_placeholders_reject_mixed_requiredness() { + let error = "arg \" [END]\"\n".parse::().unwrap_err(); + assert!( + format!("{error:?}") + .contains("fixed-arity placeholders must be either all required or all optional"), + "{error:?}" + ); + } + #[test] fn test_arg_child_nodes() { let spec = Spec::parse( diff --git a/lib/src/spec/builder.rs b/lib/src/spec/builder.rs index 186e944c7..003471cf4 100644 --- a/lib/src/spec/builder.rs +++ b/lib/src/spec/builder.rs @@ -426,6 +426,25 @@ impl SpecArgBuilder { self } + /// Set the ordered placeholders for a fixed-arity value. + pub fn value_names(mut self, names: I) -> Self + where + I: IntoIterator, + S: Into, + { + self.inner.value_names = names.into_iter().map(Into::into).collect(); + if let Some(first) = self.inner.value_names.first() { + self.inner.name.clone_from(first); + } + if self.inner.value_names.len() > 1 { + let arity = self.inner.value_names.len(); + self.inner.var = true; + self.inner.var_min = Some(arity); + self.inner.var_max = Some(arity); + } + self + } + /// Add a default value (can be called multiple times for var args) pub fn default_value(mut self, value: impl Into) -> Self { self.inner.default.push(value.into()); @@ -573,6 +592,12 @@ impl SpecArgBuilder { if self.inner.validate.is_none() { self.inner.validate_error = None; } + if self.inner.value_names.len() > 1 { + let arity = self.inner.value_names.len(); + self.inner.var = true; + self.inner.var_min = Some(arity); + self.inner.var_max = Some(arity); + } self.inner.usage = self.inner.usage(); self.inner } diff --git a/lib/src/spec/flag.rs b/lib/src/spec/flag.rs index 3b82130e0..571905cfb 100644 --- a/lib/src/spec/flag.rs +++ b/lib/src/spec/flag.rs @@ -865,7 +865,10 @@ impl FromStr for SpecFlag { } else if part.starts_with('<') && part.ends_with('>') || part.starts_with('[') && part.ends_with(']') { - flag.arg = Some(part.to_string().parse()?); + flag.arg = Some(match flag.arg.take() { + Some(existing) => format!("{} {part}", existing.usage()).parse()?, + None => part.to_string().parse()?, + }); } else { return Err(InvalidFlag { token: part.to_string(), @@ -922,12 +925,15 @@ impl From<&clap::Arg> for SpecFlag { long.extend(hidden_aliases.iter().cloned()); let name = get_name_from_short_and_long(&short, &long).unwrap_or_default(); let arg = if let clap::ArgAction::Set | clap::ArgAction::Append = c.get_action() { + let value_names = crate::spec::arg::value_names_from_clap(c); let mut arg = SpecArg::from( - c.get_value_names() - .map(|s| s.iter().map(|s| s.to_string()).join(" ")) - .unwrap_or(name.clone()) + value_names + .first() + .cloned() + .unwrap_or_else(|| name.clone()) .as_str(), ); + arg.value_names = value_names; arg.choices = crate::spec::arg::choices_from_clap(c); @@ -965,14 +971,9 @@ impl From<&clap::Arg> for SpecFlag { } } - // clap's range is per occurrence. A non-repeatable `Set` flag has one - // occurrence, so the spec's collecting bound says exactly the same thing. - // `Append` can occur several times; carrying its minimum as a total would let - // one long occurrence hide another short one, so leave that for an explicit - // per-occurrence model rather than weakening the rule silently. - if matches!(c.get_action(), clap::ArgAction::Set) { - crate::spec::arg::value_bounds(c, &mut arg, false); - } + // These bounds live on the nested value argument and are enforced per occurrence. + // That preserves both a single `Set` and each repetition of `Append`. + crate::spec::arg::value_bounds(c, &mut arg, false); Some(arg) } else { @@ -1121,6 +1122,11 @@ mod tests { assert_snapshot!("-f --flag ".parse::().unwrap(), @"-f --flag "); assert_snapshot!("-f --flag… ".parse::().unwrap(), @"-f --flag… "); assert_snapshot!("-f --flag …".parse::().unwrap(), @"-f --flag …"); + let range = "--range ".parse::().unwrap(); + let arg = range.arg.as_ref().unwrap(); + assert_eq!(arg.value_names, ["start", "end"]); + assert_eq!((arg.var_min, arg.var_max), (Some(2), Some(2))); + assert_snapshot!(range, @"--range "); assert_snapshot!("myflag: -f".parse::().unwrap(), @"myflag: -f"); assert_snapshot!("myflag: -f --flag ".parse::().unwrap(), @"myflag: -f --flag "); } @@ -1440,6 +1446,59 @@ mod tests { ); } + #[test] + fn append_value_count_bounds_are_per_occurrence() { + let cmd = clap::Command::new("ex").arg( + clap::Arg::new("pair") + .long("pair") + .action(clap::ArgAction::Append) + .num_args(2), + ); + let spec = Spec::from(&cmd); + let flag = &spec.cmd.flags[0]; + let values = flag.arg.as_ref().unwrap(); + assert!(flag.var); + assert_eq!((values.var_min, values.var_max), (Some(2), Some(2))); + + crate::parse( + &spec, + &["ex", "--pair", "a", "b", "--pair", "c", "d"].map(str::to_string), + ) + .expect("each occurrence satisfies the fixed cardinality"); + + let err = crate::parse( + &spec, + &["ex", "--pair", "a", "--pair", "c", "d"].map(str::to_string), + ) + .unwrap_err(); + assert!(format!("{err:?}").contains("requires at least 2 value(s), got 1")); + } + + #[test] + fn ranged_value_names_do_not_emit_invalid_fixed_arity() { + let cmd = clap::Command::new("ex") + .arg( + clap::Arg::new("range") + .long("range") + .action(clap::ArgAction::Set) + .num_args(2..=4) + .value_names(["START", "END"]), + ) + .arg( + clap::Arg::new("files") + .num_args(1..=3) + .value_names(["FIRST", "REST"]), + ); + let spec = Spec::from(&cmd); + assert_eq!( + spec.cmd.flags[0].arg.as_ref().unwrap().value_names, + ["START"] + ); + assert_eq!(spec.cmd.args[0].value_names, ["FIRST"]); + let rendered = spec.to_string(); + let _: Spec = rendered.parse().expect("the generated KDL must parse back"); + } + #[test] fn delimiter_value_count_bounds_are_not_mapped() { let cmd = clap::Command::new("ex") diff --git a/usage-rs/tests/facade.rs b/usage-rs/tests/facade.rs index 09d455f2b..13e8c68a1 100644 --- a/usage-rs/tests/facade.rs +++ b/usage-rs/tests/facade.rs @@ -80,6 +80,17 @@ struct ClapSpellings { path: Option, } +#[derive(Cli)] +#[command(bin = "fixed-arity")] +struct FixedArity { + #[arg(long, num_args = 2, value_names = ["START", "END"])] + pair: Vec, + #[arg(long, num_args = 2, value_name = "ITEM")] + pair_same: Vec, + #[arg(long, value_names = ["INPUT"])] + input: Option, +} + #[derive(usage::Args)] struct FlattenedRelationshipTargets { #[usage(long, default = "nested-default")] @@ -618,6 +629,40 @@ fn clap_field_ids_and_aliases_need_no_rewrite() { ); } +#[test] +fn clap_value_arity_stays_on_each_flag_occurrence() { + let parsed = FixedArity::parse_from(&[ + OsStr::new("--pair"), + OsStr::new("a"), + OsStr::new("b"), + OsStr::new("--input"), + OsStr::new("file"), + ]) + .expect("the fixed-arity occurrence should consume exactly two values"); + assert_eq!(parsed.pair, ["a", "b"]); + assert!(parsed.pair_same.is_empty()); + assert_eq!(parsed.input.as_deref(), Some("file")); + + let kdl = FixedArity::to_kdl(); + assert!(kdl.contains("flag --pair"), "{kdl}"); + assert!(kdl.contains("arg \" \""), "{kdl}"); + assert!(!kdl.contains("flag --pair var_min=2"), "{kdl}"); + assert!(kdl.contains("arg "), "{kdl}"); + let help = usage::help::render(FixedArity::spec(), FixedArity::command(), false) + .expect("the root has help to render"); + assert!(help.contains("--input "), "{help}"); + assert!(help.contains("--pair-same "), "{help}"); + assert!( + kdl.contains("flag --pair-same") && kdl.contains("arg \" \""), + "{kdl}" + ); + + assert!( + FixedArity::parse_from(&[OsStr::new("--pair"), OsStr::new("only-one")]).is_err(), + "a partial fixed-arity occurrence must fail its value minimum" + ); +} + #[test] fn relationships_resolve_targets_inside_flattened_args() { let parent_wins = FlattenedRelationships::parse_from(&[