Merge pull request #4144 from epage/collapse-bug

fix(help): Consistently use `[]` for positionals
clap-rs · Aug 29, 2022 · 567bff7 · 567bff7
2 parents 2923855 + 02db304
commit 567bff7
Show file tree

Hide file tree

Showing 34 changed files with 122 additions and 82 deletions.
diff --git a/clap_complete/tests/snapshots/aliases.bash b/clap_complete/tests/snapshots/aliases.bash
@@ -19,7 +19,7 @@ _my-app() {
 
     case "${cmd}" in
         my__app)
-            opts="-F -f -O -o -h -V --flg --flag --opt --option --help --version <positional>"
+            opts="-F -f -O -o -h -V --flg --flag --opt --option --help --version [positional]"
             if [[ ${cur} == -* || ${COMP_CWORD} -eq 1 ]] ; then
                 COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
                 return 0

diff --git a/clap_complete/tests/snapshots/feature_sample.bash b/clap_complete/tests/snapshots/feature_sample.bash
@@ -25,7 +25,7 @@ _my-app() {
 
     case "${cmd}" in
         my__app)
-            opts="-C -c -h -V --conf --config --help --version <file> first second test help"
+            opts="-C -c -h -V --conf --config --help --version [file] first second test help"
             if [[ ${cur} == -* || ${COMP_CWORD} -eq 1 ]] ; then
                 COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
                 return 0

diff --git a/clap_complete/tests/snapshots/special_commands.bash b/clap_complete/tests/snapshots/special_commands.bash
@@ -34,7 +34,7 @@ _my-app() {
 
     case "${cmd}" in
         my__app)
-            opts="-C -c -h -V --conf --config --help --version <file> first second test some_cmd some-cmd-with-hyphens some-hidden-cmd help"
+            opts="-C -c -h -V --conf --config --help --version [file] first second test some_cmd some-cmd-with-hyphens some-hidden-cmd help"
             if [[ ${cur} == -* || ${COMP_CWORD} -eq 1 ]] ; then
                 COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
                 return 0
@@ -160,7 +160,7 @@ _my-app() {
             return 0
             ;;
         my__app__some_cmd)
-            opts="-h -V --config --help --version <path>..."
+            opts="-h -V --config --help --version [path]..."
             if [[ ${cur} == -* || ${COMP_CWORD} -eq 2 ]] ; then
                 COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
                 return 0

diff --git a/clap_complete/tests/snapshots/sub_subcommands.bash b/clap_complete/tests/snapshots/sub_subcommands.bash
@@ -31,7 +31,7 @@ _my-app() {
 
     case "${cmd}" in
         my__app)
-            opts="-C -c -h -V --conf --config --help --version <file> first second test some_cmd help"
+            opts="-C -c -h -V --conf --config --help --version [file] first second test some_cmd help"
             if [[ ${cur} == -* || ${COMP_CWORD} -eq 1 ]] ; then
                 COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
                 return 0

diff --git a/clap_complete/tests/snapshots/value_hint.bash b/clap_complete/tests/snapshots/value_hint.bash
@@ -19,7 +19,7 @@ _my-app() {
 
     case "${cmd}" in
         my__app)
-            opts="-p -f -d -e -c -u -H -h --choice --unknown --other --path --file --dir --exe --cmd-name --cmd --user --host --url --email --help <command_with_args>..."
+            opts="-p -f -d -e -c -u -H -h --choice --unknown --other --path --file --dir --exe --cmd-name --cmd --user --host --url --email --help [command_with_args]..."
             if [[ ${cur} == -* || ${COMP_CWORD} -eq 1 ]] ; then
                 COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
                 return 0

diff --git a/examples/escaped-positional-derive.md b/examples/escaped-positional-derive.md
@@ -12,7 +12,7 @@ Usage:
     escaped-positional-derive[EXE] [OPTIONS] [-- <SLOP>...]
 
 Arguments:
-    <SLOP>...    
+    [SLOP]...    
 
 Options:
     -f               

diff --git a/examples/escaped-positional.md b/examples/escaped-positional.md
@@ -12,7 +12,7 @@ Usage:
     escaped-positional[EXE] [OPTIONS] [-- <SLOP>...]
 
 Arguments:
