Commit graph

69 commits

Author SHA1 Message Date
Ed Page
83d6add9aa fix(help): Shift focus to subcommands, when present
In surveying various tools and CLI parsers, I noticed they list the
subcommands first.  This puts an emphasis on them which makes sense
because that is most likely what an end user is supposed to pass in
next.

Listing them last aligns with the usage order but it probably doesn't
outweigh the value of getting a user moving forward.
2022-08-26 10:59:40 -05:00
Ed Page
9b23a09f7a fix(help): Don't rely on ALL CAPS for headers
I see them fulfilling two roles
- A form of bolding
- As a callback to their placeholder in usage

However, it is a bit of an unpolished look and no other CLI seems to do
it.  This looks a bit more proefessional.  We have colored help for
formatting and I think the sections relation to usage will be clear
enough.
2022-08-26 10:21:18 -05:00
Ed Page
df7616b820 fix(help): Match v3 usage for optional positionals 2022-08-25 13:13:09 -05:00
Ed Page
1bbf07e574 fix: Prefer more usable &str for reflection
The downside is we can't skip allocations with
- `ValueEnum` default values
- Vaid subcommands

Otherwise, this is a big ergonomic win.
2022-08-22 14:56:16 -05:00
Ed Page
85f541d789 fix: Switch to owned types
Impact:
- Binary size: 556.6 KiB to 578.4 KiB
- build time: 6.4950 us (7% slower)
- parse time: 7.7256 us
- parse sc time: 8.1580 us (5% faster)

Fixes #1041
Fixes #2150
2022-08-22 14:55:55 -05:00
Ed Page
2de59195aa fix!: Prefer IntoIterator over &[]
The main breakinge change cases:
- `&[char]`: now requires removing `&`
- All other non-ID `&[_]`: hopefully #1041 will make these non-breaking

Fixes #2870
2022-08-15 13:26:17 -05:00
Ed Page
5b5f2c1f40 fix!: Track original Ids, rather than a hash
This is a step towards #1041
- `ArgGroup` no longer takes a lifetime
- One less field type needs a lifetime

For now, we are using a more brute force type (`String`) so we can
establish performance base lines.  I was torn on whether to use `&str`
everywhere or make an `IdRef`.  The latter would add a lot of noise that
I'm concerned about, so i left it simple for now.  `IdRef` would help to
communicate the types involved though.

Speaking of communicating types, I'm also torn on whether we should use
`Id` for all strings or if we should have `Id`, `Name`, etc types to
avoid people mixing and matching.

This added 18.7 KB.

Compared to `HEAD~` on `06_rustup`:
- build: 6.23us -> 7.41us
- parse: 8.17us -> 9.36us
- parse_sc: 7.65us -> 9.29us
2022-08-12 15:46:04 -05:00
Ed Page
f84e38a4de fix!: Switch from &[] to IntoIterator
This is a part of #2870 and is prep for #1041

Oddly enough, this dropped the binary size by 200 Bytes

Compared to `HEAD~` on `06_rustup`:
- build: 6.21us -> 6.23us
- parse: 7.55us -> 8.17us
- parse_sc: 7.95us -> 7.65us
2022-08-12 15:45:02 -05:00
Ed Page
07b6e66eb7 fix: No implicit version/help actions
Documenting the existing behavior is challenging which suggests it can
cause user confusion.  So long as its not too hard to explicitly
specify actions, we should just do it.

Fixes #4057
2022-08-10 21:21:53 -05:00
Ed Page
f70ebe89a7 fix!: Require explicit help/version disabling
Before we introduced actions, it required specific setups to engage with
claps version and help printing.  With actions making that more
explicit, we don't get as much benefit from our multiple, obscure, ways
of users customizing help

Before
- Modify existing help or version with `mut_arg` which would
  automatically be pushed down the command tree like `global(true)`
- Create an new help or version and have it treated as if it was the
  built-in on (I think)
- Use the same flags as built-in and have the built-in flags
  automatically disabled
- Users could explicitly disable the built-in functionality and do what
  they want

Now
- `mut_arg` no longer works as we define help and version flags at the
  end
- If someone defines a flag that overlaps with the built-ins by id,
  long, or short, a debug assert will tell them to explicitly disable
  the built-in
- Any customization has to be done by a user providing their own.  To
  propagate through the command tree, they need to set `global(true)`.

Benefits
- Hopefully, this makes it less confusing on how to override help
  behavior.  Someone creates an arg and we then tell them how to disable
  the built-in
