Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions PLAN.md
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,10 @@ Groups are the opposite case: `Command::get_groups`, `ArgGroup::get_args` and
errors. KDL, usage-lib, the typed derive/static metadata, generated Go, and the
clap bridge carry the policy. Repeatable, variadic, and count flags retain their
collecting behavior.
- [x] **Optional flag values.** `Option<Option<T>>` distinguishes an absent flag,
a bare flag, and an explicit value. The derive infers zero-or-one value arity,
usage-argv binds all three states, and emitted KDL/help use an optional
placeholder without requiring a synthetic `default_missing`.

**Help output**

Expand Down
116 changes: 104 additions & 12 deletions argv/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,12 @@ pub struct Flag<'a> {
/// attached form (`-i9229`, `-i=9229`) still binds: only the following word
/// is refused.
pub require_equals: bool,
/// Whether this value-taking flag may be present without a value.
///
/// A missing value emits the flag event with `value: None`; bindings such as
/// `Option<Option<T>>` can therefore distinguish an absent flag from a bare
/// flag and from a flag with an explicit value.
pub value_optional: bool,
/// Value used when the flag is present but no value is given.
///
/// clap's `default_missing_value` and the spec's `default_missing`. `--color`
Expand Down Expand Up @@ -379,6 +385,7 @@ impl Flag<'_> {
allow_negative_numbers: false,
value_terminator: ::core::option::Option::None,
require_equals: false,
value_optional: false,
default_missing: ::core::option::Option::None,
global: false,
};
Expand Down Expand Up @@ -1413,15 +1420,17 @@ impl<'t: 'v, 'a, 'v> Parser<'t, 'a, 'v> {

if let Some(flag) = self.find_long(name) {
let value = if flag.takes_value {
Some(match attached {
Some(v) => v,
match attached {
Some(v) => Some(v),
None => self.take_detached_value(flag)?,
})
}
Comment thread
cursor[bot] marked this conversation as resolved.
} else {
None
};
if flag.variadic {
self.start_collecting(flag, value.unwrap_or(b""))?;
if let Some(value) = value {
self.start_collecting(flag, value)?;
}
}
return Ok(Event::Flag {
flag,
Expand Down Expand Up @@ -1511,16 +1520,18 @@ impl<'t: 'v, 'a, 'v> Parser<'t, 'a, 'v> {
let value = if rest.is_empty() {
self.take_detached_value(flag)?
} else if rest[0] == b'=' {
&rest[1..]
Some(&rest[1..])
} else {
rest
Some(rest)
};
if flag.variadic {
self.start_collecting(flag, value)?;
if let Some(value) = value {
self.start_collecting(flag, value)?;
}
}
Ok(Event::Flag {
flag,
value: Some(value),
value,
negated: false,
})
}
Expand All @@ -1531,7 +1542,10 @@ impl<'t: 'v, 'a, 'v> Parser<'t, 'a, 'v> {
/// `--jobs --force` is far more likely a forgotten value than a deliberate
/// one, and the attached form is available for the deliberate case. Declared,
/// the next token is taken whatever it looks like, including `--`.
fn take_detached_value(&mut self, flag: &'t Flag<'t>) -> Result<&'v [u8], Error<'t, 'v>> {
fn take_detached_value(
&mut self,
flag: &'t Flag<'t>,
) -> Result<Option<&'v [u8]>, Error<'t, 'v>> {
if flag.require_equals {
return self.missing_or_default(flag);
}
Expand All @@ -1542,15 +1556,16 @@ impl<'t: 'v, 'a, 'v> Parser<'t, 'a, 'v> {
|| (flag.allow_negative_numbers && is_negative_number(bytes(next))) =>
{
self.pos += 1;
Ok(bytes(next))
Ok(Some(bytes(next)))
}
_ => self.missing_or_default(flag),
}
}

fn missing_or_default(&self, flag: &'t Flag<'t>) -> Result<&'v [u8], Error<'t, 'v>> {
fn missing_or_default(&self, flag: &'t Flag<'t>) -> Result<Option<&'v [u8]>, Error<'t, 'v>> {
match flag.default_missing {
Some(value) => Ok(value),
Some(value) => Ok(Some(value)),
None if flag.value_optional => Ok(None),
Comment thread
cursor[bot] marked this conversation as resolved.
None => Err(Error::MissingFlagValue { flag }),
}
}
Expand Down Expand Up @@ -3037,6 +3052,83 @@ mod tests {
);
}

