Merge topic 'command-arg-parser'

2eba10c5ee cmArgumentParser: Drop unused parsedKeywords argument to Parse()
98cf623821 cmCTestHandlerCommand: Capture list of parsed keywords via binding
6ecd741b5f cmFileCommand: Capture list of parsed keywords via binding
f7e81802f2 cmArgumentParser: Offer binding for list of parsed keywords
f95a5832c7 cmArgumentParser: Drop unused keywordsMissingValue argument to Parse()
9a7efb6813 cmArgumentParser: Offer private binding to cmParseArgumentsCommand

Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: buildbot <buildbot@kitware.com>
Merge-request: !7508

7 files changed, 102 insertions, 58 deletions

diff --git a/Source/CTest/cmCTestHandlerCommand.cxx b/Source/CTest/cmCTestHandlerCommand.cxx
index af365ad..be952cd 100644
--- a/Source/CTest/cmCTestHandlerCommand.cxx
+++ b/Source/CTest/cmCTestHandlerCommand.cxx
@@ -82,14 +82,13 @@ bool cmCTestHandlerCommand::InitialPass(std::vector<std::string> const& args,
 
   // Process input arguments.
   std::vector<std::string> unparsedArguments;
-  std::vector<cm::string_view> parsedKeywords;
-  this->Parse(args, &unparsedArguments, /*keywordsMissingValue=*/nullptr,
-              &parsedKeywords);
+  this->Parse(args, &unparsedArguments);
   this->CheckArguments();
 
-  std::sort(parsedKeywords.begin(), parsedKeywords.end());
-  auto it = std::adjacent_find(parsedKeywords.begin(), parsedKeywords.end());
-  if (it != parsedKeywords.end()) {
+  std::sort(this->ParsedKeywords.begin(), this->ParsedKeywords.end());
+  auto it = std::adjacent_find(this->ParsedKeywords.begin(),
+                               this->ParsedKeywords.end());
+  if (it != this->ParsedKeywords.end()) {
     this->Makefile->IssueMessage(
       MessageType::FATAL_ERROR,
       cmStrCat("Called with more than one value for ", *it));
@@ -233,6 +232,7 @@ void cmCTestHandlerCommand::ProcessAdditionalValues(cmCTestGenericHandler*)
 
 void cmCTestHandlerCommand::BindArguments()
 {
+  this->BindParsedKeywords(this->ParsedKeywords);
   this->Bind("APPEND"_s, this->Append);
   this->Bind("QUIET"_s, this->Quiet);
   this->Bind("RETURN_VALUE"_s, this->ReturnValue);
diff --git a/Source/CTest/cmCTestHandlerCommand.h b/Source/CTest/cmCTestHandlerCommand.h
index 31942c8..ed6d9af 100644
--- a/Source/CTest/cmCTestHandlerCommand.h
+++ b/Source/CTest/cmCTestHandlerCommand.h
@@ -7,6 +7,8 @@
 #include <string>
 #include <vector>
 
+#include <cm/string_view>
+
 #include "cmArgumentParser.h"
 #include "cmCTestCommand.h"
 
@@ -44,6 +46,7 @@ protected:
   virtual void BindArguments();
   virtual void CheckArguments();
 
+  std::vector<cm::string_view> ParsedKeywords;
   bool Append = false;
   bool Quiet = false;
   std::string CaptureCMakeError;
diff --git a/Source/cmArgumentParser.cxx b/Source/cmArgumentParser.cxx
index 25d5c68..4379b29 100644
--- a/Source/cmArgumentParser.cxx
+++ b/Source/cmArgumentParser.cxx
@@ -83,8 +83,8 @@ void Instance::Consume(cm::string_view arg)
   if (it != this->Bindings.Keywords.end()) {
     this->FinishKeyword();
     this->Keyword = it->first;
-    if (this->ParsedKeywords != nullptr) {
-      this->ParsedKeywords->emplace_back(it->first);
+    if (this->Bindings.ParsedKeyword) {
+      this->Bindings.ParsedKeyword(*this, it->first);
     }
     it->second(*this);
     return;
@@ -113,8 +113,8 @@ void Instance::FinishKeyword()
       this->ParseResults->AddKeywordError(this->Keyword,
                                           "  missing required value\n");
     }
-    if (this->KeywordsMissingValue != nullptr) {
-      this->KeywordsMissingValue->emplace_back(this->Keyword);
+    if (this->Bindings.KeywordMissingValue) {
+      this->Bindings.KeywordMissingValue(*this, this->Keyword);
     }
   }
 }
diff --git a/Source/cmArgumentParser.h b/Source/cmArgumentParser.h
index 8fda8b7..33b8fff 100644
--- a/Source/cmArgumentParser.h
+++ b/Source/cmArgumentParser.h
@@ -64,6 +64,7 @@ AsParseResultPtr(Result&)
 
 class Instance;
 using KeywordAction = std::function<void(Instance&)>;
+using KeywordNameAction = std::function<void(Instance&, cm::string_view)>;
 
 // using KeywordActionMap = cm::flat_map<cm::string_view, KeywordAction>;
 class KeywordActionMap
@@ -79,6 +80,8 @@ class ActionMap
 {
 public:
   KeywordActionMap Keywords;
+  KeywordNameAction KeywordMissingValue;
+  KeywordNameAction ParsedKeyword;
 };
 
 class Base
@@ -100,21 +103,28 @@ public:
     assert(inserted);
     static_cast<void>(inserted);
   }
+
+  void BindParsedKeyword(KeywordNameAction action)
+  {
+    assert(!this->Bindings.ParsedKeyword);
+    this->Bindings.ParsedKeyword = std::move(action);
+  }
+
+  void BindKeywordMissingValue(KeywordNameAction action)
+  {
+    assert(!this->Bindings.KeywordMissingValue);
+    this->Bindings.KeywordMissingValue = std::move(action);
+  }
 };
 
 class Instance
 {
 public:
   Instance(ActionMap const& bindings, ParseResult* parseResult,
-           std::vector<std::string>* unparsedArguments,
-           std::vector<cm::string_view>* keywordsMissingValue,
-           std::vector<cm::string_view>* parsedKeywords,
-           void* result = nullptr)
+           std::vector<std::string>* unparsedArguments, void* result = nullptr)
     : Bindings(bindings)
     , ParseResults(parseResult)
     , UnparsedArguments(unparsedArguments)
-    , KeywordsMissingValue(keywordsMissingValue)
-    , ParsedKeywords(parsedKeywords)
     , Result(result)
   {
   }
@@ -149,8 +159,6 @@ private:
   ActionMap const& Bindings;
   ParseResult* ParseResults = nullptr;
   std::vector<std::string>* UnparsedArguments = nullptr;
-  std::vector<cm::string_view>* KeywordsMissingValue = nullptr;
-  std::vector<cm::string_view>* ParsedKeywords = nullptr;
   void* Result = nullptr;
 
   cm::string_view Keyword;
@@ -182,28 +190,34 @@ public:
     return *this;
   }
 
+  cmArgumentParser& BindParsedKeywords(
+    std::vector<cm::string_view> Result::*member)
+  {
+    this->Base::BindParsedKeyword(
+      [member](Instance& instance, cm::string_view arg) {
+        (static_cast<Result*>(instance.Result)->*member).emplace_back(arg);
+      });
+    return *this;
+  }
+
   template <typename Range>
   bool Parse(Result& result, Range const& args,
-             std::vector<std::string>* unparsedArguments,
-             std::vector<cm::string_view>* keywordsMissingValue = nullptr,
-             std::vector<cm::string_view>* parsedKeywords = nullptr) const
+             std::vector<std::string>* unparsedArguments) const
   {
     using ArgumentParser::AsParseResultPtr;
     ParseResult* parseResultPtr = AsParseResultPtr(result);
     Instance instance(this->Bindings, parseResultPtr, unparsedArguments,
-                      keywordsMissingValue, parsedKeywords, &result);
+                      &result);
     instance.Parse(args);
     return parseResultPtr ? static_cast<bool>(*parseResultPtr) : true;
   }
 
   template <typename Range>
