From ef3f77deba22c53dc5e8abe009d794caf2c04634 Mon Sep 17 00:00:00 2001 From: Thomas Zahner Date: Wed, 15 Oct 2025 15:26:56 +0200 Subject: [PATCH 1/2] test(clap_mangen): Test mangen display order Co-authored-by: ericgumba --- .../configured_display_order_args.roff | 33 ++++++++ .../snapshots/configured_subcmd_order.roff | 27 ++++++ .../tests/snapshots/default_subcmd_order.roff | 27 ++++++ clap_mangen/tests/testsuite/common.rs | 84 ++++++++++++++++--- clap_mangen/tests/testsuite/roff.rs | 35 +++++++- 5 files changed, 191 insertions(+), 15 deletions(-) create mode 100644 clap_mangen/tests/snapshots/configured_display_order_args.roff create mode 100644 clap_mangen/tests/snapshots/configured_subcmd_order.roff create mode 100644 clap_mangen/tests/snapshots/default_subcmd_order.roff diff --git a/clap_mangen/tests/snapshots/configured_display_order_args.roff b/clap_mangen/tests/snapshots/configured_display_order_args.roff new file mode 100644 index 00000000000..0e2b168d80b --- /dev/null +++ b/clap_mangen/tests/snapshots/configured_display_order_args.roff @@ -0,0 +1,33 @@ +.ie \n(.g .ds Aq \(aq +.el .ds Aq ' +.TH my-app 1 "my-app " +.SH NAME +my\-app +.SH SYNOPSIS +\fBmy\-app\fR [\fB\-Q\fR|\fB\-\-third\fR] [\fB\-\-fourth\fR] [\fB\-O\fR|\fB\-\-first\fR] [\fB\-P\fR|\fB\-\-second\fR] [\fB\-h\fR|\fB\-\-help\fR] [\fI1st\fR] [\fI2nd\fR] [\fI3rd\fR] +.SH DESCRIPTION +.SH OPTIONS +.TP +\fB\-Q\fR, \fB\-\-third\fR +Should be 3rd +.TP +\fB\-\-fourth\fR +Should be 4th +.TP +\fB\-O\fR, \fB\-\-first\fR +Should be 1st +.TP +\fB\-P\fR, \fB\-\-second\fR +Should be 2nd +.TP +\fB\-h\fR, \fB\-\-help\fR +Print help +.TP +[\fI1st\fR] +1st +.TP +[\fI2nd\fR] +2nd +.TP +[\fI3rd\fR] +3rd diff --git a/clap_mangen/tests/snapshots/configured_subcmd_order.roff b/clap_mangen/tests/snapshots/configured_subcmd_order.roff new file mode 100644 index 00000000000..f9ce5dbd7dc --- /dev/null +++ b/clap_mangen/tests/snapshots/configured_subcmd_order.roff @@ -0,0 +1,27 @@ +.ie \n(.g .ds Aq \(aq +.el .ds Aq ' +.TH my-app 1 "my-app 1" +.SH NAME +my\-app +.SH SYNOPSIS +\fBmy\-app\fR [\fB\-h\fR|\fB\-\-help\fR] [\fB\-V\fR|\fB\-\-version\fR] [\fIsubcommands\fR] +.SH DESCRIPTION +.SH OPTIONS +.TP +\fB\-h\fR, \fB\-\-help\fR +Print help +.TP +\fB\-V\fR, \fB\-\-version\fR +Print version +.SH SUBCOMMANDS +.TP +my\-app\-b1(1) +blah b1 +.TP +my\-app\-a1(1) +blah a1 +.TP +my\-app\-help(1) +Print this message or the help of the given subcommand(s) +.SH VERSION +v1 diff --git a/clap_mangen/tests/snapshots/default_subcmd_order.roff b/clap_mangen/tests/snapshots/default_subcmd_order.roff new file mode 100644 index 00000000000..f9ce5dbd7dc --- /dev/null +++ b/clap_mangen/tests/snapshots/default_subcmd_order.roff @@ -0,0 +1,27 @@ +.ie \n(.g .ds Aq \(aq +.el .ds Aq ' +.TH my-app 1 "my-app 1" +.SH NAME +my\-app +.SH SYNOPSIS +\fBmy\-app\fR [\fB\-h\fR|\fB\-\-help\fR] [\fB\-V\fR|\fB\-\-version\fR] [\fIsubcommands\fR] +.SH DESCRIPTION +.SH OPTIONS +.TP +\fB\-h\fR, \fB\-\-help\fR +Print help +.TP +\fB\-V\fR, \fB\-\-version\fR +Print version +.SH SUBCOMMANDS +.TP +my\-app\-b1(1) +blah b1 +.TP +my\-app\-a1(1) +blah a1 +.TP +my\-app\-help(1) +Print this message or the help of the given subcommand(s) +.SH VERSION +v1 diff --git a/clap_mangen/tests/testsuite/common.rs b/clap_mangen/tests/testsuite/common.rs index 2b98708b822..8ffe406e4b9 100644 --- a/clap_mangen/tests/testsuite/common.rs +++ b/clap_mangen/tests/testsuite/common.rs @@ -324,6 +324,40 @@ pub(crate) fn value_name_without_arg(name: &'static str) -> clap::Command { ) } +pub(crate) fn configured_display_order_args(name: &'static str) -> clap::Command { + clap::Command::new(name) + .arg(clap::Arg::new("1st").help("1st")) + .arg(clap::Arg::new("2nd").help("2nd")) + .arg(clap::Arg::new("3rd").help("3rd").last(true)) + .arg( + clap::Arg::new("c") + .long("third") + .short('Q') + .display_order(3) + .help("Should be 3rd"), + ) + .arg( + clap::Arg::new("d") + .long("fourth") + .display_order(4) + .help("Should be 4th"), + ) + .arg( + clap::Arg::new("a") + .long("first") + .short('O') + .display_order(1) + .help("Should be 1st"), + ) + .arg( + clap::Arg::new("b") + .long("second") + .short('P') + .display_order(2) + .help("Should be 2nd"), + ) +} + pub(crate) fn help_headings(name: &'static str) -> clap::Command { clap::Command::new(name) .arg( @@ -356,8 +390,7 @@ pub(crate) fn help_headings(name: &'static str) -> clap::Command { } pub(crate) fn value_with_required_equals(name: &'static str) -> clap::Command { - clap::Command::new(name) - .arg( + clap::Command::new(name).arg( clap::Arg::new("config") .long("config") .value_name("FILE") @@ -367,8 +400,7 @@ pub(crate) fn value_with_required_equals(name: &'static str) -> clap::Command { } pub(crate) fn optional_value_with_required_equals(name: &'static str) -> clap::Command { - clap::Command::new(name) - .arg( + clap::Command::new(name).arg( clap::Arg::new("config") .long("config") .value_name("FILE") @@ -379,8 +411,7 @@ pub(crate) fn optional_value_with_required_equals(name: &'static str) -> clap::C } pub(crate) fn optional_value(name: &'static str) -> clap::Command { - clap::Command::new(name) - .arg( + clap::Command::new(name).arg( clap::Arg::new("config") .long("config") .value_name("FILE") @@ -390,8 +421,7 @@ pub(crate) fn optional_value(name: &'static str) -> clap::Command { } pub(crate) fn multiple_optional_values(name: &'static str) -> clap::Command { - clap::Command::new(name) - .arg( + clap::Command::new(name).arg( clap::Arg::new("config") .long("config") .value_names(["FILE1", "FILE2"]) @@ -401,8 +431,7 @@ pub(crate) fn multiple_optional_values(name: &'static str) -> clap::Command { } pub(crate) fn variadic_values(name: &'static str) -> clap::Command { - clap::Command::new(name) - .arg( + clap::Command::new(name).arg( clap::Arg::new("config") .long("config") .value_names(["FILE1", "FILE2"]) @@ -410,4 +439,37 @@ pub(crate) fn variadic_values(name: &'static str) -> clap::Command { .num_args(3) .help("Optional config file"), ) -} \ No newline at end of file +} + +pub(crate) fn configured_subcmd_order(name: &'static str) -> clap::Command { + clap::Command::new(name) + .version("1") + .next_display_order(None) + .subcommands(vec![ + clap::Command::new("b1").about("blah b1").arg( + clap::Arg::new("test") + .short('t') + .action(clap::ArgAction::SetTrue), + ), + clap::Command::new("a1").about("blah a1").arg( + clap::Arg::new("roster") + .short('r') + .action(clap::ArgAction::SetTrue), + ), + ]) +} + +pub(crate) fn default_subcmd_order(name: &'static str) -> clap::Command { + clap::Command::new(name).version("1").subcommands(vec![ + clap::Command::new("b1").about("blah b1").arg( + clap::Arg::new("test") + .short('t') + .action(clap::ArgAction::SetTrue), + ), + clap::Command::new("a1").about("blah a1").arg( + clap::Arg::new("roster") + .short('r') + .action(clap::ArgAction::SetTrue), + ), + ]) +} diff --git a/clap_mangen/tests/testsuite/roff.rs b/clap_mangen/tests/testsuite/roff.rs index decd5b892ee..5864870fdb7 100644 --- a/clap_mangen/tests/testsuite/roff.rs +++ b/clap_mangen/tests/testsuite/roff.rs @@ -137,10 +137,7 @@ fn optional_value_with_required_equals() { fn optional_value() { let name = "my-app"; let cmd = common::optional_value(name); - common::assert_matches( - snapbox::file!["../snapshots/optional_value.bash.roff"], - cmd, - ); + common::assert_matches(snapbox::file!["../snapshots/optional_value.bash.roff"], cmd); } #[test] @@ -162,3 +159,33 @@ fn variadic_values() { cmd, ); } + +#[test] +fn configured_display_order_args() { + let name = "my-app"; + let cmd = common::configured_display_order_args(name); + common::assert_matches( + snapbox::file!["../snapshots/configured_display_order_args.roff"], + cmd, + ); +} + +#[test] +fn configured_subcmd_order() { + let name = "my-app"; + let cmd = common::configured_subcmd_order(name); + common::assert_matches( + snapbox::file!["../snapshots/configured_subcmd_order.roff"], + cmd, + ); +} + +#[test] +fn default_subcmd_order() { + let name = "my-app"; + let cmd = common::default_subcmd_order(name); + common::assert_matches( + snapbox::file!["../snapshots/default_subcmd_order.roff"], + cmd, + ); +} From 24dfa0d5f9af0373d08bc2db05064990ae9a47d9 Mon Sep 17 00:00:00 2001 From: Thomas Zahner Date: Sat, 11 Oct 2025 16:45:40 +0200 Subject: [PATCH 2/2] fix(clap_mangen): Take into consideration display_order In #3362 we have an issue where when we configure an arg via .display_order(int), and then generate a manpage, the synposis and options will render the order the args were provided to the App rather than the order they were configured e.g Command::new(name) arg(Arg::new("few").short('b').display_order(2)) arg(Arg::new("bar").short('a').display_order(1)) will show ... SYNOPSIS [-b] [-a] ... ... OPTIONS -b -a instead of ... SYNOPSIS [-a] [-b] ... ... OPTIONS -a -b and so on. This fix adds sorting in the synopsis and options functions responsible for generating the corresponding synopsis and options sections of the manpage. Co-authored-by: ericgumba --- clap_mangen/src/render.rs | 36 +++++++++++++++++-- .../configured_display_order_args.roff | 14 ++++---- .../snapshots/configured_subcmd_order.roff | 6 ++-- 3 files changed, 43 insertions(+), 13 deletions(-) diff --git a/clap_mangen/src/render.rs b/clap_mangen/src/render.rs index bef05731231..10c3df06c86 100644 --- a/clap_mangen/src/render.rs +++ b/clap_mangen/src/render.rs @@ -33,7 +33,11 @@ pub(crate) fn synopsis(roff: &mut Roff, cmd: &clap::Command) { let name = cmd.get_bin_name().unwrap_or_else(|| cmd.get_name()); let mut line = vec![bold(name), roman(" ")]; - for opt in cmd.get_arguments().filter(|i| !i.is_hide_set()) { + let mut opts: Vec<_> = cmd.get_arguments().filter(|i| !i.is_hide_set()).collect(); + + opts.sort_by_key(|opt| option_sort_key(opt)); + + for opt in opts { let (lhs, rhs) = option_markers(opt); match (opt.get_short(), opt.get_long()) { (Some(short), Some(long)) => { @@ -89,7 +93,10 @@ pub(crate) fn synopsis(roff: &mut Roff, cmd: &clap::Command) { } pub(crate) fn options(roff: &mut Roff, items: &[&Arg]) { - for opt in items.iter().filter(|a| !a.is_positional()) { + let mut sorted_items = items.to_vec(); + sorted_items.sort_by_key(|opt| option_sort_key(opt)); + + for opt in sorted_items.iter().filter(|a| !a.is_positional()) { let mut header = match (opt.get_short(), opt.get_long()) { (Some(short), Some(long)) => { vec![short_option(short), roman(", "), long_option(long)] @@ -217,7 +224,10 @@ fn possible_options(roff: &mut Roff, arg: &Arg, arg_help_written: bool) { } pub(crate) fn subcommands(roff: &mut Roff, cmd: &clap::Command, section: &str) { - for sub in cmd.get_subcommands().filter(|s| !s.is_hide_set()) { + let mut sorted_subcommands: Vec<_> = + cmd.get_subcommands().filter(|s| !s.is_hide_set()).collect(); + sorted_subcommands.sort_by_key(|c| subcommand_sort_key(c)); + for sub in sorted_subcommands { roff.control("TP", []); let name = format!( @@ -378,3 +388,23 @@ fn format_possible_values(possibles: &Vec<&clap::builder::PossibleValue>) -> (Ve } (lines, with_help) } + +fn subcommand_sort_key(command: &clap::Command) -> (usize, &str) { + (command.get_display_order(), command.get_name()) +} + +/// Note that this function is duplicated from `clap::builder` +fn option_sort_key(arg: &Arg) -> (usize, String) { + let key = if let Some(x) = arg.get_short() { + let mut s = x.to_ascii_lowercase().to_string(); + s.push(if x.is_ascii_lowercase() { '0' } else { '1' }); + s + } else if let Some(x) = arg.get_long() { + x.to_string() + } else { + let mut s = '{'.to_string(); + s.push_str(arg.get_id().as_str()); + s + }; + (arg.get_display_order(), key) +} diff --git a/clap_mangen/tests/snapshots/configured_display_order_args.roff b/clap_mangen/tests/snapshots/configured_display_order_args.roff index 0e2b168d80b..3a5135308d7 100644 --- a/clap_mangen/tests/snapshots/configured_display_order_args.roff +++ b/clap_mangen/tests/snapshots/configured_display_order_args.roff @@ -4,22 +4,22 @@ .SH NAME my\-app .SH SYNOPSIS -\fBmy\-app\fR [\fB\-Q\fR|\fB\-\-third\fR] [\fB\-\-fourth\fR] [\fB\-O\fR|\fB\-\-first\fR] [\fB\-P\fR|\fB\-\-second\fR] [\fB\-h\fR|\fB\-\-help\fR] [\fI1st\fR] [\fI2nd\fR] [\fI3rd\fR] +\fBmy\-app\fR [\fB\-O\fR|\fB\-\-first\fR] [\fB\-P\fR|\fB\-\-second\fR] [\fB\-Q\fR|\fB\-\-third\fR] [\fB\-\-fourth\fR] [\fB\-h\fR|\fB\-\-help\fR] [\fI1st\fR] [\fI2nd\fR] [\fI3rd\fR] .SH DESCRIPTION .SH OPTIONS .TP -\fB\-Q\fR, \fB\-\-third\fR -Should be 3rd -.TP -\fB\-\-fourth\fR -Should be 4th -.TP \fB\-O\fR, \fB\-\-first\fR Should be 1st .TP \fB\-P\fR, \fB\-\-second\fR Should be 2nd .TP +\fB\-Q\fR, \fB\-\-third\fR +Should be 3rd +.TP +\fB\-\-fourth\fR +Should be 4th +.TP \fB\-h\fR, \fB\-\-help\fR Print help .TP diff --git a/clap_mangen/tests/snapshots/configured_subcmd_order.roff b/clap_mangen/tests/snapshots/configured_subcmd_order.roff index f9ce5dbd7dc..b8128b96deb 100644 --- a/clap_mangen/tests/snapshots/configured_subcmd_order.roff +++ b/clap_mangen/tests/snapshots/configured_subcmd_order.roff @@ -15,12 +15,12 @@ Print help Print version .SH SUBCOMMANDS .TP -my\-app\-b1(1) -blah b1 -.TP my\-app\-a1(1) blah a1 .TP +my\-app\-b1(1) +blah b1 +.TP my\-app\-help(1) Print this message or the help of the given subcommand(s) .SH VERSION