Re: minor niggle with svn completion of sub-commands

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Nov 16,  2:45pm, Greg Klanderman wrote:
} Subject: Re: minor niggle with svn completion of sub-commands
}
} > The problem seems to be that we blindly clobber ca_laststate upon
} > discovering that we've reached the "rest" description.  The patch
} > below changes this behavior but I'm not certain it's complete; it
} 
} Now looking at this section of code again, I notice that in most cases
} where ca_laststate is set, it is conditioned on a predicate involving
} 'cur' (index of current word being parsed) and 'compcurrent' (index of
} word being completed) which makes some sense

Elswhere it matters whether the index of the current "definition"
being examined matches the index of the word being completed.  Here
the code has reached the "rest" of the arguments so neither index is
of consequence.

The problem is that the code appears to assume this condition won't be
reached if an earlier pair of indices matched up.  This is why the
code in parse_cadef inserts the definitions in sorted order.  This is
also why I'm increasingly convinced that the entire loop should end
as soon as any definition has been found for the index of the word
being completed.

} There's one other place that changes ca_laststate inside the loop over
} words (why is the local variable named 'line'?!)

Because it's an array of all the words that make up the entire line
from the editor (i.e., the L in ZLE).  This is just the way Sven W's
mind works.  (Now if only we had it back working for us ...)

} in the line, around line 1977:
} 
} |	    if (state.def->type == CAA_REST || state.def->type == CAA_RARGS ||
} |		state.def->type == CAA_RREST) {
} |		if (state.def->end && pattry(endpat, line)) {
} |		    state.def = NULL;
} |		    state.curopt = NULL;
} |		    state.opt = state.arg = 1;
} |		    state.argend = ca_laststate.argend = cur - 1;
} |		    goto cont;
} |		}
} 
} and it seems fishy because it is continuing the loop, not breaking out
} of it, and the 'cont' label logic seems to be responsible for when
} state data gets copied to ca_laststate.  Also, why is it setting just
} that one field in ca_laststate?

In this case AFAICT ca_laststate is being kept up to date in case NONE
of the conditions in the "if" just below "cont:" turn out to be true.
That is, ca_laststate should have something sensible in it even if we
loop through all possible definitions without finding any that apply
to the current word.
 
} It also seems strange that CAA_REST is not included in the conditions
} leading to the block of code you changed.  Why should CAA_REST be
} treated any differently here?

I'm not entirely sure but I think CAA_REST is omitted here because
the later code implements both normal-argument and argument-following-
an-option cases.

} > the entire loop elsewhere as soon as ca_laststate.def is non-NULL.
} 
} Maybe under the 'cont' label?  Is there any reason to parse the line
} past the current word being completed?

I begin to think the test really belongs in the "for" statement loop
conditions at the top.  "line && !ca_laststate.def" rather than just
"line".