PATCH: _arguments sets mixing rest/option arguments

PATCH: _arguments sets mixing rest/option arguments

[Oliver Kiddle]

With rest arguments defined in a different set, they get completed
alongside arguments to an option. This occurs with the _pkg function
distributed with FreeBSD's pkg in this case:
  pkg search --label=<tab>

Or for a minimal case try this:
  _arguments - set1 --label=:label - set2 '*:rest'

Without the sets, you wouldn't see the rest arguments.

Within _arguments, comparguments -D does the job of moving --label= from
PREFIX to IPREFIX. The number of bytes for this is in doff in the state
struct (of which every set has one). It doesn't really make sense for
doff to be per-set. There are use-cases for multiple values but it
would need to be separate for each bunch of matches you return out to
_arguments but that requires a lot of refactoring for little gain.
In all these years, I think I've only come across one command that
was ambiguous enough to come close to this being relevant (after pine
-c<tab> and even then it doesn't matter because we don't complete
numbers). Incidentally, while pondering such a solution, I noticed that
compset -p is removing bytes rather than characters to IPREFIX which
would have been useful in this case but should be corrected.

This then moves doff out of the state struct so there is only one value.
The old comment next to doff was one of those that I added earlier this
year. The new comment is more specific.

Oliver


diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c
index 0368a07d0..70cea9f27 100644
--- a/Src/Zle/computil.c
+++ b/Src/Zle/computil.c
@@ -1933,7 +1933,6 @@ struct castate {
     int inopt;		/* set to current word pos if word is a recognised option */
     int inarg;          /* in a normal argument */
     int nth;		/* number of current normal arg */
-    int doff;		/* length of current option */
     int singles;	/* argument consists of clumped options */
     int oopt;
     int actopts;	/* count of active options */
@@ -1943,6 +1942,7 @@ struct castate {
 
 static struct castate ca_laststate;
 static int ca_parsed = 0, ca_alloced = 0;
+static int ca_doff; /* no. of chars of ignored prefix (for clumped options or arg to an option) */
 
 static void
 freecastate(Castate s)
@@ -2033,7 +2033,7 @@ ca_parse_line(Cadef d, Cadef all, int multi, int first)
     state.argbeg = state.optbeg = state.nargbeg = state.restbeg = state.actopts =
 	state.nth = state.inopt = state.inarg = state.opt = state.arg = 1;
     state.argend = argend = arrlen(compwords) - 1;
-    state.doff = state.singles = state.oopt = 0;
+    state.singles = state.oopt = 0;
     state.curpos = compcurrent;
     state.args = znewlinklist();
     state.oargs = (LinkList *) zalloc(d->nopts * sizeof(LinkList));
@@ -2057,7 +2057,7 @@ ca_parse_line(Cadef d, Cadef all, int multi, int first)
 	 line; line = compwords[cur++]) {
 	ddef = adef = NULL;
 	dopt = NULL;
-	doff = state.singles = arglast = 0;
+	state.singles = arglast = 0;
 
         oline = line;
 #if 0
@@ -2114,7 +2114,7 @@ ca_parse_line(Cadef d, Cadef all, int multi, int first)
 		state.opt = 0;
 		state.argbeg = state.optbeg = state.inopt = cur;
 		state.argend = argend;
-		doff = state.doff = 0;
+		doff = 0;
 		state.singles = 1;
 		if (!state.oargs[state.curopt->num])
 		    state.oargs[state.curopt->num] = znewlinklist();
@@ -2287,7 +2287,6 @@ ca_parse_line(Cadef d, Cadef all, int multi, int first)
 		memcpy(&ca_laststate, &state, sizeof(state));
 		ca_laststate.ddef = NULL;
 		ca_laststate.dopt = NULL;
-		ca_laststate.doff = 0;
 		break;
 	    }
 	    zaddlinknode(state.args, ztrdup(line));
@@ -2324,7 +2323,6 @@ ca_parse_line(Cadef d, Cadef all, int multi, int first)
 
 		ca_laststate.ddef = NULL;
 		ca_laststate.dopt = NULL;
-		ca_laststate.doff = 0;
 		break;
 	    }
 	} else if (state.def && state.def->end)