-    <SLOP>...    
+    [SLOP]...    
 
 Options:
     -f               

diff --git a/examples/git-derive.md b/examples/git-derive.md
@@ -111,7 +111,7 @@ Usage:
     git-derive[EXE] stash pop [STASH]
 
 Arguments:
-    <STASH>    
+    [STASH]    
 
 Options:
     -h, --help    Print help information

diff --git a/examples/git.md b/examples/git.md
@@ -109,7 +109,7 @@ Usage:
     git[EXE] stash pop [STASH]
 
 Arguments:
-    <STASH>    
+    [STASH]    
 
 Options:
     -h, --help    Print help information

diff --git a/examples/pacman.md b/examples/pacman.md
@@ -59,7 +59,7 @@ Usage:
     pacman[EXE] {sync|--sync|-S} [OPTIONS] [--] [package]...
 
 Arguments:
-    <package>...    packages
+    [package]...    packages
 
 Options:
     -s, --search <search>...    search remote repositories for matching strings

diff --git a/examples/tutorial_builder/01_quick.md b/examples/tutorial_builder/01_quick.md
@@ -11,7 +11,7 @@ Subcommands:
     help    Print this message or the help of the given subcommand(s)
 
 Arguments:
-    <name>    Optional name to operate on
+    [name]    Optional name to operate on
 
 Options:
     -c, --config <FILE>    Sets a custom config file

diff --git a/examples/tutorial_builder/03_03_positional.md b/examples/tutorial_builder/03_03_positional.md
@@ -7,7 +7,7 @@ Usage:
     03_03_positional[EXE] [name]
 
 Arguments:
-    <name>    
+    [name]    
 
 Options:
     -h, --help       Print help information

diff --git a/examples/tutorial_builder/03_03_positional_mult.md b/examples/tutorial_builder/03_03_positional_mult.md
@@ -7,7 +7,7 @@ Usage:
     03_03_positional_mult[EXE] [name]...
 
 Arguments:
-    <name>...    
+    [name]...    
 
 Options:
     -h, --help       Print help information

diff --git a/examples/tutorial_builder/03_04_subcommands.md b/examples/tutorial_builder/03_04_subcommands.md
@@ -22,7 +22,7 @@ Usage:
     03_04_subcommands[EXE] add [NAME]
 
 Arguments:
-    <NAME>    
+    [NAME]    
 
 Options:
     -h, --help       Print help information

diff --git a/examples/tutorial_builder/03_05_default_values.md b/examples/tutorial_builder/03_05_default_values.md
@@ -7,7 +7,7 @@ Usage:
     03_05_default_values[EXE] [NAME]
 
 Arguments:
-    <NAME>    [default: alice]
+    [NAME]    [default: alice]
 
 Options:
     -h, --help       Print help information

diff --git a/examples/tutorial_builder/04_03_relations.md b/examples/tutorial_builder/04_03_relations.md
@@ -7,7 +7,7 @@ Usage:
     04_03_relations[EXE] [OPTIONS] <--set-ver <VER>|--major|--minor|--patch> [INPUT_FILE]
 
 Arguments:
-    <INPUT_FILE>    some regular input
+    [INPUT_FILE]    some regular input
 
 Options:
         --set-ver <VER>        set version manually

diff --git a/examples/tutorial_builder/04_04_custom.md b/examples/tutorial_builder/04_04_custom.md
@@ -7,7 +7,7 @@ Usage:
     04_04_custom[EXE] [OPTIONS] [INPUT_FILE]
 
 Arguments:
-    <INPUT_FILE>    some regular input
+    [INPUT_FILE]    some regular input
 
 Options:
         --set-ver <VER>        set version manually

diff --git a/examples/tutorial_derive/01_quick.md b/examples/tutorial_derive/01_quick.md
@@ -11,7 +11,7 @@ Subcommands:
     help    Print this message or the help of the given subcommand(s)
 
 Arguments:
-    <NAME>    Optional name to operate on
+    [NAME]    Optional name to operate on
 
 Options:
     -c, --config <FILE>    Sets a custom config file

