PATCH: Re: Assorted _arguments arguments

[Sven Wischnowsky <wischnow@xxxxxxxxxxxxxxxxxxxxxxx>]

Bart Schaefer wrote:

> On May 8, 11:00am, Sven Wischnowsky wrote:
> 
> ...
> 
> } But before I start writing it: should the default for _arguments be
> } changed? And would someone be willing to check all uses of _arguments
> } and add the option to the calls that need them?
> 
> I don't think it makes a lot of difference which way the default goes;
> do whatever would require the fewest changes to _arguments and to all
> the other functions that call it.

I didn't have much time yesterday and won't have any time today, so
this is only the change to _arguments. It adds the `-A' and `-S'
options (I couldn't think of better characters, but that's easy to
change). `-A' means: no options after the first argument and `-S'
means no options after the `--' separator. As with `-s' they can be
given to _arguments before the specs.

Changing the default behaviour would be implemented in _arguments
(this patch mostly changes the builtin behind it).


Then there is a change for this option-equal thing. I've chosen the
syntax `-opt=-' to mean: the option has to come after the equal sign,
not in a separate word. I didn't use `-opt=?' because, at least to me, 
this looks like making the `=' optional, i.e. the old behaviour, which 
is still the default. This, too, would be easy to change if we find
out that we will have to change more `-opt='s (to `-opt=-') than we
can leave untouched. I haven't had a look at the completion functions
to see which we have to change and I preferred to not change the
default behaviour.

No patches for the other two things, yet...

> ...
>
> } Exactly, the existence of `=' is the criteria used by getopt_long.  So
> } it's good.  But I vote `-opt=?' instead of `-opt=='.
> 
> I think we should try to make the syntax consistent with `getopts' and
> `zparseopts' if possible.  Maybe that _isn't_ possible do to conflicting
> use of colons ... and maybe we should have thought of that before we used
> colons everywhere, but ...

Maybe, I hope we (ok, I) can be excused for not making this more
consistent because _arguments does a whole lot more than getopts and
because of that I didn't think of getopts at that time.
 
> } Note that another common criteria is that a optional argument must be
> } some set of strings: `yes' or `no' for example.  At least, xset
> } handles optional arguments in this way.  If we can specify a pattern
> } addition to the current optional argument syntax, it can be handled.
> 
> If there's a known set of strings that can be the optional argument, then
> I think this can be handled with state changes and/or alternation and we
> don't need any more patterns jammed into the opt-spec.

I was thinking the same yesterday. Maybe someone can think of a nice
utility function that plays together with _arguments in such cases
(this is not entirely trivial because the optional option-argument
that's causing the trouble is *not* returned by the builtin, but maybe 
we could make... or something).

> ...
> 
> Returning to the original topic, perhaps a better way to think of this
> is that -i and --ignore-case are synonyms and should get the same
> treatment and description.  E.g. your suggestion does not help with
> a case like
> 
> _bzip2:    '(--decompress --compress -z --test -t)-d[decompress]' \
> _bzip2:    '(-d --compress -z --test -t)--decompress[decompress]' \
> _bzip2:    '(--compress --decompress -d --test -t)-z[compress]' \
> _bzip2:    '(-z --decompress -d --test -t)--compress[compress]' \
> 
> where you have both options that have alternate names for each other and
> are mutually exclusive with other options.  Much better would be, say,
> 
> 	'-d(--decompress)' '-z(--compress)' '-t(--test)' \
> 	'(-z -t)-d[decompress]' \
> 	'(-d -t)-z[compress]' \
> 	'(-z -d)-t[test compressed file integrity]' \
> 
> It might even be able to compute this automatically from the comma-lists
> in --help output, or some such.  (No, I don't really like the `-x(--xxx)'
> syntax, but I haven't time to think harder about it just now.)

When thinking about this yesterday, I was reminded of _argument_sets.
This does something entirely different, but the point is that this
already works with `sets'. For _argument_sets they are mutually
exclusive, but maybe we could get something similar (a wrapper
function for _arguments) with the opposite meaning, i.e. you define
multiple sets of options and the options in each set are mutually
exclusive. That with some sharing of descriptions, arguments
etc. should be enough. And even easier to read than the possibly
longish comma lists. I've got to think some more about this (and look
at the code), but it sounds promising.


Bye
 Sven

Index: Doc/Zsh/compsys.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/compsys.yo,v
retrieving revision 1.38
diff -u -r1.38 compsys.yo
--- Doc/Zsh/compsys.yo	2000/05/08 18:14:46	1.38
+++ Doc/Zsh/compsys.yo	2000/05/09 08:42:13
@@ -2951,7 +2951,10 @@
 separate argument after the option, a plus sign should be used
 instead. If the argument may be given as the next string or in same
 string as the option name but separated from it by an equal sign, a
-`tt(=)' should be used instead of the minus or plus sign.
+`tt(=)' should be used instead of the minus or plus sign and if the
+argument to the option has to be given in the same string after an
+equal sign and may not be given in the next argument, `tt(=-)' should
+be used.
 
 Note that this and the shortcut syntax with a leading tt(-+) or tt(+-) 
 means that for options like tt(-+) the second character has to be
@@ -3092,6 +3095,12 @@
 `tt(-)tt(-prefix)') are still considered to contain only one option
 name. This allows the use of the `tt(-s)' option to describe
 single-letter options together with such long option names.
+
+To simplify the specifications for commands with standard option
+parsing, the options tt(-A) and tt(-S) may be given. With tt(-A) no
+options will be completed after the first non-option argument on the
+line. With tt(-S), no option will be completed after a `tt(-)tt(-)' on 
+the line and this argument will otherwise be ignored.
 
 Another option supported is `tt(-O) var(name)'. The var(name) will be
 taken as the name of an array and its elements will be given to
Index: Src/Zle/computil.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/computil.c,v
retrieving revision 1.14
diff -u -r1.14 computil.c
--- Src/Zle/computil.c	2000/05/08 10:37:58	1.14
+++ Src/Zle/computil.c	2000/05/09 08:42:14
@@ -305,8 +305,12 @@
     int argsactive;		/* if arguments are still allowed */
 				/* used while parsing a command line */
     char *set;			/* set name, shared */
+    int flags;			/* see CDF_* below */
 };
 
+#define CDF_SEP 1
+#define CDF_ARG 2
+
 /* Description for an option. */
 
 struct caopt {
@@ -325,6 +329,7 @@
 #define CAO_DIRECT  2
 #define CAO_ODIRECT 3
 #define CAO_EQUAL   4
+#define CAO_OEQUAL  5
 
 /* Description for an argument */
 
@@ -515,7 +520,7 @@
     Caarg argp;
     char **oargs = args, *p, *q, *match = "r:|[_-]=* r:|=*", **xor;
     char *adpre, *adsuf, *set = NULL, *doset = NULL;
-    int single = 0, anum = 1, xnum, nopts, ndopts, nodopts;
+    int single = 0, anum = 1, xnum, nopts, ndopts, nodopts, flags = 0;
 
     nopts = ndopts = nodopts = 0;
 
@@ -536,23 +541,40 @@
     } else
 	adpre = adsuf = NULL;
 
-    /* Now get the -s and -M options. */
+    /* Now get the -s, -A, -S and -M options. */
 
     args++;
