Warn if an --option is specified multiple times

5 files changed, 69 insertions, 22 deletions

diff --git a/paludis/args/args_handler.cc b/paludis/args/args_handler.cc
index a1e9939d2..c5734f580 100644
--- a/paludis/args/args_handler.cc
+++ b/paludis/args/args_handler.cc
@@ -24,6 +24,7 @@
 #include <paludis/util/system.hh>
 #include <paludis/util/join.hh>
 #include <paludis/util/accept_visitor.hh>
+#include <paludis/util/iterator_funcs.hh>
 
 #include <paludis/util/pimp-impl.hh>
 #include <paludis/util/wrapped_forward_iterator-impl.hh>
@@ -37,6 +38,7 @@
 #include <sstream>
 #include <list>
 #include <map>
+#include <set>
 
 using namespace paludis;
 using namespace paludis::args;
@@ -197,6 +199,9 @@ ArgsHandler::run(
             args.push_back(option);
     }
 
+    ArgsIterator last_weak_arg(args.empty() ? args.end() : prev(args.end()));
+    ArgsOptionSpecifiedness specifiedness(args.empty() ? aos_specified : aos_weak);
+
     args.insert(args.end(), argseq->begin(), argseq->end());
 
     ArgsIterator argit(args.begin()), arge(args.end());
@@ -229,13 +234,13 @@ ArgsHandler::run(
                     throw BadArgument("--no-" + arg);
 
                 std::string remaining_chars;
-                ArgsVisitor visitor(&argit, arge, env_prefix, remaining_chars, true);
+                ArgsVisitor visitor(&argit, arge, env_prefix, remaining_chars, true, specifiedness);
                 it->second->accept(visitor);
             }
             else
             {
                 std::string remaining_chars;
-                ArgsVisitor visitor(&argit, arge, env_prefix, remaining_chars, false);
+                ArgsVisitor visitor(&argit, arge, env_prefix, remaining_chars, false, specifiedness);
                 it->second->accept(visitor);
             }
         }
@@ -258,7 +263,7 @@ ArgsHandler::run(
                     throw BadArgument(std::string("-") + *c);
                 }
 
-                ArgsVisitor visitor(&argit, arge, env_prefix, remaining_chars, false);
+                ArgsVisitor visitor(&argit, arge, env_prefix, remaining_chars, false, specifiedness);
                 it->second->accept(visitor);
 
                 if (maybe_second_char_used && remaining_chars.empty())
@@ -272,6 +277,9 @@ ArgsHandler::run(
             else
                 _imp->parameters.push_back(arg);
         }
+
+        if (aos_weak == specifiedness && last_weak_arg == argit)
+            specifiedness = aos_specified;
     }
 
     _imp->parameters.insert(_imp->parameters.end(), argit, ArgsIterator(args.end()));
diff --git a/paludis/args/args_option.cc b/paludis/args/args_option.cc
index 1140448ec..7628b6195 100644
--- a/paludis/args/args_option.cc
+++ b/paludis/args/args_option.cc
@@ -64,7 +64,7 @@ ArgsOption::ArgsOption(ArgsGroup * const g, const std::string & our_long_name,
     _long_name(our_long_name),
     _short_name(our_short_name),
     _description(our_description),
-    _specified(false)
+    _specified(aos_not)
 {
     g->add(this);
     g->section()->handler()->add_option(this, our_long_name, our_short_name);
diff --git a/paludis/args/args_option.hh b/paludis/args/args_option.hh
index 1a35d6d06..6131cccfd 100644
--- a/paludis/args/args_option.hh
+++ b/paludis/args/args_option.hh
@@ -59,6 +59,13 @@ namespace paludis
         class StringSetArg;
         class StringSequenceArg;
 
+        enum ArgsOptionSpecifiedness
+        {
+            aos_not,
+            aos_weak,
+            aos_specified
+        };
+
         /**
          * Base class for a command line option.
          *
@@ -77,7 +84,7 @@ namespace paludis
                 const char _short_name;
                 const std::string _description;
 
-                bool _specified;
+                ArgsOptionSpecifiedness _specified;
 
                 ArgsOption(const ArgsOption &);
                 void operator= (const ArgsOption &);
@@ -127,17 +134,27 @@ namespace paludis
 
                 /**
                  * Fetch whether or not we were specified on the
-                 * command line.
+                 * command line (or as an env var).
                  */
                 virtual bool specified() const
                 {
-                    return _specified;
+                    return _specified != aos_not;
+                }
+
+                /**
+                 * Fetch whether or not we were explicitly (not
+                 * an env var) specified. Used to catch -x y -x z
+                 * where -x takes a single value.
+                 */
+                virtual bool explicitly_specified() const
+                {
+                    return _specified == aos_specified;
                 }
 
                 /**
                  * Set the value returned by specified().
                  */
-                virtual void set_specified(const bool value)
+                virtual void set_specified(const ArgsOptionSpecifiedness value)
                 {
                     _specified = value;
                 }
@@ -435,7 +452,12 @@ namespace paludis
                     return _other->specified();
                 }
 
-                virtual void set_specified(const bool value)
+                virtual bool explicitly_specified() const
+                {
+                    return _other->explicitly_specified();
+                }
+
+                virtual void set_specified(const ArgsOptionSpecifiedness value)
                 {
                     _other->set_specified(value);
                 }
diff --git a/paludis/args/args_visitor.cc b/paludis/args/args_visitor.cc
index af8ec032f..d39cccb58 100644
--- a/paludis/args/args_visitor.cc
+++ b/paludis/args/args_visitor.cc
@@ -27,6 +27,7 @@
 #include <paludis/util/join.hh>
 #include <paludis/util/wrapped_forward_iterator-impl.hh>
 #include <paludis/util/pimp-impl.hh>
+#include <paludis/util/log.hh>
 
 #include <algorithm>
 #include <sstream>
@@ -45,26 +46,29 @@ namespace paludis
         std::string env_prefix;
         std::string & remaining_chars;
         bool no;
+        ArgsOptionSpecifiedness specifiedness;
 
         Imp(
                 ArgsHandler::ArgsIterator * i,
                 ArgsHandler::ArgsIterator e,
                 std::string p,
                 std::string & s,
-                bool n) :
+                bool n,
+                ArgsOptionSpecifiedness sp) :
             args_index(i),
             args_end(e),
             env_prefix(p),
             remaining_chars(s),
