Re: 'compadd -P' matches $PREFIX greedily

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

Oliver Kiddle wrote on Sun, Sep 18, 2016 at 03:28:07 +0200:
> Daniel Shahaf wrote:
> > The following completion function:
> >
> > % compdef 'compadd -P p_____ papa mama nana aaaa' f
> > % f pa<TAB>   → p_____aaaa
> 
> Out of interest, was there a real prefix where you came across this.

Yes, I ran into it in _bts.  I'll send that patch separately for X-Seq
purposes, but the tl;dr is:

    _f() { _email_addresses -c -P "from:" }
    _email-foo() { compadd eee fee }
    f fe<TAB>  → from:eee

where «from:fee» was the expected result.

> I was trying to think of more realistic prefixes to see how this
> might affect them:
> 
> % compdef 'compadd -P file:// /papa //mama /nana aaaa filter' f
> 
> before: f fil<tab> → file://
> after : f fil<tab> → file://filter
> 
> That now seems a bit too aggressive.
> 

Agreed.  The ability to complete the prefix "file://" shouldn't depend
on what hostnames (matches) are available.  Especially as hostnames (and
email addresses) are added and removed over time.

> How about allowing the longest common prefix to be equal to either
> the -P prefix or equal to $PREFIX.

In both of our examples, the last character of $compadd_args[-P] is
a delimiter.

For fil<TAB> I'm not sure what's right.  Perhaps we should offer both
«file://<CURSOR>» (offering all five matches) and «file://filter».  I'm
not certain how the UI for this would look.  Conceptually, it would
involve trying two different values for the common prefix: both 0 (the
empty string) and min(${#PREFIX}, ${#compadd_args[-P]}) if one of those
two is a prefix of the other.  I'm not sure how easy or hard trying two
values would be to implement.

In the meantime, an interdiff implementing your suggestion is attached.
I'm inclined to commit it, and we can tweak it further later if need be.

Cheers,

Daniel

Interdiff:
diff --git a/Src/Zle/compcore.c b/Src/Zle/compcore.c
index 05d2706..5443018 100644
--- a/Src/Zle/compcore.c
+++ b/Src/Zle/compcore.c
@@ -2194,8 +2194,10 @@ addmatches(Cadata dat, char **argv)
 	    /* Test if there is an existing -P prefix. */
 	    if (dat->pre && *dat->pre) {
 		int prefix_length = pfxlen(dat->pre, lpre);
-		if (dat->pre[prefix_length] == '\0') {
-		    /* $compadd_args[-P] is a prefix of ${PREFIX}. */
+		if (dat->pre[prefix_length] == '\0' ||
+		    lpre[prefix_length] == '\0') {
+		    /* $compadd_args[-P] is a prefix of ${PREFIX}, or
+		     * vice-versa. */
 		    llpl -= prefix_length;
 		    lpre += prefix_length;
 		}