Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for a minimum number of repeated arguments #110

Open
woody77 opened this issue Oct 28, 2021 · 4 comments
Open

Support for a minimum number of repeated arguments #110

woody77 opened this issue Oct 28, 2021 · 4 comments

Comments

@woody77
Copy link
Collaborator

woody77 commented Oct 28, 2021

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
Copy link
Collaborator Author

woody77 commented Oct 28, 2021

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.

@Yaulendil
Copy link

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()).

@alexonea
Copy link

alexonea commented Nov 8, 2023

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.

@andreinonea
Copy link

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>>).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants