Repository: clap-rs/clap (rust)

Issue:
Argument descriptions not aligned when mixing positional arguments with others under a help heading
### Please complete the following tasks

- [X] I have searched the [discussions](https://github.com/clap-rs/clap/discussions)
- [X] I have searched the [open](https://github.com/clap-rs/clap/issues) and [rejected](https://github.com/clap-rs/clap/issues?q=is%3Aissue+label%3AS-wont-fix+is%3Aclosed) issues

### Rust Version

rustc 1.61.0 (fe5b13d68 2022-05-18)

### Clap Version

3.2.2

### Minimal reproducible code

```rust
use clap::Parser;
use std::path::PathBuf;

#[derive(Parser)]
struct Cli {
    /// One or more test cases separated by blank space.
    #[clap(value_name = "INPUT", value_parser, help_heading = "INPUT")]
    input: Vec<String>,

    /// Reads test cases on separate lines from a file.
    #[clap(
        name = "file",
        value_name = "FILE",
        short,
        long,
        value_parser,
        help_heading = "INPUT"
    )]
    file_path: Option<PathBuf>
}

fn main() {
    let cli = Cli::parse();
}
```


### Steps to reproduce the bug with the above code

1. Compile the binary.
2. Print the help using `binary -h`.

### Actual Behaviour

The help messages are not properly aligned.

```
INPUT:
    -f, --file <FILE>    Reads test cases on separate lines from a file
    <INPUT>...       One or more test cases separated by blank space
```

### Expected Behaviour

The help messages should be properly aligned.

```
INPUT:
    -f, --file <FILE>    Reads test cases on separate lines from a file
    <INPUT>...           One or more test cases separated by blank space
```

### Additional Context

I understand that proper alignment is not always possible or appropriate. Grouped arguments, however, should always be aligned correctly, in my opinion. The problem here is probably that help messages of flags are aligned differently than help messages of arguments receiving arbitrary input.

### Debug Output

_No response_

Pull request:
fix(help): Align mixed argument types
Fixes the alignment of sections with mixed argument types, including those without short arguments. This is done by adding an extra 4 to the length of arguments that will show a short flag (by calling `fn short`) instead of trying to account for it elsewhere.

For example:

```
Options:
      --from <FROM_VER>  The version to upgrade from
  [TO_VER]           The version to upgrade to
```

Will now show as:

```
Options:
  --from <FROM_VER>  The version to upgrade from
  [TO_VER]           The version to upgrade to
```


Fixes #3835.

Patch:
diff --git a/clap_builder/src/output/help_template.rs b/clap_builder/src/output/help_template.rs
index 18f38d15..adfc3b97 100644
--- a/clap_builder/src/output/help_template.rs
+++ b/clap_builder/src/output/help_template.rs
@@ -74,6 +74,8 @@ const DEFAULT_NO_ARGS_TEMPLATE: &str = "\
 {usage-heading} {usage}{after-help}\
     ";
 
+const SHORT_SIZE: usize = 4; // See `fn short` for the 4
+
 /// Help template writer
 ///
 /// Wraps a writer stream providing different methods to generate help for `clap` objects.
@@ -468,6 +470,8 @@ impl HelpTemplate<'_, '_> {
         debug!("HelpTemplate::write_args {_category}");
         // The shortest an arg can legally be is 2 (i.e. '-x')
         let mut longest = 2;
+        let mut longest_without_short = 2;
+        let mut has_short = false;
         let mut ord_v = BTreeMap::new();
 
         // Determine the longest
@@ -477,8 +481,21 @@ impl HelpTemplate<'_, '_> {
             // args alignment
             should_show_arg(self.use_long, arg)
         }) {
+            if !has_short && arg.get_short().is_some() {
+                has_short = true;
+            }
+
             if longest_filter(arg) {
-                longest = longest.max(display_width(&arg.to_string()));
+                let width = display_width(&arg.to_string());
+                let actual_width = if arg.is_positional() {
+                    width
+                } else {
+                    width + SHORT_SIZE
+                };
+                longest = longest.max(actual_width);
+                if !has_short {
+                    longest_without_short = longest_without_short.max(width);
+                }
                 debug!(
                     "HelpTemplate::write_args: arg={:?} longest={}",
                     arg.get_id(),
@@ -490,6 +507,10 @@ impl HelpTemplate<'_, '_> {
             ord_v.insert(key, arg);
         }
 
+        if !has_short {
+            longest = longest_without_short;
+        }
+
         let next_line_help = self.will_args_wrap(args, longest);
 
         for (i, (_, arg)) in ord_v.iter().enumerate() {
@@ -499,20 +520,22 @@ impl HelpTemplate<'_, '_> {
                     self.writer.push_str("\n");
                 }
             }
-            self.write_arg(arg, next_line_help, longest);
+            self.write_arg(arg, next_line_help, longest, has_short);
         }
     }
 
     /// Writes help for an argument to the wrapped stream.
-    fn write_arg(&mut self, arg: &Arg, next_line_help: bool, longest: usize) {
+    fn write_arg(&mut self, arg: &Arg, next_line_help: bool, longest: usize, has_short: bool) {
         let spec_vals = &self.spec_vals(arg);
 
         self.writer.push_str(TAB);
-        self.short(arg);
+        if has_short {
+            self.short(arg);
+        }
         self.long(arg);
         self.writer
             .push_styled(&arg.stylize_arg_suffix(self.styles, None));
-        self.align_to_about(arg, next_line_help, longest);
+        self.align_to_about(arg, next_line_help, longest, has_short);
 
         let about = if self.use_long {
             arg.get_long_help()
@@ -555,7 +578,7 @@ impl HelpTemplate<'_, '_> {
     }
 
     /// Write alignment padding between arg's switches/values and its about message.
-    fn align_to_about(&mut self, arg: &Arg, next_line_help: bool, longest: usize) {
+    fn align_to_about(&mut self, arg: &Arg, next_line_help: bool, longest: usize, has_short: bool) {
         debug!(
             "HelpTemplate::align_to_about: arg={}, next_line_help={}, longest={}",
             arg.get_id(),
@@ -567,7 +590,10 @@ impl HelpTemplate<'_, '_> {
             debug!("HelpTemplate::align_to_about: printing long help so skip alignment");
             0
         } else if !arg.is_positional() {
-            let self_len = display_width(&arg.to_string());
+            let mut self_len = display_width(&arg.to_string());
+            if has_short {
+                self_len += SHORT_SIZE;
+            }
             // Since we're writing spaces from the tab point we first need to know if we
             // had a long and short, or just short
             let padding = if arg.get_long().is_some() {
@@ -620,10 +646,8 @@ impl HelpTemplate<'_, '_> {
 
         let spaces = if next_line_help {
             TAB.len() + NEXT_LINE_INDENT.len()
-        } else if arg.map(|a| a.is_positional()).unwrap_or(true) {
-            longest + TAB_WIDTH * 2
         } else {
-            longest + TAB_WIDTH * 2 + 4 // See `fn short` for the 4
+            longest + TAB_WIDTH * 2
         };
         let trailing_indent = spaces; // Don't indent any further than the first line is indented
         let trailing_indent = self.get_spaces(trailing_indent);
@@ -724,11 +748,7 @@ impl HelpTemplate<'_, '_> {
                 .or_else(|| arg.get_long_help())
                 .unwrap_or_default();
             let h_w = h.display_width() + display_width(spec_vals);
-            let taken = if arg.is_positional() {
-                longest + TAB_WIDTH * 2
-            } else {
-                longest + TAB_WIDTH * 2 + 4 // See `fn short` for the 4
-            };
+            let taken = longest + TAB_WIDTH * 2;
             self.term_w >= taken
                 && (taken as f32 / self.term_w as f32) > 0.40
                 && h_w > (self.term_w - taken)
diff --git a/examples/find.md b/examples/find.md
index c52cbd39..f88cbad8 100644
--- a/examples/find.md
+++ b/examples/find.md
@@ -11,9 +11,9 @@ Options:
   -V, --version  Print version
 
 TESTS:
-      --empty        File is empty and is either a regular file or a directory
-      --name <name>  Base of file name (the path with the leading directories removed) matches shell
-                     pattern pattern
+  --empty        File is empty and is either a regular file or a directory
+  --name <name>  Base of file name (the path with the leading directories removed) matches shell
+                 pattern pattern
 
 OPERATORS:
   -o, --or   expr2 is not evaluate if exp1 is true
diff --git a/examples/git-derive.md b/examples/git-derive.md
index 119a1978..490f4e2e 100644
--- a/examples/git-derive.md
+++ b/examples/git-derive.md
@@ -87,12 +87,12 @@ git-derive[EXE] stash push:
   -h, --help               Print help
 
 git-derive[EXE] stash pop:
-  -h, --help   Print help
-  [STASH]  
+  -h, --help  Print help
+  [STASH]     
 
 git-derive[EXE] stash apply:
-  -h, --help   Print help
-  [STASH]  
+  -h, --help  Print help
+  [STASH]     
 
 git-derive[EXE] stash help:
 Print this message or the help of the given subcommand(s)
diff --git a/examples/git.md b/examples/git.md
index 61a9403f..f389c8cb 100644
--- a/examples/git.md
+++ b/examples/git.md
@@ -85,12 +85,12 @@ git[EXE] stash push:
   -h, --help               Print help
 
 git[EXE] stash pop:
-  -h, --help   Print help
-  [STASH]  
+  -h, --help  Print help
+  [STASH]     
 
 git[EXE] stash apply:
-  -h, --help   Print help
-  [STASH]  
+  -h, --help  Print help
+  [STASH]     
 
 git[EXE] stash help:
 Print this message or the help of the given subcommand(s)


Test patch:
diff --git a/tests/builder/help.rs b/tests/builder/help.rs
index 8341b7e4..26bb1308 100644
--- a/tests/builder/help.rs
+++ b/tests/builder/help.rs
@@ -3967,3 +3967,79 @@ Print this message or the help of the given subcommand(s)
 "#]];
     utils::assert_output(cmd, "parent -h", expected, false);
 }
+
+#[test]
+fn mixed_argument_types() {
+    let cmd = Command::new("myprog")
+        .about("mixed arguments")
+        .next_help_heading("Mixed")
+        .arg(arg!(-b --both "Both long and short"))
+        .arg(arg!(--long "Long only"))
+        .arg(arg!(<POSITIONAL> "Positional"));
+
+    let expected = str![[r#"
+mixed arguments
+
+Usage: myprog [OPTIONS] <POSITIONAL>
+
+Options:
+  -h, --help  Print help
+
+Mixed:
+  -b, --both    Both long and short
+      --long    Long only
+  <POSITIONAL>  Positional
+
+"#]];
+    utils::assert_output(cmd, "myprog --help", expected, false);
+}
+
+#[test]
+fn mixed_argument_types_short_positional() {
+    let cmd = Command::new("myprog")
+        .about("mixed arguments")
+        .next_help_heading("Mixed")
+        .arg(arg!(-b --both "Both long and short"))
+        .arg(arg!(--long "Long only"))
+        .arg(arg!(<S> "Short positional"));
+
+    let expected = str![[r#"
+mixed arguments
+
+Usage: myprog [OPTIONS] <S>
+
+Options:
+  -h, --help  Print help
+
+Mixed:
+  -b, --both  Both long and short
+      --long  Long only
+  <S>         Short positional
+
+"#]];
+    utils::assert_output(cmd, "myprog --help", expected, false);
+}
+
+#[test]
+fn mixed_argument_types_no_short() {
+    let cmd = Command::new("myprog")
+        .about("mixed arguments")
+        .next_help_heading("Mixed")
+        .arg(arg!(--long "Long only"))
+        .arg(arg!(<POSITIONAL> "Positional"));
+
+    let expected = str![[r#"
+mixed arguments
+
+Usage: myprog [OPTIONS] <POSITIONAL>
+
+Options:
+  -h, --help  Print help
+
+Mixed:
+  --long        Long only
+  <POSITIONAL>  Positional
+
+"#]];
+    utils::assert_output(cmd, "myprog --help", expected, false);
+}