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

fix(mangen): Honor the bin_name when rendering manpages #4758

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arusahni
Copy link

@arusahni arusahni commented Mar 11, 2023

This feels like an inoffensive update to the manpage renderer. As the synopsis section provides a sample invocation, we should prefer to use the bin_name over the Crate's name if one has been specified.

This did cause an interesting diff in the help subcommand ROFF snapshot test, where I discovered that the help command was not being prefixed with the parent command in the synopsis. Nor were spaces being used to delimit the app command from the subcommand name. What's introduced in this PR feels right to me, but I'll be interested to hear y'alls thoughts.

Thanks for the review!

Fixes #4757

Use the user-supplied bin_name over the command name for the synopsis
and subcommand name. This presents a sample invocation more in line with
how the CLI will be used.
@arusahni arusahni changed the title fix(mangen): Prefer the bin_name over the command name when rendering the synopsis fix(mangen): Honor the bin_name when rendering manpages Mar 12, 2023
Comment on lines +12 to +18
my/-app help/-test(1)
tests things
.TP
my/-app/-help/-some_cmd(1)
my/-app help/-some_cmd(1)
top level subcommand
.TP
my/-app/-help/-help(1)
my/-app help/-help(1)
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't these have - all the way through?

Comment on lines +31 to +35
fn get_command_name(cmd: &clap::Command) -> &str {
cmd.get_bin_name()
.or_else(|| cmd.get_display_name())
.unwrap_or_else(|| cmd.get_name())
}
Copy link
Member

Choose a reason for hiding this comment

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

The main risk for this is if the user parsed with the Command before calling this, leading it to have a .exe in the root bin name which can cause problems with this approach

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.

Man page synopsis doesn't use the bin_name
2 participants