Support specifying options/variables/buildspec in any order

6 files changed, 198 insertions, 176 deletions

diff --git a/build2/b-options b/build2/b-options
index c641ce0..32a8709 100644
--- a/build2/b-options
+++ b/build2/b-options
@@ -356,37 +356,42 @@ namespace build2
     public:
     options ();
 
-    options (int& argc,
-             char** argv,
-             bool erase = false,
-             ::build2::cl::unknown_mode option = ::build2::cl::unknown_mode::fail,
-             ::build2::cl::unknown_mode argument = ::build2::cl::unknown_mode::stop);
-
-    options (int start,
-             int& argc,
-             char** argv,
-             bool erase = false,
-             ::build2::cl::unknown_mode option = ::build2::cl::unknown_mode::fail,
-             ::build2::cl::unknown_mode argument = ::build2::cl::unknown_mode::stop);
-
-    options (int& argc,
-             char** argv,
-             int& end,
-             bool erase = false,
-             ::build2::cl::unknown_mode option = ::build2::cl::unknown_mode::fail,
-             ::build2::cl::unknown_mode argument = ::build2::cl::unknown_mode::stop);
-
-    options (int start,
-             int& argc,
-             char** argv,
-             int& end,
-             bool erase = false,
-             ::build2::cl::unknown_mode option = ::build2::cl::unknown_mode::fail,
-             ::build2::cl::unknown_mode argument = ::build2::cl::unknown_mode::stop);
-
-    options (::build2::cl::scanner&,
-             ::build2::cl::unknown_mode option = ::build2::cl::unknown_mode::fail,
-             ::build2::cl::unknown_mode argument = ::build2::cl::unknown_mode::stop);
+    void
+    parse (int& argc,
+           char** argv,
+           bool erase = false,
+           ::build2::cl::unknown_mode option = ::build2::cl::unknown_mode::fail,
+           ::build2::cl::unknown_mode argument = ::build2::cl::unknown_mode::stop);
+
+    void
+    parse (int start,
+           int& argc,
+           char** argv,
+           bool erase = false,
+           ::build2::cl::unknown_mode option = ::build2::cl::unknown_mode::fail,
+           ::build2::cl::unknown_mode argument = ::build2::cl::unknown_mode::stop);
+
+    void
+    parse (int& argc,
+           char** argv,
+           int& end,
+           bool erase = false,
+           ::build2::cl::unknown_mode option = ::build2::cl::unknown_mode::fail,
+           ::build2::cl::unknown_mode argument = ::build2::cl::unknown_mode::stop);
+
+    void
+    parse (int start,
+           int& argc,
+           char** argv,
+           int& end,
+           bool erase = false,
+           ::build2::cl::unknown_mode option = ::build2::cl::unknown_mode::fail,
+           ::build2::cl::unknown_mode argument = ::build2::cl::unknown_mode::stop);
+
+    void
+    parse (::build2::cl::scanner&,
+           ::build2::cl::unknown_mode option = ::build2::cl::unknown_mode::fail,
+           ::build2::cl::unknown_mode argument = ::build2::cl::unknown_mode::stop);
 
     // Option accessors.
     //