-            no(n)
+            no(n),
+            specifiedness(sp)
         {
         }
     };
 }
 
 ArgsVisitor::ArgsVisitor(ArgsHandler::ArgsIterator * ai, ArgsHandler::ArgsIterator ae,
-        const std::string & env_prefix, std::string & s, bool n) :
-    _imp(ai, ae, env_prefix, s, n)
+        const std::string & env_prefix, std::string & s, bool n, ArgsOptionSpecifiedness p) :
+    _imp(ai, ae, env_prefix, s, n, p)
 {
 }
 
@@ -85,8 +89,12 @@ void ArgsVisitor::visit(StringArg & arg)
 {
     if (! _imp->no)
     {
+        if (arg.explicitly_specified())
+            Log::get_instance()->message("args.specified_twice", ll_warning, lc_context)
+                <<  "Option '--" << arg.long_name() << "' was specified more than once, but it does not take multiple values";
+
         std::string p(get_param(arg));
-        arg.set_specified(true);
+        arg.set_specified(_imp->specifiedness);
         arg.set_argument(p);
         if (! _imp->env_prefix.empty())
             setenv(env_name(arg.long_name()).c_str(), p.c_str(), 1);
@@ -95,7 +103,7 @@ void ArgsVisitor::visit(StringArg & arg)
         throw BadArgument("--no-" + arg.long_name());
     else
     {
-        arg.set_specified(false);
+        arg.set_specified(aos_not);
         if (! _imp->env_prefix.empty())
             unsetenv(env_name(arg.long_name()).c_str());
     }
@@ -110,7 +118,7 @@ void ArgsVisitor::visit(SwitchArg & arg)
 {
     if (! _imp->no)
     {
-        arg.set_specified(true);
+        arg.set_specified(_imp->specifiedness);
         if (! _imp->env_prefix.empty())
             setenv(env_name(arg.long_name()).c_str(), "1", 1);
     }
@@ -118,7 +126,7 @@ void ArgsVisitor::visit(SwitchArg & arg)
         throw BadArgument("--no-" + arg.long_name());
     else
     {
-        arg.set_specified(false);
+        arg.set_specified(aos_not);
         if (! _imp->env_prefix.empty())
             unsetenv(env_name(arg.long_name()).c_str());
     }
@@ -128,7 +136,11 @@ void ArgsVisitor::visit(IntegerArg & arg)
 {
     if (! _imp->no)
     {
-        arg.set_specified(true);
+        if (arg.explicitly_specified())
+            Log::get_instance()->message("args.specified_twice", ll_warning, lc_context)
+                <<  "Option '--" << arg.long_name() << "' was specified more than once, but it does not take multiple values";
+
+        arg.set_specified(_imp->specifiedness);
         std::string param;
 
         if ((! _imp->remaining_chars.empty()) && (std::string::npos == _imp->remaining_chars.find_first_not_of("0123456789")))
@@ -160,7 +172,11 @@ void ArgsVisitor::visit(EnumArg & arg)
 {
     if (! _imp->no)
     {
-        arg.set_specified(true);
+        if (arg.explicitly_specified())
+            Log::get_instance()->message("args.specified_twice", ll_warning, lc_context)
+                <<  "Option '--" << arg.long_name() << "' was specified more than once, but it does not take multiple values";
+
+        arg.set_specified(_imp->specifiedness);
 
         std::string p;
         if (_imp->remaining_chars.length() == 1)
@@ -183,7 +199,7 @@ void ArgsVisitor::visit(StringSetArg & arg)
 {
     if (! _imp->no)
     {
-        arg.set_specified(true);
+        arg.set_specified(_imp->specifiedness);
 
         std::string param = get_param(arg);
         arg.add_argument(param);
@@ -200,7 +216,7 @@ void ArgsVisitor::visit(StringSequenceArg & arg)
 {
     if (! _imp->no)
     {
-        arg.set_specified(true);
+        arg.set_specified(_imp->specifiedness);
 
         std::string param = get_param(arg);
         arg.add_argument(param);
diff --git a/paludis/args/args_visitor.hh b/paludis/args/args_visitor.hh
index 8f950369d..fa68a0875 100644
--- a/paludis/args/args_visitor.hh
+++ b/paludis/args/args_visitor.hh
@@ -76,7 +76,8 @@ namespace paludis
                         ArgsHandler::ArgsIterator,
                         const std::string &,
                         std::string & remaining_chars,
-                        bool no);
+                        bool no,
+                        ArgsOptionSpecifiedness specifiedness);
 
                 ~ArgsVisitor();