- This greatly simplifies the arg handling by pushing more
  responsibility onto the developer in what are hopefully just corner
  cases
- This removes about 1Kb from .text

Fixes #3405
Fixes #4033
2022-08-10 20:33:21 -05:00
Ed Page
c801e4e56e fix!: Make ArgAction::Set the default
This removes the need for `TakesValue` bookkeeping for when we knew we
took values but didn't know how many we should take.

Fixes #2687
2022-08-05 13:31:33 -05:00
Ed Page
c62d3f0cfd fix!: Replace takes_value with number_of_values 2022-08-04 09:39:25 -05:00
Ed Page
29753b6798 fix: Remove references to number_of_values 2022-08-04 09:35:59 -05:00
Ed Page
85ad452c9b fix!: Remove Arg::rwquire_value_delimiter
In clap v3, `require_value_delimiter` activated an alternative parse
mode where
- `multiple_values` meant "multiple values within a single arg"
- `number_of_values` having no parse impact, only validation impact
- `value_names` being delimited values

For unbounded `number_of_values`, this is exactly what `value_delimiter`
provides.  The only value is if someone wanted `value_name` to be
`<file1>,<file2>,...` which can be useful and we might look into adding
back in.

Alternatively, this could be used for cases like key-value pairs but
that has issues like not allowing the delimiter in the value which might
be ok in some cases but not others.  We already instead document that
people should instead use `ValueParser` for this case.

In removing this, we remove points of confusion at how the different
multiple values and delimited value calls interact with each other.  I
know I would set `require_value_delimiter(true).multiple_values(true)`
when it turns out all I needed was `value_delimiter(',')`.

This also reduces the API surface area which makes it easier to discover
what features we do provide.

While this isn't big, this is also yet another small step towards
reducing binary size and compile times.
2022-08-03 21:26:36 -05:00
Ed Page
ba15b5f430 fix!: Rename Arg::number_of_values to Arg::num_args 2022-08-03 14:45:47 -05:00
Ed Page
0664c6db37 fix!: Remove Arg::use_value_delimiter in favor of Arg::value_delimiter 2022-08-03 11:15:29 -05:00
Ed Page
470531b515 fix!: Replace multiple_values with number_of_values
This reduces ambiguity in how the different "multiple" parts of the API
interact and lowrs the amount of API surface area users have to dig
through to use clap.

For now, this is only a matter of cleaning up the public API.  Cleaning
up the implementation is the next step.
2022-08-01 15:50:23 -05:00
Ed Page
76bff6f34e fix!: Remove rest of deprecated APIs
Fixes #4009
2022-08-01 15:21:33 -05:00
Ed Page
81bc351cfc fix(help): Show when a flag 'ArgAction::Count's 2022-07-29 09:56:26 -05:00
Ed Page
ac32c831fc fix(help): Remove '...' for optional values 2022-07-29 09:44:29 -05:00
Ed Page
ccf35ff70c fix!: Replace min_values (total) with number_of_values (per occurrence) 2022-07-28 17:13:41 -05:00
Ed Page
41535d5c46 feat: Extend number_of_values to support min/max per occurrence 2022-07-28 16:52:25 -05:00
Ed Page
355a8ff90c fix!: number_of_values doesn't always imply multiple_values(true)
With `number_of_values` being per-occurrence now, its doesn't make sense
for `number_of_values(0)` to set `takes_value(true)` or for
`number_of_values(1)` to set `multiple_values(true)`.

In addition, an assert is made if the user works around this
2022-07-28 14:40:58 -05:00
Ed Page
5f20fe1930 docs: Shift focus from takes_value to actions 2022-07-26 14:50:51 -05:00
Ed Page
11076a5c70 fix(help)!: Make DeriveDisplayOrder the default, removing it
Force sorting with `next_display_order(None)`

Fixes #2808
2022-07-22 15:52:03 -05:00
Ed Page
389ff4ff21 fix(help): Subcommand display order respects Command::next_display_order
Previous behavior:
- They'd be sorted by default
- They'd derive display order if `DeriveDisplayOrder` was set
  - This could be set recursively
- The initial display order value for subcommands was 0

New behavior:
- Sorted order is derived by default
- Sorting is turned on by `cmd.next_display_order(None)`
  - This is not recursive, it must be set on each level
- The display order incrementing is mixed with arguments
  - This does make it slightly more difficult to predict