diff --git a/build2/b-options.cxx b/build2/b-options.cxx
index c841f8a..8c06f59 100644
--- a/build2/b-options.cxx
+++ b/build2/b-options.cxx
@@ -420,13 +420,15 @@ namespace build2
       static void
       parse (X& x, bool& xs, scanner& s)
       {
-        std::string o (s.next ());
+        using namespace std;
+
+        string o (s.next ());
 
         if (s.more ())
         {
-          std::string v (s.next ());
-          std::istringstream is (v);
-          if (!(is >> x && is.eof ()))
+          string v (s.next ());
+          istringstream is (v);
+          if (!(is >> x && is.peek () == istringstream::traits_type::eof ()))
             throw invalid_value (o, v);
         }
         else
@@ -579,110 +581,60 @@ namespace build2
   {
   }
 
-  options::
-  options (int& argc,
-           char** argv,
-           bool erase,
-           ::build2::cl::unknown_mode opt,
-           ::build2::cl::unknown_mode arg)
-  : v_ (),
-    q_ (),
-    verbose_ (1),
-    verbose_specified_ (false),
-    pager_ (),
-    pager_specified_ (false),
-    pager_option_ (),
-    pager_option_specified_ (false),
-    help_ (),
-    version_ ()
+  void options::
+  parse (int& argc,
+         char** argv,
+         bool erase,
+         ::build2::cl::unknown_mode opt,
+         ::build2::cl::unknown_mode arg)
   {
     ::build2::cl::argv_scanner s (argc, argv, erase);
     _parse (s, opt, arg);
   }
 
-  options::
-  options (int start,
-           int& argc,
-           char** argv,
-           bool erase,
-           ::build2::cl::unknown_mode opt,
-           ::build2::cl::unknown_mode arg)
-  : v_ (),
-    q_ (),
-    verbose_ (1),
-    verbose_specified_ (false),
-    pager_ (),
-    pager_specified_ (false),
-    pager_option_ (),
-    pager_option_specified_ (false),
-    help_ (),
-    version_ ()
+  void options::
+  parse (int start,
+         int& argc,
+         char** argv,
+         bool erase,
+         ::build2::cl::unknown_mode opt,
+         ::build2::cl::unknown_mode arg)
   {
     ::build2::cl::argv_scanner s (start, argc, argv, erase);
     _parse (s, opt, arg);
   }
 
-  options::
-  options (int& argc,
-           char** argv,
-           int& end,
-           bool erase,
-           ::build2::cl::unknown_mode opt,
-           ::build2::cl::unknown_mode arg)
-  : v_ (),
-    q_ (),
-    verbose_ (1),
-    verbose_specified_ (false),
-    pager_ (),
-    pager_specified_ (false),
-    pager_option_ (),
-    pager_option_specified_ (false),
-    help_ (),
-    version_ ()
+  void options::
+  parse (int& argc,
+         char** argv,
+         int& end,
+         bool erase,
+         ::build2::cl::unknown_mode opt,
+         ::build2::cl::unknown_mode arg)
   {
     ::build2::cl::argv_scanner s (argc, argv, erase);
     _parse (s, opt, arg);
     end = s.end ();
   }
 
-  options::
-  options (int start,
-           int& argc,
-           char** argv,
-           int& end,
-           bool erase,
-           ::build2::cl::unknown_mode opt,
-           ::build2::cl::unknown_mode arg)
-  : v_ (),
-    q_ (),
-    verbose_ (1),
-    verbose_specified_ (false),
-    pager_ (),
-    pager_specified_ (false),
-    pager_option_ (),
-    pager_option_specified_ (false),
-    help_ (),
-    version_ ()
+  void options::
+  parse (int start,
+         int& argc,
+         char** argv,
+         int& end,
+         bool erase,
+         ::build2::cl::unknown_mode opt,
+         ::build2::cl::unknown_mode arg)
   {
     ::build2::cl::argv_scanner s (start, argc, argv, erase);
     _parse (s, opt, arg);
     end = s.end ();
   }
 
-  options::
-  options (::build2::cl::scanner& s,
-           ::build2::cl::unknown_mode opt,
-           ::build2::cl::unknown_mode arg)
-  : v_ (),
-    q_ (),
-    verbose_ (1),
-    verbose_specified_ (false),
-    pager_ (),
-    pager_specified_ (false),
-    pager_option_ (),
-    pager_option_specified_ (false),
-    help_ (),
-    version_ ()
+  void options::
+  parse (::build2::cl::scanner& s,
+         ::build2::cl::unknown_mode opt,
+         ::build2::cl::unknown_mode arg)
   {
     _parse (s, opt, arg);
   }
@@ -875,8 +827,13 @@ namespace build2
        << "\033[1mDESCRIPTION\033[0m" << ::std::endl
        << ::std::endl
        << "The \033[1mbuild2\033[0m driver performs a set of meta-operations on operations on targets" << ::std::endl
-       << "according to the build specification, or \033[4mbuildspec\033[0m for short. Before \033[4mbuildspec\033[0m" << ::std::endl
-       << "(but after \033[4moptions\033[0m) you can set one or more \033[1mbuild2\033[0m \033[4mvariables\033[0m." << ::std::endl;
+       << "according to the build specification, or \033[4mbuildspec\033[0m for short. This process can" << ::std::endl
+       << "be controlled by specifying driver \033[4moptions\033[0m and build system \033[4mvariables\033[0m." << ::std::endl
+       << ::std::endl
+       << "Note that \033[4moptions\033[0m, \033[4mvariables\033[0m and \033[4mbuildspec\033[0m fragments can be specified in any" << ::std::endl
+       << "order. To avoid treating an argument that starts with \033[1m'-'\033[0m as an option, add the" << ::std::endl
+       << "\033[1m'--'\033[0m separator. To avoid treating an argument that contains \033[1m'='\033[0m as a variable," << ::std::endl
+       << "add the second \033[1m'--'\033[0m separator." << ::std::endl;
 
     p = ::build2::options::print_usage (os, ::build2::cl::usage_para::text);
 
diff --git a/build2/b.cli b/build2/b.cli
index 2657872..84daee9 100644
--- a/build2/b.cli
+++ b/build2/b.cli
@@ -1,4 +1,4 @@
-// file      : build2/options.cli
+// file      : build2/b.cli
 // copyright : Copyright (c) 2014-2016 Code Synthesis Ltd
 // license   : MIT; see accompanying LICENSE file
 
@@ -23,8 +23,14 @@ namespace build2
 
      The \cb{build2} driver performs a set of meta-operations on operations on
      targets according to the build specification, or <buildspec> for short.
-     Before <buildspec> (but after <options>) you can set one or more
-     \cb{build2} <variables>."
+     This process can be controlled by specifying driver <options> and build
+     system <variables>.
+
+     Note that <options>, <variables>, and <buildspec> fragments can be
+     specified in any order. To avoid treating an argument that starts with
+     \cb{'-'} as an option, add the \cb{'--'} separator. To avoid treating an
+     argument that contains \cb{'='} as a variable, add the second \cb{'--'}
+     separator."
   }
 
   // For usage it's nice to see the list of options on the first page. So
