From 8c84b8be73b24eff88550642aa9523fbb2d7a65d Mon Sep 17 00:00:00 2001
From: "Peter D. Barnes, Jr." <barnes26@llnl.gov>
Date: Thu, 5 Apr 2018 17:32:03 -0700
Subject: [PATCH] core: bug 2461: CommandLine should handle non-option
 arguments

---
 CHANGES.html                              |   1 +
 RELEASE_NOTES                             |   1 +
 src/core/examples/command-line-example.cc |  31 ++-
 src/core/model/command-line.cc            | 265 +++++++++++++++++-----
 src/core/model/command-line.h             | 101 ++++++++-
 src/core/test/command-line-test-suite.cc  |  51 +++++
 6 files changed, 384 insertions(+), 66 deletions(-)

diff --git a/CHANGES.html b/CHANGES.html
index 0388a992a..07e8a3f8c 100644
--- a/CHANGES.html
+++ b/CHANGES.html
@@ -53,6 +53,7 @@ us a note on ns-developers mailing list.</p>
 <hr>
 <h1>Changes from ns-3.28 to ns-3-dev</h1>
 <h2>New API:</h2>
+  <li> CommandLine can now handle non-option (positional) arguments. </li>
   <li> Added CommandLine::Parse (const std::vector<std::string>> args) </li>
   <li> NS_LOG_FUNCTION can now log the contents of vectors </li>
 <ul>
diff --git a/RELEASE_NOTES b/RELEASE_NOTES
index 0bb081eb3..0756f2a5e 100644
--- a/RELEASE_NOTES
+++ b/RELEASE_NOTES
@@ -29,6 +29,7 @@ New user-visible features
 Bugs fixed
 ----------
 - Bug 2901 - Add CommandLine::Parse (const std::vector<std::string>> args)
+- Bug 2461 - CommandLine should handle non-option arguments
 
 Known issues
 ------------
diff --git a/src/core/examples/command-line-example.cc b/src/core/examples/command-line-example.cc
index a8ea8433d..9622f91b9 100644
--- a/src/core/examples/command-line-example.cc
+++ b/src/core/examples/command-line-example.cc
@@ -67,7 +67,10 @@ int main (int argc, char *argv[])
   const std::string attrClass = "ns3::RandomVariableStream";
   const std::string attrName  = "Antithetic";
   const std::string attrPath  = attrClass + "::" + attrName;
- 
+  // Non-option arguments
+  int         nonOpt1 = 1;
+  int         nonOpt2 = 1;
+   
   // Cache the initial values.  Normally one wouldn't do this,
   // but we want to demonstrate that CommandLine has changed them.
   const int intDef = intArg;
@@ -82,6 +85,8 @@ int main (int argc, char *argv[])
     tid.LookupAttributeByName (attrName, &info);
     attrDef = info.originalInitialValue->SerializeToString (info.checker);
   }
+  const int nonOpt1Def = nonOpt1;
+  const int nonOpt2Def = nonOpt2;
   
   
   CommandLine cmd;
@@ -93,6 +98,8 @@ int main (int argc, char *argv[])
   cmd.AddValue ("strArg",  "a string argument",     strArg);
   cmd.AddValue ("anti",    attrPath);
   cmd.AddValue ("cbArg",   "a string via callback", MakeCallback (SetCbArg));
+  cmd.AddNonOption ("nonOpt1", "first non-option", nonOpt1);
+  cmd.AddNonOption ("nonOpt2", "first non-option", nonOpt2);
   cmd.Parse (argc, argv);
 
   // Show initial values:
@@ -116,6 +123,11 @@ int main (int argc, char *argv[])
   std::cout << std::setw (10)              << "cbArg:"
             << "\""           << cbDef     << "\""
             << std::endl;
+  std::cout << std::left << std::setw (10) << "nonOpt1:"
+            <<                   nonOpt1Def
+            << std::endl;
+  std::cout << std::left << std::setw (10) << "nonOpt2:"
+            <<                   nonOpt2Def
             << std::endl;
   std::cout << std::endl;
 
@@ -147,6 +159,23 @@ int main (int argc, char *argv[])
   std::cout << std::setw (10)              << "cbArg:"
             << "\""           << g_cbArg   << "\""
             << std::endl;
