Merge topic 'cmCommandLineArgument_understands_exact_versus_matching_variables'

396e0a840e cmCommandLineArgument: OneOrMore mode supports `=` separator
372bf1bcc4 cmCommandLineArgument: Understands which commands require partial matching

Acked-by: Kitware Robot <[email protected]>
Merge-request: !6142

Source/cmCommandLineArgument.h

@@ -17,10 +17,25 @@ struct cmCommandLineArgument
     OneOrMore
   };
 
+  enum class RequiresSeparator
+  {
+    Yes,
+    No
+  };
+
+  enum class ParseMode
+  {
+    Valid,
+    Invalid,
+    SyntaxError,
+    ValueError
+  };
+
   std::string InvalidSyntaxMessage;
   std::string InvalidValueMessage;
   std::string Name;
   Values Type;
+  RequiresSeparator SeparatorNeeded;
   std::function<FunctionSignature> StoreCall;
 
   template <typename FunctionType>
@@ -29,6 +44,19 @@ struct cmCommandLineArgument
     , InvalidValueMessage(cmStrCat("Invalid value used with ", n))
     , Name(std::move(n))
     , Type(t)
+    , SeparatorNeeded(RequiresSeparator::Yes)
+    , StoreCall(std::forward<FunctionType>(func))
+  {
+  }
+
+  template <typename FunctionType>
+  cmCommandLineArgument(std::string n, Values t, RequiresSeparator s,
+                        FunctionType&& func)
+    : InvalidSyntaxMessage(cmStrCat(" is invalid syntax for ", n))
+    , InvalidValueMessage(cmStrCat("Invalid value used with ", n))
+    , Name(std::move(n))
+    , Type(t)
+    , SeparatorNeeded(s)
     , StoreCall(std::forward<FunctionType>(func))
   {
   }
@@ -40,14 +68,39 @@ struct cmCommandLineArgument
     , InvalidValueMessage(std::move(failedMsg))
     , Name(std::move(n))
     , Type(t)
+    , SeparatorNeeded(RequiresSeparator::Yes)
+    , StoreCall(std::forward<FunctionType>(func))
+  {
+  }
+
+  template <typename FunctionType>
+  cmCommandLineArgument(std::string n, std::string failedMsg, Values t,
+                        RequiresSeparator s, FunctionType&& func)
+    : InvalidSyntaxMessage(cmStrCat(" is invalid syntax for ", n))
+    , InvalidValueMessage(std::move(failedMsg))
+    , Name(std::move(n))
+    , Type(t)
+    , SeparatorNeeded(s)
     , StoreCall(std::forward<FunctionType>(func))
   {
   }
 
   bool matches(std::string const& input) const
   {
-    return (this->Type == Values::Zero) ? (input == this->Name)
-                                        : cmHasPrefix(input, this->Name);
+    bool matched = false;
+    if (this->Type == Values::Zero) {
+      matched = (input == this->Name);
+    } else if (this->SeparatorNeeded == RequiresSeparator::No) {
+      matched = cmHasPrefix(input, this->Name);
+    } else if (cmHasPrefix(input, this->Name)) {
+      if (input.size() == this->Name.size()) {
+        matched = true;
+      } else {
+        matched =
+          (input[this->Name.size()] == '=' || input[this->Name.size()] == ' ');
+      }
+    }
+    return matched;
   }
 
   template <typename T, typename... CallState>
@@ -55,13 +108,6 @@ struct cmCommandLineArgument
              std::vector<std::string> const& allArgs,
              CallState&&... state) const
   {
-    enum class ParseMode
-    {
-      Valid,
-      Invalid,
-      SyntaxError,
-      ValueError
-    };
     ParseMode parseState = ParseMode::Valid;
 
     if (this->Type == Values::Zero) {
@@ -95,23 +141,10 @@ struct cmCommandLineArgument
           index = nextValueIndex;
         }
       } else {
-        // parse the string to get the value
-        auto possible_value = cm::string_view(input).substr(this->Name.size());
-        if (possible_value.empty()) {
-          parseState = ParseMode::ValueError;
-        } else if (possible_value[0] == '=') {
-          possible_value.remove_prefix(1);
-          if (possible_value.empty()) {
-            parseState = ParseMode::ValueError;
-          }
-        }
+        auto value = this->extract_single_value(input, parseState);
         if (parseState == ParseMode::Valid) {
-          if (possible_value[0] == ' ') {
-            possible_value.remove_prefix(1);
-          }
-
-          parseState = this->StoreCall(std::string(possible_value),
-                                       std::forward<CallState>(state)...)
+          parseState =
+            this->StoreCall(value, std::forward<CallState>(state)...)
             ? ParseMode::Valid
             : ParseMode::Invalid;
         }
@@ -149,7 +182,13 @@ struct cmCommandLineArgument
           index = (nextValueIndex - 1);
         }
       } else {
-        parseState = ParseMode::SyntaxError;
+        auto value = this->extract_single_value(input, parseState);
+        if (parseState == ParseMode::Valid) {
+          parseState =
+            this->StoreCall(value, std::forward<CallState>(state)...)
+            ? ParseMode::Valid
+            : ParseMode::Invalid;
+        }
       }
     }
 