-    while ((p = *args)) {
-	if (!strcmp(p, "-s"))
-	    single = 1;
-	else if (p[0] == '-' && p[1] == 'M') {
-	    if (p[2])
-		match = p + 2;
-	    else if (args[1])
-		match = *++args;
-	    else {
-		args++;
+    while ((p = *args) && *p == '-') {
+	for (q = ++p; *q; q++)
+	    if (*q == 'M') {
+		q = "";
+		break;
+	    } else if (*q != 's' && *q != 'S' && *q != 'A')
 		break;
+
+	if (*q)
+	    break;
+
+	for (; *p; p++) {
+	    if (*p == 's')
+		single = 1;
+	    else if (*p == 'S')
+		flags |= CDF_SEP;
+	    else if (*p == 'A')
+		flags |= CDF_ARG;
+	    else if (*p == 'M') {
+		if (p[1]) {
+		    match = p + 1;
+		    p = "" - 1;
+		} else if (args[1])
+		    match = *++args;
+		else
+		    break;
 	    }
-	} else
+	}
+	if (*p)
 	    break;
+
 	args++;
     }
     if (!*args)
@@ -574,6 +596,7 @@
     } else
 	ret->single = NULL;
     ret->match = ztrdup(match);
+    ret->flags = flags;
 
     /* Get the definitions. */
 
@@ -659,8 +682,10 @@
 
 	    /* Skip over the name. */
 	    for (p++; *p && *p != ':' && *p != '[' &&
-		     ((*p != '-' && *p != '+' && *p != '=') ||
-		      (p[1] != ':' && p[1] != '[')); p++)
+		     ((*p != '-' && *p != '+') ||
+		      (p[1] != ':' && p[1] != '[')) &&
+		     (*p != '=' ||
+		      (p[1] != ':' && p[1] != '[' && p[1] != '-')); p++)
 		if (*p == '\\' && p[1])
 		    p++;
 
