Re: PATCH: compadd (+ questions)

Re: PATCH: compadd (+ questions)

[Sven Wischnowsky]

Bart Schaefer wrote:

> On Feb 11, 10:11am, Peter Stephenson wrote:
>
> ...
>
> } I, too, found the new [[-tests a little confusing at first sight.  I'm
> } inclined to think maybe they should be limited to those that can't be
> } done so easily in the standard way.  For example, I don't see why
> } people shouldn't use (( NMATCHES )) rather than [[ ! -nmatches 0 ]] .
> } But I haven't really got a proper grip on using this yet.
> 
> This is what I'm thinking, too.  I may be getting my chronology confused,
> but wasn't it the case that the new condition codes showed up before all
> the variables for NMATCHES and CURRENT etc. were available?
> 
> There's a short list of stuff you can't easily do with the variables:
> 
> 1.  Automatic shifting of bits of PREFIX into IPREFIX, as -iprefix does
>     and as -string and -class do with two args.
> 2.  Adjusting the range of words used, as -position does with two args.
> 3.  Skipping exactly N occurrences of an embedded substring, as -string
>     and -class do with two args.
> 
> Have I missed any others?  Sven didn't answer my question about whether
> the remaining condition codes are faster or have other side-effects:

(Sorry, I was rather busy yesterday and had to leave earlier...)

The -[m]between and -[m]after conditions also adjust the range of
words to use and these are the ones that are most complicated to
re-implement in shell-code.
Changing the values of the user-visible parameters is the only
side-effect they have.
As for the speed: they may be a bit faster than the equivalent tests
with parameter expansion but I don't think this is a argument for
keeping the condition codes since normally you'll have only few such
tests.

> The point being that, although being able to add conditions in modules is
> a cool thing, perhaps we should drop the ones that are easily replicated
> using simple tests on the variables, and then look again at the rest to
> see if there's a better way to express them.

Agreed, although I'm not sure if I'll produce a patch for this any
time soon.

> Unless there's a reason not to, such as, interpolating the string values
> of the variables during parsing of the expression is significantly slower
> than recognizing the new codes themselves (which I don't know whether or
> not is the case).

Again, I don't think that this is a problem, due to the expected
number of such tests.

> } For really clean completion functions we would need a new control
> } structure to be able to put all tests into one function and having
> } everything reset to the previous state automatically. Doing something
> } like:
> } 
> }   if comptest iprefix '-'; then
> }     ...
> }     compreset
> }   else
> } 
> } wouldn't be that easy to glark, too.
> 
> I still think the right way to do this is with shell-function scoping.
> 
> 	tryprefix () {
> 	    complocal	# Equivalent of "local CURRENT NMATCHES ...",
> 	    		# if "local" worked on special parameters
> 	    comptest ignored-prefix $1 && { shift ; "$@" }
> 	}
> 	tryprefix - complist -k "($signals[1,-3])"
>
> The unfortunate thing about this is that it clashes with $argv holding
> the command line (but that's true of any function called from within the
> top-level main-complete function, isn't it?).  There should probably be
> another array that also holds the words, but I guess it isn't that hard
> to have the main completion function set up a global for it.

Ah, now I finally understand why you wanting restting in function
scopes. One problem I have with this is that we can come up with a few 
functions for the tests, but the things to do if the test succeeds can 
often not easily be put in a command line.
Also we often need combinations of tests, for and'ed tests this would
lead to things like `tryrange -exec \; tryprefix - ...', but for or'ed 
tests. Also, will the non-parameter-modifying tests have equivalent
functions or will this lead to combinations of such function calls and 
`[[...]]' conditions? This can get very hard to read, I think.

About using argv for the stuff from the command line. I'm not too
happy with this anyway. If the new style completion is slower than the 
old one, this may partly be caused by several calls to sub-functions where 
we have to use "$@" to propagate the positional parameters.

> Looking at the above gives me a nagging idea ... it's half-formed at the
> moment, but it goes something like this ...
> 
> The effect of `compsave; [[ -iprefix ... ]] ; compreset` is that we want
> to try a completion with a particular prefix and if that fails, start
> over with the original state and try again.  Similarly for -position to 
> limit the range of words, etc.
> 
> So why don't we actually DO that?  That is, make a "recursive" call to
> the whole completion system?  Add -iprefix, -position, -string, -class
> options to "compcall" (or some similar new command) and have THAT adjust
> the variables, re-invoke the appropriate main completion function, and
> then restore everything if that function returns nonzero.  It would then
> be an advertized side-effect of compcall that, if it returns nonzero,
> the state of the completion variables has been adjusted -- which makes a
> lot more sense to me than having a conditional do it.

Only seldom do we want to do normal completion after adjusting the
word/words to use. In most cases we want to do some very specialised
calls to complist/compadd or whatever and we want only those
matches. Especially we normally don't want the main completion
function be called, it will use the `first'-completion-definition
again and things like that.

But, maybe we are on a completely wrong track. I forgot to mention it
yesterday, but when splitting the examples in different files I had
the unpleasent feeling that all this is the wrong way to do things.
The problem is that the current approach is completion-centered, not
command-centered. Just think about someone who wants to use one of the
example completions for another command. He would have to either
edit the file, adding the command name to the first line, or call
`defcomp' by hand.
I have the feeling, that this and your remarks about the `tryprefix'
function and about this recursive calling (and, btw, some of the mails
that started all this new ocmpletion stuff) point in the same
direction. Maybe we should re-build the example code in a completely
context-based way.
I haven't thought that much about all this yet, but it would go like
this:

We have a collection of context-names (completely shell-code based,
this is only vaguely connected to the CONTEXT parameter). For each
such context we have a definition saying what should be completed in
this context (such a definition could again be a function or an array, 
re-using some of the code). There would be a dispatcher-function that
gets the context-name as its argument and then invokes the
corresponding handler. Some of these handler-functions will do some
testing, and decide on the result of the tests, which contexts are
really to be used. The task of the main completion widget (note: the
widget, not the dispatcher) would be to check parameters like CONTEXT, 
decide which context(s) to try and then call the dispatcher.
We would then come up with files containing context-definitions (some
of the files are exactly that already and have names that show it, I
think that the fact that for other files names of commands seemed more
natural may be a hint that something went wrong).
Then we would need a way to connect command-names to context-names,
preferably in some automagically collected way.
When implementing the example stuff I had already had this kind of
feeling and thus the code already looks a bit like the above, but not
throughout. So, we wouldn't need that much changes (I think), we would
mainly need a shift in our perception of the problem (well, at least I
would need it) and change the code to reflect this everywhere.

Bye
 Sven


--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: PATCH: compadd (+ questions)

[Bart Schaefer]

On Feb 12,  9:42am, Sven Wischnowsky wrote:
} Subject: Re: PATCH: compadd (+ questions)
}
} > The point being that, although being able to add conditions in modules is
} > a cool thing, perhaps we should drop the ones that are easily replicated
} > using simple tests on the variables, and then look again at the rest to
} > see if there's a better way to express them.
} 
} Agreed, although I'm not sure if I'll produce a patch for this any
} time soon.

If you're about to do new doc, though, you shouldn't spend time documenting
something that's going to go away ...

} > 	tryprefix () {
} > 	    complocal	# Equivalent of "local CURRENT NMATCHES ...",
} > 	    		# if "local" worked on special parameters
} > 	    comptest ignored-prefix $1 && { shift ; "$@" }
} > 	}
} > 	tryprefix - complist -k "($signals[1,-3])"
} 
} Ah, now I finally understand why you wanting restting in function
} scopes. One problem I have with this is that we can come up with a few 
} functions for the tests, but the things to do if the test succeeds can 
} often not easily be put in a command line.

That's just a matter of more shell functions:

    listsignals() { complist -k "($signals[1,-3])" }
    tryprefix - listsignals

but you're right that a real syntactic construct would be much cleaner.

Maybe we can figure out a way to make "local" work inside { ... } ?  I've
seen some postings on zsh-users that lead me to believe some people think
it does already.

} Also we often need combinations of tests, for and'ed tests this would
} lead to things like `tryrange -exec \; tryprefix - ...', but for or'ed 
} tests.