diff --git a/examples/tutorial_derive/03_03_positional.md b/examples/tutorial_derive/03_03_positional.md
@@ -7,7 +7,7 @@ Usage:
     03_03_positional_derive[EXE] [NAME]
 
 Arguments:
-    <NAME>    
+    [NAME]    
 
 Options:
     -h, --help       Print help information

diff --git a/examples/tutorial_derive/03_03_positional_mult.md b/examples/tutorial_derive/03_03_positional_mult.md
@@ -7,7 +7,7 @@ Usage:
     03_03_positional_mult_derive[EXE] [NAME]...
 
 Arguments:
-    <NAME>...    
+    [NAME]...    
 
 Options:
     -h, --help       Print help information

diff --git a/examples/tutorial_derive/03_04_subcommands.md b/examples/tutorial_derive/03_04_subcommands.md
@@ -22,7 +22,7 @@ Usage:
     03_04_subcommands_derive[EXE] add [NAME]
 
 Arguments:
-    <NAME>    
+    [NAME]    
 
 Options:
     -h, --help       Print help information

diff --git a/examples/tutorial_derive/03_05_default_values.md b/examples/tutorial_derive/03_05_default_values.md
@@ -7,7 +7,7 @@ Usage:
     03_05_default_values_derive[EXE] [NAME]
 
 Arguments:
-    <NAME>    [default: alice]
+    [NAME]    [default: alice]
 
 Options:
     -h, --help       Print help information

diff --git a/examples/tutorial_derive/04_03_relations.md b/examples/tutorial_derive/04_03_relations.md
@@ -7,7 +7,7 @@ Usage:
     04_03_relations_derive[EXE] [OPTIONS] <--set-ver <VER>|--major|--minor|--patch> [INPUT_FILE]
 
 Arguments:
-    <INPUT_FILE>    some regular input
+    [INPUT_FILE]    some regular input
 
 Options:
         --set-ver <VER>        set version manually

diff --git a/examples/tutorial_derive/04_04_custom.md b/examples/tutorial_derive/04_04_custom.md
@@ -7,7 +7,7 @@ Usage:
     04_04_custom_derive[EXE] [OPTIONS] [INPUT_FILE]
 
 Arguments:
-    <INPUT_FILE>    some regular input
+    [INPUT_FILE]    some regular input
 
 Options:
         --set-ver <VER>        set version manually

diff --git a/src/builder/arg.rs b/src/builder/arg.rs
@@ -3970,7 +3970,7 @@ impl Arg {
         }
     }
 
