[core] Use optparse in TApplication's option parsing

core/CMakeLists.txt

@@ -34,11 +34,6 @@ elseif(NOT WIN32)
   set_property(TARGET Core APPEND PROPERTY BUILD_RPATH $ORIGIN)
 endif()
 
-generateHeader(Core
-  ${CMAKE_SOURCE_DIR}/core/base/src/root-argparse.py
-  ${CMAKE_BINARY_DIR}/ginclude/TApplicationCommandLineOptionsHelp.h
-)
-
 add_dependencies(Core CLING rconfigure)
 
 target_link_libraries(Core

core/base/CMakeLists.txt

@@ -223,6 +223,7 @@ endif()
 target_include_directories(Core PUBLIC
   $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/inc>
 )
+target_include_directories(Core PRIVATE ${CMAKE_SOURCE_DIR}/core/utils/inc)
 
 if(ROOT_NEED_STDCXXFS)
     target_link_libraries(Core PRIVATE stdc++fs)

core/foundation/inc/ROOT/StringUtils.hxx

@@ -19,11 +19,38 @@
 #include <vector>
 #include <numeric>
 #include <iterator>
+#include <utility>
 
 namespace ROOT {
 
 std::vector<std::string> Split(std::string_view str, std::string_view delims, bool skipEmpty = false);
 
+/// Given a string `str`, returns a pair of string views into it: the first containing the substring preceding the
+/// first instance of `splitter`  and the second containing the substring following it. Note that:
+///   1. the first instance of `splitter` will not appear in either string;
+///   2. if `splitter` does not appear, or appears as the last character, the second string view will be empty;
+///   3. if `splitter` appears as the first character, the first string view will be empty;
+///   4. if `str` is empty so will both views be.
+/// IMPORTANT: The lifetime of the returned string views is the same as `str`.
+std::pair<std::string_view, std::string_view> SplitAt(std::string_view str, char splitter);
+
+/**
+ * \brief Concatenate a list of strings with a separator
+ * \tparam StringCollection_t Any container of strings (vector, initializer_list, ...)
+ * \param[in] sep Separator inbetween the strings.
+ * \param[in] begin Beginning of the strings container
+ * \param[in] end Past-the-end iterator of the strings container
+ * \return the sep-delimited concatenation of strings
+ */
+template <typename InputIt_t>
+std::string Join(const std::string &sep, InputIt_t begin, InputIt_t end)
+{
+   if (begin == end)
+      return "";
+
+   return std::accumulate(std::next(begin), end, *begin, [&sep](auto const &a, auto const &b) { return a + sep + b; });
+}
+
 /**
  * \brief Concatenate a list of strings with a separator
  * \tparam StringCollection_t Any container of strings (vector, initializer_list, ...)
@@ -34,10 +61,7 @@ std::vector<std::string> Split(std::string_view str, std::string_view delims, bo
 template <class StringCollection_t>
 std::string Join(const std::string &sep, StringCollection_t &&strings)
 {
-   if (strings.empty())
-      return "";
-   return std::accumulate(std::next(std::begin(strings)), std::end(strings), strings[0],
-                          [&sep](auto const &a, auto const &b) { return a + sep + b; });
+   return Join(sep, std::begin(strings), std::end(strings));
 }
 
 std::string Round(double value, double error, unsigned int cutoff = 1, std::string_view delim = "#pm");

core/foundation/src/StringUtils.cxx

@@ -40,6 +40,14 @@ std::vector<std::string> Split(std::string_view str, std::string_view delims, bo
    return out;
 }
 
+std::pair<std::string_view, std::string_view> SplitAt(std::string_view str, char splitter)
+{
+   auto pos = str.find(splitter);
+   if (pos == std::string_view::npos)
+      return {str, ""};
+   return {str.substr(0, pos), str.substr(pos + 1)};
+}
+
 /**
  * \brief Convert (round) a value and its uncertainty to string using one or two significant digits of the error
  * \param error the error. If the error is negative or zero, only the value is returned with no specific rounding

core/foundation/test/testStringUtils.cxx

@@ -81,3 +81,14 @@ TEST(StringUtils, Round)
    EXPECT_EQ(ROOT::Round(-30000., 1000000000., 0), "(-0#pm1)*1e9");
    EXPECT_EQ(ROOT::Round(110., 0.24, 1, "+-"), "110.0+-0.2");
 }
+
+TEST(StringUtils, SplitAt) {
+   const auto p = [] (auto a, auto b) { return std::make_pair<std::string_view, std::string_view>(a, b); };
+
+   EXPECT_EQ(ROOT::SplitAt("foo, bar", ','), p("foo", " bar"));
+   EXPECT_EQ(ROOT::SplitAt("foo, bar,", ','), p("foo", " bar,"));
+   EXPECT_EQ(ROOT::SplitAt(", bar,", ','), p("", " bar,"));
+   EXPECT_EQ(ROOT::SplitAt("foo,bar", ' '), p("foo,bar", ""));
+   EXPECT_EQ(ROOT::SplitAt("", ' '), p("", ""));
+   EXPECT_EQ(ROOT::SplitAt(",,,,", ','), p("", ",,,"));
+}

core/utils/inc/optparse.hxx

@@ -111,7 +111,10 @@ public:
 
    struct RSettings {
       /// Affects how flags are parsed (\see EFlagTreatment).
-      EFlagTreatment fFlagTreatment;
+      EFlagTreatment fFlagTreatment = EFlagTreatment::kDefault;
+      bool fIgnoreUnknownFlags = false;
+
+      RSettings() {}
    };
 
    enum class EFlagType {
@@ -136,8 +139,14 @@ public:
 
 private:
    RSettings fSettings;
+   /// Flags, in order of appearance
    std::vector<RFlag> fFlags;
+   /// Positional arguments, in order of appearance
    std::vector<std::string> fArgs;
+   /// Index of the first element in fArgs that appeared after `--`.
+   std::optional<std::size_t> fFirstPostDashDashArg;
+   /// Indices of all args passed to Parse() that were skipped.
+   std::vector<std::size_t> fUnprocessedArgsIndices;
 
    struct RExpectedFlag {
       EFlagType fFlagType = EFlagType::kSwitch;
@@ -174,7 +183,7 @@ private:
    }
 
 public:
-   explicit RCmdLineOpts(RSettings settings = {EFlagTreatment::kDefault}) : fSettings(settings) {}
+   explicit RCmdLineOpts(RSettings settings = {}) : fSettings(settings) {}
 
    /// Returns all parsing errors
    const std::vector<std::string> &GetErrors() const { return fErrors; }
@@ -422,7 +431,7 @@ public:
       return values;
    }
 
-   void Parse(const char **args, std::size_t nArgs)
+   void Parse(const char *const *const args, std::size_t nArgs)
    {
       bool forcePositional = false;
 
@@ -435,18 +444,21 @@ public:
       // into flags "a", "b", and "c", which will be stored in `argStr`).
       std::vector<std::string_view> argStr;
 
-      for (std::size_t i = 0; i < nArgs && fErrors.empty(); ++i) {
-         const char *arg = args[i];
+      for (std::size_t argIndex = 0; argIndex < nArgs && fErrors.empty(); ++argIndex) {
+         const char *arg = args[argIndex];
          const char *const argOrig = arg;
+         const auto argIndexOrig = argIndex;
 
-         if (strcmp(arg, "--") == 0) {
+         if (strcmp(arg, "--") == 0 && !forcePositional) {
             forcePositional = true;
             continue;
          }
 
          bool isFlag = !forcePositional && arg[0] == '-';
          if (!isFlag) {
             // positional argument
+            if (forcePositional && !fFirstPostDashDashArg)
+               fFirstPostDashDashArg = fArgs.size();
             fArgs.push_back(arg);
             continue;
          }
@@ -484,9 +496,9 @@ public:
                   nxtArgIsTentative = false;
                } else {
                   argStr.push_back(std::string_view(arg));
-                  if (i < nArgs - 1 && args[i + 1][0] != '-') {
-                     nxtArgStr = args[i + 1];
-                     ++i;
+                  if (argIndex < nArgs - 1 && args[argIndex + 1][0] != '-') {
+                     nxtArgStr = args[argIndex + 1];
+                     ++argIndex;
                   }
                }
             }
@@ -500,9 +512,9 @@ public:
             }
 
             argStr.push_back(std::string_view(arg));
-            if (i < nArgs - 1 && args[i + 1][0] != '-') {
-               nxtArgStr = args[i + 1];
-               ++i;
+            if (argIndex < nArgs - 1 && args[argIndex + 1][0] != '-') {
+               nxtArgStr = args[argIndex + 1];
+               ++argIndex;
             }
          }
 
@@ -513,8 +525,13 @@ public:
 
             const auto *exp = GetExpectedFlag(argS);
             if (!exp) {
-               fErrors.push_back(std::string("Unknown flag: ") + argOrig);
-               break;
+               if (fSettings.fIgnoreUnknownFlags) {
+                  fUnprocessedArgsIndices.push_back(argIndexOrig);
+                  continue;
+               } else {
+                  fErrors.push_back(std::string("Unknown flag: ") + argOrig);
+                  break;
+               }
             }
 
             // In Prefix mode, check if the returned expected flag is shorter than `argS`. This can mean two things:
@@ -526,9 +543,12 @@ public:
             // {flag: "-Dfoo", arg: "bar"}, rather than {flag: "-D", arg: "foo=bar"}.
             if ((exp->fOpts & kFlagPrefixArg) && argS.size() > exp->fName.size()) {
                if (nxtArgIsTentative) {
-                  i -= !nxtArgStr.empty(); // if we had already picked a candidate next arg, undo that.
+                  argIndex -= !nxtArgStr.empty(); // if we had already picked a candidate next arg, undo that.
                   nxtArgStr = argS.substr(exp->fName.size());
                   nxtArgIsTentative = false;
+               } else if (fSettings.fIgnoreUnknownFlags) {
+                  fUnprocessedArgsIndices.push_back(argIndexOrig);
+                  continue;
                } else {
                   fErrors.push_back(std::string("Unknown flag: ") + argOrig);
                   break;
@@ -571,7 +591,7 @@ public:
             } else {
                if (!nxtArg.empty()) {
                   if (nxtArgIsTentative)
-                     --i;
+                     --argIndex;
                   else
                      fErrors.push_back("Flag " + exp->AsStr() + " does not expect an argument");
                }
@@ -587,6 +607,15 @@ public:
             break;
       }
    }
+
+   /// Returns the index of the first positional argument that appeared after `--`. If not null, it is guaranteed
+   /// to be less than the size of GetArgs().
+   std::optional<std::size_t> GetFirstPostDashDashArg() const { return fFirstPostDashDashArg; }
+
+   /// Returns a list of the indices of all args passed to Parse() that were not parsed.
+   /// This will typically be empty, unless the RCmdLineOpts was created with `fIgnoreUnknownFlags == true`, in which
+   /// case this will contain all skipped unknown flags.
+   const std::vector<std::size_t> &GetUnprocessedArgsIndices() const { return fUnprocessedArgsIndices; }
 };
 
 } // namespace ROOT

core/utils/test/optparse_test.cxx

@@ -771,3 +771,42 @@ TEST(OptParse, DuplicateFlagAlias)
       EXPECT_STREQ(ex.what(), "The same flag `--a` was passed multiple times to AddFlag().");
    }
 }
+
+TEST(OptParse, IgnoreUnknownFlags)
+{
+   ROOT::RCmdLineOpts::RSettings settings;
+   settings.fIgnoreUnknownFlags = true;
+   ROOT::RCmdLineOpts opts{settings};
+
+   const char *args[] = {"somename", "-f", "foo", "--help"};
+   opts.Parse(args, std::size(args));
+
+   EXPECT_TRUE(opts.GetErrors().empty());
+   EXPECT_EQ(opts.GetUnprocessedArgsIndices(), std::vector<std::size_t>({1, 3}));
+}
+
+TEST(OptParse, MultipleDashDash)
+{
+   ROOT::RCmdLineOpts opts;
+   opts.AddFlag({"-a"});
+   const char *args[] = {"--", "--", "--"};
+   opts.Parse(args, std::size(args));
+   EXPECT_EQ(opts.GetArgs(), std::vector<std::string>({"--", "--"}));
+}
+
+TEST(OptParse, PostDashDash)
+{
+   ROOT::RCmdLineOpts opts;
+   opts.AddFlag({"--foo"});
+   opts.AddFlag({"-a"});
+   opts.AddFlag({"--with-arg"}, ROOT::RCmdLineOpts::EFlagType::kWithArg);
+
+   // Note that only the first `--` is interpreted as special. Also, all "flags" coming after it are considered
+   // positional arguments.
+   const char *args[] = {"--foo", "first", "-a", "second", "--with-arg", "arg",
+                         "--",    "third", "-c", "fourth", "-a",         "--"};
+   opts.Parse(args, std::size(args));
+
+   EXPECT_EQ(opts.GetArgs(), std::vector<std::string>({"first", "second", "third", "-c", "fourth", "-a", "--"}));
+   EXPECT_EQ(opts.GetFirstPostDashDashArg(), 2);
+}