Merge pull request #2682 from clap-rs/checker

Set TakesValue to true for positional args when building

src/build/app/debug_asserts.rs

@@ -200,7 +200,7 @@ pub(crate) fn assert_app(app: &App) {
 
         if arg.value_hint == ValueHint::CommandWithArguments {
             assert!(
-                arg.short.is_none() && arg.long.is_none(),
+                arg.is_positional(),
                 "Argument '{}' has hint CommandWithArguments and must be positional.",
                 arg.name
             );

src/build/app/mod.rs

@@ -2276,7 +2276,7 @@ impl<'help> App<'help> {
                     self.settings.set(AppSettings::DontCollapseArgsInUsage);
                 }
                 a._build();
-                if a.short.is_none() && a.long.is_none() && a.index.is_none() {
+                if a.is_positional() && a.index.is_none() {
                     a.index = Some(pos_counter);
                     pos_counter += 1;
                 }
@@ -2496,7 +2496,7 @@ impl<'help> App<'help> {
             for (i, a) in self
                 .args
                 .args_mut()
-                .filter(|a| a.has_switch())
+                .filter(|a| !a.is_positional())
                 .filter(|a| a.disp_ord == 999)
                 .enumerate()
             {

src/build/arg/debug_asserts.rs

@@ -29,7 +29,7 @@ pub(crate) fn assert_arg(arg: &Arg) {
 
     if arg.index.is_some() {
         assert!(
-            !arg.has_switch(),
+            arg.is_positional(),
             "Argument '{}' is a positional argument and can't have short or long name versions",
             arg.name
         );

src/build/arg/mod.rs

@@ -4623,24 +4623,24 @@ impl<'help> Arg<'help> {
     }
 
     // FIXME: (@CreepySkeleton)
-    #[doc(hidden)]
-    pub fn _build(&mut self) {
+    pub(crate) fn _build(&mut self) {
         if (self.is_set(ArgSettings::UseValueDelimiter)
             || self.is_set(ArgSettings::RequireDelimiter))
             && self.val_delim.is_none()
         {
             self.val_delim = Some(',');
         }
-        if !(self.index.is_some() || (self.short.is_none() && self.long.is_none()))
+
+        if !(self.index.is_some() || self.is_positional())
             && self.is_set(ArgSettings::TakesValue)
             && self.val_names.len() > 1
         {
             self.num_vals = Some(self.val_names.len());
         }
-    }
 
-    pub(crate) fn has_switch(&self) -> bool {
-        self.short.is_some() || self.long.is_some()
+        if self.is_positional() {
+            self.settings.set(ArgSettings::TakesValue);
+        }
     }
 
     pub(crate) fn longest_filter(&self) -> bool {
@@ -4847,7 +4847,7 @@ impl<'help> PartialEq for Arg<'help> {
 
 impl<'help> Display for Arg<'help> {
     fn fmt(&self, f: &mut Formatter) -> fmt::Result {
-        if self.index.is_some() || (self.long.is_none() && self.short.is_none()) {
+        if self.index.is_some() || self.is_positional() {
             // Positional
             let mut delim = String::new();
 

src/build/usage_parser.rs

@@ -69,9 +69,8 @@ impl<'help> UsageParser<'help> {
         } else {
             None
         };
-        if !arg.has_switch() && arg.is_set(ArgSettings::MultipleOccurrences) {
+        if arg.is_positional() && arg.is_set(ArgSettings::MultipleOccurrences) {
             // We had a positional and need to set mult vals too
-            arg.settings.set(ArgSettings::TakesValue);
             arg.settings.set(ArgSettings::MultipleValues);
             arg.settings.unset(ArgSettings::MultipleOccurrences);
         }

src/output/help.rs

@@ -284,7 +284,7 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> {
 
         if let Some(s) = arg.short {
             self.good(&format!("-{}", s))
-        } else if arg.has_switch() {
+        } else if !arg.is_positional() {
             self.none(TAB)
         } else {
             Ok(())
@@ -294,7 +294,7 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> {
     /// Writes argument's long command to the wrapped stream.
     fn long(&mut self, arg: &Arg<'help>) -> io::Result<()> {
         debug!("Help::long");
-        if !arg.has_switch() {
+        if arg.is_positional() {
             return Ok(());
         }
         if arg.is_set(ArgSettings::TakesValue) {
@@ -372,7 +372,7 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> {
                 if mult && num == 1 {
                     self.good("...")?;
                 }
-            } else if arg.has_switch() {
+            } else if !arg.is_positional() {
                 self.good(&format!("<{}>", arg.name))?;
                 if mult {
                     self.good("...")?;
@@ -386,7 +386,7 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> {
         if self.use_long {
             // long help prints messages on the next line so it don't need to align text
             debug!("Help::val: printing long help so skip alignment");
-        } else if arg.has_switch() {
+        } else if !arg.is_positional() {
             debug!("Yes");
             debug!("Help::val: nlh...{:?}", next_line_help);
             if !next_line_help {
@@ -451,7 +451,7 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> {
     /// Writes argument's help to the wrapped stream.
     fn help(
         &mut self,
-        has_switch: bool,
+        is_not_positional: bool,
         about: &str,
         spec_vals: &str,
         next_line_help: bool,
@@ -493,7 +493,7 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> {
             self.none("\n")?;
             if next_line_help {
                 self.none(&format!("{}{}{}", TAB, TAB, TAB))?;
-            } else if has_switch {
+            } else if is_not_positional {
                 self.spaces(longest + 12)?;
             } else {
                 self.spaces(longest + 8)?;
@@ -521,7 +521,13 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> {
             arg.about.unwrap_or_else(|| arg.long_about.unwrap_or(""))
         };
 
-        self.help(arg.has_switch(), about, spec_vals, next_line_help, longest)?;
+        self.help(
+            !arg.is_positional(),
+            about,
+            spec_vals,
+            next_line_help,
+            longest,
+        )?;
         Ok(())
     }
 
@@ -831,7 +837,7 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> {
                 .app
                 .args
                 .args()
-                .filter(|a| a.has_switch())
+                .filter(|a| !a.is_positional())
                 .collect::<Vec<_>>();
             if !first {
                 self.none("\n\n")?;
@@ -1061,7 +1067,7 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> {
                             .app
                             .args
                             .args()
-                            .filter(|a| a.has_switch())
+                            .filter(|a| !a.is_positional())
                             .collect::<Vec<_>>();
                         self.write_args(&opts_flags)?;
                     }

src/parse/parser.rs

@@ -1613,7 +1613,7 @@ impl<'help, 'app> Parser<'help, 'app> {
                 if (self.is_set(AS::AllArgsOverrideSelf) || override_self)
                     && !overrider.is_set(ArgSettings::MultipleValues)
                     && !overrider.is_set(ArgSettings::MultipleOccurrences)
-                    && overrider.has_switch()
+                    && !overrider.is_positional()
                 {
                     debug!(
                         "Parser::remove_overrides:iter:{:?}:iter:{:?}: self override",

tests/env.rs

@@ -28,9 +28,9 @@ fn env_bool_literal() {
     env::set_var("CLP_TEST_FLAG_FALSE", "nO");
 
     let r = App::new("df")
-        .arg(Arg::from("[present] 'some opt'").env("CLP_TEST_FLAG_TRUE"))
-        .arg(Arg::from("[negated] 'some another opt'").env("CLP_TEST_FLAG_FALSE"))
-        .arg(Arg::from("[absent] 'some third opt'").env("CLP_TEST_FLAG_ABSENT"))
+        .arg(Arg::new("present").short('p').env("CLP_TEST_FLAG_TRUE"))
+        .arg(Arg::new("negated").short('n').env("CLP_TEST_FLAG_FALSE"))
+        .arg(Arg::new("absent").short('a').env("CLP_TEST_FLAG_ABSENT"))
         .try_get_matches_from(vec![""]);
 
     assert!(r.is_ok());