-  Result Parse(Range const& args, std::vector<std::string>* unparsedArguments,
-               std::vector<cm::string_view>* keywordsMissingValue = nullptr,
-               std::vector<cm::string_view>* parsedKeywords = nullptr) const
+  Result Parse(Range const& args,
+               std::vector<std::string>* unparsedArguments) const
   {
     Result result;
-    this->Parse(result, args, unparsedArguments, keywordsMissingValue,
-                parsedKeywords);
+    this->Parse(result, args, unparsedArguments);
     return result;
   }
 };
@@ -219,20 +233,27 @@ public:
     return *this;
   }
 
+  cmArgumentParser& BindParsedKeywords(std::vector<cm::string_view>& ref)
+  {
+    this->Base::BindParsedKeyword(
+      [&ref](Instance&, cm::string_view arg) { ref.emplace_back(arg); });
+    return *this;
+  }
+
   template <typename Range>
-  ParseResult Parse(
-    Range const& args, std::vector<std::string>* unparsedArguments,
-    std::vector<cm::string_view>* keywordsMissingValue = nullptr,
-    std::vector<cm::string_view>* parsedKeywords = nullptr) const
+  ParseResult Parse(Range const& args,
+                    std::vector<std::string>* unparsedArguments) const
   {
     ParseResult parseResult;
-    Instance instance(this->Bindings, &parseResult, unparsedArguments,
-                      keywordsMissingValue, parsedKeywords);
+    Instance instance(this->Bindings, &parseResult, unparsedArguments);
     instance.Parse(args);
     return parseResult;
   }
 
 protected:
