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

Add has_no_matched_args to ArgMatches #3265

Closed

Conversation

ohsayan
Copy link

@ohsayan ohsayan commented Jan 6, 2022

This member function provides a simple way to check if no arguments
were matched at all, in v3. The same in v2 could be achieved by
accessing the args field in ArgMatches and checking if it was
empty. However, in v3 the field was made private in the public API
and for the same reason, this method was added.

@ohsayan ohsayan force-pushed the argmatches/has_no_matched_args branch from a9593b1 to d2909aa Compare January 6, 2022 04:47
This member function provides a simple way to check if no arguments
were matched at all, in v3. The same in v2 could be achieved by
accessing the `args` field in `ArgMatches` and checking if it was
empty. However, in v3 the field was made private in the public API
and for the same reason, this method was added.
@ohsayan ohsayan force-pushed the argmatches/has_no_matched_args branch from d2909aa to 0fc3fcb Compare January 6, 2022 04:50
@ohsayan ohsayan marked this pull request as ready for review January 6, 2022 04:58
ohsayan added a commit to skytable/skytable that referenced this pull request Jan 6, 2022
All deps except for `clap` has been upgraded. Due to the removal of
the `args` field in `ArgMatches` in v3, and our dependence on the
field, we cannot upgrade to the latest version.

A PR has been created and once it is merged or a workaround
suggested, we can upgrade. (see clap-rs/clap#3265)
Copy link
Member

@epage epage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you post your use case on #3136 so we can explore what the ArgMatches API should look like?

My hope is to make it more container-like over time as we work on issues like #2683

@ohsayan ohsayan requested a review from epage January 26, 2022 04:21
epage added a commit to epage/clap that referenced this pull request Feb 1, 2022
In clap2, `ArgMatches.args` was public but hidden.  We made it private
in clap3, giving us more implementation flexibility but many people
relied on it, like to short-circuit defaulting, providing their own
`ArgRequiredElseHelp`, etc.

The main problem was how to expose this
- If we think of `ArgMatches` as a container (a DAG), should we have an
  `is_empty` and what all is included in that, like subcommands?
- If we focus on only args, what term how do we refer to this to convey
  the right intent?

In the end, I realized that this aligns most with our existing
`is_present` check and reporting if args are present fits the best
within the existing API.

I looked into also exposing iterating over the args (`present_arg_ids`)
but we have no way to expose the Id.  The Id is currently private and if
we made it public, it can't be used to access any arg because it can't
implement `Key`.

This supersedes clap-rs#3265
@epage
Copy link
Member

epage commented Feb 1, 2022

How to represent this in the abstraction was my biggest concern (which included understanding how it'd be used). I was exploring how this should be done and created #3382 which is now available in v3.0.14.

@epage epage closed this Feb 1, 2022
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

Successfully merging this pull request may close these issues.

2 participants