#[test]
fn optional_flag_value_distinguishes_bare_and_explicit_forms() {
static BUMP: Flag = Flag {
key: 11,
name: "bump",
longs: &["bump"],
takes_value: true,
value_optional: true,
..Flag::BOOL
};
static OPTIONAL: Command = Command {
name: "ex",
flags: &[&BUMP],
..Command::EMPTY
};

assert_eq!(parse(&OPTIONAL, &argv([])).unwrap(), vec![]);
assert_eq!(
parse(&OPTIONAL, &argv(["--bump"])).unwrap(),
vec![Event::Flag {
flag: &BUMP,
value: None,
negated: false,
}]
);
assert_eq!(
parse(&OPTIONAL, &argv(["--bump=5"])).unwrap(),
vec![Event::Flag {
flag: &BUMP,
value: Some(b"5"),
negated: false,
}]
);

static INCLUDE: Flag = Flag {
key: 12,
name: "include",
longs: &["include"],
takes_value: true,
variadic: true,
value_optional: true,
..Flag::BOOL
};
static VERBOSE: Flag = Flag {
key: 13,
name: "verbose",
longs: &["verbose"],
..Flag::BOOL
};
static VARIADIC: Command = Command {
name: "ex",
flags: &[&INCLUDE, &VERBOSE],
args: &[&REST],
..Command::EMPTY
};
assert_eq!(
parse(&VARIADIC, &argv(["--include", "--verbose", "file"])).unwrap(),
vec![
Event::Flag {
flag: &INCLUDE,
value: None,
negated: false,
},
Event::Flag {
flag: &VERBOSE,
value: None,
negated: false,
},
Event::Arg {
arg: &REST,
value: b"file",
delimit: true,
},
]
);
}

