From: Martin Mares <mj@ucw.cz>
Date: Mon, 27 Jan 2014 16:13:46 +0000 (+0100)
Subject: Opt: Simpified and corrected processing of OPT_SINGLE
X-Git-Tag: v5.99~25^2~13
X-Git-Url: http://mj.ucw.cz/gitweb/?a=commitdiff_plain;h=f89145376775da7f15fe6d654a287c57ea3d38da;p=libucw.git

Opt: Simpified and corrected processing of OPT_SINGLE

OPT_SINGLE is now checked in opt_parse_value(). Previously, short,
long, and positional options had their own checks.

OPT_CL_SWITCH did not allow changing the value of the switch once
it is set, although documentation in opt.h claimed quite the opposite.
Changes are currently allowed unless OPT_SINGLE is set.

Test cases have been updated accordingly.
---

diff --git a/ucw/opt-test.c b/ucw/opt-test.c
index 0d5337df..cd7acc38 100644
--- a/ucw/opt-test.c
+++ b/ucw/opt-test.c
@@ -128,10 +128,10 @@ static struct opt_section help = {
     OPT_HELP(""),
     OPT_BOOL('e', "english-style", english, 0, "\tEnglish style (with milk)"),
     OPT_INT('s', "sugar", sugar, OPT_REQUIRED_VALUE, "<spoons>\tAmount of sugar (in teaspoons)"),
-    OPT_SWITCH(0, "standard-set", set, TEAPOT_STANDARD, 0, "\tStandard teapot"),
-    OPT_SWITCH('x', "exclusive-set", set, TEAPOT_EXCLUSIVE, 0, "\tExclusive teapot"),
-    OPT_SWITCH('g', "glass-set", set, TEAPOT_GLASS, 0, "\tTransparent glass teapot"),
-    OPT_SWITCH('h', "hands", set, TEAPOT_HANDS, 0, "\tUse user's hands as a teapot (a bit dangerous)"),
+    OPT_SWITCH(0, "standard-set", set, TEAPOT_STANDARD, OPT_SINGLE, "\tStandard teapot"),
+    OPT_SWITCH('x', "exclusive-set", set, TEAPOT_EXCLUSIVE, OPT_SINGLE, "\tExclusive teapot"),
+    OPT_SWITCH('g', "glass-set", set, TEAPOT_GLASS, OPT_SINGLE, "\tTransparent glass teapot"),
+    OPT_SWITCH('h', "hands", set, TEAPOT_HANDS, OPT_SINGLE, "\tUse user's hands as a teapot (a bit dangerous)"),
     OPT_USER('t', "temperature", temperature, teapot_temperature_t, OPT_REQUIRED_VALUE | OPT_REQUIRED,
 		  "<value>\tWanted final temperature of the tea to be served (required)\n"
 	      "\t\tSupported scales:  Celsius [60C], Fahrenheit [140F],\n"
diff --git a/ucw/opt.c b/ucw/opt.c
index 4cddc621..5ba7954d 100644
--- a/ucw/opt.c
+++ b/ucw/opt.c
@@ -255,6 +255,9 @@ static struct opt_precomputed * opt_find_item_longopt(struct opt_context * oc, c
 static void opt_parse_value(struct opt_context * oc, struct opt_precomputed * opt, char * value) {
   struct opt_item * item = opt->item;
 
+  if (opt->count++ && (opt->flags & OPT_SINGLE))
+    opt_failure("Option %s must be specified at most once.", THIS_OPT);
+
   for (int i = 0; i < oc->hooks_before_value_count; i++)
     oc->hooks_before_value[i]->u.call(item, value, oc->hooks_before_value[i]->ptr);
 
@@ -315,8 +318,7 @@ static void opt_parse_value(struct opt_context * oc, struct opt_precomputed * op
 	break;
       }
     case OPT_CL_SWITCH:
-      // FIXME: Really? And who sets the default to -1?
-      if (*((int *)item->ptr) != -1)
+      if ((opt->flags & OPT_SINGLE) && *((int *)item->ptr) != -1)
 	opt_failure("Multiple switches: %s", THIS_OPT);
       else
 	*((int *)item->ptr) = item->u.value;
@@ -355,10 +357,6 @@ static int opt_longopt(struct opt_context * oc, char ** argv, int index) {
 
   opt->flags |= OPT_SEEN_AS_LONG;
 
-  // FIXME: Move to opt_parse_value()?
-  if (opt->count++ && (opt->flags & OPT_SINGLE))
-    opt_failure("Option %s must be specified at most once.", THIS_OPT);
-
   if (opt->item->cls == OPT_CL_BOOL && !strncmp(name_in, "no-", 3) && !strncmp(name_in+3, opt->item->name, pos-3)) {
     if (name_in[pos])
       opt_failure("Option --%s must not have any value.", name_in);
@@ -398,10 +396,6 @@ static int opt_shortopt(struct opt_context * oc, char ** argv, int index) {
 
     opt->flags &= ~OPT_SEEN_AS_LONG;
 
-    if (opt->count && (opt->flags & OPT_SINGLE))
-      opt_failure("Option -%c must be specified at most once.", o);
-    opt->count++;
-
     if (opt->flags & OPT_NO_VALUE)
       opt_parse_value(oc, opt, NULL);
     else if (opt->flags & OPT_REQUIRED_VALUE) {
@@ -432,11 +426,10 @@ static void opt_positional(struct opt_context * oc, char * value) {
   oc->positional_count++;
   uns id = oc->positional_count > oc->positional_max ? OPT_POSITIONAL_TAIL : OPT_POSITIONAL(oc->positional_count);
   struct opt_precomputed * opt = oc->shortopt[id];
-  if (!opt || (opt->flags & OPT_SINGLE) && opt->count)
+  if (!opt)
     opt_failure("Too many positional arguments.");
   else {
     opt->flags &= OPT_SEEN_AS_LONG;
-    opt->count++;
     opt_parse_value(oc, opt, value);
   }
 }
diff --git a/ucw/opt.t b/ucw/opt.t
index d7fac7ec..aeafd199 100644
--- a/ucw/opt.t
+++ b/ucw/opt.t
@@ -19,7 +19,8 @@ Run:	../obj/ucw/opt-test -t120F -w4 puerh darjeeling earl-grey
 Out:	English style: no|Temperature: 120F|Verbosity: 1|Prayer: no|Water amount: 4|Gas: no|First tea: puerh|Boiling a tea: darjeeling|Boiling a tea: earl-grey|Everything OK. Bye.
 
 Name:	Opt-5
-Run:	../obj/ucw/opt-test -ghx 2>&1 1>/dev/null || [ $? -eq "2" ]
+Run:	../obj/ucw/opt-test -ghx 2>&1 1>/dev/null
+Exit:	2
 Out:	Multiple switches: -h
 	Run with argument --help for more information.
 
@@ -32,7 +33,8 @@ Run:	../obj/ucw/opt-test -t120F -w4 -b15 -b54 -he -- --puerh darjeeling earl-gre
 Out:	English style: yes|Chosen teapot: hands|Temperature: 120F|Verbosity: 1|Black magic: 15|Black magic: 54|Prayer: no|Water amount: 4|Gas: no|First tea: --puerh|Boiling a tea: darjeeling|Boiling a tea: earl-grey|Everything OK. Bye.
 
 Name:	Opt-Conf-1
-Run:	../obj/ucw/opt-test -h --dumpconfig 2>&1 1>/dev/null || [ $? -eq "2" ]
+Run:	../obj/ucw/opt-test -h --dumpconfig 2>&1 1>/dev/null
+Exit:	2
 Out:	Config options (-C, -S) must stand before other options.
 	Run with argument --help for more information.