You wouldn't do it that way.  You'd write a special function for each
particular combination of tests you wanted, and then just call them in
the appropriate spots.  Yes, this is crude, but it's easy to explain and
use, in terms of existing zsh scripting concepts.

} Also, will the non-parameter-modifying tests have equivalent
} functions or will this lead to combinations of such function calls and 
} `[[...]]' conditions? This can get very hard to read, I think.

My thought was that only the tests with side-effects get a builtin.  I
don't think the mixture will occur often enough to get that confusing.
If you don't need to "local"-ize the side-effects, which in at least
some cases you won't, you can just write stuff like

    if test $NMATCHES -eq 0; then
	...
    elif comptest ignored-prefix - ; then
	...
    elif ...
	...
    fi

I.e., use "test" if it's really important to avoid mixing [[ ... ]] with
plain ol' command syntax.

} About using argv for the stuff from the command line. I'm not too
} happy with this anyway. If the new style completion is slower than the 
} old one, this may partly be caused by several calls to sub-functions where 
} we have to use "$@" to propagate the positional parameters.

So don't propagate it via positional parameters after the first level.
Having them in the positionals is a convenience for when the top-level
completion function doesn't need to be as complex as __main_complete,
e.g., when writing special key completions.

For Functions/Completion/*, something like this:

    alias compsub 'words=( "$@" ) __normal || return 1'

or just have __main_complete assign to a local $words and then use it in
place of $argv in all of the sub-functions.

} > So why don't we actually DO that?  That is, make a "recursive" call to
} > the whole completion system?  Add -iprefix, -position, -string, -class
} > options to "compcall" (or some similar new command) and have THAT adjust
} > the variables, re-invoke the appropriate main completion function, and
} > then restore everything
} 
} Only seldom do we want to do normal completion after adjusting the
} word/words to use. In most cases we want to do some very specialised
} calls to complist/compadd or whatever and we want only those
} matches.

OK, then, give compcall another parameter that's the name of a different
top-level completion function to use.  That'd also solve the positional
parameter propagation problem we just talked about.

Note that I'm talking here about adding functionality to the compcall
builtin, not to some new piece in Functions/Completion/.

More later ...

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Thinking about Functions/Completion/* differently

[Bart Schaefer]

On Feb 12,  9:42am, Sven Wischnowsky wrote:
} Subject: Re: PATCH: compadd (+ questions)
}
} The problem is that the current approach is completion-centered, not
} command-centered. Just think about someone who wants to use one of the
} example completions for another command.

I think we need to be neither command-centered nor completion-centered.
We need a relational rather than operational view.

We have all these files, many of which amount to nothing more than a map
from a name like __aliases to an old compctl option letter.  We need to
recognize that this mapping is even more important than what the files
contain.

} I haven't thought that much about all this yet, but it would go like
} this:
} 
} We have a collection of context-names (completely shell-code based,
} this is only vaguely connected to the CONTEXT parameter). For each
} such context we have a definition saying what should be completed in
} this context (such a definition could again be a function or an array, 
} re-using some of the code).

This sounds OK so far ...

} There would be a dispatcher-function that
} gets the context-name as its argument and then invokes the
} corresponding handler.

What's the reason to do this with a dispatcher-function that takes an
argument, rather than simply using a "case ... esac"?

} We would then come up with files containing context-definitions (some
} of the files are exactly that already and have names that show it, I
} think that the fact that for other files names of commands seemed more
} natural may be a hint that something went wrong).

Not entirely.  By contexts, here, you mean things like "now we are
completing path names," "here we complete user names," "here we want
alias names," "now file names ending in .gz," etc.  Right?

There are only a few such contexts that are general enough to apply to
a wide range of commands.  Most of those are already represented by old
compctl options that take no arguments, and therefore can be directly
implemented by calling complist.  I don't think it's very useful to
have separate files for defining each of these cases.

Then there are a few other cases, like __x_options and __dvi, that are
specific to a particular class of commands; and lastly there are the
commands like __dd that have so many unique options that they form a
context of their own.  So if the goal was to have one file per context,
you aren't far off.

The hint that something is wrong is not the file names; it's the fact that
some only do match generation, whereas others do what might be called
"second-level dispatch."  Match generation is actually NOT the hard part!
The hard part is determining, from the command line, which context is the
one in which you're completing.

} Then we would need a way to connect command-names to context-names,
} preferably in some automagically collected way.

Connecting command-names to context-names isn't enough.  A single command
may have multiple contexts.

So it seems that what we need (with the usual caveats about contexts like
parameter substitution that need some other key than "command name") are:

1.  For each command, something that maps the command line and cursor
    postion to one of a set of a contexts.  (Some commands will never
    have more than one context.)

2.  For each context that can be generated by (1), something that maps
    the command name to the set of possible matches for that context.
    (Some contexts will never care about the command name or other key.)

3.  For each set of matches that can be generated by (2), something that
    maps the command and context to the code to insert the matches into
    the command line.  (There's lots of builtin support for this already.)

Some of those mappings may actually be multi-level, e.g., to implement
completion after the -exec for "find ... -exec ...".

Does this make sense?

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Re: PATCH: compadd (+ questions)

[Sven Wischnowsky]

Bart Schaefer wrote:

> [ completion tests and resetting the completion parameters... ]
>
> That's just a matter of more shell functions:
> 
>     listsignals() { complist -k "($signals[1,-3])" }
>     tryprefix - listsignals
> 
> but you're right that a real syntactic construct would be much cleaner.
> 
> Maybe we can figure out a way to make "local" work inside { ... } ?  I've
> seen some postings on zsh-users that lead me to believe some people think
> it does already.

We could use aliases in the meantime:

  alias compsave='local _oprefix$_level _oiprefix$_level _oargv$_level _ocurrent$_level; \
                eval "_oprefix${_level}=\"\$PREFIX\"; \
                _oiprefix${_level}=\"\$IPREFIX\"; \
                _oargv${_level}=( \"\$@\" ); \
                _ocurrent${_level}=\"\$CURRENT\""'
  alias compreset='eval "PREFIX=\"\$_oprefix${_level}\"; \
                 IPREFIX=\"\$_oiprefix${_level}\"; \
                 argv=( \"\$_oargv${_level}[@]\" ); \
		 CURRENT=\"\$_ocur${_level}\""'

  alias __if='(( $+_level )) || local _level=0; (( _level++ )); compsave; if'
  alias __elif='compreset; elif'
  alias __else='compreset; else'
  alias __fi='compreset; fi; unset _oprefix$_level _oiprefix$_level _oargv$_level _ocurrent$_level; (( _level-- ))'

With this one can use `__if ... __elif ... __else ... __fi' for tests
in completion functions (even nested) without having to worry about
calling comp{save,reset}.

But of course that's quite expensive...

Bye
 Sven


--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: PATCH: compadd (+ questions)

[Sven Wischnowsky]

I wrote:

> > > And yet another one (this one may still be experimental): `-r <str>'
> > > says that the suffix should be removed if the next character typed is
> > > one of those given in `<str>'.
> > 
> > Is there a decision on how this is going to work after Bart got at
> > the last proposal?
> 
> I'm currently thinking about keeping the `-r ...' option and adding a
> `-R func' option that gives a function to be called to remove the
> suffix when it is left unchanged by the `-r ...' stuff or the internal 
> suffix-removal-code (the non-inserting stuff known from `compctl').

The patch below implements almost this. The function given to `compadd -R'
will be called from zle the first time it thinks that there might be a 
suffix to remove. This function gets the length of the suffix inserted 
as a argument and can then use the widget-special parameters to test
what caused it to be called and to modify the command line.

I don't know if this is considered to be a general-enough solution to
be included, though.

Bye
 Sven

diff -u os/Zle/comp.h Src/Zle/comp.h
--- os/Zle/comp.h	Fri Feb 12 12:03:20 1999
+++ Src/Zle/comp.h	Fri Feb 12 13:55:52 1999
@@ -211,6 +211,7 @@
     int brpl;			/* the place where to put the brace prefix */
     int brsl;			/* ...and the suffix */
     char *rems;			/* when to remove the suffix */
+    char *remf;			/* shell function to call for suffix-removal */
 };
 
 #define CMF_FILE     1		/* this is a file */
diff -u os/Zle/comp1.c Src/Zle/comp1.c
--- os/Zle/comp1.c	Fri Feb 12 13:46:37 1999
+++ Src/Zle/comp1.c	Fri Feb 12 13:53:54 1999
@@ -49,7 +49,7 @@
 /* pointers to functions required by compctl and defined by zle */
 
 /**/
-void (*addmatchesptr) _((char *, char *, char *, char *, char *, char *, char *, char *, char *, int, int, Cmatcher, char **));
+void (*addmatchesptr) _((char *, char *, char *, char *, char *, char *, char *, char *, char *, char *, int, int, Cmatcher, char **));
 
 /**/
 char *(*comp_strptr) _((int*,int*));
diff -u os/Zle/compctl.c Src/Zle/compctl.c
--- os/Zle/compctl.c	Fri Feb 12 13:46:37 1999
+++ Src/Zle/compctl.c	Fri Feb 12 13:54:53 1999
@@ -1676,7 +1676,7 @@
     char *p, **sp, *e;
     char *ipre = NULL, *ppre = NULL, *psuf = NULL, *prpre = NULL;
     char *pre = NULL, *suf = NULL, *group = NULL, *m = NULL, *rs = NULL;
-    char *ign = NULL;
+    char *ign = NULL, *rf = NULL;
     int f = 0, a = 0, dm;
     Cmatcher match = NULL;
 
@@ -1754,9 +1754,15 @@
 		dm = 1;
 		break;
 	    case 'r':
+		f |= CMF_REMOVE;
 		sp = &rs;
 		e = "string expected after -%c";
 		break;
+	    case 'R':
+		f |= CMF_REMOVE;
+		sp = &rf;
+		e = "function name expected after -%c";
+		break;
 	    case '-':
 		argv++;
 		goto ca_args;
@@ -1792,7 +1798,7 @@
 
     match = cpcmatcher(match);
     addmatchesptr(ipre, ppre, psuf, prpre, pre, suf, group,
-		  rs, ign, f, a, match, argv);
+		  rs, rf, ign, f, a, match, argv);
     freecmatcher(match);
 
     return 0;
diff -u os/Zle/zle_misc.c Src/Zle/zle_misc.c
--- os/Zle/zle_misc.c	Fri Feb 12 13:46:38 1999
+++ Src/Zle/zle_misc.c	Fri Feb 12 14:31:10 1999
@@ -764,6 +764,11 @@
 /**/
 int suffixlen[257];
 
+/* Shell function to call to remove the suffix. */
+
+/**/
+static char *suffixfunc;
+
 /* Set up suffix: the last n characters are a suffix that should be *
  * removed in the usual word end conditions.                        */
 
@@ -798,9 +803,13 @@
 
 /**/
 void
