Merge pull request #6142 from thomas-zahner/respect-display-order

epage · web-flow · commit 419244bc84be · 2025-10-22T10:14:13.000-05:00
Respect display order
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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,13 +431,45 @@ 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"])
             .require_equals(false)
             .num_args(3)
             .help("Optional config 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
@@ -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,
+    );
+}
