From dd2c4c6346fa2f9366da7943a1a4f967d96e5b0b Mon Sep 17 00:00:00 2001
From: Ed Page <eopage@gmail.com>
Date: Sat, 30 Oct 2021 13:40:52 -0500
Subject: [PATCH] fix(usage)!: Switch positionals... from multi-val to
 mulit-occur

I noticed this while investigating #2692.  Since we are making
multiple-occurrences a thing for positional arguments, this allows us to
remove a special case.

Another way to look at this is that we should make the default whatever
we do for dervies (#1772).  I'm going to propose we make the derive
always turn `Vec<i32>` into multiple occurences and not multiple values
(with users being able to change it through attributes), but that is an
in-work proposal and not decided yet.

BREAKING CHANGE: `Arg::from(...)` will now use `multiple_occurrences`
for a positional `...`, rather than `multiple_values`.
---
 src/build/usage_parser.rs | 38 ++++++++++++++++----------------------
 1 file changed, 16 insertions(+), 22 deletions(-)

diff --git a/src/build/usage_parser.rs b/src/build/usage_parser.rs
index 40440712..7b2f76cd 100644
--- a/src/build/usage_parser.rs
+++ b/src/build/usage_parser.rs
@@ -64,12 +64,6 @@ impl<'help> UsageParser<'help> {
             }
         }
 
-        if arg.is_positional() && arg.is_set(ArgSettings::MultipleOccurrences) {
-            // We had a positional and need to set mult vals too
-            arg.settings.set(ArgSettings::MultipleValues);
-            arg.settings.unset(ArgSettings::MultipleOccurrences);
-        }
-
         debug!("UsageParser::parse: vals...{:?}", arg.val_names);
         arg
     }
@@ -1109,8 +1103,8 @@ mod test {
         let a = Arg::from("[pos]... 'some help info'");
         assert_eq!(a.name, "pos");
         assert_eq!(a.about.unwrap(), "some help info");
-        assert!(!a.is_set(ArgSettings::MultipleOccurrences));
-        assert!(a.is_set(ArgSettings::MultipleValues));
+        assert!(a.is_set(ArgSettings::MultipleOccurrences));
+        assert!(!a.is_set(ArgSettings::MultipleValues));
         assert!(!a.is_set(ArgSettings::Required));
         assert!(a.val_names.is_empty());
     }
@@ -1120,8 +1114,8 @@ mod test {
         let a = Arg::from("[pos]... 'some help\' info'");
         assert_eq!(a.name, "pos");
         assert_eq!(a.about.unwrap(), "some help' info");
-        assert!(!a.is_set(ArgSettings::MultipleOccurrences));
-        assert!(a.is_set(ArgSettings::MultipleValues));
+        assert!(a.is_set(ArgSettings::MultipleOccurrences));
+        assert!(!a.is_set(ArgSettings::MultipleValues));
         assert!(!a.is_set(ArgSettings::Required));
         assert!(a.val_names.is_empty());
     }
@@ -1131,8 +1125,8 @@ mod test {
         let a = Arg::from("[pos]... 'some \'help\' info'");
         assert_eq!(a.name, "pos");
         assert_eq!(a.about.unwrap(), "some 'help' info");
-        assert!(!a.is_set(ArgSettings::MultipleOccurrences));
-        assert!(a.is_set(ArgSettings::MultipleValues));
+        assert!(a.is_set(ArgSettings::MultipleOccurrences));
+        assert!(!a.is_set(ArgSettings::MultipleValues));
         assert!(!a.is_set(ArgSettings::Required));
         assert!(a.val_names.is_empty());
     }
@@ -1145,8 +1139,8 @@ mod test {
         );
         assert_eq!(a.name, "pos");
         assert_eq!(a.about.unwrap(), "some help\ninfo");
-        assert!(!a.is_set(ArgSettings::MultipleOccurrences));
-        assert!(a.is_set(ArgSettings::MultipleValues));
+        assert!(a.is_set(ArgSettings::MultipleOccurrences));
+        assert!(!a.is_set(ArgSettings::MultipleValues));
         assert!(!a.is_set(ArgSettings::Required));
         assert!(a.val_names.is_empty());
     }
@@ -1159,8 +1153,8 @@ mod test {
         );
         assert_eq!(a.name, "pos");
         assert_eq!(a.about.unwrap(), "some help' stuff\ninfo");
-        assert!(!a.is_set(ArgSettings::MultipleOccurrences));
-        assert!(a.is_set(ArgSettings::MultipleValues));
+        assert!(a.is_set(ArgSettings::MultipleOccurrences));
+        assert!(!a.is_set(ArgSettings::MultipleValues));
         assert!(!a.is_set(ArgSettings::Required));
         assert!(a.val_names.is_empty());
     }
@@ -1170,8 +1164,8 @@ mod test {
         let a = Arg::from("<pos>... 'some help info'");
         assert_eq!(a.name, "pos");
         assert_eq!(a.about.unwrap(), "some help info");
-        assert!(!a.is_set(ArgSettings::MultipleOccurrences));
-        assert!(a.is_set(ArgSettings::MultipleValues));
+        assert!(a.is_set(ArgSettings::MultipleOccurrences));
+        assert!(!a.is_set(ArgSettings::MultipleValues));
         assert!(a.is_set(ArgSettings::Required));
         assert!(a.val_names.is_empty());
     }
@@ -1190,8 +1184,8 @@ mod test {
     fn pos_mult() {
         let a = Arg::from("[pos]...");
         assert_eq!(a.name, "pos");
-        assert!(!a.is_set(ArgSettings::MultipleOccurrences));
-        assert!(a.is_set(ArgSettings::MultipleValues));
+        assert!(a.is_set(ArgSettings::MultipleOccurrences));
+        assert!(!a.is_set(ArgSettings::MultipleValues));
         assert!(!a.is_set(ArgSettings::Required));
         assert!(a.val_names.is_empty());
     }
@@ -1201,8 +1195,8 @@ mod test {
         let a = Arg::from("<pos>... @a 'some help info'");
         assert_eq!(a.name, "pos");
         assert_eq!(a.about.unwrap(), "some help info");
-        assert!(!a.is_set(ArgSettings::MultipleOccurrences));
-        assert!(a.is_set(ArgSettings::MultipleValues));
+        assert!(a.is_set(ArgSettings::MultipleOccurrences));
+        assert!(!a.is_set(ArgSettings::MultipleValues));
         assert!(a.is_set(ArgSettings::Required));
         assert!(a.val_names.is_empty());
         assert_eq!(a.default_vals, vec![std::ffi::OsStr::new("a")]);