-makesuffixstr(char *s, int n)
+makesuffixstr(char *f, char *s, int n)
 {
-    if (s) {
+    if (f) {
+	zsfree(suffixfunc);
+	suffixfunc = ztrdup(f);
+	suffixlen[0] = n;
+    } else if (s) {
 	int inv, i, v, z = 0;
 
 	if (*s == '^' || *s == '!') {
@@ -842,10 +851,33 @@
 void
 iremovesuffix(int c)
 {
-    int sl = suffixlen[c];
-    if(sl) {
-	backdel(sl);
-	invalidatelist();
+    if (suffixfunc) {
+	List l = getshfunc(suffixfunc);
+
+	if (l != &dummy_list) {
+	    LinkList args = newlinklist();
+	    char buf[20];
+	    int osc = sfcontext;
+
+	    sprintf(buf, "%d", suffixlen[0]);
+	    addlinknode(args, suffixfunc);
+	    addlinknode(args, buf);
+
+	    startparamscope();
+	    makezleparams(0);
+	    sfcontext = SFC_COMPLETE;
+	    doshfunc(suffixfunc, l, args, 0, 1);
+	    sfcontext = osc;
+	    endparamscope();
+	}
+	zsfree(suffixfunc);
+	suffixfunc = NULL;
+    } else {
+	int sl = suffixlen[c];
+	if(sl) {
+	    backdel(sl);
+	    invalidatelist();
+	}
     }
     fixsuffix();
 }
diff -u os/Zle/zle_tricky.c Src/Zle/zle_tricky.c
--- os/Zle/zle_tricky.c	Fri Feb 12 13:46:39 1999
+++ Src/Zle/zle_tricky.c	Fri Feb 12 14:08:31 1999
@@ -2458,7 +2458,7 @@
 /**/
 void
 addmatches(char *ipre, char *ppre, char *psuf, char *prpre, char *pre,
-	   char *suf, char *group, char *rems, char *ign,
+	   char *suf, char *group, char *rems, char *remf, char *ign,
 	   int flags, int aflags, Cmatcher match, char **argv)
 {
     char *s, *t, *e, *te, *ms, *lipre = NULL, *lpre, *lsuf, **aign = NULL;
@@ -2528,7 +2528,10 @@
 		if (aflags & CAF_NOSORT)
 		    mgroup->flags |= CGF_NOSORT;
 	    }
-	    if (rems)
+	    if (remf) {
+		remf = dupstring(remf);
+		rems = NULL;
+	    } else if (rems)
 		rems = dupstring(rems);
     	    if (ai->pprefix) {
 		if (pre)
@@ -2643,6 +2646,7 @@
 		cm->flags = flags;
 		cm->brpl = bpl;
 		cm->brsl = bsl;
+		cm->remf = remf;
 		cm->rems = rems;
 		addlinknode(l, cm);
 
@@ -2944,7 +2948,7 @@
     cm->flags = mflags | isf;
     cm->brpl = bpl;
     cm->brsl = bsl;
-    cm->rems = NULL;
+    cm->rems = cm->remf = NULL;
     addlinknode(l, cm);
 
     /* One more match for this explanation. */
@@ -5266,6 +5270,7 @@
     r->brpl = m->brpl;
     r->brsl = m->brsl;
     r->rems = ztrdup(m->rems);
+    r->remf = ztrdup(m->remf);
 
     return r;
 }
@@ -5378,6 +5383,7 @@
     zsfree(m->psuf);
     zsfree(m->prpre);
     zsfree(m->rems);
+    zsfree(m->remf);
 
     zfree(m, sizeof(m));
 }
@@ -5632,7 +5638,7 @@
 	if (menuwe) {
 	    menuend += menuinsc;
 	    if (m->flags & CMF_REMOVE) {
-		makesuffixstr(m->rems, menuinsc);
+		makesuffixstr(m->remf, m->rems, menuinsc);
 		if (menuinsc == 1)
 		    suffixlen[STOUC(m->suf[0])] = 1;
 	    }

--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: PATCH: compadd (+ questions)

[Sven Wischnowsky]

Peter Stephenson wrote:

> > Another small addition: the option `-F' says that fignore should be
> > used (in the same way as it's done by `compctl').
> 
> Would it not be just as easy to be able to specify a $fignore-type
> variable (maybe in the `compctl -k ...' fmt so you can add literals)?
> If it isn't, it doesn't matter, but I thought maybe that's no
> harder than hard-wiring $fignore in.

The patch below changes the `-F' option to compadd to take such a
string. It also changes the `__files' example function to accept a
optional `-F <string>' option which uses this (the handling in the
function could be improved). E.g. now you can do things like 
`__files -F "(.o .pro)"'.

Bye
 Sven

diff -u os/Zle/comp.h Src/Zle/comp.h
--- os/Zle/comp.h	Fri Feb 12 10:55:40 1999
+++ Src/Zle/comp.h	Fri Feb 12 11:09:22 1999
@@ -281,5 +281,4 @@
 #define CAF_MENU     2
 #define CAF_NOSORT   4
 #define CAF_ALT      8
-#define CAF_FIGNORE 16
-#define CAF_MATCH   32
+#define CAF_MATCH   16
diff -u os/Zle/comp1.c Src/Zle/comp1.c
--- os/Zle/comp1.c	Fri Feb 12 10:55:40 1999
+++ Src/Zle/comp1.c	Fri Feb 12 11:09:53 1999
@@ -49,7 +49,7 @@
 /* pointers to functions required by compctl and defined by zle */
 
 /**/
-void (*addmatchesptr) _((char *, char *, char *, char *, char *, char *, char *, char *, int, int, Cmatcher, char **));
+void (*addmatchesptr) _((char *, char *, char *, char *, char *, char *, char *, char *, char *, int, int, Cmatcher, char **));
 
 /**/
 char *(*comp_strptr) _((int*,int*));
diff -u os/Zle/compctl.c Src/Zle/compctl.c
--- os/Zle/compctl.c	Fri Feb 12 10:55:40 1999
+++ Src/Zle/compctl.c	Fri Feb 12 11:09:41 1999
@@ -1676,6 +1676,7 @@
     char *p, **sp, *e;
     char *ipre = NULL, *ppre = NULL, *psuf = NULL, *prpre = NULL;
     char *pre = NULL, *suf = NULL, *group = NULL, *m = NULL, *rs = NULL;
+    char *ign = NULL;
     int f = 0, a = 0, dm;
     Cmatcher match = NULL;
 
@@ -1698,7 +1699,8 @@
 		f |= CMF_FILE;
 		break;
 	    case 'F':
-		a |= CAF_FIGNORE;
+		sp = &ign;
+		e = "string expected after -%c";
 		break;
 	    case 'n':
 		f |= CMF_NOLIST;
@@ -1790,7 +1792,7 @@
 
     match = cpcmatcher(match);
     addmatchesptr(ipre, ppre, psuf, prpre, pre, suf, group,
-		  rs, f, a, match, argv);
+		  rs, ign, f, a, match, argv);
     freecmatcher(match);
 
     return 0;
diff -u os/Zle/zle_tricky.c Src/Zle/zle_tricky.c
--- os/Zle/zle_tricky.c	Fri Feb 12 10:55:42 1999
+++ Src/Zle/zle_tricky.c	Fri Feb 12 11:12:39 1999
@@ -2458,10 +2458,10 @@
 /**/
 void
 addmatches(char *ipre, char *ppre, char *psuf, char *prpre, char *pre,
-	   char *suf, char *group, char *rems, 
+	   char *suf, char *group, char *rems, char *ign,
 	   int flags, int aflags, Cmatcher match, char **argv)
 {
-    char *s, *t, *e, *te, *ms, *lipre = NULL, *lpre, *lsuf;
+    char *s, *t, *e, *te, *ms, *lipre = NULL, *lpre, *lsuf, **aign = NULL;
     int lpl, lsl, i, pl, sl, test, bpl, bsl, lsm, llpl;
     Aminfo ai;
     Cline lc = NULL;
@@ -2486,6 +2486,9 @@
     }
     if ((aflags & CAF_MENU) && isset(AUTOMENU))
 	usemenu = 1;
+    if (ign)
+	aign = get_user_var(ign);
+
     SWITCHHEAPS(compheap) {
 	HEAPALLOC {
 	    if (aflags & CAF_MATCH) {
@@ -2540,8 +2543,8 @@
 		ms = NULL;
 		bpl = brpl;
 		bsl = brsl;
-		if ((!psuf || !*psuf) && (aflags & CAF_FIGNORE)) {
-		    char **pt = fignore;
+		if ((!psuf || !*psuf) && aign) {
+		    char **pt = aign;
 		    int filell;
 
 		    for (test = 1; test && *pt; pt++)
diff -u om/Completion/__path_files Misc/Completion/__path_files
--- om/Completion/__path_files	Fri Feb 12 10:55:16 1999
+++ Misc/Completion/__path_files	Fri Feb 12 11:30:08 1999
@@ -12,13 +12,18 @@
 # You may also give the `-W <spec>' option as with `compctl' and `complist',
 # but only as the first argument.
 #
+# This function also accepts an optional `-F <string>' option as its first
+# argument or just after the `-W <spec>'. This can be used to define file
+# name extension (a la `fignore'). Files with such an extension will not
+# be considered possible completions.
+#
 # This function behaves as if you have a matcher definition like:
 #   compctl -M 'r:|[-.,_/]=* r:|=* m:{a-z}={A-Z} m:-=_ m:.=,' \
 #              'm:{a-z}={A-Z} l:|=* r:|=*'
 # so you may want to modify this.
 
 local nm prepaths str linepath realpath donepath patstr prepath testpath rest
-local tmp1 collect tmp2 suffixes i
+local tmp1 collect tmp2 suffixes i ignore
 
 setopt localoptions nullglob rcexpandparam globdots extendedglob
 unsetopt markdirs globsubst shwordsplit nounset
@@ -38,38 +43,54 @@
   prepaths=( '' )
 fi
 
+# Get the optional `-F' option and its argument.
+if [[ "$1" = -F ]]; then
+  ignore=(-F "$2")
+  shift 2
+else
+  ignore=''
+fi
+
 # str holds the whole string from the command line with a `*' between
 # the prefix and the suffix.
 
 str="${PREFIX:q}*${SUFFIX:q}"
 
-# We will first try normal completion called with `complist', first build
-# an array containing the `-W' option, if there is any and we want to use
-# it. We don't want to use it the string from the command line is a non-
-# relative path.
+# We will first try normal completion called with `complist', but only if we
+# weren't given a `-F' option.
 
-if [[ -z "$tmp1[1]" || "$str[1]" = [~/] || "$str" = (.|..)/* ]]; then
-  tmp1=()
-else
-  tmp1=(-W "( $ppres )")
-fi
+if [[ -z "$ignore" ]]; then
+  # First build an array containing the `-W' option, if there is any and we
+  # want to use it. We don't want to use it if the string from the command line
+  # is a absolute path or relative to the current directory.
+
+  if [[ -z "$tmp1[1]" || "$str[1]" = [~/] || "$str" = (.|..)/* ]]; then
+    tmp1=()
+  else
+    tmp1=(-W "( $ppres )")
+  fi
 
-# Now call complist.
+  # Now call complist.
 
-nm=$NMATCHES
-if [[ $# -eq 0 ]]; then
-  complist "$tmp1[@]" -f
-elif [[ "$1" = -g ]]; then
-  complist "$tmp1[@]" -g "$argv[2,-1]"
-  shift
-else
-  complist "$tmp1[@]" $1
-  shift
-fi
+  nm=$NMATCHES
+  if [[ $# -eq 0 ]]; then
+    complist "$tmp1[@]" -f
+  elif [[ "$1" = -g ]]; then
+    complist "$tmp1[@]" -g "$argv[2,-1]"
+    shift
+  else
+    complist "$tmp1[@]" $1
+    shift
+  fi
+
+  # If this generated any matches, we don't wnat to do in-path completion.
 
-# If this generated any matches, we don't wnat to do in-path completion.
+  [[ -nmatches nm ]] || return
 
-[[ -nmatches nm ]] || return
+  # No `-F' option, so we want to use `fignore'.
+
+  ignore=(-F fignore)
+fi
 
 # If we weren't given any file patterns as arguments, we trick ourselves
 # into believing that we were given the pattern `*'. This is just to simplify
@@ -215,7 +236,7 @@
 	# (the `-f' and `-F' options).
 
         for i in $collect; do
-          compadd -p "$linepath$testpath" -W "$tmp1" -s "/${i#*/}" -fF -- "${i%%/*}"
+          compadd -p "$linepath$testpath" -W "$tmp1" -s "/${i#*/}" -f "$ignore[@]" -- "${i%%/*}"
         done
 
 	# We have just finished handling all the matches from above, so we
@@ -247,5 +268,5 @@
   suffixes=( $str$@ )
   suffixes=( "${(@)suffixes:gs.**.*.}" )
   tmp2=( $~tmp1(#l)$~suffixes )
-  compadd -p "$linepath$testpath" -W "$prepath$realpath$testpath" -fF -- ${tmp2#$tmp1}
+  compadd -p "$linepath$testpath" -W "$prepath$realpath$testpath" -f "$ignore[@]" -- ${tmp2#$tmp1}
 done

--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: PATCH: compadd (+ questions)

[Sven Wischnowsky]

Peter Stephenson wrote:

> Just some queries about the new-style completion.  I'll put the more
> general questions first, then there are some more specific questions
> mainly to do with compadd.
> 
> First... I was looking at new-completion-examples and wondered:
> 
> 1) I suspect most people are going to use much of it as it stands, and
> just alter what they want.  (Who the *other* people might be, I cannot
> imagine.)  So might it be an idea to find a more general name than
> my-comp, and also to define widgets using main-complete for all the
> standard completion functions, not just expand-or-complete?  And
> perhaps do some trickery with bindkey output to rebind any keys using
> the old ones to the new ones?

The new-completion-example file is still a hack. I was planning to
clean it up anyway. As for the name: yes, I took that name to make it
the same I used in the description I sent in the mail. I hadn't
thought about the bindkey-trickery, though.
I also thought about adding a function that lets you bind special
completions to certain keys even more easily than in your replacement
functions for the old `zle -C'.

> Some way of supplying options to
> new-completion-examples would be good.

I don't understand what kind of options you mean here.

> 2) Currently, existing compctl's take precedence over any of the new
> style functions for given commands.  That's sort of reasonable at the
> moment, since many of the new-style functions are examples rather than
> complete systems.  I wonder if there's a better way of deciding this.
> If we implement something like 3), it might be better for the
> new-style ones to take precedence.

YES. Of course I made it this way to get people to test the new style
completion stuff. To those who want the new completion functions to
take precedence I suggust removing the call to `compcall' and the
replacing the function __default with:

  [[ -nmatches 0 ]] && compcall -TD

Until we find the final solution.

> 3) As far as the example functions are concerned, it might be an idea
> to separate them out into their own directory so they can be
> autoloaded or copied for autoloading.  How about this:  they get
> separated into a directory, and then some code in the main file scans
> that directory and decides file by file how to call defcomp.
> 
> For example, the file __default might contain
>   #defcomp --default--
>   files
> and then new-completion-examples can contain something like
>   local d f a
>   for d in $fpath; do
>     for f in $d/__*(N); do
>       read -A a <$f
>       if [[ $a[1] = '#defcomp' ]]; then
>   	defcomp $f:t $a[2]
>       fi
>     done
>   done
> Users can then put this directory where they want in their $fpath and
> add or subtract from it to get autoloading of completion functions.
> Also new-completion-examples can have a shorter name in its own
> directory.

I wanted to split the file into separate files and make them
autoloaded, too (that's why I added the `-a' option to defcomp).
Again, I hadn't thought about something like this loop, but I like it.

> Next... I'm late replying to this, but I was just looking over the
> new-style completion again and had some niggling questions...  I have
> half an eye on documentation for this new stuff, which Sven should
> probably have a go at first, although I think with something this
> involved it's good if someone other than the author makes sure they
> can understand it.

When I first sent the patch for all this, I hadn't tested it that
much and thought that probably some things might still change. After
using only the new style completion since the beginning of this year
(and after several fixes, of course) I'm starting to have more
confidence. So I already began to think about writing the manual, but
I don't know when I'll find the time for it.
And having a reputation for writing short and hard-to-understand
manual entries (in a bad English), I always hope others have a look at 
what I write and amend it.

> Sven Wischnowsky wrote:
> > Since `compadd' is the interface to the internals of the completion
> > stuff, it should be possible to use to add any words as matches. But
> > sometimes you probably want to use it only to have control over the
> > prefix/suffix- or the match-type stuff.
> 
> Does this mean a compadd without new matches alters what's there
> already, or what's to come, or are you simply saying you can have
> better control over what you are adding?

The last one. Only with `compadd' you can control all the strings and
flags stored for each match directly.

> > So, the patch below adds the `-m' option to `compadd'
> > which says that the completion code should use normal matching rules
> > to the words given. Of course, for this the `-M' option known from
> > `compctl' and `complist' is also supported.
> 
> This is fine.  Tests with with complist (compcall must be similar)
> suggests that uses the standard rules, is that right?  That would also
> be what I would guess.

Yes, `complist' behaves like `compctl' (and one could say that
`compcall' is `compctl').

> > Another small addition: the option `-F' says that fignore should be
> > used (in the same way as it's done by `compctl').
> 
> Would it not be just as easy to be able to specify a $fignore-type
> variable (maybe in the `compctl -k ...' fmt so you can add literals)?
> If it isn't, it doesn't matter, but I thought maybe that's no
> harder than hard-wiring $fignore in.

Hm, there are a lot of things I hadn't thought about, this is another
one. Meaning: yes, it would be easy to add.

> (Does this do any more than
>   for suf in $fignore; do
>     for match in $matches1; do
>       [[ $match = *$suf ]] || matches2=($matches2 $match)
>     done
>   done
> ?  I can see it's a lot more efficient, anyway.)

It does this plus the scheduling into what I have sometimes called the
list of alternative matches. These alternative matches are those that
match with fignore ignored and are used only if there are no matches
with fignore used.

The options `-m' and `-F' belong together, I wanted to make `compadd'
more user-friendly for those cases where one wants to add some matches
with, e.g. a generated path-prefix, and get all the matching
`complist' does without having to program it all by yourself.

> > And yet another one (this one may still be experimental): `-r <str>'
> > says that the suffix should be removed if the next character typed is
> > one of those given in `<str>'.
> 
> Is there a decision on how this is going to work after Bart got at
> the last proposal?

I'm currently thinking about keeping the `-r ...' option and adding a
`-R func' option that gives a function to be called to remove the
suffix when it is left unchanged by the `-r ...' stuff or the internal 
suffix-removal-code (the non-inserting stuff known from `compctl').

Bye
 Sven


--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: PATCH: compadd (+ questions)

[Peter Stephenson]

Sven Wischnowsky wrote:
> > Some way of supplying options to
> > new-completion-examples would be good.
> 
> I don't understand what kind of options you mean here.

Sorry, I wasn't clear about that.  I meant that if people are going to
be using whatever-it's-going-to-be-called more or less en bloc, it
would be helpful to have ways of telling it whether or not to bind the
widgets it's just defined (and maybe other things like that).
However, since it's almost certainly going to be sourced, it's hard to
supply options in a simple way.  Maybe I'm looking too far ahead, but
if it becomes necessary, a single assoc array would probably be the
thing.  (I spellt that necessarray the first time.)

I hope gradually to move over to the new completion forms and was
simply wondering about things I ought to be looking out for.

I, too, found the new [[-tests a little confusing at first sight.  I'm
inclined to think maybe they should be limited to those that can't be
done so easily in the standard way.  For example, I don't see why
people shouldn't use (( NMATCHES )) rather than [[ ! -nmatches 0 ]] .
But I haven't really got a proper grip on using this yet.

-- 
Peter Stephenson <pws@ibmth.df.unipi.it>       Tel: +39 050 844536
WWW:  http://www.ifh.de/~pws/
Dipartimento di Fisica, Via Buonarroti 2, 56127 Pisa, Italy

Re: PATCH: compadd (+ questions)

[Sven Wischnowsky]

Bart Schaefer wrote:

> On Feb 10,  5:26pm, Peter Stephenson wrote:
> } Subject: Re: PATCH: compadd (+ questions)
> }
> } 1) I suspect most people are going to use much of it as it stands, and
> } just alter what they want.  (Who the *other* people might be, I cannot
> } imagine.)  So might it be an idea to find a more general name than
> } my-comp, and also to define widgets using main-complete for all the
> } standard completion functions, not just expand-or-complete?  And
> } perhaps do some trickery with bindkey output to rebind any keys using
> } the old ones to the new ones?  Some way of supplying options to
> } new-completion-examples would be good.
> } 
> } 2) Currently, existing compctl's take precedence over any of the new
> } style functions for given commands.  That's sort of reasonable at the
> } moment, since many of the new-style functions are examples rather than
> } complete systems.  I wonder if there's a better way of deciding this.
> } If we implement something like 3), it might be better for the
> } new-style ones to take precedence.
> 
> Addressing both of these questions together:
> 
> I do think it's a good idea to make the examples as directly usable as
> possible, and therefore to carefully choose descriptive names for the
> various functions (nothing so uninformative as "my-comp" nor as generic
> as "files").  It's probably also a good idea to have separate examples
> emulating as many of the old built-in completions as possible, so that
> people have "building blocks" from which to construct completions that
> have effects similar to the built-in set or combinations thereof.

As I have said, the current examples are a mixture of hacking and
re-implementing stuff from the old completion examples. This was
thought to show to people on the workers-list how to use all this
stuff in real-world examples.

> That'll also provide a good set of test cases to see whether we've (a)
> achieved the goal of writing something more understandable than compctl,
> and (b) made all the necessary bits of the functionality available to
> the shell script programmer.

This was another goal of the example file. Some of the patches I sent
after the initial patch for the new style completion were results of
using the examples and finding bugs, omissions, or better ways. But I
simply need some help in evaluating all this.

> However, for the simple cases I'm concerned that calling interpreted
> shell functions will be "too slow" and we shouldn't encourage people to
> simply load up the examples en masse.  Has anybody done any speed
> comparisons on having zsh call the old-style compctls directly, vs. the
> new-style stuff firing `compcall`, vs. doing the entire thing with the
> new stuff?  I confess I haven't had much chance to play with this.

The difference between `compctl' and `compcall' shouldn't be too
big. As I already said, I have been using only new style completions
all this year. For now, the speed is acceptable (meaning that it
doesn't interrupt things noticeably). Also, my zsh is compiled with
all debugging and without any optimisation, so in a zsh for work, this 
will get faster (and of course this also depends on the machine you
use, I have a older Alpha, dunno how this compares to a Ultra or
Pentium).

> } 3) As far as the example functions are concerned, it might be an idea
> } to separate them out into their own directory so they can be
> } autoloaded or copied for autoloading.  How about this:  they get
> } separated into a directory, and then some code in the main file scans
> } that directory and decides file by file how to call defcomp.
> 
> This sounds like a good idea.  BTW, defcomp and friends could use some
> more commenting up front.  The meaning of, say, "--default--" as the
> second parameter to defcomp should be explained near the definition of
> defcomp, either instead of or as well as down in main-complete when the
> associative array is finally referenced.
> 
>  ----------
> 
> As to goal (a) above:  This is the first time I've really taken the time
> to read through the new-completion-examples file, and I find myself
> disappointed by a couple of things that sounded good in theory.  Part
> of it may just be Sven's coding style; can we use better variable names
> than "a", "b", c", "opa", "epre", and "ppres", please?  It's bad enough
> having that in the C code, without exposing mere mortals to it.

This (and the comments) will certainly be cleaned up in the final
version.

> I think the worst are the new switches for the conditional expressions,
> particularly the ones that take two following strings; it looks wrong.
> I just spent several minutes pondering whether [[ -word CURRENT-1 -ef ]]
> would look better to me than [[ -current -1 -ef ]], and decided it's
> only marginally better.  It using -current or -word a lot faster than
> using [[ $@[CURRENT-1] == '-ef' ]]?  Are they doing some other magic?
> 
> For the conditionals that don't use two strings, I think some of the
> keywords could be better, but I don't yet have a list of suggestions.
> Maybe it's just the 'm' prefix on the ones that take a pattern that
> bothers me, and which also makes me wonder for a moment whether there's
> supposed to be a -matches is when I see that there's a -nmatches.

All these are things I didn't like that much, too. Most of the
conditions were added for completeness' sake.

> Also, and I apologize for not responding more immediately to Sven from
> zsh-workers/5083, I must say that the way that -iprefix, -position and
> -string have parameter-modifying side-effects makes me cringe.  It'd be
> fine in another new builtin, but a [[ ]] with side-effects?!?  Yipes!
> 
> Don't go changing anything yet just on the basis of my opinion, though;
> I'm only trying to hear some discussion about this stuff.  I realize
> that the side-effects are merely mirroring the state of the completion
> internals, I just question letting this happen from [[ ]], and maybe
> whether there should be a builtin equivalent of compsave/compreset.

This is what I was questioning again and again from the beginning. For
really clean completion functions we would need a new control
structure to be able to put all tests into one function and having
everything reset to the previous state automatically. Doing something
like:

  if comptest iprefix '-'; then
    ...
    compreset
  else

wouldn't be that easy to glark, too. I have been thinking a lot about
better ways to do all this but haven't found an answer yet (given that 
we won't want to add a new syntactical construct for this).

> I should close by saying that I *am* impressed, overall, with the new
> completion scheme and the possibilities it presents.  I'm particularly
> glad of way it and PWS's completion widgets were able to collapse into
> one system.  I think the progress is good, and that we should keep on
> speeding towards something even more marvelous.

Yes, and now that Peter has started, maybe we can get a discussion
about all this.

Bye
 Sven


--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: PATCH: compadd (+ questions)

[Bart Schaefer]

On Feb 11, 10:11am, Peter Stephenson wrote:
} Subject: Re: PATCH: compadd (+ questions)
}
} Sven Wischnowsky wrote:
} > > Some way of supplying options to
} > > new-completion-examples would be good.
} > 
} > I don't understand what kind of options you mean here.
} 
} Sorry, I wasn't clear about that.  I meant that if people are going to
} be using whatever-it's-going-to-be-called more or less en bloc, it
} would be helpful to have ways of telling it whether or not to bind the
} widgets it's just defined (and maybe other things like that).
} However, since it's almost certainly going to be sourced, it's hard to
} supply options in a simple way.

Eh?

    zsh% source =(echo 'echo $0: $*') these are the args
    /tmp/zsha00834: these are the args

} I, too, found the new [[-tests a little confusing at first sight.  I'm
} inclined to think maybe they should be limited to those that can't be
} done so easily in the standard way.  For example, I don't see why
} people shouldn't use (( NMATCHES )) rather than [[ ! -nmatches 0 ]] .
} But I haven't really got a proper grip on using this yet.

This is what I'm thinking, too.  I may be getting my chronology confused,
but wasn't it the case that the new condition codes showed up before all
the variables for NMATCHES and CURRENT etc. were available?

There's a short list of stuff you can't easily do with the variables:

1.  Automatic shifting of bits of PREFIX into IPREFIX, as -iprefix does
    and as -string and -class do with two args.
2.  Adjusting the range of words used, as -position does with two args.
3.  Skipping exactly N occurrences of an embedded substring, as -string
    and -class do with two args.

Have I missed any others?  Sven didn't answer my question about whether
the remaining condition codes are faster or have other side-effects:

On Feb 11, 10:38am, Sven Wischnowsky wrote:
} Subject: Re: PATCH: compadd (+ questions)
}
} Bart Schaefer wrote:
} 
} > I think the worst are the new switches for the conditional expressions,
} > particularly the ones that take two following strings; it looks wrong.
} > I just spent several minutes pondering whether [[ -word CURRENT-1 -ef ]]
} > would look better to me than [[ -current -1 -ef ]], and decided it's
} > only marginally better.  It using -current or -word a lot faster than
} > using [[ $@[CURRENT-1] == '-ef' ]]?  Are they doing some other magic?

Sven?

The point being that, although being able to add conditions in modules is
a cool thing, perhaps we should drop the ones that are easily replicated
using simple tests on the variables, and then look again at the rest to
see if there's a better way to express them.

Unless there's a reason not to, such as, interpolating the string values
of the variables during parsing of the expression is significantly slower
than recognizing the new codes themselves (which I don't know whether or
not is the case).

} For really clean completion functions we would need a new control
} structure to be able to put all tests into one function and having
} everything reset to the previous state automatically. Doing something
} like:
} 
}   if comptest iprefix '-'; then
}     ...
}     compreset
}   else
} 
} wouldn't be that easy to glark, too.

I still think the right way to do this is with shell-function scoping.

	tryprefix () {
	    complocal	# Equivalent of "local CURRENT NMATCHES ...",
	    		# if "local" worked on special parameters
	    comptest ignored-prefix $1 && { shift ; "$@" }
	}
	tryprefix - complist -k "($signals[1,-3])"

The unfortunate thing about this is that it clashes with $argv holding
the command line (but that's true of any function called from within the
top-level main-complete function, isn't it?).  There should probably be
another array that also holds the words, but I guess it isn't that hard
to have the main completion function set up a global for it.

Looking at the above gives me a nagging idea ... it's half-formed at the
moment, but it goes something like this ...

The effect of `compsave; [[ -iprefix ... ]] ; compreset` is that we want
to try a completion with a particular prefix and if that fails, start
over with the original state and try again.  Similarly for -position to 
limit the range of words, etc.

So why don't we actually DO that?  That is, make a "recursive" call to
the whole completion system?  Add -iprefix, -position, -string, -class
options to "compcall" (or some similar new command) and have THAT adjust
the variables, re-invoke the appropriate main completion function, and
then restore everything if that function returns nonzero.  It would then
be an advertized side-effect of compcall that, if it returns nonzero,
the state of the completion variables has been adjusted -- which makes a
lot more sense to me than having a conditional do it.

Then most if not all of the completion-related conditionals can go away.

However, I don't really understand yet the interaction of (say) adding
matches for menucomplete that rely on a prefix in one place, and adding
more matches for the same completion in another place that rely on a
different prefix, with the state of the completion variables in both
places.  Maybe the variables should simply ALWAYS be restored when the
recursive compcall unwinds; or maybe there's some other interaction that
makes this whole idea unworkable.

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

PATCH: compadd

[Sven Wischnowsky]

I already said that I wanted to make `compadd' more powerful.

Since `compadd' is the interface to the internals of the completion
stuff, it should be possible to use to add any words as matches. But
sometimes you probably want to use it only to have control over the
prefix/suffix- or the match-type stuff. In this case you would have to 
do all the matching with what's on the line yourself which can't be
done in a generic way since you'll never know what matchers other
people use. So, the patch below adds the `-m' option to `compadd'
which says that the completion code should use normal matching rules
to the words given. Of course, for this the `-M' option known from
`compctl' and `complist' is also supported.

Another small addition: the option `-F' says that fignore should be
used (in the same way as it's done by `compctl').

And yet another one (this one may still be experimental): `-r <str>'
says that the suffix should be removed if the next character typed is
one of those given in `<str>'. This is parsed like a key-string to
`bindkey', with one more backslash operator: if a `\-' apears in the
string, this is taken to represent all characters that do not insert
anything (e.g. `return' bound to `accept-line'). The whole string is
taken as a character class, i.e. `a-z' means `all lower case letters'
and a `6' or `!' at the beginning means `all characters but...'.

Bye
 Sven

diff -u os/utils.c Src/utils.c
--- os/utils.c	Fri Jan 29 15:56:16 1999
+++ Src/utils.c	Sat Jan 30 15:33:38 1999
@@ -3349,12 +3349,19 @@
 	    case Meta:
 		*t++ = '\\', s--;
 		break;
+	    case '-':
+		if (fromwhere == 5) {
+		    *misc  = 1;
+		    break;
+		}
+		goto def;
 	    case 'c':
 		if (fromwhere < 2) {
 		    *misc = 1;
 		    break;
 		}
 	    default:
+	    def:
 		if ((idigit(*s) && *s < '8') || *s == 'x') {
 		    if (!fromwhere) {
 			if (*s == '0')
@@ -3386,7 +3393,7 @@
 	} else if (fromwhere == 4 && *s == Snull) {
 	    for (u = t; (*u++ = *s++););
 	    return t + 1;
-	} else if (*s == '^' && fromwhere == 2) {
+	} else if (*s == '^' && (fromwhere == 2 || fromwhere == 5)) {
 	    control = 1;
 	    continue;
 	} else if (*s == Meta)
diff -u os/Zle/comp.h Src/Zle/comp.h
--- os/Zle/comp.h	Fri Jan 29 15:56:32 1999
+++ Src/Zle/comp.h	Sat Jan 30 15:46:36 1999
@@ -210,6 +210,7 @@
     int flags;			/* see CMF_* below */
     int brpl;			/* the place where to put the brace prefix */
     int brsl;			/* ...and the suffix */
+    char *rems;			/* when to remove the suffix */
 };
 
 #define CMF_FILE     1		/* this is a file */
@@ -272,3 +273,12 @@
 
 #define CFN_FIRST   1
 #define CFN_DEFAULT 2
+
+/* Flags for compadd and addmatches(). */
+
+#define CAF_QUOTE    1
+#define CAF_MENU     2
+#define CAF_NOSORT   4
+#define CAF_ALT      8
+#define CAF_FIGNORE 16
+#define CAF_MATCH   32
diff -u os/Zle/comp1.c Src/Zle/comp1.c
--- os/Zle/comp1.c	Fri Jan 29 15:56:32 1999
+++ Src/Zle/comp1.c	Sat Jan 30 15:51:10 1999
@@ -55,7 +55,7 @@
 /* pointers to functions required by compctl and defined by zle */
 
 /**/
-void (*addmatchesptr) _((char *, char *, char *, char *, char *, char *, char *, int, int, int, int, int, char **));
+void (*addmatchesptr) _((char *, char *, char *, char *, char *, char *, char *, char *, int, int, Cmatcher, char **));
 
 /**/
 char *(*comp_strptr) _((int*,int*));
diff -u os/Zle/compctl.c Src/Zle/compctl.c
--- os/Zle/compctl.c	Fri Jan 29 15:56:32 1999
+++ Src/Zle/compctl.c	Sat Jan 30 15:52:33 1999
@@ -1703,8 +1703,9 @@
 {
     char *p, **sp, *e;
     char *ipre = NULL, *ppre = NULL, *psuf = NULL, *prpre = NULL;
-    char *pre = NULL, *suf = NULL, *group = NULL;
-    int f = 0, q = 0, m = 0, ns = 0, a = 0;
+    char *pre = NULL, *suf = NULL, *group = NULL, *m = NULL, *rs = NULL;
+    int f = 0, a = 0, dm;
+    Cmatcher match = NULL;
 
     if (incompfunc != 1) {
 	zerrnam(name, "can only be called from completion function", NULL, 0);
@@ -1713,21 +1714,25 @@
     for (; *argv && **argv ==  '-'; argv++) {
 	for (p = *argv + 1; *p; p++) {
 	    sp = NULL;
+	    dm = 0;
 	    switch (*p) {
 	    case 'q':
 		f |= CMF_REMOVE;
 		break;
 	    case 'Q':
-		q = 1;
+		a |= CAF_QUOTE;
 		break;
 	    case 'f':
 		f |= CMF_FILE;
 		break;
+	    case 'F':
+		a |= CAF_FIGNORE;
+		break;
 	    case 'n':
 		f |= CMF_NOLIST;
 		break;
 	    case 'U':
-		m = 1;
+		a |= CAF_MENU;
 		break;
 	    case 'P':
 		sp = &pre;
@@ -1743,7 +1748,7 @@
 		break;
 	    case 'V':
 		if (!group)
-		    ns = 1;
+		    a |= CAF_NOSORT;
 		sp = &group;
 		e = "group name expected after -%c";
 		break;
@@ -1764,7 +1769,19 @@
 		e = "string expected after -%c";
 		break;
 	    case 'a':
-		a = 1;
+		a |= CAF_ALT;
+		break;
+	    case 'm':
+		a |= CAF_MATCH;
+		break;
+	    case 'M':
+		sp = &m;
+		e = "matching specification expected after -%c";
+		dm = 1;
+		break;
+	    case 'r':
+		sp = &rs;
+		e = "string expected after -%c";
 		break;
 	    case '-':
 		argv++;
@@ -1788,6 +1805,10 @@
 		    zerrnam(name, e, NULL, *p);
 		    return 1;
 		}
+		if (dm && (match = parse_cmatcher(name, m)) == pcm_err) {
+		    match = NULL;
+		    return 1;
+		}
 	    }
 	}
     }
@@ -1796,7 +1817,7 @@
 	return 1;
 
     addmatchesptr(ipre, ppre, psuf, prpre, pre, suf, group,
-		  f, q, m, ns, a, argv);
+		  rs, f, a, match, argv);
     return 0;
 }
 
--- os/Zle/zle_misc.c	Mon Feb  1 09:47:35 1999
+++ Src/Zle/zle_misc.c	Mon Feb  1 09:56:58 1999
@@ -792,6 +792,49 @@
     }
 }
 
+/* Set up suffix given a string containing the characters on which to   *
+ * remove the suffix. */
+
+/**/
+void
+makesuffixstr(char *s, int n)
+{
+    if (s) {
+	int inv, i, v, z = 0;
+
+	if (*s == '^' || *s == '!') {
+	    inv = 1;
+	    s++;
+	} else
+	    inv = 0;
+	s = getkeystring(s, &i, 5, &z);
+	s = metafy(s, i, META_USEHEAP);
+
+	if (inv) {
+	    v = 0;
+	    for (i = 0; i < 257; i++)
+		 suffixlen[i] = n;
+	} else
+	    v = n;
+
+	if (z)
+	    suffixlen[256] = v;
+
+	while (*s) {
+	    if (s[1] == '-' && s[2]) {
+		int b = (int) *s, e = (int) s[2];
+
+		while (b <= e)
+		    suffixlen[b++] = v;
+		s += 2;
+	    } else
+		suffixlen[*s] = v;
+	    s++;
+	}
+    } else
+	makesuffix(n);
+}
+
 /* Remove suffix, if there is one, when inserting character c. */
 
 /**/
diff -u os/Zle/zle_tricky.c Src/Zle/zle_tricky.c
--- os/Zle/zle_tricky.c	Fri Jan 29 15:56:35 1999
+++ Src/Zle/zle_tricky.c	Sat Jan 30 16:05:25 1999
@@ -2408,20 +2412,45 @@
 /**/
 void
 addmatches(char *ipre, char *ppre, char *psuf, char *prpre, char *pre,
-	   char *suf, char *group,
-	   int flags, int quote, int menu, int nosort, int alt, char **argv)
+	   char *suf, char *group, char *rems, 
+	   int flags, int aflags, Cmatcher match, char **argv)
 {
-    char *s, *t;
-    int lpl, lsl, i;
-    Aminfo ai = (alt ? fainfo : ainfo);
+    char *s, *t, *e, *te, *ms, *lipre = NULL, *lpre, *lsuf;
+    int lpl, lsl, i, pl, sl, test, bpl, bsl, lsm, llpl;
+    Aminfo ai;
+    Cline lc = NULL;
+    LinkList l;
     Cmatch cm;
+    struct cmlist mst;
+    Cmlist oms = mstack;
 
-    if (menu && isset(AUTOMENU))
+    if (aflags & CAF_ALT) {
+	l = fmatches;
+	ai = fainfo;
+    } else {
+	l = matches;
+	ai = ainfo;
+    }
+    if (match) {
+	mst.next = mstack;
+	mst.matcher = match;
+	mstack = &mst;
+    }
+    if ((aflags & CAF_MENU) && isset(AUTOMENU))
 	usemenu = 1;
     SWITCHHEAPS(compheap) {
 	HEAPALLOC {
+	    if (aflags & CAF_MATCH) {
+		ctokenize(lipre = dupstring(compiprefix));
+		remnulargs(lipre);
+		ctokenize(lpre = dupstring(compprefix));
+		remnulargs(lpre);
+		llpl = strlen(lpre);
+		ctokenize(lsuf = dupstring(compsuffix));
+		remnulargs(lsuf);
+	    }
 	    if (ipre)
-		ipre = dupstring(ipre);
+		ipre = (lipre ? dyncat(lipre, ipre) : dupstring(ipre));
 	    if (ppre) {
 		ppre = dupstring(ppre);
 		lpl = strlen(ppre);
@@ -2432,6 +2461,8 @@
 		lsl = strlen(psuf);
 	    } else
 		lsl = 0;
+	    if (aflags & CAF_MATCH)
+		lsm = (psuf ? !strcmp(psuf, lsuf) : (!lsuf || !*lsuf));
 	    if (pre)
 		pre = dupstring(pre);
 	    if (suf)
@@ -2443,10 +2474,12 @@
 		prpre = dupstring(prpre);
 	    if (group) {
 		endcmgroup(NULL);
-		begcmgroup(group, nosort);
-		if (nosort)
+		begcmgroup(group, (aflags & CAF_NOSORT));
+		if (aflags & CAF_NOSORT)
 		    mgroup->flags |= CGF_NOSORT;
 	    }
+	    if (rems)
+		rems = dupstring(rems);
     	    if (ai->pprefix) {
 		if (pre)
 		    ai->pprefix[pfxlen(ai->pprefix, pre)] = '\0';
@@ -2455,8 +2488,52 @@
 	    } else
 		ai->pprefix = dupstring(pre ? pre : "");
 
-	    for (; (s = *argv); argv++) {
-		if (ai->firstm) {
+	    for (; (s = dupstring(*argv)); argv++) {
+		sl = strlen(s);
+		ms = NULL;
+		bpl = brpl;
+		bsl = brsl;
+		if ((!psuf || !*psuf) && (aflags & CAF_FIGNORE)) {
+		    char **pt = fignore;
+		    int filell;
+
+		    for (test = 1; test && *pt; pt++)
+			if ((filell = strlen(*pt)) < sl
+			    && !strcmp(*pt, s + sl - filell))
+			    test = 0;
+
+		    if (!test) {
+			l = fmatches;
+			ai = fainfo;
+		    } else {
+			l = matches;
+			ai = ainfo;
+		    }
+		}
+		if (aflags & CAF_MATCH) {
+		    t = (ppre ? dyncat(ppre, s) : s);
+		    pl = sl + lpl;
+		    if ((test = (llpl <= pl && !strncmp(t, lpre, pl))))
+			test = lsm;
+		    if (!test && mstack &&
+			(ms = comp_match(lpre, lsuf,
+					 (psuf ? dyncat(t, psuf) : t),
+					 &lc, (aflags & CAF_QUOTE),
+					 &bpl, &bsl)))
+			test = 1;
+		    if (!test)
+			continue;
+		    e = s + sl;
+		} else {
+		    e = s;
+		    pl = lpl;
+		}
+		if (!(aflags & CAF_QUOTE)) {
+		    te = s + pl;
+		    s = quotename(s, &e, te, &pl);
+		    sl = strlen(s);
+		}
+		if (!ms && ai->firstm) {
 		    if ((i = pfxlen(ai->firstm->str, s)) < ai->prerest)
 			ai->prerest = i;
 		    if ((i = sfxlen(ai->firstm->str, s)) < ai->suflen)
@@ -2466,15 +2543,20 @@
 		if (ppre)
 		    t = dyncat(ppre, t);
 		if (ipre && *ipre) {
+		    Cline tlc = prepend_cline(ipre, lc);
+
 		    ai->noipre = 0;
-		    if (ai->icpl > lpl)
-			ai->icpl = lpl;
-		    if (ai->icsl > lsl)
-			ai->icsl = lsl;
-		    if (ai->iaprefix)
-			ai->iaprefix[pfxlen(ai->iaprefix, t)] = '\0';
-		    else
-			ai->iaprefix = dupstring(t);
+		    if (!ms) {
+			if ((aflags & CAF_MATCH) || ai->icpl > pl)
+			    ai->icpl = pl;
+			if ((aflags & CAF_MATCH) || ai->icsl > lsl)
+			    ai->icsl = lsl;
+			if (ai->iaprefix)
+			    ai->iaprefix[pfxlen(ai->iaprefix, t)] = '\0';
+			else
+			    ai->iaprefix = dupstring(t);
+		    } else
+			ai->ilinecl = join_clines(ai->ilinecl, lc);
 		    if (ai->iprefix) {
 			if (strcmp(ipre, ai->iprefix))
 			    ai->iprefix = "";
@@ -2482,16 +2564,21 @@
 			ai->iprefix = dupstring(ipre);
 
 		    t = dyncat(ipre, t);
+		    lc = tlc;
 		} else
 		    ai->iprefix = "";
-		if (ai->cpl > lpl)
-		    ai->cpl = lpl;
-		if (ai->csl > lsl)
-		    ai->csl = lsl;
-		if (ai->aprefix)
-		    ai->aprefix[pfxlen(ai->aprefix, t)] = '\0';
-		else
-		    ai->aprefix = dupstring(t);
+		if (!ms) {
+		    if ((aflags & CAF_MATCH) || ai->cpl > pl)
+			ai->cpl = pl;
+		    if ((aflags & CAF_MATCH) || ai->csl > lsl)
+			ai->csl = lsl;
+		    if (ai->aprefix)
+			ai->aprefix[pfxlen(ai->aprefix, t)] = '\0';
+		    else
+			ai->aprefix = dupstring(t);
+		} else
+		    ai->linecl = join_clines(ai->linecl, lc);
+
 		mnum++;
 		ai->count++;
 
@@ -2499,25 +2586,41 @@
 		cm->ppre = ppre;
 		cm->psuf = psuf;
 		cm->prpre = prpre;
-		if (!quote)
-		    s = quotename(s, NULL, NULL, NULL);
-		cm->str = dupstring(s);
+		cm->str = (ms ? ms : dupstring(s));
 		cm->ipre = cm->ripre = ipre;
 		cm->pre = pre;
 		cm->suf = suf;
 		cm->flags = flags;
-		cm->brpl = brpl;
-		cm->brsl = brsl;
-		addlinknode((alt ? fmatches : matches), cm);
-
-		if (expl)
-		    expl->fcount++;
-		if (!ai->firstm)
-		    ai->firstm = cm;
+		cm->brpl = bpl;
+		cm->brsl = bsl;
+		cm->rems = rems;
+		addlinknode(l, cm);
+
+		if (expl) {
+		    if (l == matches)
+			expl->count++;
+		    else
+			expl->fcount++;
+		}
+		if (!ms) {
+		    if (!ai->firstm)
+			ai->firstm = cm;
+		    if ((aflags & CAF_MATCH) && !(e - (s + pl))) {
+			if (!ai->exact)
+			    ai->exact = 1;
+			else {
+			    ai->exact = 2;
+			    cm = NULL;
+			}
+			ai->exactm = cm;
+		    }
+		}
 	    }
 	    compnmatches = mnum;
 	} LASTALLOC;
     } SWITCHBACKHEAPS;
+
+    mstack = oms;
 }
 
 /* This adds a match to the list of matches.  The string to add is given   *
@@ -2729,8 +2832,7 @@
 		ai->iaprefix[pfxlen(ai->iaprefix, t)] = '\0';
 	    else
 		ai->iaprefix = dupstring(t);
-	}
-	else
+	} else
 	    ai->ilinecl = join_clines(ai->ilinecl, lc);
 	if (ai->iprefix) {
 	    if (strcmp(ipre, ai->iprefix))
@@ -2750,8 +2852,7 @@
 	    ai->aprefix[pfxlen(ai->aprefix, t)] = '\0';
 	else
 	    ai->aprefix = dupstring(t);
-    }
-    else
+    } else
 	ai->linecl = join_clines(ai->linecl, lc);
 
     mnum++;
@@ -2793,6 +2894,7 @@
     cm->flags = mflags | isf;
     cm->brpl = bpl;
     cm->brsl = bsl;
+    cm->rems = NULL;
     addlinknode(l, cm);
 
     /* One more match for this explanation. */
@@ -3377,6 +3479,7 @@
 
 /* This should probably be moved into tokenize(). */
 
+/**/
 static char *
 ctokenize(char *p)
 {
@@ -5072,6 +5175,7 @@
     r->flags = m->flags;
     r->brpl = m->brpl;
     r->brsl = m->brsl;
+    r->rems = ztrdup(m->rems);
 
     return r;
 }
@@ -5183,6 +5287,7 @@
     zsfree(m->ppre);
     zsfree(m->psuf);
     zsfree(m->prpre);
+    zsfree(m->rems);
 
     zfree(m, sizeof(m));
 }
@@ -5435,7 +5540,7 @@
 	if (menuwe) {
 	    menuend += menuinsc;
 	    if (m->flags & CMF_REMOVE) {
-		makesuffix(menuinsc);
+		makesuffixstr(m->rems, menuinsc);
 		if (menuinsc == 1)
 		    suffixlen[STOUC(m->suf[0])] = 1;
 	    }
diff -u om/new-completion-examples Misc/new-completion-examples
--- om/new-completion-examples	Sat Jan 30 21:37:37 1999
+++ Misc/new-completion-examples	Sat Jan 30 21:23:38 1999
@@ -276,7 +276,6 @@
         s=( "${(@)s:gs.**.*.}" )
         for i in $a; do
           b=( $~i/(#l)$~s )
-          eval b\=\( \$\{b:/\*\(${(j:|:)fignore}\)\} \)
           [[ $#b -ne 0 ]] && c=( $c $i )
         done
         if [[ $#c -eq 0 ]] then
@@ -284,10 +283,9 @@
         elif [[ $#c -ne 1 ]] then
           a="$ppre$epre$pa"
           c=( $~c/(#l)$~s )
-          eval c\=\( \$\{c:/\*\(${(j:|:)fignore}\)\} \)
   	  c=( ${c#$a} )
           for i in $c; do
-            compadd -p "$pre$pa" -W "$a" -s "/${i#*/}" -f "${i%%/*}"
+            compadd -p "$pre$pa" -W "$a" -s "/${i#*/}" -fF -- "${i%%/*}"
           done
   	  continue 2
         fi
@@ -301,8 +299,7 @@
     s=( $str$@ )
     s=( "${(@)s:gs.**.*.}" )
     b=( $~a(#l)$~s )
-    eval b\=\( \$\{b:/\*\(${(j:|:)fignore}\)\} \)
-    compadd -p "$pre$pa" -W "$ppre$epre$pa" -f ${b#$a}
+    compadd -p "$pre$pa" -W "$ppre$epre$pa" -fF -- ${b#$a}
   done
 }
 

--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: PATCH: compadd (+ questions)

[Peter Stephenson]

Just some queries about the new-style completion.  I'll put the more
general questions first, then there are some more specific questions
mainly to do with compadd.

First... I was looking at new-completion-examples and wondered:

1) I suspect most people are going to use much of it as it stands, and
just alter what they want.  (Who the *other* people might be, I cannot
imagine.)  So might it be an idea to find a more general name than
my-comp, and also to define widgets using main-complete for all the
standard completion functions, not just expand-or-complete?  And
perhaps do some trickery with bindkey output to rebind any keys using
the old ones to the new ones?  Some way of supplying options to
new-completion-examples would be good.

2) Currently, existing compctl's take precedence over any of the new
style functions for given commands.  That's sort of reasonable at the
moment, since many of the new-style functions are examples rather than
complete systems.  I wonder if there's a better way of deciding this.
If we implement something like 3), it might be better for the
new-style ones to take precedence.

3) As far as the example functions are concerned, it might be an idea
to separate them out into their own directory so they can be
autoloaded or copied for autoloading.  How about this:  they get
separated into a directory, and then some code in the main file scans
that directory and decides file by file how to call defcomp.

For example, the file __default might contain
  #defcomp --default--
  files
and then new-completion-examples can contain something like
  local d f a
  for d in $fpath; do
    for f in $d/__*(N); do
      read -A a <$f
      if [[ $a[1] = '#defcomp' ]]; then
  	defcomp $f:t $a[2]
      fi
    done
  done
Users can then put this directory where they want in their $fpath and
add or subtract from it to get autoloading of completion functions.
Also new-completion-examples can have a shorter name in its own
directory.


Next... I'm late replying to this, but I was just looking over the
new-style completion again and had some niggling questions...  I have
half an eye on documentation for this new stuff, which Sven should
probably have a go at first, although I think with something this
involved it's good if someone other than the author makes sure they
can understand it.

Sven Wischnowsky wrote:
> Since `compadd' is the interface to the internals of the completion
> stuff, it should be possible to use to add any words as matches. But
> sometimes you probably want to use it only to have control over the
> prefix/suffix- or the match-type stuff.

Does this mean a compadd without new matches alters what's there
already, or what's to come, or are you simply saying you can have
better control over what you are adding?

> So, the patch below adds the `-m' option to `compadd'
> which says that the completion code should use normal matching rules
> to the words given. Of course, for this the `-M' option known from
> `compctl' and `complist' is also supported.

This is fine.  Tests with with complist (compcall must be similar)
suggests that uses the standard rules, is that right?  That would also
be what I would guess.

> Another small addition: the option `-F' says that fignore should be
> used (in the same way as it's done by `compctl').

Would it not be just as easy to be able to specify a $fignore-type
variable (maybe in the `compctl -k ...' fmt so you can add literals)?
If it isn't, it doesn't matter, but I thought maybe that's no
harder than hard-wiring $fignore in.  (Does this do any more than
  for suf in $fignore; do
    for match in $matches1; do
      [[ $match = *$suf ]] || matches2=($matches2 $match)
    done
  done
?  I can see it's a lot more efficient, anyway.)

> And yet another one (this one may still be experimental): `-r <str>'
> says that the suffix should be removed if the next character typed is
> one of those given in `<str>'.

Is there a decision on how this is going to work after Bart got at
the last proposal?

-- 
Peter Stephenson <pws@ibmth.df.unipi.it>       Tel: +39 050 844536
WWW:  http://www.ifh.de/~pws/
Dipartimento di Fisica, Via Buonarroti 2, 56127 Pisa, Italy

Re: PATCH: compadd (+ questions)

[Bart Schaefer]

On Feb 10,  5:26pm, Peter Stephenson wrote:
} Subject: Re: PATCH: compadd (+ questions)
}
} 1) I suspect most people are going to use much of it as it stands, and
} just alter what they want.  (Who the *other* people might be, I cannot
} imagine.)  So might it be an idea to find a more general name than
} my-comp, and also to define widgets using main-complete for all the
} standard completion functions, not just expand-or-complete?  And
} perhaps do some trickery with bindkey output to rebind any keys using
} the old ones to the new ones?  Some way of supplying options to
} new-completion-examples would be good.
} 
} 2) Currently, existing compctl's take precedence over any of the new
} style functions for given commands.  That's sort of reasonable at the
} moment, since many of the new-style functions are examples rather than
} complete systems.  I wonder if there's a better way of deciding this.
} If we implement something like 3), it might be better for the
} new-style ones to take precedence.

Addressing both of these questions together:

I do think it's a good idea to make the examples as directly usable as
possible, and therefore to carefully choose descriptive names for the
various functions (nothing so uninformative as "my-comp" nor as generic
as "files").  It's probably also a good idea to have separate examples
emulating as many of the old built-in completions as possible, so that
people have "building blocks" from which to construct completions that
have effects similar to the built-in set or combinations thereof.

That'll also provide a good set of test cases to see whether we've (a)
achieved the goal of writing something more understandable than compctl,
and (b) made all the necessary bits of the functionality available to
the shell script programmer.

However, for the simple cases I'm concerned that calling interpreted
shell functions will be "too slow" and we shouldn't encourage people to
simply load up the examples en masse.  Has anybody done any speed
comparisons on having zsh call the old-style compctls directly, vs. the
new-style stuff firing `compcall`, vs. doing the entire thing with the
new stuff?  I confess I haven't had much chance to play with this.

} 3) As far as the example functions are concerned, it might be an idea
} to separate them out into their own directory so they can be
} autoloaded or copied for autoloading.  How about this:  they get
} separated into a directory, and then some code in the main file scans
} that directory and decides file by file how to call defcomp.

This sounds like a good idea.  BTW, defcomp and friends could use some
more commenting up front.  The meaning of, say, "--default--" as the
second parameter to defcomp should be explained near the definition of
defcomp, either instead of or as well as down in main-complete when the
associative array is finally referenced.

 ----------

As to goal (a) above:  This is the first time I've really taken the time
to read through the new-completion-examples file, and I find myself
disappointed by a couple of things that sounded good in theory.  Part
of it may just be Sven's coding style; can we use better variable names
than "a", "b", c", "opa", "epre", and "ppres", please?  It's bad enough
having that in the C code, without exposing mere mortals to it.

I think the worst are the new switches for the conditional expressions,
particularly the ones that take two following strings; it looks wrong.
I just spent several minutes pondering whether [[ -word CURRENT-1 -ef ]]
would look better to me than [[ -current -1 -ef ]], and decided it's
only marginally better.  It using -current or -word a lot faster than
using [[ $@[CURRENT-1] == '-ef' ]]?  Are they doing some other magic?

For the conditionals that don't use two strings, I think some of the
keywords could be better, but I don't yet have a list of suggestions.
Maybe it's just the 'm' prefix on the ones that take a pattern that
bothers me, and which also makes me wonder for a moment whether there's
supposed to be a -matches is when I see that there's a -nmatches.

Also, and I apologize for not responding more immediately to Sven from
zsh-workers/5083, I must say that the way that -iprefix, -position and
-string have parameter-modifying side-effects makes me cringe.  It'd be
fine in another new builtin, but a [[ ]] with side-effects?!?  Yipes!

Don't go changing anything yet just on the basis of my opinion, though;
I'm only trying to hear some discussion about this stuff.  I realize
that the side-effects are merely mirroring the state of the completion
internals, I just question letting this happen from [[ ]], and maybe
whether there should be a builtin equivalent of compsave/compreset.

I should close by saying that I *am* impressed, overall, with the new
completion scheme and the possibilities it presents.  I'm particularly
glad of way it and PWS's completion widgets were able to collapse into
one system.  I think the progress is good, and that we should keep on
speeding towards something even more marvelous.

(BTW, the contract I was supposed to be starting was postponed until
this coming Monday.  Once it starts, my contributions will be rather
limited for several weeks.)

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com