-    pub(crate) fn stylized(&self) -> StyledStr {
+    pub(crate) fn stylized(&self, required: Option<bool>) -> StyledStr {
         let mut styled = StyledStr::new();
         // Write the name such --long or -l
         if let Some(l) = self.get_long() {
@@ -3980,11 +3980,11 @@ impl Arg {
             styled.literal("-");
             styled.literal(s);
         }
-        styled.extend(self.stylize_arg_suffix().into_iter());
+        styled.extend(self.stylize_arg_suffix(required).into_iter());
         styled
     }
 
-    pub(crate) fn stylize_arg_suffix(&self) -> StyledStr {
+    pub(crate) fn stylize_arg_suffix(&self, required: Option<bool>) -> StyledStr {
         let mut styled = StyledStr::new();
 
         let mut need_closing_bracket = false;
@@ -4005,7 +4005,8 @@ impl Arg {
             }
         }
         if self.is_takes_value_set() || self.is_positional() {
-            let arg_val = self.render_arg_val();
+            let required = required.unwrap_or_else(|| self.is_required_set());
+            let arg_val = self.render_arg_val(required);
             styled.placeholder(arg_val);
         } else if matches!(*self.get_action(), ArgAction::Count) {
             styled.placeholder("...");
@@ -4018,7 +4019,7 @@ impl Arg {
     }
 
     /// Write the values such as <name1> <name2>
-    fn render_arg_val(&self) -> String {
+    fn render_arg_val(&self, required: bool) -> String {
         let mut rendered = String::new();
 
         let num_vals = self.get_num_args().expect(INTERNAL_ERROR_MSG);
@@ -4036,7 +4037,7 @@ impl Arg {
 
         debug_assert!(self.is_takes_value_set());
         for (n, val_name) in val_names.iter().enumerate() {
-            let arg_name = if self.is_positional() && num_vals.min_values() == 0 {
+            let arg_name = if self.is_positional() && (num_vals.min_values() == 0 || !required) {
                 format!("[{}]", val_name)
             } else {
                 format!("<{}>", val_name)
@@ -4098,7 +4099,7 @@ impl Eq for Arg {}
 
 impl Display for Arg {
     fn fmt(&self, f: &mut Formatter) -> fmt::Result {
-        self.stylized().fmt(f)
+        self.stylized(None).fmt(f)
     }
 }
 
@@ -4372,6 +4373,14 @@ mod test {
         let mut p = Arg::new("pos").index(1).num_args(1..);
         p._build();
 
+        assert_eq!(p.to_string(), "[pos]...");
+    }
+
+    #[test]
+    fn positional_display_multiple_values_required() {
+        let mut p = Arg::new("pos").index(1).num_args(1..).required(true);
+        p._build();
+
         assert_eq!(p.to_string(), "<pos>...");
     }
 
@@ -4388,6 +4397,14 @@ mod test {
         let mut p = Arg::new("pos").index(1).num_args(1..);
         p._build();
 
+        assert_eq!(p.to_string(), "[pos]...");
+    }
+
+    #[test]
+    fn positional_display_one_or_more_values_required() {
+        let mut p = Arg::new("pos").index(1).num_args(1..).required(true);
+        p._build();
+
         assert_eq!(p.to_string(), "<pos>...");
     }
 
@@ -4407,6 +4424,17 @@ mod test {
         let mut p = Arg::new("pos").index(1).action(ArgAction::Append);
         p._build();
 
+        assert_eq!(p.to_string(), "[pos]...");
+    }
+
+    #[test]
+    fn positional_display_multiple_occurrences_required() {
+        let mut p = Arg::new("pos")
+            .index(1)
+            .action(ArgAction::Append)
+            .required(true);
+        p._build();
+
         assert_eq!(p.to_string(), "<pos>...");
     }
 
@@ -4423,6 +4451,17 @@ mod test {
         let mut p = Arg::new("pos").index(1).value_names(["file1", "file2"]);
         p._build();
 
+        assert_eq!(p.to_string(), "[file1] [file2]");
+    }
+
+    #[test]
+    fn positional_display_val_names_required() {
+        let mut p = Arg::new("pos")
+            .index(1)
+            .value_names(["file1", "file2"])
+            .required(true);
+        p._build();
+
         assert_eq!(p.to_string(), "<file1> <file2>");
     }
 

diff --git a/src/output/help.rs b/src/output/help.rs
@@ -447,7 +447,7 @@ impl<'cmd, 'writer> Help<'cmd, 'writer> {
         self.none(TAB);
         self.short(arg);
         self.long(arg);
-        self.writer.extend(arg.stylize_arg_suffix().into_iter());
+        self.writer.extend(arg.stylize_arg_suffix(None).into_iter());
         self.align_to_about(arg, next_line_help, longest);
 
         let about = if self.use_long {

diff --git a/src/output/usage.rs b/src/output/usage.rs
@@ -418,10 +418,11 @@ impl<'cmd> Usage<'cmd> {
                 if !is_present {
                     if arg.is_positional() {
                         if incl_last || !arg.is_last_set() {
-                            required_positionals.insert((arg.index.unwrap(), arg.stylized()));
+                            required_positionals
+                                .insert((arg.index.unwrap(), arg.stylized(Some(true))));
                         }
                     } else {
-                        required_opts.insert(arg.stylized());
+                        required_opts.insert(arg.stylized(Some(true)));
                     }
                 }
             } else {
@@ -445,7 +446,7 @@ impl<'cmd> Usage<'cmd> {
                         group_members
                             .iter()
                             .flat_map(|id| self.cmd.find(id))
-                            .map(|arg| arg.stylized()),
+                            .map(|arg| arg.stylized(Some(true))),
                     );
                 }
             }