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

Default help grouping is confusing and unwanted #2807

Closed
epage opened this issue Oct 4, 2021 · 11 comments · Fixed by #2867
Closed

Default help grouping is confusing and unwanted #2807

epage opened this issue Oct 4, 2021 · 11 comments · Fixed by #2867
Labels
A-help Area: documentation, including docs.rs, readme, examples, etc... C-enhancement Category: Raise on the bar on expectations M-breaking-change Meta: Implementing or merging this will introduce a breaking change.
Milestone

Comments

@epage
Copy link
Member

epage commented Oct 4, 2021

Rust Version

1.55.0

Affected Version of clap

v3.0.0-beta.4

Expected Behavior Summary

Args would be in a shared group by default

Actual Behavior Summary

Args are split up by their type (flags, arguments, positional)

Context

UnifiedHelpMessage is a very popular flag to turn on. When further researching this, only tools using clap's defaults have this styling.

One thing to keep in mind is ideally people can use help_heading out of the gate (see #2847). This is why I recommend we remove UnifiedHelpMessage and the default help headings, giving us the behavior of UnifiedHelpMessage while also allowing help_heading to be used by default.

Going into more detail for why we should unify the approaches:

Problems with the default

  • It doesn't match the users workflow. It focuses on how the flag is used (bool or enum) and not the users task. help_heading and DeriveDisplayOrder are other examples of organizing --help around the users task. Even sorting alphabetically is an okay-ish fallback when explicit organization falls flat because someone can scan for arguments that start with a letter for a word related to their task ("I need to see files, let's scan for --f"), see rg (which fails frequently for me...). Once I find the argument, I then care about how to use it (did they use a flag or an option with an enum and what are the values).
    • Someone might say "isn't this also true for argument and subcommand" but it isn't. Generally, when using positional arguments or subcommands, all flags/options are subordinate to them and having them separate-by-default goes alongwith the task-oriented organization.
  • It is frequently disabled and not following what people want with a default requires people be "in the known" and go through the effort of copy/pasting all of the non-defaults they set between apps they write (and frequently miss this and leave a less than desired experience)
  • The distinction between "flag" and "option" are confusing because
    • People tend to be imprecise in their language, muddying definitions
    • Sometimes something is both like --color[=<when>]

Problems with UnifiedHelpMessage

  • The name is a misnomer; it only applies to Flags vs Options. It had been a while since I've dealt with my own tools help that I assumed this would also include Arguments and thought clap3 had a bug because it didn't.
    • Yes, this is "solved" by documentation but that only helps when people read it which they are unlikely to do when the name already documents the behavior
  • It can't be used with help_heading (Using help_heading in one place requires using it on all Flags to get a Unified experience #2847) and making it do so makes the name even more of a misnomer
  • At least for me, its difficult to come up with an accurate name that encompasses current behavior or if we add help_heading which suggests something is off about the feature.
@epage epage added C-bug Category: Updating dependencies M-breaking-change Meta: Implementing or merging this will introduce a breaking change. A-help Area: documentation, including docs.rs, readme, examples, etc... D: medium labels Oct 4, 2021
@epage epage added this to the 4.0 milestone Oct 4, 2021
@pksunkara
Copy link
Member

I agree with this being v4. I have a few opinions on this and especially modernizing the help message formats but don't want to focus on it for v3.

@pksunkara pksunkara added C-enhancement Category: Raise on the bar on expectations and removed C-bug Category: Updating dependencies labels Oct 9, 2021
@epage
Copy link
Member Author

epage commented Oct 9, 2021

I have a few opinions on this and especially modernizing the help message formats

Please share these thoughts somewhere so we can all be ruminating on them.

@epage
Copy link
Member Author

epage commented Oct 10, 2021

In thinking about this, I think my main problem is with the splitting of Options and Flags. If those were combined, I probably wouldn't care or notice a problem.

EDIT: nevermind, that is what Unified is. I forgot it doesn't impact positional arguments.

@kbknapp
Copy link
Member

kbknapp commented Oct 10, 2021

clap history time; the original splitting of flags and options was based on a
bunch of tools I used at work who's usage string was something like program [flags] [options] [args] to distinguish boolean settings (flags), setting values
(options), and free arguments. Looking back I don't actually remember if their
help message also split the groups or not. That was a distinction that made
sense to me, and originally made the parser more simple because of assumptions
that could be made. Most of those have since gone away or been abstracted over
so simplifying parsing isn't really an argument for or against any longer.

And I've had quite a few consumers ping me about what is the difference between
flags and options or being confused by that distinction. Turns out there isn't
standard nomenclature for these things. Some place flags are booleans, some
places flags are anything that starts with -. Other places switches are
booleans, and still in others switches are just the -foo part of the option.

Having said all that I actually don't have strong feelings about this issue
either way. I can appreciate those wanting to make UnifiedHelpMessage the
default, and I can also appreciate those that prefer the current default.

No matter what direction we ultimately decide on, I believe a setting should be
provided for the opposite preference just as a backwards "compatibility"
option. If we change the default, I'd like to provide an option for the old
behavior. Likewise, if we keep the current default we're obviously not
considering removing UnifiedHelpMessage.

My gut says I think the least churn option is the best for the short term just
to get v3 going. Being that we have a decent amount of polish left for v3 and
want to get v3 out as soon as possible. This would mean keep the current
default, gather more usage of help_heading and worth through any issues it
has with unified help while we flesh out those details. If we want to change
the default, do it for v4. I say this because I don't plan on making 3.x
another 2.x where we wait 5 years. Additionally, all the high-profile CLIs
using clap are already using the settings they want. It's new users we don't
want to confuse. Perhaps this can be done through documentation (i.e. calling
out unified help or help headings more frequently or more rapidly into the
quick start).

@epage
Copy link
Member Author

epage commented Oct 11, 2021

FYI I've updated the Issue with a more detailed description of why the current dichotomy falls flat:

Problems with the default

  • It doesn't match the users workflow. It focuses on how the flag is used (bool or enum) and not the users task. help_heading and DeriveDisplayOrder are other examples of organizing --help around the users task. Even sorting alphabetically is an okay-ish fallback when explicit organization falls flat because someone can scan for arguments that start with a letter for a word related to their task ("I need to see files, let's scan for --f"), see rg (which fails frequently for me...). Once I find the argument, I then care about how to use it (did they use a flag or an option with an enum and what are the values).
    • Someone might say "isn't this also true for argument and subcommand" but it isn't. Generally, when using positional arguments or subcommands, all flags/options are subordinate to them and having them separate-by-default goes alongwith the task-oriented organization.
  • It is frequently disabled and not following what people want with a default requires people be "in the known" and go through the effort of copy/pasting all of the non-defaults they set between apps they write (and frequently miss this and leave a less than desired experience)
  • The distinction between "flag" and "option" are confusing because
    • People tend to be imprecise in their language, muddying definitions
    • Sometimes something is both like --color[=<when>]

Problems with UnifiedHelpMessage

  • The name is a misnomer; it only applies to Flags vs Options. It had been a while since I've dealt with my own tools help that I assumed this would also include Arguments and thought clap3 had a bug because it didn't.
    • Yes, this is "solved" by documentation but that only helps when people read it which they are unlikely to do when the name already documents the behavior
  • It can't be used with help_heading (Using help_heading in one place requires using it on all Flags to get a Unified experience #2847) and making it do so makes the name even more of a misnomer
    • At least for me, its difficult to come up with an accurate name that encompasses current behavior or if we add help_heading which suggests something is off about the feature.

@kbknapp

Having said all that I actually don't have strong feelings about this issue
either way. I can appreciate those wanting to make UnifiedHelpMessage the
default, and I can also appreciate those that prefer the current default.

No matter what direction we ultimately decide on, I believe a setting should be
provided for the opposite preference just as a backwards "compatibility"
option. If we change the default, I'd like to provide an option for the old
behavior. Likewise, if we keep the current default we're obviously not
considering removing UnifiedHelpMessage.

My concerns for keeping the feature around, even if off-by-default:

  • Can we give it a meaningful name? As I called out above, the current name has some shortcomings, to say the least
  • Is it worth the cost, both in terms of code to maintain and the user cognitive load of our sprawling Settings documentation compared to the cost of the users who want it to recreate it with help_heading?

Personally, I've only seen one person say they personally prefer it. It'd help me a lot to feel this is worth it if I understood why. How does this help? What problem do they feel it solves?

. This would mean keep the current
default, gather more usage of help_heading and worth through any issues it
has with unified help while we flesh out those details

I've gone ahead and created #2847 to describe the issue. The challenge is the straightforward option (add support for help_heading to UnifiedHelpMessage) makes the name more of a misnomer, making its use even less clear to users.

Maybe this less-than-ideal middle road is good enough for 3.0 and we fix this in 4.0.

Additionally, all the high-profile CLIs
using clap are already using the settings they want. It's new users we don't
want to confuse

Perhaps this can be done through documentation (i.e. calling
out unified help or help headings more frequently or more rapidly into the
quick start).

I frequently forget what my incantation of clap settings are and end up "regressing" when creating a new CLI. As I'm not a new user, I'm also less likely to look through the "basic" docs to remember.

In addition, my concern with solving through docs:

  • The more we put in, the less useful they become because its overwhelming
  • The API design is the primary form of documentation people rely on and are unlikely to dig through enough detail in the docs (maybe if there is a serious enough of a "bug").

@pksunkara
Copy link
Member

The name is a misnomer; it only applies to Flags vs Options.

I 100% agree with the name being a misnomer.

@BurntSushi
Copy link
Contributor

I don't really have any skin in this since I can disable it, but I'll add my voice and say that as someone who has been using CLI tools for a very long time, the dichotomy that Clap was drawing between "options" and "flags" made zero sense to me. In fact, it confused me. I had to go and look up what was meant, since "option" and "flag" are synonyms in my head. I do not think the current grouping should be the default in v3. Even if I understood the distinction being drawn, it's hard for me to imagine how it is a useful one. And I can't think of any tool that I use off the top of my head that draws this distinction in their help text. (And if it did, it would annoy me, because it doesn't seem like a useful way of categorizing flags from an end user's perspective.)

@kbknapp
Copy link
Member

kbknapp commented Oct 11, 2021

the dichotomy that Clap was drawing between "options" and "flags" made zero sense to me. In fact, it confused me

Yeah this is the sentiment I was referring to, I've had more than a few people express similar thoughts. Even if the dichotomy made sense to me at inception, like I mentioned many of the benefits and reasons behind the original splitting have gone away over the years.

Having ruminated on this for a bit more time I 100% agree the name is a misnomer. I would be all on board for a re-name, however doing so seems like nearly as much churn than just making this the default setting and removing all complexities around it.

Therefore I think we go with making it the default behavior and removing this setting/complexities around the split with the assumption that help_heading works well enough to allow those to who wish to keep the old behavior to do so (even if somewhat manually). I would like to mention how to get the old behavior back through help_heading in whatever v2->v3 change document we publish/deprecation message.

This might turn out to be a decent undertaking at least in src/output/help.rs, so I'll suggest we keep the current behavior and deprecate for v3 and make the change in v4. Meaning this would stay in the v4 milestone. However I'm also not in charge of anyone's time, so if someone tackles this prior to v3 release I'm OK including it there as well.

@pksunkara pksunkara self-assigned this Oct 11, 2021
@pksunkara
Copy link
Member

Therefore I think we go with making it the default behavior

Agreed.

I would like to mention how to get the old behavior back through help_heading in whatever v2->v3 change document we publish/deprecation message.

Instead of that, let's keep the current behaviour under the inverse setting UseAutoHelpHeading. The current behaviour is as follows:

  • Boolean args are grouped as FLAGS
  • Option args are grouped as OPTIONS

Later on after 3.0, I would want to add more groups to this setting like boolean toggle args, subcommand flag args etc...

@epage
Copy link
Member Author

epage commented Oct 12, 2021

If people can use help_heading to get the same behavior, how worth it is it for us to support a UseAutoHelpHeading? I suspect the group is small enough that we shouldn't maintain the extra implementation and API complexity.

If we do want a UseAutoHelpHeading, I don't think we should do it yet. There is enough stuff to figure out, like a clear name, that we shouldn't distract ourselves with it at this current time. For example, the suggestion was originally UseSmartHelpHeading but I felt that suggested a level of endorsement that didn't seem appropriate. That name and the new UseAutoHelpHeadingmake it sound like it trumpshelp_heading`. We can iterate on names and figure out what is good enough but I feel like that'll detract us from moving forward with everything else.

@pksunkara
Copy link
Member

If people can use help_heading to get the same behavior, how worth it is it for us to support a UseAutoHelpHeading?

Can't we say that with the default help too since we provide template? It's all about easier way of doing it instead of asking people to do extra work. Here they would need to categorise their args one by one.

I suspect the group is small enough that we shouldn't maintain the extra implementation and API complexity.

You can't say that because it is currently a default behavior. In fact there were a few people who created issues around this behavior because they want it improved.

If we do want a UseAutoHelpHeading, I don't think we should do it yet. There is enough stuff to figure out, like a clear name, that we shouldn't distract ourselves with it at this current time.

Feel free to suggest a name. If we should not distract ourselves with this, let's do this as part of v4 then as @kbknapp proposed.

That name and the new UseAutoHelpHeading make it sound like it trumps help_heading.

It would work with help_heading as that is the current behavior. I don't see anything in the name that contradicts that.

We can iterate on names and figure out what is good enough but I feel like that'll detract us from moving forward with everything else.

This is about inverting the option and making the current behavior still be available to users. If we can't do that right now, let's fix this issue in v4 then. I don't mind. All I am asking is not to remove the current behavior.

epage added a commit to epage/clap that referenced this issue Oct 12, 2021
For those that want the original behavior, you can usxe
`arg.help_heading(Some("FLAGS"))` on your flags.  Limitations:
- This will not give you a special sort order
- This will not get a `[FLAGS]` added to usage

For templates, we removed `{unified}` and `{flags}`.  To help people
catch these, a debug_assert was added.

I'm unsure but I think there might be a change in behavior in calcuating
when to show `[OPTION]` in usage.  The old code only looked at
`required` while flags looked only at arg groups.  We now look at both.

Ideally we'd add these in `_build` and remove special casing for
no-groups except in the sort order of groups.  I feel like thats best
left for later.

This also reduced the scope of `App`s public API.
`get_*_with_no_heading` seemed a bit specialized to be in the public
API.  clap-rs#2853 looks at splitting it out into its own PR.

BREAKING CHANGE: Multiple
- `UnifiedHelpMessage` removed
- `{flags}` and `{unified}` are removed and will assert when present.
- `get_*_with_no_heading` removed

Fixes clap-rs#2807
epage added a commit to epage/clap that referenced this issue Oct 12, 2021
For those that want the original behavior, you can usxe
`arg.help_heading(Some("FLAGS"))` on your flags.  Limitations:
- This will not give you a special sort order
- This will not get a `[FLAGS]` added to usage

For templates, we removed `{unified}` and `{flags}`.  To help people
catch these, a debug_assert was added.

I'm unsure but I think there might be a change in behavior in calcuating
when to show `[OPTION]` in usage.  The old code only looked at
`required` while flags looked only at arg groups.  We now look at both.

Ideally we'd add these in `_build` and remove special casing for
no-groups except in the sort order of groups.  I feel like thats best
left for later.

This also reduced the scope of `App`s public API.
`get_*_with_no_heading` seemed a bit specialized to be in the public
API.  clap-rs#2853 looks at splitting it out into its own PR.

BREAKING CHANGE: Multiple
- `UnifiedHelpMessage` removed
- `{flags}` and `{unified}` are removed and will assert when present.
- `get_*_with_no_heading` removed

Fixes clap-rs#2807
@pksunkara pksunkara modified the milestones: 4.0, 3.0 Oct 13, 2021
epage added a commit to epage/clap that referenced this issue Oct 13, 2021
For those that want the original behavior, you can usxe
`arg.help_heading(Some("FLAGS"))` on your flags.  Limitations:
- This will not give you a special sort order
- This will not get a `[FLAGS]` added to usage

For templates, we removed `{unified}` and `{flags}`.  To help people
catch these, a debug_assert was added.

I'm unsure but I think there might be a change in behavior in calcuating
when to show `[OPTION]` in usage.  The old code only looked at
`required` while flags looked only at arg groups.  We now look at both.

Ideally we'd add these in `_build` and remove special casing for
no-groups except in the sort order of groups.  I feel like thats best
left for later.

This also reduced the scope of `App`s public API.
`get_*_with_no_heading` seemed a bit specialized to be in the public
API.  clap-rs#2853 looks at splitting it out into its own PR.

BREAKING CHANGE: Multiple
- `UnifiedHelpMessage` removed
- `{flags}` and `{unified}` are removed and will assert when present.
- `get_*_with_no_heading` removed

Fixes clap-rs#2807
epage added a commit to epage/clap that referenced this issue Oct 13, 2021
For those that want the original behavior, you can usxe
`arg.help_heading(Some("FLAGS"))` on your flags.  Limitations:
- This will not give you a special sort order
- This will not get a `[FLAGS]` added to usage

For templates, we removed `{unified}` and `{flags}`.  To help people
catch these, a debug_assert was added.

I'm unsure but I think there might be a change in behavior in calcuating
when to show `[OPTION]` in usage.  The old code only looked at
`required` while flags looked only at arg groups.  We now look at both.

Ideally we'd add these in `_build` and remove special casing for
no-groups except in the sort order of groups.  I feel like thats best
left for later.

This also reduced the scope of `App`s public API.
`get_*_with_no_heading` seemed a bit specialized to be in the public
API.  clap-rs#2853 looks at splitting it out into its own PR.

BREAKING CHANGE: Multiple
- `UnifiedHelpMessage` removed
- `{flags}` and `{unified}` are removed and will assert when present.
- `get_*_with_no_heading` removed

Fixes clap-rs#2807
epage added a commit to epage/clap that referenced this issue Oct 13, 2021
For those that want the original behavior, you can usxe
`arg.help_heading(Some("FLAGS"))` on your flags.  Limitations:
- This will not give you a special sort order
- This will not get a `[FLAGS]` added to usage

For templates, we removed `{unified}` and `{flags}`.  To help people
catch these, a debug_assert was added.

I'm unsure but I think there might be a change in behavior in calcuating
when to show `[OPTION]` in usage.  The old code only looked at
`required` while flags looked only at arg groups.  We now look at both.

Ideally we'd add these in `_build` and remove special casing for
no-groups except in the sort order of groups.  I feel like thats best
left for later.

This also reduced the scope of `App`s public API.
`get_*_with_no_heading` seemed a bit specialized to be in the public
API.  clap-rs#2853 looks at splitting it out into its own PR.

BREAKING CHANGE: Multiple
- `UnifiedHelpMessage` removed
- `{flags}` and `{unified}` are removed and will assert when present.
- `get_*_with_no_heading` removed

Fixes clap-rs#2807
epage added a commit to epage/clap that referenced this issue Oct 13, 2021
For those that want the original behavior, you can usxe
`arg.help_heading(Some("FLAGS"))` on your flags.  Limitations:
- This will not give you a special sort order
- This will not get a `[FLAGS]` added to usage

For templates, we removed `{unified}` and `{flags}`.  To help people
catch these, a debug_assert was added.

I'm unsure but I think there might be a change in behavior in calcuating
when to show `[OPTION]` in usage.  The old code only looked at
`required` while flags looked only at arg groups.  We now look at both.

Ideally we'd add these in `_build` and remove special casing for
no-groups except in the sort order of groups.  I feel like thats best
left for later.

This also reduced the scope of `App`s public API.
`get_*_with_no_heading` seemed a bit specialized to be in the public
API.  clap-rs#2853 looks at splitting it out into its own PR.

BREAKING CHANGE: Multiple
- `UnifiedHelpMessage` removed
- `{flags}` and `{unified}` are removed and will assert when present.
- `get_*_with_no_heading` removed

Fixes clap-rs#2807
@bors bors bot closed this as completed in 61c9e62 Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-help Area: documentation, including docs.rs, readme, examples, etc... C-enhancement Category: Raise on the bar on expectations M-breaking-change Meta: Implementing or merging this will introduce a breaking change.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants