Skip to content

Conversation

@Fiedzia
Copy link

@Fiedzia Fiedzia commented Nov 14, 2024

Group subcommands displayed in help (#1553)
This is for help only, does not affect matching.

I've done the builder part, still figuring out derive support
(ah - it works automatically by setting subcommand_help_header=Some("foo"). I'd probably should just document it somewhere).

This is done the same way grouping args works: subcommand has optional subcommand_help_heading.
If provided, it will be used to group commands visually when displaying subcommand help.

so that we can go from

Commands:
cmd1 ...
cmd2 ...
help ...

to

Commands:
help ...

Utility commands:
cmd1 ...
cmd2 ....

Nothing changes if groups aren't used. See tests for examples.

Issues:
having subcommand_heading and subcommand_help_heading may be confusing, maybe naming should change.

@epage epage changed the title Command groups #1553 Add Subcommand help headings Nov 15, 2024
/// [`Command::subcommand_help_heading`]: crate::Command::subcommand_help_heading()
#[inline]
#[must_use]
pub fn next_subcommand_help_heading(mut self, heading: impl IntoResettable<Str>) -> Self {
Copy link
Member

Choose a reason for hiding this comment

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

This will also need a way to set the help heading for the current command

Copy link
Member

Choose a reason for hiding this comment

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

This will likely run into name conflicts with subcommand_help_heading. We should probably deprecate that and migrate people over to setting next_subcommand_help_heading.

/// [`Command::subcommand_help_heading`]: crate::Command::subcommand_help_heading()
#[inline]
#[must_use]
pub fn next_subcommand_help_heading(mut self, heading: impl IntoResettable<Str>) -> Self {
Copy link
Member

Choose a reason for hiding this comment

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

The derive has special handling for regular help headings, so we should probably have the same for these

/// [`Command::subcommand_help_heading`]: crate::Command::subcommand_help_heading()
#[inline]
#[must_use]
pub fn next_subcommand_help_heading(mut self, heading: impl IntoResettable<Str>) -> Self {
Copy link
Member

Choose a reason for hiding this comment

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

Should we only have one next_help_heading?

Copy link
Author

Choose a reason for hiding this comment

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

Good question, that would make api simpler, but I'd rather have clear separation between subcommand and parameter heading

Copy link
Member

Choose a reason for hiding this comment

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

Could you explain why?

Comment on lines +640 to +641
Commands:
help Print this message or the help of the given subcommand(s)
Copy link
Member

Choose a reason for hiding this comment

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

Before this can move forward, we need to come to a resolution on help. The initial discussion should be on #1553. If we need to make new issues off of that, then we can migrate from there.

@Kamuno
Copy link

Kamuno commented Apr 21, 2025

Hey, I needed this feature for a tool I am writing, that has a lot of complex commands. So I rebased the PR in Kamuno@400dd85

Thank you all for the hard work you are doing. :) Great library and really useful PR!

@berkus
Copy link

berkus commented Jun 23, 2025

Please merge this, it fixes a painful problem.

Without it, I would need a serious code enshittification to change my code from declarative derive subcommands (all 27 of them) to an ugly mix with builder patterns that will not work well anyway, just to show these section headings.

@epage
Copy link
Member

epage commented Jun 23, 2025

Please merge this, it fixes a painful problem.

This can't be merged as is. Someone will need to step up and finish this work

Without it, I would need a serious code enshittification to change my code from declarative derive subcommands (all 27 of them) to an ugly mix with builder patterns that will not work well anyway, just to show these section headings.

Why would the lack of this require mixing derive with builder?

@berkus
Copy link

berkus commented Jun 24, 2025

Why would the lack of this require mixing derive with builder?

Because I cannot make section headings on subcommands enum in current clap version.
If you look at the setup here.

The disparate bits and pieces of clap suggestions are saying something like "You can have subcommand section headings if you use builder, but not derive, so if you have a bunch of derived structures, you will have to glue them with builder to have the section headings". It loses abiiltiy to make back Opts structure from parsed args though, so will require a lot of noisy refactoring anyway.

With the changes in this PR I can make section headings, the only two thing that are not possible is to 1) order them properly like I need to, 2) rename section headings back to "Commands" in the subcommand helps (it looks weird).

@berkus
Copy link

berkus commented Jun 24, 2025

Related discussion and changes.

@jerusdp
Copy link

jerusdp commented Oct 31, 2025

This can't be merged as is. Someone will need to step up and finish this work

What is the process for taking over the PR to finish up the work and get it ready to merge?
I see the PR is already many commits behind the master so perhaps starting from a fresh master is best? - if there is some git "magic" to help with this a steer towards it would be good.

I am interested in seeing this to merging if possible.

@epage
Copy link
Member

epage commented Oct 31, 2025

Note that there is still design discussion that needs to happen before moving forward on this PR, see #5819 (comment) (the issue is marked "waiting on design", not one of our "ready to implement labels).

After that, I've not checked to see how bad the conflicts are to see if this just needs a git rebase master or if it would be easier to port the concepts over.

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.

6 participants