@@ -161,4 +200,24 @@ struct cmCommandLineArgument
     }
     return (parseState == ParseMode::Valid);
   }
+
+private:
+  std::string extract_single_value(std::string const& input,
+                                   ParseMode& parseState) const
+  {
+    // parse the string to get the value
+    auto possible_value = cm::string_view(input).substr(this->Name.size());
+    if (possible_value.empty()) {
+      parseState = ParseMode::ValueError;
+    } else if (possible_value[0] == '=') {
+      possible_value.remove_prefix(1);
+      if (possible_value.empty()) {
+        parseState = ParseMode::ValueError;
+      }
+    }
+    if (parseState == ParseMode::Valid && possible_value[0] == ' ') {
+      possible_value.remove_prefix(1);
+    }
+    return std::string(possible_value);
+  }
 };

Source/cmake.cxx

@@ -529,25 +529,29 @@ bool cmake::SetCacheArgs(const std::vector<std::string>& args)
 
   std::vector<CommandArgument> arguments = {
     CommandArgument{ "-D", "-D must be followed with VAR=VALUE.",
-                     CommandArgument::Values::One, DefineLambda },
+                     CommandArgument::Values::One,
+                     CommandArgument::RequiresSeparator::No, DefineLambda },
     CommandArgument{ "-W", "-W must be followed with [no-]<name>.",
-                     CommandArgument::Values::One, WarningLambda },
+                     CommandArgument::Values::One,
+                     CommandArgument::RequiresSeparator::No, WarningLambda },
     CommandArgument{ "-U", "-U must be followed with VAR.",
-                     CommandArgument::Values::One, UnSetLambda },
-    CommandArgument{ "-C", "-C must be followed by a file name.",
                      CommandArgument::Values::One,
-                     [&](std::string const& value, cmake* state) -> bool {
-                       cmSystemTools::Stdout("loading initial cache file " +
-                                             value + "\n");
-                       // Resolve script path specified on command line
-                       // relative to $PWD.
-                       auto path = cmSystemTools::CollapseFullPath(value);
-                       state->ReadListFile(args, path);
-                       return true;
-                     } },
+                     CommandArgument::RequiresSeparator::No, UnSetLambda },
+    CommandArgument{
+      "-C", "-C must be followed by a file name.",
+      CommandArgument::Values::One, CommandArgument::RequiresSeparator::No,
+      [&](std::string const& value, cmake* state) -> bool {
+        cmSystemTools::Stdout("loading initial cache file " + value + "\n");
+        // Resolve script path specified on command line
+        // relative to $PWD.
+        auto path = cmSystemTools::CollapseFullPath(value);
+        state->ReadListFile(args, path);
+        return true;
+      } },
 
     CommandArgument{ "-P", "-P must be followed by a file name.",
-                     CommandArgument::Values::One, ScriptLambda },
+                     CommandArgument::Values::One,
+                     CommandArgument::RequiresSeparator::No, ScriptLambda },
     CommandArgument{ "--toolchain", "No file specified for --toolchain",
                      CommandArgument::Values::One, ToolchainLambda },
     CommandArgument{ "--install-prefix",
@@ -830,31 +834,44 @@ void cmake::SetArgs(const std::vector<std::string>& args)
 
   std::vector<CommandArgument> arguments = {
     CommandArgument{ "-S", "No source directory specified for -S",
-                     CommandArgument::Values::One, SourceArgLambda },
+                     CommandArgument::Values::One,
+                     CommandArgument::RequiresSeparator::No, SourceArgLambda },
     CommandArgument{ "-H", "No source directory specified for -H",
-                     CommandArgument::Values::One, SourceArgLambda },
+                     CommandArgument::Values::One,
+                     CommandArgument::RequiresSeparator::No, SourceArgLambda },
     CommandArgument{ "-O", CommandArgument::Values::Zero,
                      IgnoreAndTrueLambda },
     CommandArgument{ "-B", "No build directory specified for -B",
-                     CommandArgument::Values::One, BuildArgLambda },
+                     CommandArgument::Values::One,
+                     CommandArgument::RequiresSeparator::No, BuildArgLambda },
     CommandArgument{ "-P", "-P must be followed by a file name.",
                      CommandArgument::Values::One,
+                     CommandArgument::RequiresSeparator::No,
                      [&](std::string const&, cmake*) -> bool {
                        scriptMode = true;
                        return true;
                      } },
     CommandArgument{ "-D", "-D must be followed with VAR=VALUE.",
-                     CommandArgument::Values::One, IgnoreAndTrueLambda },
+                     CommandArgument::Values::One,
+                     CommandArgument::RequiresSeparator::No,
+                     IgnoreAndTrueLambda },
     CommandArgument{ "-C", "-C must be followed by a file name.",
-                     CommandArgument::Values::One, IgnoreAndTrueLambda },
-    CommandArgument{ "-U", "-U must be followed with VAR.",
-                     CommandArgument::Values::One, IgnoreAndTrueLambda },
+                     CommandArgument::Values::One,
+                     CommandArgument::RequiresSeparator::No,
+                     IgnoreAndTrueLambda },
+    CommandArgument{
+      "-U", "-U must be followed with VAR.", CommandArgument::Values::One,
+      CommandArgument::RequiresSeparator::No, IgnoreAndTrueLambda },
     CommandArgument{ "-W", "-W must be followed with [no-]<name>.",
-                     CommandArgument::Values::One, IgnoreAndTrueLambda },
+                     CommandArgument::Values::One,
+                     CommandArgument::RequiresSeparator::No,
+                     IgnoreAndTrueLambda },
     CommandArgument{ "-A", "No platform specified for -A",
-                     CommandArgument::Values::One, PlatformLambda },
+                     CommandArgument::Values::One,
+                     CommandArgument::RequiresSeparator::No, PlatformLambda },
     CommandArgument{ "-T", "No toolset specified for -T",
-                     CommandArgument::Values::One, ToolsetLamda },
+                     CommandArgument::Values::One,
+                     CommandArgument::RequiresSeparator::No, ToolsetLamda },
     CommandArgument{ "--toolchain", "No file specified for --toolchain",
                      CommandArgument::Values::One, IgnoreAndTrueLambda },
     CommandArgument{ "--install-prefix",
@@ -1079,6 +1096,7 @@ void cmake::SetArgs(const std::vector<std::string>& args)
   bool badGeneratorName = false;
   CommandArgument generatorCommand(
     "-G", "No generator specified for -G", CommandArgument::Values::One,
+    CommandArgument::RequiresSeparator::No,
     [&](std::string const& value, cmake* state) -> bool {
       bool valid = state->CreateAndSetGlobalGenerator(value, true);
       badGeneratorName = !valid;

Source/cmakemain.cxx

@@ -271,6 +271,7 @@ int do_cmake(int ac, char const* const* av)
                      } },
     CommandArgument{ "-P", "No script specified for argument -P",
                      CommandArgument::Values::One,
+                     CommandArgument::RequiresSeparator::No,
                      [&](std::string const& value) -> bool {
                        workingMode = cmake::SCRIPT_MODE;
                        parsedArgs.emplace_back("-P");
@@ -476,9 +477,10 @@ int do_build(int ac, char const* const* av)
                        listPresets = true;
                        return true;
                      } },
-    CommandArgument{ "-j", CommandArgument::Values::ZeroOrOne, jLambda },
+    CommandArgument{ "-j", CommandArgument::Values::ZeroOrOne,
+                     CommandArgument::RequiresSeparator::No, jLambda },
     CommandArgument{ "--parallel", CommandArgument::Values::ZeroOrOne,
-                     parallelLambda },
+                     CommandArgument::RequiresSeparator::No, parallelLambda },
     CommandArgument{ "-t", CommandArgument::Values::OneOrMore, targetLambda },
     CommandArgument{ "--target", CommandArgument::Values::OneOrMore,
                      targetLambda },

Tests/RunCMake/CommandLine/build-invalid-target-syntax-stderr.txt

@@ -1,2 +1 @@
-^CMake Error: '--target=invalid' is invalid syntax for --target
-Usage: cmake --build \[<dir> \| --preset <preset>\] \[options\] \[-- \[native-options\]\]
+^Error: could not load cache

Tests/RunCMake/CommandLine/build-unknown-command-partial-match-stderr.txt

@@ -1,2 +1,2 @@
-^CMake Error: '--targetinvalid' is invalid syntax for --target
+^Unknown argument --targetinvalid
 Usage: cmake --build \[<dir> \| --preset <preset>\] \[options\] \[-- \[native-options\]\]