feat(help): line the flag column up, and give the short page a column at all

argv/src/help.rs

@@ -252,28 +252,55 @@ pub fn short_help(spec: &Spec<'_>, path: &[&str], meta: &CommandMeta<'_>) -> Str
     // `tool-alias get <TOOL>` under `mise tool-alias`, the whole path from the root rather
     // than the child's own name.
     commands_section(&mut out, &path[1.min(path.len())..], meta);
+
+    // The short page lines its columns up too. It did not: every description began directly
+    // after the name it belonged to, so nothing in `-h` lined up with anything — and `-h` is
+    // the form most people type. One column per section over its visible entries, which is
+    // the rule the long page already follows.
+    let args: Vec<&ArgMeta<'_>> = meta.args.iter().filter(|a| !a.hide).collect();
+    let arg_col = args
+        .iter()
+        .map(|a| arg_usage(a).chars().count())
+        .max()
+        .unwrap_or(0);
     groups_section(
         &mut out,
         "Arguments",
-        meta.args.iter().filter(|a| !a.hide),
+        args.iter().copied(),
         |a| a.help_heading,
         |out, a| {
-            let _ = write!(out, "  {}", arg_usage(a));
-            if let Some(help) = a.help {
-                let _ = write!(out, "  {help}");
+            let usage = arg_usage(a);
+            match a.help.filter(|h| !h.trim().is_empty()) {
+                Some(help) => {
+                    let _ = write!(out, "  {usage:<arg_col$}  {help}");
+                }
+                None => {
+                    let _ = write!(out, "  {usage}");
+                }
             }
             annotations(out, a.choices, a.env, a.default);
         },
     );
+    let flags: Vec<&FlagMeta<'_>> = meta.flags.iter().filter(|f| !f.hide).collect();
+    let flag_col = flags
+        .iter()
+        .map(|f| column_usage(f).chars().count())
+        .max()
+        .unwrap_or(0);
     groups_section(
         &mut out,
         "Flags",
-        meta.flags.iter().filter(|f| !f.hide),
+        flags.iter().copied(),
         |f| f.help_heading,
         |out, f| {
-            let _ = write!(out, "  {}", display_usage(f));
-            if let Some(help) = f.help {
-                let _ = write!(out, "  {help}");
+            let usage = column_usage(f);
+            match f.help.filter(|h| !h.trim().is_empty()) {
+                Some(help) => {
+                    let _ = write!(out, "  {usage:<flag_col$}  {help}");
+                }
+                None => {
+                    let _ = write!(out, "  {usage}");
+                }
             }
             annotations(out, f.choices, f.env, &[]);
         },
@@ -410,6 +437,45 @@ fn display_usage(meta: &FlagMeta<'_>) -> String {
     }
 }
 
+/// The width of the short column: `-x, `, or the blank that stands in for it.
+///
+/// Fixed, because a short form is one character. clap's, measured.
+const SHORT_COL: usize = 4;
+
+/// A flag as the *flags section* lists it, with its long form in a column of its own.
+///
+/// Separate from [`flag_usage`], which feeds the usage line — `Usage: ex [-f --force]` must
+/// not be padded, and this must be. clap's shape, measured from clap 4:
+///
+/// ```text
+///       --github-release
+///   -n, --dry-run
+///   -o, --output <OUTPUT>
+///   -j <JOBS>
+/// ```
+///
+/// Two rules in there worth stating. The short column is only spent where there is a long form
+/// to line up *with*: a flag with no long one writes `-j <JOBS>` and does not pad, which is
+/// what clap does. And a flag with neither — usage can name one the forms do not imply,
+/// `verbose: -v`, which clap has no equivalent for — takes the same path as short-only.
+fn column_usage(meta: &FlagMeta<'_>) -> String {
+    let rest = display_usage(meta);
+    let Some(long) = meta.flag.longs.first() else {
+        return rest;
+    };
+    // Only when the text actually begins with the long form. The `name:` prefix case does not,
+    // and splitting it would put `verbose:` in a column meant for `-v, `.
+    let Some(at) = rest.find(&format!("--{long}")) else {
+        return rest;
+    };
+    let (before, after) = rest.split_at(at);
+    let short = match before.trim() {
+        "" => String::new(),
+        s => format!("{s},"),
+    };
+    format!("{short:<SHORT_COL$}{after}")
+}
+
 fn examples_section(out: &mut String, spec: &Spec<'_>, meta: &CommandMeta<'_>) {
     let examples = page_examples(spec, meta);
     if examples.is_empty() {
@@ -520,7 +586,7 @@ pub fn long_help(spec: &Spec<'_>, path: &[&str], meta: &CommandMeta<'_>) -> Stri
     let flags: Vec<&FlagMeta<'_>> = meta.flags.iter().filter(|f| !f.hide).collect();
     let flag_col = flags
         .iter()
-        .map(|f| display_usage(f).chars().count())
+        .map(|f| column_usage(f).chars().count())
         .max()
         .unwrap_or(0);
     groups_section(
@@ -530,7 +596,7 @@ pub fn long_help(spec: &Spec<'_>, path: &[&str], meta: &CommandMeta<'_>) -> Stri
         |f| f.help_heading,
         |out, f| {
             let text = f.long_help.or(f.help);
-            entry(out, &display_usage(f), text, flag_col, width);
+            entry(out, &column_usage(f), text, flag_col, width);
             long_annotations(out, f.choices, f.env, &[]);
         },
     );

conformance/tests/flag_column.rs

@@ -0,0 +1,133 @@
+//! Where a flag's name sits in the flags section.
+//!
+//! Measured from clap 4 rather than remembered. This is the whole rule, in five lines:
+//!
+//! ```text
+//! Options:
+//!   -j <JOBS>
+//!       --github-release
+//!   -n, --dry-run
+//!   -o, --output <OUTPUT>
+//!   -h, --help             Print help
+//! ```
+//!
+//! A four-character column holds `-x, ` or the blank standing in for it — but *only* where
+//! there is a long form to line up with. `-j <JOBS>` has none, so it does not pad; that is
+//! clap's behaviour and not an oversight in it.
+
+use usage_argv::help;
+use usage_derive::Cli;
+
+/// A tool with one of each shape
+#[derive(Cli)]
+#[usage(bin = "ex")]
+struct Ex {
+    /// Short only, so nothing to align with
+    #[usage(short = 'j')]
+    jobs: Option<String>,
+    /// Long only
+    #[usage(long)]
+    github_release: bool,
+    /// Both
+    #[usage(long, short = 'n')]
+    dry_run: bool,
+    /// Both, and takes a value
+    #[usage(long, short)]
+    output: Option<String>,
+    /// Its description is long enough to need wrapping at any sane width
+    #[usage(long)]
+    describe: bool,
+}
+
+fn page(long: bool) -> String {
+    help::render(Ex::spec(), Ex::spec().root.cmd, long).expect("a page")
+}
+
+#[test]
+fn a_long_form_starts_in_the_same_column_whatever_precedes_it() {
+    for long in [false, true] {
+        let page = page(long);
+        for line in [
+            "      --github-release",
+            "  -n, --dry-run",
+            "  -o, --output <OUTPUT>",
+        ] {
+            assert!(
+                page.lines().any(|l| l.starts_with(line)),
+                "long={long}: no line starts `{line}`:\n{page}"
+            );
+        }
+        // A comma, which is the near-universal convention and what clap prints.
+        assert!(!page.contains("-n --dry-run"), "long={long}: {page}");
+    }
+}
+
+#[test]
+fn a_flag_with_no_long_form_does_not_pay_for_the_column() {
+    // clap writes `-j <JOBS>` at the indent, not padded out to where the long forms begin:
+    // there is nothing to line it up with, and the padding would only push it away from its
+    // own description.
+    for long in [false, true] {
+        let page = page(long);
+        assert!(
+            page.lines().any(|l| l.starts_with("  -j <JOBS>")),
+            "long={long}: {page}"
+        );
+    }
+}
+
+#[test]
+fn the_short_page_lines_its_descriptions_up_too() {
+    // It did not. Every description began directly after the name it belonged to, so nothing
+    // in `-h` lined up with anything — and `-h` is the form most people type. One column per
+    // section, which is the rule the long page already followed.
+    let page = page(false);
+    // Where each description begins, for three flags whose names are three different lengths.
+    // If the column is real they are equal; before this they were 2 + the length of the name.
+    let column_of = |needle: &str, help: &str| -> usize {
+        let line = page
+            .lines()
+            .find(|l| l.contains(needle))
+            .unwrap_or_else(|| panic!("no line for {needle}:\n{page}"));
+        line.find(help)
+            .unwrap_or_else(|| panic!("no help on {line:?}"))
+    };
+    let a = column_of("--github-release", "Long only");
+    let b = column_of("--dry-run", "Both");
+    let c = column_of("--describe", "Its description");
+    assert!(
+        a == b && b == c,
+        "descriptions should start in one column, got {a}, {b}, {c}:\n{page}"
+    );
+}
+
+#[test]
+fn the_usage_line_is_not_padded() {
+    // `column_usage` is separate from the usage line's own rendering for this reason: a
+    // `Usage: ex [    --describe]` would be absurd.
+    let page = page(true);
+    let usage = page
+        .lines()
+        .find(|l| l.starts_with("Usage:"))
+        .expect("a usage line");
+    assert!(!usage.contains("  --"), "padded usage line: {usage}");
+}
+
+#[test]
+fn the_fields_are_bound() {
+    use std::ffi::OsStr;
+    let argv = [
+        "-j",
+        "4",
+        "--github-release",
+        "-n",
+        "--output",
+        "o",
+        "--describe",
+    ]
+    .map(OsStr::new);
+    let ex = Ex::parse_from(&argv).expect("should parse");
+    assert_eq!(ex.jobs.as_deref(), Some("4"));
+    assert!(ex.github_release && ex.dry_run && ex.describe);
+    assert_eq!(ex.output.as_deref(), Some("o"));
+}