@@ -116,7 +122,8 @@ namespace build2
      b 'clean(foo-out/exe{foo})'   # no need to specify src_base
      \
 
-     \cb{build2} has the following built-in and pre-defined meta-operations:
+     The build system has the following built-in and pre-defined
+     meta-operations:
 
      \dl|
 
@@ -141,7 +148,7 @@ namespace build2
          Prepare a distribution containing all files necessary to perform all
          operations in a project. Implemented by the \cb{dist} module.||
 
-     \cb{build2} has the following built-in and pre-defined operations:
+     The build system has the following built-in and pre-defined operations:
 
      \dl|
 
@@ -163,7 +170,7 @@ namespace build2
          Install a target. Performs \cb{update} as a pre-operation. Implemented
          by the \cb{install} module.||
 
-     The ability to specify \c{build2} variables as part of the command line
+     The ability to specify \cb{build2} variables as part of the command line
      is normally used to pass configuration values, for example:
 
      \
diff --git a/build2/b.cxx b/build2/b.cxx
index 0cfb6d8..417008e 100644
--- a/build2/b.cxx
+++ b/build2/b.cxx
@@ -12,6 +12,7 @@
 
 #include <sstream>
 #include <cassert>
+#include <cstring>     // strcmp(), strchr()
 #include <typeinfo>
 #include <iostream>
 #include <system_error>
