diff --git a/clap_builder/src/builder/mod.rs b/clap_builder/src/builder/mod.rs index 9f871d7ff7c..5113a1dd9d3 100644 --- a/clap_builder/src/builder/mod.rs +++ b/clap_builder/src/builder/mod.rs @@ -69,3 +69,49 @@ pub(crate) use self::str::Inner as StrInner; pub(crate) use action::CountType; pub(crate) use arg_settings::{ArgFlags, ArgSettings}; pub(crate) use command::AppExt; + +/// Strategies for ordering arguments +pub enum ArgOrder { + /// Preserve argument order and skip sorting + Preserve, + /// Sort arguments by short flags, then by long flags and finally by name + LongShortName, +} + +impl ArgOrder { + /// Sort arguments as specified by `ArgOrder` + pub fn sort(self, mut args: Vec<&Arg>) -> Vec<&Arg> { + let comparison = match self { + ArgOrder::Preserve => None, + ArgOrder::LongShortName => Some(option_sort_key), + }; + + if let Some(c) = comparison { + args.sort_by_key(|e| c(e)); + } + args + } +} + +fn option_sort_key(arg: &Arg) -> (usize, String) { + // Formatting key like this to ensure that: + // 1. Argument has long flags are printed just after short flags. + // 2. For two args both have short flags like `-c` and `-C`, the + // `-C` arg is printed just after the `-c` arg + // 3. For args without short or long flag, print them at last(sorted + // by arg name). + // Example order: -a, -b, -B, -s, --select-file, --select-folder, -x + + 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_builder/src/output/help_template.rs b/clap_builder/src/output/help_template.rs index 5ac11fe71cf..7b2f3a52c4b 100644 --- a/clap_builder/src/output/help_template.rs +++ b/clap_builder/src/output/help_template.rs @@ -13,7 +13,7 @@ use crate::builder::PossibleValue; use crate::builder::Str; use crate::builder::StyledStr; use crate::builder::Styles; -use crate::builder::{Arg, Command}; +use crate::builder::{Arg, ArgOrder, Command}; use crate::output::display_width; use crate::output::wrap; use crate::output::Usage; @@ -220,14 +220,14 @@ impl<'cmd, 'writer> HelpTemplate<'cmd, 'writer> { self.write_args( &self.cmd.get_non_positionals().collect::>(), "options", - option_sort_key, + ArgOrder::LongShortName, ); } "positionals" => { self.write_args( &self.cmd.get_positionals().collect::>(), "positionals", - positional_sort_key, + ArgOrder::Preserve, ); } "subcommands" => { @@ -422,7 +422,7 @@ impl HelpTemplate<'_, '_> { // Write positional args if any let help_heading = "Arguments"; let _ = write!(self.writer, "{header}{help_heading}:{header:#}\n",); - self.write_args(&pos, "Arguments", positional_sort_key); + self.write_args(&pos, "Arguments", ArgOrder::Preserve); } if !non_pos.is_empty() { @@ -432,7 +432,7 @@ impl HelpTemplate<'_, '_> { first = false; let help_heading = "Options"; let _ = write!(self.writer, "{header}{help_heading}:{header:#}\n",); - self.write_args(&non_pos, "Options", option_sort_key); + self.write_args(&non_pos, "Options", ArgOrder::LongShortName); } if !custom_headings.is_empty() { for heading in custom_headings { @@ -454,7 +454,7 @@ impl HelpTemplate<'_, '_> { } first = false; let _ = write!(self.writer, "{header}{heading}:{header:#}\n",); - self.write_args(&args, heading, option_sort_key); + self.write_args(&args, heading, ArgOrder::LongShortName); } } } @@ -466,11 +466,10 @@ impl HelpTemplate<'_, '_> { } /// Sorts arguments by length and display order and write their help to the wrapped stream. - fn write_args(&mut self, args: &[&Arg], _category: &str, sort_key: ArgSortKey) { + fn write_args(&mut self, args: &[&Arg], _category: &str, order: ArgOrder) { debug!("HelpTemplate::write_args {_category}"); // The shortest an arg can legally be is 2 (i.e. '-x') let mut longest = 2; - let mut ord_v = BTreeMap::new(); // Determine the longest for &arg in args.iter().filter(|arg| { @@ -493,14 +492,10 @@ impl HelpTemplate<'_, '_> { longest ); } - - let key = (sort_key)(arg); - ord_v.insert(key, arg); } let next_line_help = self.will_args_wrap(args, longest); - - for (i, (_, arg)) in ord_v.iter().enumerate() { + for (i, arg) in order.sort(args.to_vec()).iter().enumerate() { if i != 0 { self.writer.push_str("\n"); if next_line_help && self.use_long { @@ -914,7 +909,7 @@ impl HelpTemplate<'_, '_> { term_w: self.term_w, use_long: self.use_long, }; - sub_help.write_args(&args, heading, option_sort_key); + sub_help.write_args(&args, heading, ArgOrder::LongShortName); if subcommand.is_flatten_help_set() { sub_help.write_flat_subcommands(subcommand, first); } @@ -1058,35 +1053,6 @@ impl HelpTemplate<'_, '_> { const NEXT_LINE_INDENT: &str = " "; -type ArgSortKey = fn(arg: &Arg) -> (usize, String); - -fn positional_sort_key(arg: &Arg) -> (usize, String) { - (arg.get_index().unwrap_or(0), String::new()) -} - -fn option_sort_key(arg: &Arg) -> (usize, String) { - // Formatting key like this to ensure that: - // 1. Argument has long flags are printed just after short flags. - // 2. For two args both have short flags like `-c` and `-C`, the - // `-C` arg is printed just after the `-c` arg - // 3. For args without short or long flag, print them at last(sorted - // by arg name). - // Example order: -a, -b, -B, -s, --select-file, --select-folder, -x - - 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) -} - pub(crate) fn dimensions() -> (Option, Option) { #[cfg(not(feature = "wrap_help"))] return (None, None); diff --git a/clap_mangen/src/render.rs b/clap_mangen/src/render.rs index c48d13a4812..2aeae526990 100644 --- a/clap_mangen/src/render.rs +++ b/clap_mangen/src/render.rs @@ -1,4 +1,4 @@ -use clap::{Arg, ArgAction}; +use clap::{builder::ArgOrder, Arg, ArgAction}; use roff::{bold, italic, roman, Inline, Roff}; pub(crate) fn subcommand_heading(cmd: &clap::Command) -> &str { @@ -33,7 +33,10 @@ 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 opts: Vec<_> = + ArgOrder::LongShortName.sort(cmd.get_arguments().filter(|i| !i.is_hide_set()).collect()); + + for opt in opts { let (lhs, rhs) = option_markers(opt); match (opt.get_short(), opt.get_long()) { (Some(short), Some(long)) => { @@ -89,7 +92,9 @@ 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 sorted_items: Vec<_> = ArgOrder::LongShortName.sort(items.to_vec()); + + 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)] @@ -199,7 +204,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!( @@ -335,3 +343,7 @@ 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()) +} 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..3a5135308d7 --- /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\-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\-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 +[\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..b8128b96deb --- /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\-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 +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 e4ed80c535b..2bfb432131b 100644 --- a/clap_mangen/tests/testsuite/common.rs +++ b/clap_mangen/tests/testsuite/common.rs @@ -273,6 +273,39 @@ pub(crate) fn env_value_command(name: &'static str) -> clap::Command { ) } +pub(crate) fn mangen_output(cmd: &clap::Command) -> String { + let mut buf = vec![]; + clap_mangen::Man::new(cmd.clone()).render(&mut buf).unwrap(); + + let s = match std::str::from_utf8(&buf) { + Ok(s) => s, + Err(e) => panic!("Failed to convert output to UTF-8: {e}"), + }; + + s.to_string() +} + +// Go through the output and assert that the keywords +// appear in the expected order +pub(crate) fn is_correct_ordering( + keywords: &[&'static str], + text: &str, +) -> bool { + let mut s = text; + for keyword in keywords { + if !s.contains(keyword) { + return false; + } + // everytime we find a match, + // we only look at the rest of the string + s = match s.split(keyword).last() { + Some(rest) => rest, + None => return false, + }; + } + true +} + pub(crate) fn assert_matches(expected: impl IntoData, cmd: clap::Command) { let mut buf = vec![]; clap_mangen::Man::new(cmd).render(&mut buf).unwrap(); @@ -324,6 +357,41 @@ 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( @@ -354,3 +422,30 @@ pub(crate) fn help_headings(name: &'static str) -> clap::Command { .value_parser(["always", "never", "auto"]), ) } + +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)), + ]) +} \ No newline at end of file diff --git a/clap_mangen/tests/testsuite/roff.rs b/clap_mangen/tests/testsuite/roff.rs index 6adfcd238fc..34c2365633b 100644 --- a/clap_mangen/tests/testsuite/roff.rs +++ b/clap_mangen/tests/testsuite/roff.rs @@ -112,3 +112,64 @@ fn value_name_without_arg() { cmd, ); } + +#[test] +fn configured_display_order_args() { + let name = "my-app"; + let cmd = common::configured_display_order_args(name); + + let s = common::mangen_output(&cmd); + + let ordered_keywords = [ + "first", "second", "third", "fourth", "1st", "2nd", "3rd", + ]; + let default_ordered_keywords = [ + "third", "fourth", "first", "second", "1st", "2nd", "3rd", + ]; + + assert!(common::is_correct_ordering(&ordered_keywords, &s)); + assert!(!common::is_correct_ordering(&default_ordered_keywords, &s)); + + 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); + + let s = common::mangen_output(&cmd); + + let ordered_keywords = ["a1", "b1"]; + let default_ordered_keywords = ["b1", "a1"]; + + assert!(common::is_correct_ordering(&ordered_keywords, &s)); + assert!(!common::is_correct_ordering(&default_ordered_keywords, &s)); + + 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); + + let s = common::mangen_output(&cmd); + + let ordered_keywords = ["a1", "b1"]; + let default_ordered_keywords = ["b1", "a1"]; + + assert!(!common::is_correct_ordering(&ordered_keywords, &s)); + assert!(common::is_correct_ordering(&default_ordered_keywords, &s)); + + common::assert_matches( + snapbox::file!["../snapshots/default_subcmd_order.roff"], + cmd, + ); +} \ No newline at end of file