Potential Design Direction

[epage]

Looking at [NoAutoHelp](Inspired by https://github.com/clap-rs/clap/pull/2831/commits/1bb038a04fcb713bee03151e859f4ea6ea5cc625)s docs reminds me of a feeling that has been growing in me: that a part of clap's API surface and complexity stems from us allowing built-in machinery to be customized by end-users. I feel like we are hitting a local maxima with the approach we are taking and we need to re-evaluate this to keep clap's API and code base more approachable.

In general, I've been feeling like clap has a composability issue. Instead of new use cases falling out of the API design, we have to do things like support every combination of looking up a value (and we are missing a lot of "grouped" combinations).

I am always cautious of "I came from X, therefore they do it better" but I think we could simplify things with a couple of concepts from Python's argparse:

• Instead of an ArgMatches, it has a dumb Namespace container that is effectively a HashMap<&str, Option<Box<Any>>> and validation is replaced by parsing
  • This gets rid of the redundant work of validation and parsing
  • This gets rid of the need for AllowInvalidUtf8
• argparse.ArgumentParser.add_argument accepts an action parameter to customize what happens when the argument is being processed. We only have support for action="store" with special-cases for --version and --about. What if we allow an action parameter that accepts a fn(value, &App, &mut ArgMatches?
  • While we might have default policy for adding -h and -V, how help and version behave can be exposed as Actions for reuse
  • Other API simplifications are possible, like occurrences being replaced by an append action.
  • This, combined with a dumb Namespace, allows users to do value-grouping without any native support! Of course, we can look into upstreaming these kind of generally useful helpers.

I fully admit this is all hand-waivy and there are a lot of details to work out. We might even be able to prototype this as a parallel to clap with the proposed modularization effort, like splitting out the lexer and creating a general reflection API that completion generation (and maybe even help output) are built on.

What are people's thoughts? This seem worth creating a roadmap to get to this over the next couple of major versions?

[pksunkara]

One of the issues of your proposal (if I understand it correctly) is that all the inter-arg dependencies would be difficult to model. But this is actually a good start to the discussion.

What do you think about parser plugins that actually do stuff while parsing and people can implement their own plugin to get desired effect?

[epage]

What do you think about parser plugins that actually do stuff while parsing and people can implement their own plugin to get desired effect?

Could you give an idea of what you mean?

For example, this idea has several plugins included, state or not stated

• The value parser could be viewed as a plugin (turn ValueX into TypedValueX)
• The action could be viewed as a plugin (the lexer matched ValueX with ArgY, pass ValueX to the ActionY to do with as appropriate, store, append, count, etc)
  • Of course there is some question on how the value parser and value action would interact
• I could also see us supporting stateful validation plugins that have full access to both App and ArgMatches. We could implement ArgGroups and other features in terms of this (though we'd have to evaluate impact on ergonomics).

[pksunkara]

Yeah, that is what I mean. Your original comment didn't expand into the stateful validation plugins which is why I was saying that it wasn't enough.

[kbknapp]

A couple of quick thoughts (Sorry this'll be long 😄 ):

I generally agree that we can (and should) do a better job of saying no to all feature requests and instead build something that can compose better and allow people to use those features to implement their desired functionality. A silly example I use is the original shell completions generating code. I designed and wrote that to a be a starting point for people to generate shell completions. I.e. you generate 90% of your shell completions and then tweak it to get your final script (i.e. iron out any oddities, or fix areas where we do something slightly different from the norm like not defaulting to local file globs on positional arguments*). (Un?)fortunately it worked well enough that people just used it. But since many wanted that last 5-10%, we got feature requests and bug reports to fix/support those edge cases, and sometimes it meant additions to the API in areas we didn't intend or think through fully so it feels bolted on.

argparse and actions

I'm not a super huge fan of Python's argparse (I use it very heavily at work), so I'm trying not to let that taint my response at all because there are things I think we could take from it. The actions @epage is talking about are something I've floated in a few issues although when I spoke about them I had them broken down a little more such as Filters, or Maps (and some other more actiony style closures that I can't remember what I called them). Tangentially related was also the "Partial Parsing" issue which stemmed from wanting to be able to run actions (generate completions, etc.) at the CLI based on flags already passed.

I mostly walked off the action style API (but not from the Filter or Map idea entirely) because I don't like when an argument can spawn significant work or effects (in a functional sense). The validator API comes about as close to that as I was wanting to go, because technically they do anything they want inside that closure except have access to the partially parsed ArgMatches.

A Quick Digression into Partial Parsing

I haven't found a good solution to this request yet, in part because that seems ultra flaky to me, as we'd be handing back a partially formed ArgMatches that has been through no validation, de-duplication (from overrides, conflicts, etc.) (more on this below), etc. for the user to make a decision on. Likewise, this partial ArgMatches could include the "forward" args, or args not yet parsed at that point, but then how do we (or do we even) split up subcommands without having gone through the full ARGV yet? Another route would have been to parse the entire ARGV, do our validation, de-duplication, etc. and rewalk the ARGV handing off the ArgMatches to each Arg that wants to do partial parsing. But that has it's own issues as well.

clap's Validation

In the linked blog post "Parse, Don't Validate" I think clap is dealing with a different layer and problem set. The blog post's thesis is making an assumption about the input early and not having to re-hash said assumption. Specifically it refers to parsing the data as ensuring correctness through datatypes and not "validating" data by post processing. I don't really think that is what clap is doing. CLIs are unbelievably ambiguous and we (clap) can't actually make a determination about whether a CLI is valid or invalid until the entire thing has been seen (parsed).

I'm not aware of any redundant parsing we're doing. If there are areas, I'd be for cleaning those up. But in the Parse, Don't Validate sense I don't believe we're re-hashing past assumptions. (Sans the InvalidUtf8 since that is an outlier that almost certainly has a better way to be done!).

Part of the issue here is I think we're overloading terms. clap's version of "parse" is more like lexing as @epage pointed out. It's validation phase is more like evaluating developer supplied rules. It blurs a little because clap also keeps a squish state machine**** running to decide what type of "token" a particular argument is once it encounters that. That encounter updates the state machine and it moves forward. Once the entire ARGV has been "lexed" clap them applies the developer supplied rules (requirements, conflicts, overrides, etc.) against that tokenized ARGV. That applying of user supplied rules is the "validation" phase.

Where I will concede is that internally clap could possibly use different datatypes to enforce certain developer supplied rules. Maybe. I'm not entirely sure, because in the blog it gives the simple cases like "I want a list that has no duplicates, so instead of writing a validating function to check for duplicates I just a [Set] which disallows duplicates at the datatype level." I'm not sure how we'd apply that same logic to, "Arg X must be used" or "Arg Y cannot be used with Arg Z but if Y is used, the user most also supply Arg A which itself requires at least two, but no more than three values." (or some other complex ruleset).

clap can't fully scan forward in an ARGV. Meaning, it evaluates each argument based on it's current state in the parsing, then relies on a final validation to not only ensure correctness, but also to correct earlier assumptions (like removing overridden arguments). So all the partial parsing proposals thus far have a hard time of deailing with that because clap isn't really sure about anything until after the validation phase. And what good is handing back a partial ArgMatches if it's incorrect.

This phase is also arguably clap's most important phase. As any work you can encode in your CLI and not have to include in your user code is a big benefit. It makes CLIs feel consistent, and we (as clap) can focus on the edge cases that most developers would blast right past because they're rightly focused on their core product, not argument parsing. And as we all know here, argument parsing is actually pretty involved with literally tons of edge cases. We also need to keep certain constraints (more on this in a minute).

This phase could certainly be cleaned up, and I'd welcome that effort. But I think we also need to take into account clap's current API and how to do get from here, to where we want to be? A break the world throw everything away and start fresh while tempting isn't really an option here.

clap's API has grown over the years organically. That means we'll need to do some re-work and trimming but do so gradually. Most of the API surface was added under the constraints of the time that it was added. I.e. not all Rust features that exist today existed then. Likewise, not all best practices known today were known then. Finally, the API wasn't as big then when feature X was added as it is today.

Namespace

An early decision was to not have all args present in some form once parsing completes. I.e. once clap is done parsing and hands back the ArgMatches, that matches struct knows just about nothing about the original CLI.

That's by design. Is it the right design? I still think so, but I'm open to being swayed.

Reason being, for big CLIs I didn't want to hold on to a big chunk of memory, etc. for a small number of args. Take ripgrep for example. It has a bunch of arguments, probably over 100. Yet most invocations use less than a handful of them. Most my invocations of ripgrep probably use two or three at most. So I didn't want ripgrep to get an ArgMatches that knows about and can answer about all 100+ arguments, when it really just needs to hold the info for a few. This not only reduces final memory, but also speeds up checks for things like value_of or is_present. Is this significant or not? It depends.

And doesn't Custom Derive just undo that? Kinda, yeah.*** But at least the user has the option.

Namespaces with user action code

I wasn't super clear on how a namespace combined with an action would allow for grouping of values, unless we mean that the user can make the call to group an argument or not? Or perhaps add to/remove from the final "matches" struct? I think the "danger" is in a partially parsed CLI is essentially a lie. In a hypothetical (yet very realistic...i.e. shell aliases) CLI imagine an option --foo=bar, which can be overridden so the final CLI ARGV is something like myprog --foo=bar --foo=baz, currently clap will erase the first --foo=bar so the final ARGV ends up myprog --foo=baz which is what the final matches struct represents, but how would that get passed as a partially parsed CLI? Because we'd end up passing incorrect info the fn in the form of the first bar value.

There are also the complexities around the user having to handle multiple edge cases or keep some kind of state beyond just the App or PartialMatches because clap needs more context, I'd imagine the user would too. At that point they're essentially just implementing parts of the parsing/lexing themselves which is super error prone.

This was why I mentioned doing a full parse phase, then just re-iterating over the args and allowing a fn to run on each arg that defined one. However, that loses the ability to run arbitrary code in the middle of parsing for some reason (i.e. such as for generating completions). This is also more complex because we'd have to consider all kinds of user niches concerns (I say niche as in not every user will have the same concerns/requirements) like order of the arguments, etc.

Constraints

I've had a few guiding constraints that I've tried to go by while doing anything with clap (in no order):

• Be fast
• Try to be ergonomic
• Try to be intuitive
• Make iteration easy**

I feel that we've mostly kept those. clap has grown a lot, and with that growth definitely came some losses to the intuitive nature. But I still feel for the most post that someone could approach clap with no prior experience and auto-complete/browse two or three structs in the docs and write a functional CLI. They for sure wouldn't be using all the advanced features, and probably wouldn't even be doing everything the most efficiently, but it would be a great starting point that they could iterate on easily.

Something I want to add to that list is I really want to tackle our binary size. I 100% our binary size is justified for the amount of things clap does. I've said it publicly before, but many other argument parsing libraries have a smaller binary size because they leave the majority of the work up to the consumer. Which as noted above is rife with errors sneaking in and making CLIs feel less cohesive. But we still need to find ways to allow those who want no features and a slim binary to be able to use clap too. I believe that means giving every feature request a suspicious eye (which will also help our API growth), and some good design/thought. It also means breaking clap into smaller crates that can be opted into or out of.

Summary

That's a lot of words that can be summed up in:

• Our API has grown (too much?) and I agree with tackling this issue
• I'm not a big fan of "take this action on Arg X appearing" because the user can already do that easily once we complete (i.e. check for Arg X and do the action)
  • With the caveat that I know partial parsing is still desired. I haven't entirely given up on this, but it's a very hard problem in clap's current design...and by design I mean like core design, not just a small implementation detail.
  • I would be interested in exploring a action= like enum/API that would allow us to perhaps clean up other APIs...but I'm unsure of exactly how to accomplish this and if it would ultimately fit well or not
• We can't throw it all away and start over (well we can, but that's called a new crate 😜 ), so we have to deal with what we have now. And where there are major differences between what we have and what we want, we have to incrementally work to get there
• Our validation phase could certainly use some clean up, but not at the expense of requiring the consumer to learn/use a dozen different struct or enum types just to make our job easier

That was way too long and I probably failed to address all the summary points. We can break down these topics further and I can probably address them better individually 😜

---

* aside; at the time that made sense because for a general abstracted CLI we have no idea if local file globs are safe or a good idea for the end utility
** CLIs evolve. Especially in early development. Many times when I start a CLI I start with the "feel" and that takes a while to get down. I wanted clap to not be a barrier to that iteration. I didn't want changes to cascade through the CLI definition.
*** Supporting bitflags for any non-multiple boolean flags would be a cool way to condense down the CLI a little.
**** This is one area I'd like to see some big internal refactoring. To take advantage of an actual state machine.

[epage]

@kbknapp

In the linked blog post "Parse, Don't Validate" I think clap is dealing with a different layer and problem set. The blog post's thesis is making an assumption about the input early and not having to re-hash said assumption. Specifically it refers to parsing the data as ensuring correctness through datatypes and not "validating" data by post processing. I don't really think that is what clap is doing. CLIs are unbelievably ambiguous and we (clap) can't actually make a determination about whether a CLI is valid or invalid until the entire thing has been seen (parsed).

I'm not aware of any redundant parsing we're doing. If there are areas, I'd be for cleaning those up. But in the Parse, Don't Validate sense I don't believe we're re-hashing past assumptions. (Sans the InvalidUtf8 since that is an outlier that almost certainly has a better way to be done!).

With Parser, Don't Validate, I was referring to the user parsing the OsStr we give them back, like what clap_derive does automatically but without the two phases (#2206) . Right now, we will "parse" from an OsString to str (user can opt out). If the user wants to parse a number, clap_derive will add a validator for that number using the parser and then later parse. Looking at the API, it seems this will also cause an extra allocation because Arg::vaidator takes a function that takes String.

This feature would replace and consolidate Arg::validator and Arg::valdiator_os

This would directly let us remove

• The myriad of ArgMatches getters (and we are missing some)
• AllowInvalidUtf8

For the user, this would

• Make it trivial to leverage clap's error reporting machinery (a general weakness of the current API design)
• Remove a small performance hit
• Provide a more discoverable route for accepting non-utf8 paths (we rely purely on docs atm)

[kbknapp]

Gotcah, yeah for the OsStr UTF-8 areas of the code-base I definitely agree with that sentiment. I think the hard part is everyone wants UTF-8...until they don't. And Rust in particular makes working with OsStr values more painful, so we end up using String/str anywhere possible in the API just so it isn't terrible to use for the 99% of people who actually don't care about invalid UTF-8 (even when some of them technically should). So finding a way to expose that flexibility without losing either the ability to handle invalid UTF-8 when required, or making the API generally worse for the majority of people is hard. I think this is a major reason many arg parsers out there opt out of handling invalid UTF-8 period.

[ssokolow]

As someone trying to polish up some reusable validators for release as a crate, there are two parts that jump out at me as sub-optimal there:

1. clap's API may make it necessary to run the same code twice with different APIs, just because validating often is parsing, except that it throws out the result for API reasons. (eg. Is this a valid SocketAddr? Try parsing it and then applying whatever other restrictions you want... just remember that you can't pass the parsed form back in the Ok response.)

2. clap's API presents no way to catch "Validated a path with .validator_os, attempted to access it with .value_of" at compile time, opening programs up to runtime panics.

I'd also argue that intra-argument invariants and inter-argument invariants are categorically different enough that the difficulties with the latter shouldn't impact how the former are handled.

How much of that is something that's open to be changed is a different question, but I think it deserves to be thought about.

[epage]

I think #2683 addresses your concerns and is one of our top priorities after the dust settles with clap v3 (whether at release of 3.0 or up to a 3.1, depending on how critical we view some work deferred out of 3.0).

[ssokolow]

Hmm. My main concern there is that this line looks like it could easily become a footgun if not handled carefully:

Change the default parser from OsString to String, deprecating AllowInvalidUtf8, gated behind a feature flag.

Windows may make it difficult to get paths with un-paired surrogates, but I've found mojibake'd filenames to be easy on POSIX platforms... to the point where it led me to report a bug in Serde because a mojibake'd Japanese ID3 tag had put mojibake'd characters in media file names during auto-renaming and Serde doesn't complain if you want to put a PathBuf into a JSON file without explicitly specifying how to handle un-UTF8-able contents. (Was it ID3v1 that left the tag's encoding to be communicated out-of-band? I think Audacious Media Player actually has a browser-style encoding detector for that.)

[epage]

Replied over at #2683 to keep conversation related to that in one place.

[epage]

@kbknapp

I mostly walked off the action style API (but not from the Filter or Map idea entirely) because I don't like when an argument can spawn significant work or effects (in a functional sense). The validator API comes about as close to that as I was wanting to go, because technically they do anything they want inside that closure except have access to the partially parsed ArgMatches.
...
I'm not a big fan of "take this action on Arg X appearing" because the user can already do that easily once we complete (i.e. check for Arg X and do the action)

I would be interested in exploring a action= like enum/API that would allow us to perhaps clean up other APIs...but I'm unsure of exactly how to accomplish this and if it would ultimately fit well or not

My intention with action is to allow pluggable storage mechanism (Python's isn't pluggable).

Map I see being more related to the Parse, Don't Validate thread.

The most processing I had in mind was --version and --help. I know users can then abuse it to do whatever they want. We can encourage this, somewhat, by making the easy path a clearer path, like say the Action trait has a blanket impl on fn(App, &mut Entry) -> Result so it only can mutate the relevant entry.

Maybe we start off with just making actions an Enum to gradually explore the API design. Even an Enum would give us an advantages of filtering the available API to browse down to what is relevant for the current action, making it more discoverable (a form of progressive disclosure)

[kbknapp]

Yeah I'd be up to exploring this idea either in an unstable feature gate (in case we decide to yank it) or a related crate that we can explore bounce ideas around in.