+  using Base::Instance;
+  using Base::BindKeywordMissingValue;
+
   template <typename T>
   bool Bind(cm::string_view name, T& ref)
   {
diff --git a/Source/cmFileCommand.cxx b/Source/cmFileCommand.cxx
index c7ba424..330d991 100644
--- a/Source/cmFileCommand.cxx
+++ b/Source/cmFileCommand.cxx
@@ -2482,6 +2482,7 @@ bool HandleGenerateCommand(std::vector<std::string> const& args,
     bool NoSourcePermissions = false;
     bool UseSourcePermissions = false;
     ArgumentParser::NonEmpty<std::vector<std::string>> FilePermissions;
+    std::vector<cm::string_view> ParsedKeywords;
   };
 
   static auto const parser =
@@ -2494,13 +2495,12 @@ bool HandleGenerateCommand(std::vector<std::string> const& args,
       .Bind("NO_SOURCE_PERMISSIONS"_s, &Arguments::NoSourcePermissions)
       .Bind("USE_SOURCE_PERMISSIONS"_s, &Arguments::UseSourcePermissions)
       .Bind("FILE_PERMISSIONS"_s, &Arguments::FilePermissions)
-      .Bind("NEWLINE_STYLE"_s, &Arguments::NewLineStyle);
+      .Bind("NEWLINE_STYLE"_s, &Arguments::NewLineStyle)
+      .BindParsedKeywords(&Arguments::ParsedKeywords);
 
   std::vector<std::string> unparsedArguments;
-  std::vector<cm::string_view> parsedKeywords;
   Arguments const arguments =
-    parser.Parse(cmMakeRange(args).advance(1), &unparsedArguments,
-                 /*keywordsMissingValue=*/nullptr, &parsedKeywords);
+    parser.Parse(cmMakeRange(args).advance(1), &unparsedArguments);
 
   if (arguments.MaybeReportError(status.GetMakefile())) {
     return true;
@@ -2511,7 +2511,7 @@ bool HandleGenerateCommand(std::vector<std::string> const& args,
     return false;
   }
 
-  if (!arguments.Output || parsedKeywords[0] != "OUTPUT"_s) {
+  if (!arguments.Output || arguments.ParsedKeywords[0] != "OUTPUT"_s) {
     status.SetError("GENERATE requires OUTPUT as first option.");
     return false;
   }
@@ -2521,8 +2521,8 @@ bool HandleGenerateCommand(std::vector<std::string> const& args,
     status.SetError("GENERATE requires INPUT or CONTENT option.");
     return false;
   }
-  const bool inputIsContent = parsedKeywords[1] == "CONTENT"_s;
-  if (!inputIsContent && parsedKeywords[1] == "INPUT") {
+  const bool inputIsContent = arguments.ParsedKeywords[1] == "CONTENT"_s;
+  if (!inputIsContent && arguments.ParsedKeywords[1] == "INPUT") {
     status.SetError("Unknown argument to GENERATE subcommand.");
   }
   std::string const& input =
diff --git a/Source/cmParseArgumentsCommand.cxx b/Source/cmParseArgumentsCommand.cxx
index 31538b6..7e19566 100644
--- a/Source/cmParseArgumentsCommand.cxx
+++ b/Source/cmParseArgumentsCommand.cxx
@@ -48,6 +48,12 @@ using options_set = std::set<cm::string_view>;
 
 struct UserArgumentParser : public cmArgumentParser<void>
 {
+  void BindKeywordsMissingValue(std::vector<cm::string_view>& ref)
+  {
+    this->cmArgumentParser<void>::BindKeywordMissingValue(
+      [&ref](Instance&, cm::string_view arg) { ref.emplace_back(arg); });
+  }
+
   template <typename T, typename H>
   void Bind(std::vector<std::string> const& names,
             std::map<std::string, T>& ref, H duplicateKey)
@@ -211,8 +217,9 @@ bool cmParseArgumentsCommand(std::vector<std::string> const& args,
   }
 
   std::vector<cm::string_view> keywordsMissingValues;
+  parser.BindKeywordsMissingValue(keywordsMissingValues);
 
-  parser.Parse(list, &unparsed, &keywordsMissingValues);
+  parser.Parse(list, &unparsed);
 
   PassParsedArguments(
     prefix, status.GetMakefile(), options, singleValArgs, multiValArgs,
diff --git a/Tests/CMakeLib/testArgumentParser.cxx b/Tests/CMakeLib/testArgumentParser.cxx
index f0ace50..5460356 100644
--- a/Tests/CMakeLib/testArgumentParser.cxx
+++ b/Tests/CMakeLib/testArgumentParser.cxx
@@ -38,6 +38,8 @@ struct Result : public ArgumentParser::ParseResult
   std::vector<std::vector<std::string>> Multi2;
   cm::optional<std::vector<std::vector<std::string>>> Multi3;
   cm::optional<std::vector<std::vector<std::string>>> Multi4;
+
+  std::vector<cm::string_view> ParsedKeywords;
 };
 
 std::initializer_list<cm::string_view> const args = {
@@ -63,14 +65,27 @@ std::initializer_list<cm::string_view> const args = {
 };
 
 bool verifyResult(Result const& result,
-                  std::vector<std::string> const& unparsedArguments,
-                  std::vector<cm::string_view> const& keywordsMissingValue)
+                  std::vector<std::string> const& unparsedArguments)
 {
   static std::vector<std::string> const foobar = { "foo", "bar" };
   static std::vector<std::string> const barfoo = { "bar", "foo" };
-  static std::vector<cm::string_view> const missing = { "STRING_1"_s,
-                                                        "LIST_1"_s,
-                                                        "LIST_4"_s };
+  static std::vector<cm::string_view> const parsedKeywords = {
+    /* clang-format off */
+    "OPTION_1",
+    "STRING_1",
+    "STRING_2",
+    "STRING_4",
+    "LIST_1",
+    "LIST_2",
+    "LIST_3",
+    "LIST_3",
+    "LIST_4",
+    "LIST_6",
+    "MULTI_2",
+    "MULTI_3",
+    "MULTI_3",
+    /* clang-format on */
+  };
   static std::map<cm::string_view, std::string> const keywordErrors = {
     { "STRING_1"_s, "  missing required value\n" },
     { "LIST_1"_s, "  missing required value\n" },
@@ -117,7 +132,8 @@ bool verifyResult(Result const& result,
 
   ASSERT_TRUE(unparsedArguments.size() == 1);
   ASSERT_TRUE(unparsedArguments[0] == "bar");
-  ASSERT_TRUE(keywordsMissingValue == missing);
+
+  ASSERT_TRUE(result.ParsedKeywords == parsedKeywords);
 
   ASSERT_TRUE(result.GetKeywordErrors().size() == keywordErrors.size());
   for (auto const& ke : result.GetKeywordErrors()) {
@@ -133,7 +149,6 @@ bool testArgumentParserDynamic()
 {
   Result result;
   std::vector<std::string> unparsedArguments;
-  std::vector<cm::string_view> keywordsMissingValue;
 
   static_cast<ArgumentParser::ParseResult&>(result) =
     cmArgumentParser<void>{}
@@ -153,9 +168,10 @@ bool testArgumentParserDynamic()
       .Bind("MULTI_2"_s, result.Multi2)
       .Bind("MULTI_3"_s, result.Multi3)
       .Bind("MULTI_4"_s, result.Multi4)
-      .Parse(args, &unparsedArguments, &keywordsMissingValue);
+      .BindParsedKeywords(result.ParsedKeywords)
+      .Parse(args, &unparsedArguments);
 
-  return verifyResult(result, unparsedArguments, keywordsMissingValue);
+  return verifyResult(result, unparsedArguments);
 }
 
 static auto const parserStatic = //
@@ -176,25 +192,22 @@ static auto const parserStatic = //
     .Bind("MULTI_2"_s, &Result::Multi2)
     .Bind("MULTI_3"_s, &Result::Multi3)
     .Bind("MULTI_4"_s, &Result::Multi4)
+    .BindParsedKeywords(&Result::ParsedKeywords)
   /* keep semicolon on own line */;
 
 bool testArgumentParserStatic()
 {
   std::vector<std::string> unparsedArguments;
-  std::vector<cm::string_view> keywordsMissingValue;
-  Result const result =
-    parserStatic.Parse(args, &unparsedArguments, &keywordsMissingValue);
-  return verifyResult(result, unparsedArguments, keywordsMissingValue);
+  Result const result = parserStatic.Parse(args, &unparsedArguments);
+  return verifyResult(result, unparsedArguments);
 }
 
 bool testArgumentParserStaticBool()
 {
   std::vector<std::string> unparsedArguments;
-  std::vector<cm::string_view> keywordsMissingValue;
   Result result;
-  ASSERT_TRUE(parserStatic.Parse(result, args, &unparsedArguments,
-                                 &keywordsMissingValue) == false);
-  return verifyResult(result, unparsedArguments, keywordsMissingValue);
+  ASSERT_TRUE(parserStatic.Parse(result, args, &unparsedArguments) == false);
+  return verifyResult(result, unparsedArguments);
 }
 
 } // namespace