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

feat(mangen): Support flatten_help #5769

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Conversation

aparcar
Copy link

@aparcar aparcar commented Oct 7, 2024

The flatten_help argument combines all subcommands on a single page. Until now this wasn't supported for mangen. With this command the sections SYNOPSIS as well as (SUB)COMMANDS are changed to imitate the style of git stash --help.

I reached out via discussions and it looks like this doesn't work just yet #5761

This is not necessarily the most beautiful implementation due to my limited knowledge of both clap and Rust itself.

I implemented this for a project I'm working on and the results look quite okay:

image

Not using man.render() but call each command manually feels a bit awkward but does work just fine, see example here https://github.com/rosenpass/rosenpass/pull/434/files#diff-352422e105afd3138a54ea14e33af782243398f1d768d271655729dd4bcb99d2R18

clap_mangen/src/lib.rs Outdated Show resolved Hide resolved
Comment on lines 225 to 228
for sc in self.cmd.get_subcommands() {
render::synopsis(roff, sc);
roff.control("br", []);
}
Copy link
Member

Choose a reason for hiding this comment

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

There are times when you should sow the usage for the current command

See

if !self.cmd.is_subcommand_required_set()
|| self.cmd.is_args_conflicts_with_subcommands_set()
{
self.write_arg_usage(styled, &[], true);
styled.trim_end();
let _ = write!(styled, "{USAGE_SEP}");
}

In general, I'd recommend checking how we use flatten_help to change output

Copy link
Author

Choose a reason for hiding this comment

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

I tried to adopt the style

Copy link
Member

Choose a reason for hiding this comment

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

I am not seeing a call to is_subcommand_required_set or is_args_conflicts_with_subcommands_set to handle this

Copy link
Member

Choose a reason for hiding this comment

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

Please document in the PR description any style differences between --help and man's handling of flatten_help

Copy link
Author

Choose a reason for hiding this comment

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

Before documenting it I want to make sure that the flatten_help implementation for --help isn't incomplete. In the picture below are subcommand details printed directly after the options without another section title, is that intended?

image

Copy link
Member

Choose a reason for hiding this comment

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

In the picture below are subcommand details printed directly after the options without another section title, is that intended?

Yes, that is intentional as we don't really have a good way of rendering sub-headers at this time.

@aparcar aparcar force-pushed the flat-man branch 3 times, most recently from 3f70e85 to c2c7c26 Compare October 16, 2024 15:15
for (_, name, cmd) in ord_v {
let mut line = command_with_args(cmd, name);

if cmd.has_subcommands() && !flatten {
Copy link
Author

Choose a reason for hiding this comment

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

I'm quite sure you don't love this way of implementing it, do you have a cleaner idea?

Copy link
Member

Choose a reason for hiding this comment

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

where possible, we should match how our regular usage renderer does things

Copy link
Author

Choose a reason for hiding this comment

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

I changed the style to be less invasive, what do you think?

@aparcar aparcar marked this pull request as ready for review October 16, 2024 16:09
clap_mangen/src/render.rs Outdated Show resolved Hide resolved
clap_mangen/src/render.rs Outdated Show resolved Hide resolved
clap_mangen/src/render.rs Outdated Show resolved Hide resolved
clap_mangen/src/render.rs Outdated Show resolved Hide resolved
clap_mangen/src/render.rs Outdated Show resolved Hide resolved
This test shows that the output stays the same independent of the value
of `flatten_help`. Further commits may implement `flatten_help` for
`clap_mangen`.

Signed-off-by: Paul Spooren <[email protected]>
The function prints the usage of a (sub)command.

Signed-off-by: Paul Spooren <[email protected]>
The `flatten_help` argument combines all subcommands on a single page.
Until now this wasn't supported for `mangen`. With this command the
sections `SYNOPSIS` as well as `SUBCOMMANDS` are changed to imitate the
style of `git stash --help`.

Signed-off-by: Paul Spooren <[email protected]>
@aparcar aparcar marked this pull request as draft October 17, 2024 13:42
@aparcar
Copy link
Author

aparcar commented Oct 17, 2024

I added a wip commit to handle is_subcommand_required. Still trying to figure out is_args_conflicts_with_subcommands_set

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