#[test]
fn default_missing_with_require_equals_leaves_the_following_word() {
static INSPECT: Flag = Flag {
Expand Down
3 changes: 3 additions & 0 deletions argv/src/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1641,6 +1641,9 @@ fn write_flag(out: &mut String, meta: &FlagMeta<'_>, depth: usize) -> core::fmt:
if meta.flag.require_equals {
out.push_str(" require_equals=#true");
}
if meta.flag.value_optional {
out.push_str(" value_optional=#true");
}
if let Some(missing) = meta.flag.default_missing {
write!(
out,
Expand Down
1 change: 1 addition & 0 deletions conformance/src/tables.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,7 @@ fn build_flag(f: &SpecFlag) -> &'static Flag<'static> {
.and_then(|arg| arg.value_terminator.as_deref())
.map(|value| leak(value).as_bytes()),
require_equals: f.require_equals,
value_optional: f.value_optional,
default_missing: f.default_missing.as_deref().map(|s| leak(s).as_bytes()),
global: f.global,
}))
Expand Down
35 changes: 35 additions & 0 deletions conformance/tests/optional_flag_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,44 @@ fn the_reference_renders_it_the_same_way() {
fn the_emitted_spec_says_both_halves() {
let kdl = Ex::to_kdl();
assert!(kdl.contains(r#"arg "[BUMP]" required=#false"#), "{kdl}");
assert!(!kdl.contains("value_optional=#true"), "{kdl}");
assert!(kdl.contains("arg <PORT>"), "{kdl}");
}

#[test]
fn portable_tables_keep_help_optionality_separate_from_binding() {
let presentation: LibSpec = Ex::to_kdl().parse().unwrap();
let presentation = usage_conformance::tables::build_spec(&presentation);
let bare = [OsStr::new("--bump")];
let mut parser = usage_argv::Parser::new(presentation.root.cmd, &bare);
let presentation_error = loop {
match parser.next_event() {
Some(Ok(_)) => {}
Some(Err(_)) => break true,
None => break false,
}
};
assert!(presentation_error);

let executable: LibSpec = r#"
name "ex"
flag "--bump [BUMP]" value_optional=#true
"#
.parse()
.unwrap();
let executable = usage_conformance::tables::build_spec(&executable);
let mut parser = usage_argv::Parser::new(executable.root.cmd, &bare);
let mut events = Vec::new();
while let Some(event) = parser.next_event() {
events.push(event.unwrap());
}
assert_eq!(events.len(), 1);
assert!(matches!(
events[0],
usage_argv::Event::Flag { value: None, .. }
));
}

#[test]
fn it_still_takes_its_value() {
// Nothing about binding changed: the value is read where it is given.
Expand Down
38 changes: 38 additions & 0 deletions derive/src/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1070,6 +1070,11 @@ fn flag_table(i: usize, field: &Field) -> TokenStream {
None => quote!(::core::option::Option::None),
};
let require_equals = field.require_equals;
// `value_optional` can be a presentation-only declaration for clap/spec
// compatibility. Only a nested Option can represent a genuinely bare value
// in the typed result; `default_missing` turns the bare form into a value
// before binding.
let value_optional = field.optional_value_type || field.default_missing.is_some();
let default_missing = match field.default_missing.as_deref() {
Some(value) => quote!(::core::option::Option::Some(#value.as_bytes())),
None => quote!(::core::option::Option::None),
Expand All @@ -1089,6 +1094,7 @@ fn flag_table(i: usize, field: &Field) -> TokenStream {
allow_negative_numbers: #allow_negative_numbers,
value_terminator: #value_terminator,
require_equals: #require_equals,
value_optional: #value_optional,
Comment thread
jdx marked this conversation as resolved.
default_missing: #default_missing,
global: #global,
};
Expand Down Expand Up @@ -1819,6 +1825,9 @@ fn flag_arm(cli: &Cli, i: usize, field: &Field) -> TokenStream {
// Saturating, because a `u8` field given 256 occurrences would otherwise
// panic in debug and wrap to zero in release.
Shape::Count => quote!(partial.#ident = partial.#ident.saturating_add(1);),
Shape::Optional if field.optional_value_type => quote! {
partial.#ident = value.map(__usage_text);
},
Shape::Optional => quote! {
partial.#ident = ::std::option::Option::Some(__usage_value_text(value));
},
Expand Down Expand Up @@ -2810,6 +2819,7 @@ fn partial_defaults(cli: &Cli) -> TokenStream {
/// number, or a type of the adopter's own.
fn field_final(field: &Field) -> TokenStream {
let ident = &field.ident;
let given = format_ident!("__given_{}", ident);
let name = &field.name;
if matches!(field.kind, Kind::Skip) {
// clap's skip: not parsed, filled from Default when the struct is built.
Expand Down Expand Up @@ -2902,6 +2912,20 @@ fn field_final(field: &Field) -> TokenStream {
// A `match` rather than `.map`, and a loop rather than `.collect`, for the same
// reason the text path below uses them: the conversion can fail, and a `return`
// inside a closure would leave the error in the closure's own return type.
Shape::Optional if field.optional_value_type => {
let value = converted(quote!(__usage_value));
quote! {
#ident: match partial.#ident {
::std::option::Option::Some(__usage_value) => {
::std::option::Option::Some(::std::option::Option::Some(#value))
}
::std::option::Option::None if partial.#given => {
::std::option::Option::Some(::std::option::Option::None)
}
::std::option::Option::None => ::std::option::Option::None,
}
}
}
Shape::Optional => {
let value = converted(quote!(__usage_value));
quote! {
Expand Down Expand Up @@ -2991,6 +3015,20 @@ fn field_final(field: &Field) -> TokenStream {
let one = converted(quote!(partial.#ident));
quote!(#ident: #one)
}
Shape::Optional if field.optional_value_type => {
let one = converted(quote!(__usage_value));
quote! {
#ident: match partial.#ident {
::std::option::Option::Some(__usage_value) => {
::std::option::Option::Some(::std::option::Option::Some(#one))
}
::std::option::Option::None if partial.#given => {
::std::option::Option::Some(::std::option::Option::None)
}
::std::option::Option::None => ::std::option::Option::None,
}
}
}
Shape::Optional => {
let one = converted(quote!(__usage_value));
quote! {
Expand Down
Loading