Support new type of option value (#1504)

* support `std::vector<std::string>` type for option
* support multiple values for option in command line parser
* write unit test for new functionality

Signed-off-by: Roman Rusyaev <r.rusyaev@samsung.com>

diff --git a/contrib/nnc/include/support/CommandLine.h b/contrib/nnc/include/support/CommandLine.h
index 8469419..158cee9 100644 (file)
--- a/contrib/nnc/include/support/CommandLine.h
+++ b/contrib/nnc/include/support/CommandLine.h
@@ -121,6 +121,19 @@ private:
    */
   const char *findOptionValue(const BaseOption *opt, const char **argv, int cur_argv);
 
+  /**
+   * @brief figure out value for option with multiple values
+   * @param opt - option for which value is looked for
+   * @param opt_name - option name which taken from command line
+   * @param argv - array of command line arguments
+   * @param val_argv - position in argv for current option value
+   * @return position in argv where option value begins or nullptr if option doesn't have value anymore
+   * @throw BadOption throw exception if value for option is incorrect
+   */
+  const char *findValueForMultOption(const BaseOption *opt,
+                                     const std::string &opt_name,
+                                     const char **argv, int val_argv);
+
   // allow object constructor only for methods
   CommandLine() = default;
 
@@ -243,7 +256,12 @@ public:
   /**
    * @brief is option disabled?
    */
-   virtual bool isDisabled() const = 0;
+  virtual bool isDisabled() const = 0;
+
+  /**
+   * @brief can option have several values?
+   */
+  virtual bool canHaveSeveralVals() const = 0;
 };
 
 /**
@@ -315,6 +333,8 @@ public:
   const std::vector<char> &getSeparators() const override { return _seps; }
 
   bool isDisabled() const override { return !_is_enabled; }
+
+  bool canHaveSeveralVals() const override { return _can_have_several_vals; }
   // end overridden methods
 
 private:
@@ -326,6 +346,7 @@ private:
   option_checker_t _checker; // function verifies option and its value
   std::vector<char> _seps; // these symbols separate option name and its value
   bool _is_enabled;
+  bool _can_have_several_vals; // can option take several values?
 };
 
 // the following functions are helpers for users that declare new options
@@ -468,9 +489,11 @@ Option<T>::Option(const std::vector<std::string> &optnames,
   _checker = checker;
 
   _is_enabled = enabled;
-
   assert((_is_enabled || _is_optional) && "disabled option can't be required");
 
+  _can_have_several_vals = std::is_same<T, std::vector<std::string>>::value;
+  assert(!(_can_have_several_vals && !_seps.empty()) && "option with several values can't have separators");
+
   // register new option for parser
   CommandLine::getParser()->registerOption(this);
 

diff --git a/contrib/nnc/support/CommandLine.cpp b/contrib/nnc/support/CommandLine.cpp
index 4470858..e3f4594 100644 (file)
--- a/contrib/nnc/support/CommandLine.cpp
+++ b/contrib/nnc/support/CommandLine.cpp
@@ -209,6 +209,29 @@ BaseOption *CommandLine::findOption(const char *optname)
 
 } // findOption
 
+// check that option value is correct
+static void checkOptionValue(const BaseOption *opt, const std::string &opt_name, const std::string &val)
+{
+  auto valid_vals = opt->getValidVals();
+  bool is_valid = valid_vals.empty();
+
+  for ( const auto &v: valid_vals )
+  {
+    if ( v == val )
+    {
+      // value is valid
+      is_valid = true;
+      break;
+    }
+  }
+
+  if ( !is_valid )
+  {
+    throw BadOption(opt_name, val);
+  }
+
+} // checkOptionValue
+
 const char *CommandLine::findOptionValue(const BaseOption *opt, const char **argv, int cur_argv)
 {
   auto seps = opt->getSeparators();
@@ -261,27 +284,40 @@ const char *CommandLine::findOptionValue(const BaseOption *opt, const char **arg
   }
 
   // check that option value is correct
-  auto valid_vals = opt->getValidVals();
-  bool is_valid = valid_vals.empty();
+  checkOptionValue(opt, opt_name, val_pos);
 
-  for ( const auto &v: valid_vals )
+  return val_pos;
+
+} // findOptionValue
+
+const char *CommandLine::findValueForMultOption(const BaseOption *opt,
+                                                const std::string &opt_name,
+                                                const char **argv, int cur_argv)
+{
+  const char *val_pos = nullptr;
+
+  if ( cur_argv >= _args_num )
   {
-    if ( v == val_pos )
-    {
-      // value is valid
-      is_valid = true;
-      break;
-    }
+    return nullptr;
   }
 
-  if ( !is_valid )
+  val_pos = argv[cur_argv];
+
+  if (val_pos[0] == '-')
   {
-    throw BadOption(opt_name, val_pos);
+    // it can be a value for numeric (negative numbers)
+    // or symbolic (contains value `-`) option
+    if (!isdigit(val_pos[1]) && val_pos[1])
+    {
+      return nullptr;
+    }
   }
 
+  checkOptionValue(opt, opt_name, val_pos);
+
   return val_pos;
 
-} // findOptionValue
+} // findValueForMultOption
 
 void CommandLine::checkRegisteredOptions(const std::set<std::string> &cmd_args)
 {
@@ -394,10 +430,27 @@ void CommandLine::parseCommandLine(int argc, const char **argv, bool check_nonop
     // figure out value for option
     try
     {
-      arg_val = findOptionValue(opt, argv, i);
+      if ( opt->canHaveSeveralVals() )
+      {
+        int j = i + 1;
+        for ( arg_val = findValueForMultOption(opt, argv[i], argv, j);
+              arg_val;
+              arg_val = findValueForMultOption(opt, argv[i], argv, j) )
+        {
+          // set value for option
+          opt->setValue(arg_val);
+          j++;
+        }
+
+        i = j - 1;
+      }
+      else
+      {
+        arg_val = findOptionValue(opt, argv, i);
 
-      // set value for option
-      opt->setValue(arg_val);
+        // set value for option
+        opt->setValue(arg_val);
+      }
     }
     catch ( BadOption &e )
     {
@@ -435,6 +488,14 @@ void Option<std::string>::setValue(const std::string &val)
     this->setRawValue(val);
 }
 
+// vector of strings
+template <>
+void Option<std::vector<std::string>>::setValue(const std::string &val)
+{
+  if ( !val.empty() )
+    this->push_back(val);
+}
+
 // bool
 template <>
 void Option<bool>::setValue(const std::string &val)

diff --git a/contrib/nnc/unittests/support/CommandLineTest.cpp b/contrib/nnc/unittests/support/CommandLineTest.cpp
index a9e278b..74666bb 100644 (file)
--- a/contrib/nnc/unittests/support/CommandLineTest.cpp
+++ b/contrib/nnc/unittests/support/CommandLineTest.cpp
@@ -113,6 +113,19 @@ Option<bool> BoolOpt(optname("-bool_opt"),
 Option<bool> BoolOpt2(optname("-bool-opt2"),
                       overview("description of bool option with value"));
 
+//
+// declare vector<string> option
+//
+Option<std::vector<std::string>> VecStrOpt1(optname("-vec_opt1"),
+                                            overview("description of vector option"));
+Option<std::vector<std::string>> VecStrOpt2(optname("-vec_opt2"),
+                                            overview("description of vector option"));
+Option<std::vector<std::string>> VecStrOptWithVals(optname("--vec_opt_with_vals"),
+                                                   overview("description of vector option"),
+                                                   std::vector<std::string>(),
+                                                   optional(false),
+                                                   optvalues("abc, 123, xxx"));
+
 // test options
 TEST(SUPPORT_NNC, verify_cl_options)
 {
@@ -136,6 +149,10 @@ TEST(SUPPORT_NNC, verify_cl_options)
                         // bool options
                         "-bool_opt=false",
                         "-bool-opt2",
+                        // vector of strings options
+                        "-vec_opt1", "1", "c", "222", "ABC", "857",
+                        "-vec_opt2",
+                        "--vec_opt_with_vals", "abc", "123", "xxx", "abc", "xxx",
                         nullptr};
   int argc = (sizeof(argv) / sizeof(argv[0])) - 1;
 
@@ -147,6 +164,7 @@ TEST(SUPPORT_NNC, verify_cl_options)
   int32_t tmp_sint = NNegOpt;
   char tmp_char = CharOpt;
   bool tmp_bool = BoolOpt;
+  std::vector<std::string> tmp_vec = VecStrOpt1;
 
   //
   // string options
@@ -218,4 +236,24 @@ TEST(SUPPORT_NNC, verify_cl_options)
   ASSERT_EQ(tmp_bool, true);
   ASSERT_EQ(BoolOpt2, tmp_bool);
 
+  //
+  // vector of strings options
+  //
+  ASSERT_EQ(tmp_vec, VecStrOpt1);
+  ASSERT_EQ(VecStrOpt1[0], "1");
+  ASSERT_EQ(tmp_vec[1], "c");
+  ASSERT_EQ(VecStrOpt1[2], "222");
+  ASSERT_EQ(tmp_vec[3], "ABC");
+  ASSERT_EQ(VecStrOpt1[4], "857");
+
+  tmp_vec = VecStrOpt2;
+  ASSERT_EQ(VecStrOpt2, tmp_vec);
+  ASSERT_TRUE(VecStrOpt2.empty());
+
+  ASSERT_EQ(VecStrOptWithVals[0], "abc");
+  ASSERT_EQ(VecStrOptWithVals[1], "123");
+  ASSERT_EQ(VecStrOptWithVals[2], "xxx");
+  ASSERT_EQ(VecStrOptWithVals[3], "abc");
+  ASSERT_EQ(VecStrOptWithVals[4], "xxx");
+
 }