+  std::cout << std::left << std::setw (10) << "nonOpt1:"
+            <<                   nonOpt1
+            << std::endl;
+  std::cout << std::left << std::setw (10) << "nonOpt2:"
+            <<                   nonOpt2
+            << std::endl;
+  std::cout << std::left << "Number of extra non-option arguments:"
+            <<                   cmd.GetNExtraNonOptions ()
+            << std::endl;
+
+  for (std::size_t i = 0; i < cmd.GetNExtraNonOptions (); ++i)
+    {
+      std::cout << std::left << std::setw (10) << "extra:"
+                << "\""       << cmd.GetExtraNonOption (i) << "\""
+                << std::endl;
+    }
+
 
   return 0;
 }
diff --git a/src/core/model/command-line.cc b/src/core/model/command-line.cc
index 9956cb07e..217462e23 100644
--- a/src/core/model/command-line.cc
+++ b/src/core/model/command-line.cc
@@ -46,6 +46,8 @@ namespace ns3 {
 NS_LOG_COMPONENT_DEFINE ("CommandLine");
 
 CommandLine::CommandLine ()
+  : m_NNonOptions (0),
+    m_nonOptionCount (0)
 {
   NS_LOG_FUNCTION (this);
 }
@@ -70,11 +72,10 @@ CommandLine::Copy (const CommandLine &cmd)
 {
   NS_LOG_FUNCTION (&cmd);
 
-  for (Items::const_iterator i = cmd.m_items.begin (); 
-       i != cmd.m_items.end (); ++i)
-    {
-      m_items.push_back (*i);
-    }
+  std::copy (cmd.m_options.begin (), cmd.m_options.end (), m_options.end ());
+  std::copy (cmd.m_nonOptions.begin (), cmd.m_nonOptions.end (), m_nonOptions.end ());
+
+  m_NNonOptions = cmd.m_NNonOptions;
   m_usage = cmd.m_usage;
   m_name  = cmd.m_name;
 }
@@ -83,11 +84,17 @@ CommandLine::Clear (void)
 {
   NS_LOG_FUNCTION (this);
 
-  for (Items::const_iterator i = m_items.begin (); i != m_items.end (); ++i)
+  for (auto i : m_options)
     {
-      delete *i;
+      delete i;
     }
-  m_items.clear ();
+  for (auto i : m_nonOptions)
+    {
+      delete i;
+    }
+  m_options.clear ();
+  m_nonOptions.clear ();
+  m_NNonOptions = 0;
   m_usage = "";
   m_name  = "";
 }
@@ -114,42 +121,25 @@ CommandLine::Parse (std::vector<std::string> args)
 {
   NS_LOG_FUNCTION (this << args.size () << args);
 
-  m_name = SystemPath::Split (args[0]).back ();
-  
-  for (auto param : args)
+  m_nonOptionCount = 0;
+  m_name = "";
+
+  if (args.size () > 0)
     {
-      // remove "--" or "-" heading.
-      std::string::size_type cur = param.find ("--");
-      if (cur == 0)
+      m_name = SystemPath::Split (args[0]).back ();
+      args.erase (args.begin ());  // discard the program name
+      
+      for (auto param : args)
         {
-          param = param.substr (2, param.size () - 2);
+          if (HandleOption (param)) continue;
+          if (HandleNonOption (param)) continue;
+          
+          // is this possible?
+          NS_ASSERT_MSG (false,
+                         "unexpected error parsing command line parameter: '"
+                         << param << "'");
+
         }
-      else
-        {
-          cur = param.find ("-");
-          if (cur == 0)
-            {
-              param = param.substr (1, param.size () - 1);
-            }
-          else
-            {
-              // invalid argument. ignore.
-              continue;
-            }
-        }
-      cur = param.find ("=");
-      std::string name, value;
-      if (cur == std::string::npos)
-        {
-          name = param;
-          value = "";
-        }
-      else
-        {
-          name = param.substr (0, cur);
-          value = param.substr (cur + 1, param.size () - (cur+1));
-        }
-      HandleArgument (name, value);
     }
 
 #ifdef ENABLE_DES_METRICS
@@ -158,6 +148,78 @@ CommandLine::Parse (std::vector<std::string> args)
   
 }
 
+bool
+CommandLine::HandleOption (const std::string & param) const
+{
+  // remove leading "--" or "-"
+  std::string arg = param;
+  std::string::size_type cur = arg.find ("--");
+  if (cur == 0)
+    {
+      arg = arg.substr (2, arg.size () - 2);
+    }
+  else
+    {
+      cur = arg.find ("-");
+      if (cur == 0)
+        {
+          arg = arg.substr (1, arg.size () - 1);
+        }
+      else
+        {
+          // non-option argument?
+          return false;
+        }
+    }
+  // find any value following '='
+  cur = arg.find ("=");
+  std::string name, value;
+  if (cur == std::string::npos)
+    {
+      name = arg;
+      value = "";
+    }
+  else
+    {
+      name = arg.substr (0, cur);
+      value = arg.substr (cur + 1, arg.size () - (cur+1));
+    }
+  HandleArgument (name, value);
+
+  return true;
+}
+
+bool
+CommandLine::HandleNonOption (const std::string &value)
+{
+  NS_LOG_FUNCTION (this << value);
+
+  if (m_nonOptionCount == m_nonOptions.size())
+    {
+      // Add an unspecified non-option as a string
+      NS_LOG_LOGIC ("adding StringItem, NOCount:" << m_nonOptionCount
+                    << ", NOSize:" << m_nonOptions.size ());
+      StringItem * item = new StringItem;
+      item->m_name = "extra-non-option-argument";
+      item->m_help = "Extra non-option argument encountered.";
+      item->m_value = value;
+      m_nonOptions.push_back (item);
+    }
+
+  auto i = m_nonOptions[m_nonOptionCount];
+  if (!i->Parse (value))
+    {
+      std::cerr << "Invalid non-option argument value "
+                << value << " for " << i->m_name
+                << std::endl;
+      std::exit (1);
+    }
+  ++m_nonOptionCount;
+  return true;
+}
+
+
+
 void
 CommandLine::Parse (int argc, char *argv[])
 {
@@ -171,8 +233,14 @@ CommandLine::PrintHelp (std::ostream &os) const
 {
   NS_LOG_FUNCTION (this);
 
-  os << m_name << " [Program Arguments] [General Arguments]"
-            << std::endl;
+  // Hack to show just the declared non-options
+  Items nonOptions (m_nonOptions.begin (),
+                    m_nonOptions.begin () + m_NNonOptions);
+  os << m_name
+     << (m_options.size ()  ? " [Program Options]" : "")
+     << (nonOptions.size () ? " [Program Arguments]" : "")
+     << " [General Arguments]"
+     << std::endl;
   
   if (m_usage.length ())
     {
@@ -180,27 +248,51 @@ CommandLine::PrintHelp (std::ostream &os) const
       os << m_usage << std::endl;
     }
   
-  if (!m_items.empty ())
+  std::size_t width = 0;
+  for (auto it : m_options)
     {
-      size_t width = 0;
-      for (Items::const_iterator i = m_items.begin (); i != m_items.end (); ++i)
-        {
-          width = std::max (width, (*i)->m_name.size ());
-        }
-      width += 3;
+      width = std::max (width, it->m_name.size ());
+    }
+  for (auto it : nonOptions)
+    {
+      width = std::max (width, it->m_name.size ());
+    }
+  width += 3;  // room for ":  " betwen option and help
 
+  if (!m_options.empty ())
+    {
       os << std::endl;
-      os << "Program Arguments:" << std::endl;
-      for (Items::const_iterator i = m_items.begin (); i != m_items.end (); ++i)
+      os << "Program Options:" << std::endl;
+      for (auto i : m_options)
         {
           os << "    --"
-                    << std::left << std::setw (width) << ( (*i)->m_name + ":")
-                    << std::right
-                    << (*i)->m_help;
+             << std::left << std::setw (width) << ( i->m_name + ":")
+             << std::right
+             << i->m_help;
 
-          if ( (*i)->HasDefault ())
+          if ( i->HasDefault ())
             {
-              os << " [" << (*i)->GetDefault () << "]";
+              os << " [" << i->GetDefault () << "]";
+            }
+          os << std::endl;
+        }
+    }
+
+  if (!nonOptions.empty ())
+    {
+      width += 2;  // account for "--" added above
+      os << std::endl;
+      os << "Program Arguments:" << std::endl;
+      for (auto i : nonOptions)
+        {
+          os << "    "
+             << std::left << std::setw (width) << ( i->m_name + ":")
+             << std::right
+             << i->m_help;
+
+          if ( i->HasDefault ())
+            {
+              os << " [" << i->GetDefault () << "]";
             }
           os << std::endl;
         }
@@ -368,6 +460,8 @@ CommandLine::HandleArgument (const std::string &name, const std::string &value)
   NS_LOG_FUNCTION (this << name << value);
 
   NS_LOG_DEBUG ("Handle arg name=" << name << " value=" << value);
+
+  // Hard-coded options
   if (name == "PrintHelp" || name == "help")
     {
       // method below never returns.
@@ -406,11 +500,11 @@ CommandLine::HandleArgument (const std::string &name, const std::string &value)
     }
   else
     {
-      for (Items::const_iterator i = m_items.begin (); i != m_items.end (); ++i)
+      for (auto i : m_options)
         {
-          if ((*i)->m_name == name)
+          if (i->m_name == name)
             {
-              if (!(*i)->Parse (value))
+              if (! i->Parse (value))
                 {
                   std::cerr << "Invalid argument value: "
                             << name << "=" << value << std::endl;
@@ -423,6 +517,7 @@ CommandLine::HandleArgument (const std::string &name, const std::string &value)
             }
         }
     }
+  // Global or ConfigPath options
   if (!Config::SetGlobalFailSafe (name, StringValue (value))
       && !Config::SetDefaultFailSafe (name, StringValue (value)))
     {
@@ -451,7 +546,7 @@ CommandLine::AddValue (const std::string &name,
   item->m_name = name;
   item->m_help = help;
   item->m_callback = callback;
-  m_items.push_back (item);
+  m_options.push_back (item);
 }
 
 void
@@ -486,6 +581,35 @@ CommandLine::AddValue (const std::string &name,
             MakeBoundCallback (CommandLine::HandleAttribute, attributePath)) ;
 }
 
+std::string
+CommandLine::GetExtraNonOption (std::size_t i) const
+{
+  std::string value;
+  
+  if (m_nonOptions.size () >= i + m_NNonOptions)
+    {
+      auto ip = dynamic_cast<StringItem *> (m_nonOptions[i + m_NNonOptions]);
+      if (ip != NULL)
+        {
+          value = ip->m_value;
+        }
+    }
+  return value;
+}
+
+std::size_t
+CommandLine::GetNExtraNonOptions (void) const
+{
+  if (m_nonOptions.size () > m_NNonOptions)
+    {
+      return m_nonOptions.size () - m_NNonOptions;
+    }
+  else
+    {
+      return 0;
+    }
+}
+
 
 /* static */
 bool
@@ -514,6 +638,25 @@ CommandLine::Item::GetDefault () const
   return "";
 }
 
+bool
+CommandLine::StringItem::Parse (const std::string value)
+{
+  m_value = value;
+  return true;
+}
+
+bool
+CommandLine::StringItem::HasDefault (void) const
+{
+  return false;
+}
+ 
+std::string
+CommandLine::StringItem::GetDefault (void) const
+{
+  return "";
+}
+
 template <>
 std::string
 CommandLineHelper::GetDefault<bool> (const bool & val)
diff --git a/src/core/model/command-line.h b/src/core/model/command-line.h
index 932c0a607..cc895fbf7 100644
--- a/src/core/model/command-line.h
+++ b/src/core/model/command-line.h
@@ -68,13 +68,19 @@ namespace ns3 {
  * The more common \c --help is a synonym for \c --PrintHelp; an example
  * is given below.
  *
+ * CommandLine can also handle non-option arguments
+ * (often called simply "positional" parameters: arguments which don't begin
+ * with "-" or "--").  These can be parsed directly in to variables,
+ * by registering arguments with AddNonOption in the order expected.
+ * Additional non-option arguments encountered will be captured as strings.
+ *
  * Finally, CommandLine processes Attribute and GlobalValue arguments.
  * Default values for specific attributes can be set using a shorthand
  * argument name.
  *
  * In use, arguments are given in the form
  * \verbatim
-   --arg=value --toggle \endverbatim
+   --arg=value --toggle first-non-option\endverbatim
  * Most arguments expect a value, as in the first form, \c --arg=value.
  * Toggles, corresponding to boolean arguments, can be given in any of
  * the forms
@@ -83,6 +89,9 @@ namespace ns3 {
  * The first form changes the state of toggle1 from its default; 
  * all the rest set the corresponding boolean variable to true.
  * \c 0, \c f and \c false are accepted to set the variable to false.
+ * Option arguments can appear in any order on the command line,
+ * even intermixed with non-option arguments.
+ * The order of non-option arguments is preserved.
  *
  * Option arguments can be repeated on the command line; the last value given
  * will be the final value used.  For example,
@@ -116,6 +125,7 @@ namespace ns3 {
    --SchedulerType=HeapScheduler \endverbatim
  *
  * A simple example of CommandLine is in `src/core/example/``command-line-example.cc`
+ * See that file for an example of handling non-option arguments.
  *
  * The heart of that example is this code:
  *
@@ -265,6 +275,41 @@ public:
    */
   void AddValue (const std::string &name,
                  const std::string &attributePath);
+  
+  /**
+   * Add a non-option argument, assigning to POD
+   *
+   * \param [in] name The name of the program-supplied argument
+   * \param [in] help The help text used by \c \-\-PrintHelp
+   * \param [out] value A reference to the variable where the
+   *        value parsed will be stored (if no value
+   *        is parsed, this variable is not modified).
+   */
+  template <typename T>
+  void AddNonOption (const std::string name, const std::string help, T & value);
+
+  /**
+   * Get extra non-option arguments by index.
+   * This allows CommandLine to accept more non-option arguments than
+   * have been configured explictly with AddNonOption().
+   *
+   * This is only valid after calling Parse().
+   *
+   * \param [in] i The index of the non-option argument to return.
+   * \return The i'th non-option argument, as a string.
+   */
+  std::string GetExtraNonOption (std::size_t i) const;
+  
+  /**
+   * Get the total number of non-option arguments found,
+   * including those configured with AddNonOption() and extra non-option
+   * arguments.
+   *
+   * This is only valid after calling Parse().
+   *
+   * \returns the number of non-option arguments found.
+   */
+  std::size_t GetNExtraNonOptions (void) const;
 
   /**
    * Parse the program arguments
@@ -364,6 +409,16 @@ private:
     std::string m_default;    /**< String representation of default value */
   };  // class UserItem
 
+  class StringItem : public Item
+  {
+  public:
+    // Inherited
+    bool Parse (const std::string value);
+    bool HasDefault (void) const;
+    std::string GetDefault (void) const;
+    
+    std::string m_value;     /**< The argument value. */
+  };  // class StringItem
 
   /**
    * \ingroup commandline
@@ -383,6 +438,22 @@ private:
   };  // class CallbackItem
 
 
+  /**
+   * Handle an option in the form \c param=value.
+   *
+   * \param [in] param The option string. 
+   * \returns \c true if this was really an option.
+   */
+  bool HandleOption (const std::string & param) const;
+  
+  /**
+   * Handle a non-option
+   *
+   * \param [in] value The command line non-option value.
+   * \return \c true if \c value could be parsed correctly.
+   */
+  bool HandleNonOption (const std::string &value);
+  
   /**
    * Match name against the program or general arguments,
    * and dispatch to the appropriate handler.
@@ -440,10 +511,14 @@ private:
   /** Remove all arguments, Usage(), name */
   void Clear (void);
 
-  typedef std::list<Item *> Items;      /**< Argument list container */
-  Items m_items;                        /**< The list of arguments */
+  typedef std::vector<Item *> Items;    /**< Argument list container */
+  Items m_options;                      /**< The list of option arguments */
+  Items m_nonOptions;                   /**< The list of non-option arguments */
+  std::size_t m_NNonOptions;            /**< The expected number of non-option arguments */
+  std::size_t m_nonOptionCount;         /**< The number of actual non-option arguments seen so far. */
   std::string m_usage;                  /**< The Usage string */
   std::string m_name;                   /**< The program name */
+
 };  // class CommandLine
 
 
@@ -513,9 +588,27 @@ CommandLine::AddValue (const std::string &name,
   ss << value;
   ss >> item->m_default;
     
-  m_items.push_back (item);
+  m_options.push_back (item);
 }
 
+template <typename T>
+void
+CommandLine::AddNonOption (const std::string name,
+                           const std::string help,
+                           T & value)
+{
+  UserItem<T> *item = new UserItem<T> ();
+  item->m_name = name;
+  item->m_help = help;
+  item->m_valuePtr = &value;
+  
+  std::stringstream ss;
+  ss << value;
+  ss >> item->m_default;
+  m_nonOptions.push_back (item);
+  ++m_NNonOptions;
+    
+}
 
 template <typename T>
 bool
diff --git a/src/core/test/command-line-test-suite.cc b/src/core/test/command-line-test-suite.cc
index 9dbb33695..6f75dd439 100644
--- a/src/core/test/command-line-test-suite.cc
+++ b/src/core/test/command-line-test-suite.cc
@@ -342,6 +342,56 @@ CommandLineInvalidTestCase::DoRun (void)
   NS_TEST_ASSERT_MSG_EQ (myUint32, 5, "CommandLine did not correctly set an unsigned integer value to 5");
 }
 
+/**
+ * \ingroup commandline-tests
+ * Test non-option arguments
+ */
+class CommandLineNonOptionTestCase : public CommandLineTestCaseBase
+{
+public:
+  CommandLineNonOptionTestCase ();              /**< Constructor */
+  virtual ~CommandLineNonOptionTestCase () {}   /**< Destructor */
+
+private:
+  virtual void DoRun (void);                 /**< Run the test */
+
+};
+
+CommandLineNonOptionTestCase::CommandLineNonOptionTestCase ()
+  : CommandLineTestCaseBase ("nonoption")
+{
+}
+
+void
+CommandLineNonOptionTestCase::DoRun (void)
+{
+  CommandLine cmd;
+  bool myBool = false;
+  int32_t myInt = 1;
+  std::string myStr = "MyStr";
+
+  cmd.AddNonOption ("my-bool", "help", myBool);
+  cmd.AddNonOption ("my-int", "help", myInt);
+  cmd.AddNonOption ("my-str", "help", myStr);
+
+  Parse (cmd, 2, "true", "5");
+
+  NS_TEST_ASSERT_MSG_EQ (myBool, true, "CommandLine did not correctly set a boolean non-option");
+  NS_TEST_ASSERT_MSG_EQ (myInt, 5, "CommandLine did not correctly set an integer non-option value to 5");
+  NS_TEST_ASSERT_MSG_EQ (myStr, "MyStr", "CommandLine did not leave a non-option unmodified.");
+
+  Parse (cmd, 5, "false", "6", "newValue", "extraVal1", "extraVal2");
+  
+  NS_TEST_ASSERT_MSG_EQ (myBool, false, "CommandLine did not correctly set a boolean non-option");
+  NS_TEST_ASSERT_MSG_EQ (myInt
+, 6, "CommandLine did not correctly set an integer non-option value to 5");
+  NS_TEST_ASSERT_MSG_EQ (myStr, "newValue", "CommandLine did not leave a non-option unmodified.");
+
+  NS_TEST_ASSERT_MSG_EQ (cmd.GetNExtraNonOptions (), 2, "CommandLine did not parse the correct number of extra non-options.");
+  NS_TEST_ASSERT_MSG_EQ (cmd.GetExtraNonOption (0), "extraVal1", "CommandLine did not correctly get one extra non-option");
+  NS_TEST_ASSERT_MSG_EQ (cmd.GetExtraNonOption (1), "extraVal2", "CommandLine did not correctly get two extra non-option");
+}
+
 /**
  * \ingroup commandline-tests
  * The Test Suite that glues all of the Test Cases together.
@@ -361,6 +411,7 @@ CommandLineTestSuite::CommandLineTestSuite ()
   AddTestCase (new CommandLineStringTestCase);
   AddTestCase (new CommandLineOrderTestCase);
   AddTestCase (new CommandLineInvalidTestCase);
+  AddTestCase (new CommandLineNonOptionTestCase);
 }
 
 /**