argh icon indicating copy to clipboard operation
argh copied to clipboard

Support for a minimum number of repeated arguments

Open woody77 opened this issue 3 years ago • 4 comments

Whether positional or option arguments that are of type Vec<T> are always treated as "0 or more", with no way to specify "one or more", or "two or more".

This is similar to issue #13, but a more general case.

I can make two proposals here:

Proposal 1:

add an attribute such as #[argh(option, required)], which forces some value to be present for vec-types.

Proposal 2:

add an attribute such as #[argh(option, required(2+)] or #[argh(option, required="2+")]


Help text would need to be adjusted accordingly as well, such as:

Usage: cmdname --option <option> --option <option> [--option <option> ...]

Options:
  --option        some option (2 or more required)

woody77 avatar Oct 28 '21 18:10 woody77

Implementation-wise, this seems like it could be added to parse_attrs::FieldAttrs as a Option<usize> for minimum number of elements with Vec types.

woody77 avatar Oct 28 '21 18:10 woody77

I am not too familiar with writing proc macros, but I arrived here having just run into a need for this feature, and I have an idea for the syntax wrt your Proposal 2 — Range syntax. Like so:

  • #[argh(option, required)]: Requires a non-zero number of occurrences.
  • #[argh(option, required(1))]: Requires exactly one occurrence.
  • #[argh(option, required(2..))]: Requires two or more occurrences.
  • #[argh(option, required(..=2))]: Requires two or fewer occurrences.
  • #[argh(option, required(3..5))]: Requires either three or four (not five) occurrences.
  • #[argh(option, required(3..=5))]: Requires three, four, or five occurrences.

Assuming the supported syntax is roughly similar to standard Macros, I could see this being done with a special case for a Literal, and then a fallback case which takes whatever $x:expr is found and verifies that $x.contains(vec.len()).

Yaulendil avatar Nov 16 '21 20:11 Yaulendil

I would not complicate so much the space of possibilities. My view is the following.

At the moment there is a Optionality::Repeating value defined which translates to "zero or more positional arguments in a Vec". Obviously, there are (possibly many) cases where what we want is "one or more positional arguments in a Vec". The current implementation does not distinguish between them.

My proposal would be to start distinguishing them, as follows:

  • Option<Vec<String>> is an optional list of positional arguments represented by something like Optionality::ZeroOrMore, currently Optionality::Repeating, and
  • Vec<String> is a "required" list of positional arguments represented by something like Optionality::OneOrMore.

Then, Option<Vec<String>> will print the square brackets around the positional argument in the help message ([<files>...]) while Vec<String> will not (<files>...).

The problem is that this is a breaking change, it is not backwards compatible. However, I find it more natural to think of "optionality" in terms of Option and "cardinality" in terms of Vec.

Let me know your thoughts.

Edit: this approach is based on the presumption that a Vec will always have at least one element, which may be counter-intuitive in a programming context (after all we most of the times start with empty Vec instances and populate them later), but it actually makes sense in the command-line context. When we say "my text editor program takes as parameters a list of files to open or no file at all", we actually mean that there is either a non-empty list of files (one or more) or no list at all, so having this perspective in mind, a Vec will always have at least one element, or there will be no Vec at all. Basically Option<Vec<String>>. But I understand that this is not how programmers think of Vec.

alexonea avatar Nov 08 '23 09:11 alexonea

To @alexonea's proposal, I would add two things.

  1. I think that application-specific use-cases like the range-based method @Yaulendil proposed needn't be handled by the parser. If desired cardinality is 3..=5, the application can ignore extra items and report when it sees too few items. What stays true for the command-line interface is the "one or more" optionality.

  2. I think the current design is great in saying that for an argument Option<T> or a default value means optional, otherwise it is required.

optionality = if let Some(x) = ty_inner(&["Option"], &field.ty) {
    Optionality::Optional
} else if let Some(x) = ty_inner(&["Vec"], &field.ty) {
    Optionality::Repeating
} else {
    Optionality::None
};

However, in my view, the problem here is that the second branch creates a rule that supersedes the first one, and it shouldn't. Vec<T> should adhere as well to the first rule. If Option<Vec<T>> is given, then it is optional, if Vec<T> is given and there is no default value, it is required. And what does it mean for a Vec to be required? As @alexonea exemplified as well, it means it must have at least one element to make sense in a command-line context.

I strongly believe that in a command-line context linking required and empty makes no sense. Therefore, accepting an empty Vec as a required positional argument should not be an option.

It is either required and has at least one element, or it is optional but then it is marked accordingly (Option<Vec<T>>).

andreinonea avatar Nov 08 '23 09:11 andreinonea