From 460459c77148923084cb6579417bd68efd3fe0c1 Mon Sep 17 00:00:00 2001
From: Ajeet D'Souza <98ajeet@gmail.com>
Date: Fri, 21 May 2021 00:29:21 +0530
Subject: [PATCH] fix(usage): Move positional args to end of usage string
 (#2492)

* Move positional args to end of USAGE

* Remove -- from usage string
---
 src/output/usage.rs | 44 +++++++++++++++++++-------------------------
 tests/conflicts.rs  |  4 ++--
 tests/require.rs    |  6 +++---
 3 files changed, 24 insertions(+), 30 deletions(-)

diff --git a/src/output/usage.rs b/src/output/usage.rs
index ee8bb03e..b39ea1ec 100644
--- a/src/output/usage.rs
+++ b/src/output/usage.rs
@@ -408,23 +408,6 @@ impl<'help, 'app, 'parser> Usage<'help, 'app, 'parser> {
             .flat_map(|g| self.p.app.unroll_args_in_group(&g.id))
             .collect::<Vec<_>>();
 
-        let pmap = unrolled_reqs
-            .iter()
-            .chain(incls.iter())
-            .filter(|a| self.p.app.get_positionals().any(|p| &&p.id == a))
-            .filter(|&pos| matcher.map_or(true, |m| !m.contains(pos)))
-            .filter_map(|pos| self.p.app.find(pos))
-            .filter(|&pos| incl_last || !pos.is_set(ArgSettings::Last))
-            .filter(|pos| !args_in_groups.contains(&pos.id))
-            .map(|pos| (pos.index.unwrap(), pos))
-            .collect::<BTreeMap<usize, &Arg>>(); // sort by index
-
-        for p in pmap.values() {
-            debug!("Usage::get_required_usage_from:iter:{:?}", p.id);
-            if !args_in_groups.contains(&p.id) {
-                ret_val.push(p.to_string());
-            }
-        }
         for a in unrolled_reqs
             .iter()
             .chain(incls.iter())
@@ -434,12 +417,7 @@ impl<'help, 'app, 'parser> Usage<'help, 'app, 'parser> {
             .filter(|name| !(matcher.is_some() && matcher.as_ref().unwrap().contains(name)))
         {
             debug!("Usage::get_required_usage_from:iter:{:?}", a);
-            let arg = self
-                .p
-                .app
-                .find(&a)
-                .map(ToString::to_string)
-                .expect(INTERNAL_ERROR_MSG);
+            let arg = self.p.app.find(&a).expect(INTERNAL_ERROR_MSG).to_string();
             ret_val.push(arg);
         }
         let mut g_vec: Vec<String> = vec![];
@@ -465,8 +443,24 @@ impl<'help, 'app, 'parser> Usage<'help, 'app, 'parser> {
                 g_vec.push(elem);
             }
         }
-        for g in g_vec {
-            ret_val.push(g);
+        ret_val.extend_from_slice(&g_vec);
+
+        let pmap = unrolled_reqs
+            .iter()
+            .chain(incls.iter())
+            .filter(|a| self.p.app.get_positionals().any(|p| &&p.id == a))
+            .filter(|&pos| matcher.map_or(true, |m| !m.contains(pos)))
+            .filter_map(|pos| self.p.app.find(pos))
+            .filter(|&pos| incl_last || !pos.is_set(ArgSettings::Last))
+            .filter(|pos| !args_in_groups.contains(&pos.id))
+            .map(|pos| (pos.index.unwrap(), pos))
+            .collect::<BTreeMap<usize, &Arg>>(); // sort by index
+
+        for p in pmap.values() {
+            debug!("Usage::get_required_usage_from:iter:{:?}", p.id);
+            if !args_in_groups.contains(&p.id) {
+                ret_val.push(p.to_string());
+            }
         }
 
         debug!("Usage::get_required_usage_from: ret_val={:?}", ret_val);
diff --git a/tests/conflicts.rs b/tests/conflicts.rs
index a81a2495..d48141db 100644
--- a/tests/conflicts.rs
+++ b/tests/conflicts.rs
@@ -5,14 +5,14 @@ use clap::{App, Arg, ArgGroup, ErrorKind};
 static CONFLICT_ERR: &str = "error: The argument '-F' cannot be used with '--flag'
 
 USAGE:
-    clap-test <positional> <positional2> --flag --long-option-2 <option2>
+    clap-test --flag --long-option-2 <option2> <positional> <positional2>
 
 For more information try --help";
 
 static CONFLICT_ERR_REV: &str = "error: The argument '--flag' cannot be used with '-F'
 
 USAGE:
-    clap-test <positional> <positional2> -F --long-option-2 <option2>
+    clap-test -F --long-option-2 <option2> <positional> <positional2>
 
 For more information try --help";
 
diff --git a/tests/require.rs b/tests/require.rs
index 06eafa98..385d556f 100644
--- a/tests/require.rs
+++ b/tests/require.rs
@@ -28,11 +28,11 @@ USAGE:
 For more information try --help";
 
 static MISSING_REQ: &str = "error: The following required arguments were not provided:
-    <positional2>
     --long-option-2 <option2>
+    <positional2>
 
 USAGE:
-    clap-test <positional2> --long-option-2 <option2> -F
+    clap-test --long-option-2 <option2> -F <positional2>
 
 For more information try --help";
 
@@ -913,7 +913,7 @@ static ISSUE_1158: &str = "error: The following required arguments were not prov
     -z <Z>
 
 USAGE:
-    example <ID> -x <X> -y <Y> -z <Z>
+    example -x <X> -y <Y> -z <Z> <ID>
 
 For more information try --help";