@@ -62,8 +63,84 @@ main (int argc, char* argv[])
   {
     tracer trace ("main");
 
-    cl::argv_scanner scan (argc, argv, true);
-    options ops (scan);
+    // Parse the command line. We want to be able to specify options, vars,
+    // and buildspecs in any order (it is really handy to just add -v at the
+    // end of the command line).
+    //
+    options ops;
+    strings vars;
+    string args;
+    try
+    {
+      cl::argv_scanner scan (argc, argv);
+
+      for (bool opt (true), var (true); scan.more (); )
+      {
+        if (opt)
+        {
+          // If we see first "--", then we are done parsing options.
+          //
+          if (strcmp (scan.peek (), "--") == 0)
+          {
+            scan.next ();
+            opt = false;
+            continue;
+          }
+
+          // Parse the next chunk of options until we reach an argument (or
+          // eos).
+          //
+          ops.parse (scan);
+
+          if (!scan.more ())
+            break;
+
+          // Fall through.
+        }
+
+        const char* s (scan.next ());
+
+        // See if this is a command line variable. What if someone needs to
+        // pass a buildspec that contains '='? One way to support this would
+        // be to quote such a buildspec (e.g., "'/tmp/foo=bar/'"). Or invent
+        // another separator. Or use a second "--". Actually, let's just do
+        // the second "--".
+        //
+        if (var)
+        {
+          // If we see second "--", then we are also done parsing variables.
+          //
+          if (strcmp (s, "--") == 0)
+          {
+            var = false;
+            continue;
+          }
+
+          if (strchr (s, '=') != nullptr) // Covers =, +=, and =+.
+          {
+            vars.push_back (s);
+            continue;
+          }
+
+          // Fall through.
+        }
+
+        // Merge all the individual buildspec arguments into a single string.
+        // Instead, we could also parse them individually (and merge the
+        // result). The benefit of doing it this way is potentially better
+        // diagnostics (i.e., we could have used <buildspec-1>, <buildspec-2>
+        // to give the idea about which argument is invalid).
+        //
+        if (!args.empty ())
+          args += ' ';
+
+        args += s;
+      }
+    }
+    catch (const cl::exception& e)
+    {
+      fail << e;
+    }
 
     // Diagnostics verbosity.
     //
@@ -154,73 +231,49 @@ main (int argc, char* argv[])
     //
     reset ();
 
-    // Parse command line variables. They should come before the
-    // buildspec.
+    // Parse the command line variables.
     //
-    int argi (1);
-    for (; argi != argc; argi++)
+    for (const string& v: vars)
     {
-      const char* s (argv[argi]);
-
-      istringstream is (s);
+      istringstream is (v);
       is.exceptions (istringstream::failbit | istringstream::badbit);
       lexer l (is, path ("<cmdline>"));
-      token t (l.next ());
-
-      if (t.type == token_type::eos)
-        continue; // Whitespace-only argument.
 
-      // Unless this is a name followed by = or +=, assume it is
-      // a start of the buildspec.
+      // This should be a name followed by =, +=, or =+.
       //
-      if (t.type != token_type::name)
-        break;
-
+      token t (l.next ());
       token_type tt (l.next ().type);
 
-      if (tt != token_type::assign &&
-          tt != token_type::prepend &&
-          tt != token_type::append)
-        break;
+      if (t.type != token_type::name ||
+          (tt != token_type::assign &&
+           tt != token_type::prepend &&
+           tt != token_type::append))
+      {
+        fail << "expected variable assignment instead of '" << v << "'" <<
+          info << "use double '--' to treat this argument as buildspec";
+      }
 
       parser p;
       t = p.parse_variable (l, *global_scope, t.value, tt);
 
       if (t.type != token_type::eos)
-        fail << "unexpected " << t << " in variable " << s;
+        fail << "unexpected " << t << " in variable assignment '" << v << "'";
     }
 
     // Parse the buildspec.
     //
     buildspec bspec;
+    try
     {
-      // Merge all the individual buildspec arguments into a single
-      // string. Instead, we could also parse them individually (
-      // and merge the result). The benefit of doing it this way
-      // is potentially better diagnostics (i.e., we could have
-      // used <buildspec-1>, <buildspec-2> to give the idea about
-      // which argument is invalid).
-      //
-      string s;
-      for (; argi != argc;)
-      {
-        s += argv[argi];
-        if (++argi != argc)
-          s += ' ';
-      }
-
-      try
-      {
-        istringstream is (s);
-        is.exceptions (istringstream::failbit | istringstream::badbit);
+      istringstream is (args);
+      is.exceptions (istringstream::failbit | istringstream::badbit);
 
-        parser p;
-        bspec = p.parse_buildspec (is, path ("<buildspec>"));
-      }
-      catch (const istringstream::failure&)
-      {
-        fail << "unable to parse buildspec '" << s << "'";
-      }
+      parser p;
+      bspec = p.parse_buildspec (is, path ("<buildspec>"));
+    }
+    catch (const istringstream::failure&)
+    {
+      fail << "unable to parse buildspec '" << args << "'";
     }
 
     level5 ([&]{trace << "buildspec: " << bspec;});
diff --git a/build2/buildfile b/build2/buildfile
index 9805378..e47ca8c 100644
--- a/build2/buildfile
+++ b/build2/buildfile
@@ -77,7 +77,7 @@ if! $cli.loaded
 
 cli.options += -I $src_root --include-with-brackets --include-prefix build2 \
 --guard-prefix BUILD2 --cli-namespace build2::cl --generate-file-scanner \
---generate-specifier
+--generate-parse --generate-specifier
 
 # Usage options.
 #
diff --git a/build2/lexer b/build2/lexer
index 06f2b24..0bbd22a 100644
--- a/build2/lexer
+++ b/build2/lexer
@@ -75,7 +75,7 @@ namespace build2
     char
     pair_separator () const {return pair_separator_;}
 
-    // Scanner.
+    // Scanner. Note that it is ok to call next() again after getting eos.
     //
     token
     next ();