2022-07-22 15:03:16 -05:00
Ed Page
36dcb05d96 fix!: Change arg! to use ArgAction
Fixes #3795
2022-07-22 13:24:40 -05:00
Ed Page
0d459128d7 fix(error)!: Merge UnrecognizedSubcommand into InvalidSubcommand
Fixes #3676
2022-07-22 12:12:35 -05:00
Ed Page
01a3ea425f fix!: Remove unstable-v4 feature gate 2022-07-22 11:34:06 -05:00
Ed Page
9caec5a52d fix(parser): Deprecate ArgMatches::is_present 2022-06-10 14:21:25 -05:00
Ed Page
10bb9abb1a fix(assert): Reference help_expected 2022-06-08 12:06:15 -05:00
Ed Page
14a62e11fd fix(parser): Deprecate multiple_occurrences
Fixes #3772
2022-06-08 09:54:23 -05:00
Ed Page
4a9c4dee64 refactor(test): Make it easier to fork tests 2022-06-07 16:21:12 -05:00
Ed Page
a712adefcd fix(parser): Deprecate value_of and friends 2022-05-25 12:57:11 -05:00
Ed Page
256643f8d3 fix: Deprecate possible_values 2022-05-25 12:57:11 -05:00
Ed Page
089f96eb87 fix(help): Disallow too many value names (unstable)
I can't think of a case for this or a way to render it.

Fixes #2695
2022-05-06 13:54:07 -05:00
Ed Page
70c29e3dd8 fix(help): Use '...' when not enough value names supplied 2022-05-06 13:33:17 -05:00
Ed Page
16791ab0a7 test(help): Update for display name change 2022-05-04 20:46:49 -05:00
Ed Page
c7ff695581 fix(help): Don't wrap URLs
Confusingly, there are two similar but slightly different settings in
`textwrap`.  We need both set.

Fixes #3222
2022-05-04 16:00:08 -05:00
Ed Page
3cfea3223b fix(help): Offer a html_template variable for display name
This is a step towards #992.  When help renders the application name, it
uses the `bin` template variable which is just the `bin` name with
spaces converted to ` `.  While having `app.exe sub` makes sense,
`app.exe-sub` does not.

To get around needing this for usage, we've created a `display_name`
field that is fairly similar but
- The root name is the `name` and not `bin_name`
- We always join with `-`

This means that the derived `bin_name` will only show up in usage.

For now, the default template has not been updated as that is a minor
compatibility change and should be in a minor release, at least.  I was
worried this would be a full breaking change.  The main case I was
worried about was cargo subcommands but our tests show they should just
work.
2022-05-03 14:34:47 -05:00
Ed Page
ce727f1951 fix(error): Render actual usage for unrecognized subcommands
For some reason this code path diverged and manually constructed a
usage, not following any of our patterns for doing so.
2022-05-02 09:25:44 -05:00
Ed Page
ccf8634c19 fix(help): Provide correct context for help subcmd errors 2022-04-29 20:25:05 -05:00
Ed Page
1452c1e42b fix(help): Help subcommands usage includes requires arguments 2022-04-29 20:16:19 -05:00
Ed Page
37ebb71806 fix(help): write_help usage includes required arguments 2022-04-29 20:15:54 -05:00
Ed Page
2edc1a2bdf test(help): Verify showing of required attributes 2022-04-29 15:46:15 -05:00
Ed Page
7cdce9cabe refactor(test): Use snapbox's diffs 2022-04-29 15:32:25 -05:00
Ed Page
8f182067e3 feat(clap): Publicly expose Command::build
`Command::_build_all` started as an internal function for
`clap_complete` as a stopgap until #2911.  Overtime, we've been finding
more cases where this function needs to be called, so now we're going to
fully embrace it until #2911 so people aren't scrared off by the hidden
implementation from using it.

This was inspired by #3602

Comptibility: Though this adds a deprecation which we general reserve
for minor or major versions, this is enough of a corner case that I'm
fine doing this in a patch release.
2022-04-19 10:13:43 -05:00
Roland Fredenhagen
33949ce4de
feat(help): Show PossibleValue help in --help (#3503)
`-h` (short help) still shows the same.

This gates it behind an `unstable-v4` feature flag to avoid disrupting users who set the help without knowing where all it shows up (particularly derive users where `ArgEnum` is automatically extracting the help).

Fixes #3312
2022-03-02 09:15:28 -06:00
Ed Page
e8010e79a9 refactor: Update app variables to cmd 2022-02-14 15:55:56 -06:00
Ed Page
c4144d7d6c docs: Update App references to Command 2022-02-14 15:33:49 -06:00