@@ -2338,7 +2336,6 @@ ca_parse_line(Cadef d, Cadef all, int multi, int first)
 	    memcpy(&ca_laststate, &state, sizeof(state));
 	    ca_laststate.ddef = NULL;
 	    ca_laststate.dopt = NULL;
-	    ca_laststate.doff = 0;
 	} else if (cur == compcurrent && !ca_laststate.def) {
 	    if ((ca_laststate.def = ddef)) {
 		ca_laststate.singles = state.singles;
@@ -2349,7 +2346,7 @@ ca_parse_line(Cadef d, Cadef all, int multi, int first)
 		    ca_laststate.opt = 1;
 		    state.curopt->active = 1;
 		} else {
-		    ca_laststate.doff = doff;
+		    ca_doff = doff;
 		    ca_laststate.opt = 0;
 		}
 	    } else {
@@ -2452,11 +2449,16 @@ ca_set_data(LinkList descr, LinkList act, LinkList subc,
 		!strcmp((char *) getdata(anode), arg->action))
 		break;
 
+	/* with an ignored prefix, we're not completing any normal arguments */
+	if (single && !arg->opt)
+	    return;
+
 	if (!dnode) {
 	    addlinknode(descr, arg->descr);
 	    addlinknode(act, arg->action);
 
 	    if (!restr) {
+
 		if ((restr = (arg->type == CAA_RARGS)))
 		    restrict_range(ca_laststate.optbeg, ca_laststate.argend);
 		else if ((restr = (arg->type == CAA_RREST)))
@@ -2493,6 +2495,7 @@ ca_set_data(LinkList descr, LinkList act, LinkList subc,
 	if (arg->type == CAA_NORMAL &&
 	    opt && optdef && optdef->type == CAO_NEXT)
 	    return;
+
 	if (single)
 	    break;
 
@@ -2521,7 +2524,6 @@ ca_set_data(LinkList descr, LinkList act, LinkList subc,
     if (!single && opt && (lopt || ca_laststate.oopt)) {
 	opt = NULL;
 	arg = ca_get_arg(ca_laststate.d, ca_laststate.nth);
-
 	goto rec;
     }
     if (!opt && oopt > 0) {
@@ -2590,6 +2592,7 @@ bin_comparguments(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
 
 	    multi = !!def->snext; /* if we have sets */
 	    ca_parsed = cap;
+	    ca_doff = 0;
 
 	    while (def) { /* for each set */
 		use = !ca_parse_line(def, all, multi, first);
@@ -2629,23 +2632,20 @@ bin_comparguments(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
 	{
 	    LinkList descr, act, subc;
 	    Caarg arg;
-	    int ign = 0, ret = 1;
+	    int ret = 1;
 
 	    descr = newlinklist();
 	    act = newlinklist();
 	    subc = newlinklist();
 
+	    ignore_prefix(ca_doff);
 	    while (lstate) {
 		arg = lstate->def;
 
 		if (arg) {
 		    ret = 0;
-		    if (!ign && lstate->doff > 0) {
-			ign = 1;
-			ignore_prefix(lstate->doff);
-		    }
 		    ca_set_data(descr, act, subc, arg->opt, arg,
-				lstate->curopt, (lstate->doff > 0));
+				lstate->curopt, (ca_doff > 0));
 		}
 		lstate = lstate->snext;
 	    }
@@ -2673,7 +2673,7 @@ bin_comparguments(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
 
 	    for (; lstate; lstate = lstate->snext) {
 		if (lstate->actopts &&
-		    (lstate->opt || (lstate->doff && lstate->def) ||
+		    (lstate->opt || lstate->def ||
 		     (lstate->def && lstate->def->opt &&
 		      (lstate->def->type == CAA_OPT ||
 		       (lstate->def->type >= CAA_RARGS &&
diff --git a/Test/Y03arguments.ztst b/Test/Y03arguments.ztst
index 94ce8361e..de495ee6c 100644
--- a/Test/Y03arguments.ztst
+++ b/Test/Y03arguments.ztst
@@ -364,6 +364,14 @@
 0:repeatable options
 >line: {tst -v -v }{}
 
+ tst_arguments - set1 '--label=:arg:(a b)' - set2 ':rest:(rest args --label=not)'
+ comptest $'tst --label=\t'
+0:rest arguments from another set not completed after option from first set
+>line: {tst --label=}{}
+>DESCRIPTION:{arg}
+>NO:{a}
+>NO:{b}
+
  tst_arguments -A '-*' - help -h -V - other -a '*: :(-x more)'
  comptest $'tst -a -x m\t'
 0:continue completion after rest argument that looks like an option (with sets)

PATCH: handle multibyte chars in compset -p/-s

[Oliver Kiddle]

On 7 Oct, I wrote:
> Incidentally, while pondering such a solution, I noticed that
> compset -p is removing bytes rather than characters to IPREFIX which
> would have been useful in this case but should be corrected.

Having further checked uses of compset -p, I'm satisfied that compset -p
and -s should be handling multibyte characters so this is a patch to do
that. Is mb_metacharlenconv and backwardmetafiedchar as used here the
best way to do that?

Oliver

diff --git a/Src/Zle/complete.c b/Src/Zle/complete.c
index 68bdf2332..16f48c958 100644
--- a/Src/Zle/complete.c
+++ b/Src/Zle/complete.c
@@ -934,19 +934,45 @@ do_comp_vars(int test, int na, char *sa, int nb, char *sb, int mod)
 	}
     case CVT_PRENUM:
     case CVT_SUFNUM:
-	if (!na)
-	    return 1;
-	if (na > 0 &&
-	    (int)strlen(test == CVT_PRENUM ? compprefix : compsuffix) >= na) {
-	    if (mod) {
-		if (test == CVT_PRENUM)
-		    ignore_prefix(na);
-		else
-		    ignore_suffix(na);
-		return 1;
-	    }
+	if (na < 0)
 	    return 0;
+	if (na > 0 && mod) {
+#ifdef MULTIBYTE_SUPPORT
+	    if (isset(MULTIBYTE)) {
+		if (test == CVT_PRENUM) {
+		    const char *ptr = compprefix;
+		    int len = 1;
+		    int sum = 0;
+		    while (*ptr && na && len) {
+			wint_t wc;
+			len = mb_metacharlenconv(ptr, &wc);
+			ptr += len;
+			sum += len;
+			na--;
+		    }
+		    if (na)
+			return 0;
+		    na = sum;
+		} else {
+		    char *end = compsuffix + strlen(compsuffix);
+		    char *ptr = end;
+		    while (na-- && ptr > compsuffix)
+			 ptr = backwardmetafiedchar(compsuffix, ptr, NULL);
+		    if (na >= 0)
+			return 0;
+		    na = end - ptr;
+		}
+	    } else
+#endif
+	    if ((int)strlen(test == CVT_PRENUM ? compprefix : compsuffix) >= na)
+		return 0;
+	    if (test == CVT_PRENUM)
+		ignore_prefix(na);
+	    else
+		ignore_suffix(na);
+	    return 1;
 	}
+	return 1;
     case CVT_PREPAT:
     case CVT_SUFPAT:
 	{

Re: PATCH: handle multibyte chars in compset -p/-s

[Peter Stephenson]

On Mon, 09 Oct 2017 11:42:20 +0200
Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> On 7 Oct, I wrote:
> > Incidentally, while pondering such a solution, I noticed that
> > compset -p is removing bytes rather than characters to IPREFIX which
> > would have been useful in this case but should be corrected.
> 
> Having further checked uses of compset -p, I'm satisfied that compset -p
> and -s should be handling multibyte characters so this is a patch to do
> that. Is mb_metacharlenconv and backwardmetafiedchar as used here the
> best way to do that?

Probably --- it certainly doesn't look any more clumsy than the rest of
the multibyte handling.  Going backwards is a pain so closing your eyes
and calling the function is probably correct, and going forward that
family of function calls are much more efficient.

(BTW the real horror for multibyte characters in completion is matching
control --- it's tied to single-character array handling and at a point
where conversion from and to wide characters isn't particularly
convenient.  I gave up on this after looking for some weeks about a
decade ago.)

pws