@@ -674,8 +699,11 @@
 		otype = CAO_ODIRECT;
 		c = *++p;
 	    } else if (c == '=') {
-		otype = CAO_EQUAL;
-		c = *++p;
+		otype = CAO_OEQUAL;
+		if ((c = *++p) == '-') {
+		    otype = CAO_EQUAL;
+		    c = *++p;
+		}
 	    }
 	    /* Get the optional description, if any. */
 	    if (c == '[') {
@@ -797,9 +825,9 @@
 	    opt->args = oargs;
 	    opt->num = nopts++;
 
-	    if (otype == CAO_DIRECT)
+	    if (otype == CAO_DIRECT || otype == CAO_EQUAL)
 		ndopts++;
-	    else if (otype == CAO_ODIRECT || otype == CAO_EQUAL)
+	    else if (otype == CAO_ODIRECT || otype == CAO_OEQUAL)
 		nodopts++;
 
 	    /* If this is for single-letter option we also store a
@@ -952,7 +980,8 @@
 		    /* Return a pointer to the end of the option. */
 		    int l = strlen(p->name);
 
-		    if (p->type == CAO_EQUAL && line[l] == '=')
+		    if ((p->type == CAO_OEQUAL || p->type == CAO_EQUAL) &&
+			line[l] == '=')
 			l++;
 
 		    *end = line + l;
@@ -983,7 +1012,8 @@
 		p->args && p->type != CAO_NEXT && p->name[0] == pre) {
 		if (end) {
 		    line++;
-		    if (p->type == CAO_EQUAL && *line == '=')
+		    if ((p->type == CAO_OEQUAL || p->type == CAO_EQUAL) &&
+			*line == '=')
 			line++;
 		    *end = line;
 		}
@@ -1022,14 +1052,14 @@
 static LinkList ca_xor;
 
 static int
-ca_inactive(Cadef d, char **xor, int cur)
+ca_inactive(Cadef d, char **xor, int cur, int opts)
 {
-    if (xor && cur <= compcurrent) {
+    if ((xor || opts) && cur <= compcurrent) {
 	Caopt opt;
 	char *x;
 	int sl = (d->set ? strlen(d->set) : -1);
 
-	for (; (x = *xor); xor++) {
+	for (; (x = (opts ? "-" : *xor)); xor++) {
 	    if (ca_xor)
 		addlinknode(ca_xor, x);
 	    if (sl > 0) {
@@ -1059,6 +1089,9 @@
 		    a->active = 0;
 	    } else if ((opt = ca_get_opt(d, x, 1, NULL)))
 		opt->active = 0;
+
+	    if (opts)
+		break;
 	}
     }
     return 0;
@@ -1145,10 +1178,13 @@
 	ddef = adef = NULL;
 	doff = state.singles = 0;
 
-	if (ca_inactive(d, argxor, cur))
-	    return 1;
-
-	/* We've a definition for an argument, skip to the next. */
+	if (ca_inactive(d, argxor, cur, 0) ||
+	    ((d->flags & CDF_SEP) && !strcmp(line, "--"))) {
+	    if (ca_inactive(d, NULL, cur, 1))
+		return 1;
+	    continue;
+	}
+	/* We've got a definition for an argument, skip to the next. */
 
 	if (state.def) {
 	    state.arg = 0;
@@ -1186,10 +1222,14 @@
 	/* See if it's an option. */
 
 	if (state.opt == 2 && (state.curopt = ca_get_opt(d, line, 0, &pe)) &&
-	    (state.curopt->type != CAO_EQUAL || 
-	     compwords[cur] || pe[-1] == '=')) {
-
-	    ddef = state.def = state.curopt->args;
+	    (state.curopt->type == CAO_OEQUAL ?
+	     (compwords[cur] || pe[-1] == '=') :
+	     (state.curopt->type == CAO_EQUAL ?
+	      (pe[-1] == '=' || !pe[0]) : 1))) {
+
+	    ddef = state.def = ((state.curopt->type != CAO_EQUAL ||
+				 pe[-1] == '=') ?
+				state.curopt->args : NULL);
 	    doff = pe - line;
 	    state.optbeg = state.argbeg = state.inopt = cur;
 	    state.singles = (d->single && (!pe || !*pe) &&
@@ -1197,15 +1237,16 @@
 
 	    state.oargs[state.curopt->num] = znewlinklist();
 
-	    if (ca_inactive(d, state.curopt->xor, cur))
+	    if (ca_inactive(d, state.curopt->xor, cur, 0))
 		return 1;
 
 	    /* Collect the argument strings. Maybe. */
 
 	    if (state.def &&
 		(state.curopt->type == CAO_DIRECT ||
+		 state.curopt->type == CAO_EQUAL ||
 		 (state.curopt->type == CAO_ODIRECT && pe[0]) ||
-		 (state.curopt->type == CAO_EQUAL &&
+		 (state.curopt->type == CAO_OEQUAL &&
 		  (pe[0] || pe[-1] == '=')))) {
 		if (state.def->type != CAA_REST &&
 		    state.def->type != CAA_RARGS &&
@@ -1237,14 +1278,15 @@
 		if ((tmpopt = d->single[STOUC(*p)])) {
 		    state.oargs[tmpopt->num] = znewlinklist();
 
-		    if (ca_inactive(d, tmpopt->xor, cur))
+		    if (ca_inactive(d, tmpopt->xor, cur, 0))
 			return 1;
 		}
 	    }
 	    if (state.def &&
 		(state.curopt->type == CAO_DIRECT ||
+		 state.curopt->type == CAO_EQUAL ||
 		 (state.curopt->type == CAO_ODIRECT && pe[0]) ||
-		 (state.curopt->type == CAO_EQUAL &&
+		 (state.curopt->type == CAO_OEQUAL &&
 		  (pe[0] || pe[-1] == '=')))) {
 		if (state.def->type != CAA_REST &&
 		    state.def->type != CAA_RARGS &&
@@ -1261,6 +1303,9 @@
 	    return 1;
 	else if (state.arg) {
 	    /* Otherwise it's a normal argument. */
+	    if ((d->flags & CDF_ARG) && ca_inactive(d, NULL, cur + 1, 1))
+		return 1;
+
 	    if (state.inopt) {
 		state.inopt = 0;
 		state.nargbeg = cur - 1;
@@ -1541,7 +1586,7 @@
 		ca_xor = newlinklist();
 		if ((xor = getaparam(args[1]))) {
 		    if (arrcontains(xor, args[2], 0) ||
-			ca_inactive(def, xor, compcurrent)) {
+			ca_inactive(def, xor, compcurrent, 0)) {
 			ca_xor = cax;
 			return 1;
 		    }
@@ -1645,7 +1690,8 @@
 		      ztrdup(ca_laststate.ddef ?
 			     (ca_laststate.ddef->type == CAO_DIRECT ?
 			      "direct" :
-			      (ca_laststate.ddef->type == CAO_EQUAL ?
+			      ((ca_laststate.ddef->type == CAO_OEQUAL ||
+				ca_laststate.ddef->type == CAO_EQUAL) ?
 			       "equal" : "next")) : ""));
 	    return 0;
 	}

--
Sven Wischnowsky                         wischnow@xxxxxxxxxxxxxxxxxxxxxxx