Strange _values completion on accept-and-menu-complete and menu selection

Strange _values completion on accept-and-menu-complete and menu selection

[Andrey Borzenkov]

For a long time I have the following bindings:
{pts/1}% bindkey -M menuselect
...
"," accept-and-menu-complete
"/" accept-and-infer-next-history

Apparently it does not work any more for _values; I presume it did work once 
because _urpmi completion includes _values usage in question.

Consider:

function _foo () {
        _values -s , "test completion" 'foo: :(1 2 3)' bar baz
}

compdef _foo foo

now start completion:

{pts/2}% foo bar,
Completing test completion
bar  baz  foo
^^^ highlighted

so far so good. But if I press ',' now (assuming complete current value and go 
on) I get

{pts/2}% foo bar baz,
Completing test completion
bar  baz  foo
     ^^^ highlighted

so auto-suffix is removed while it apparently should not to be?

OTOH doing

pts/2}% foo bar,
Completing test completion
bar  baz  foo
^^^ highlighted

now press "b" TAB gives you (as expected)

pts/2}% foo bar,baz,
Completing test completion
bar  baz  foo

Even more interesting with subvalue:

{pts/2}% foo foo=
Completing test completion
bar  baz  foo

press '/'

{pts/2}% foo foo=1,
1  2  3

OK so ENTER (to accept it) TAB you get

{pts/2}% foo foo=1,bar,
Completing test completion
bar  baz

and pressing ',' now results in

{pts/2}% foo foo=1,bar foo=1,baz,
Completing test completion
bar  baz

so something strange goes on when menu selection is used.

non default settings:

bindkey -e
bindkey '^I' complete-word
bindkey '^[q' push-line-or-edit
bindkey -M menuselect , accept-and-menu-complete
bindkey -M menuselect / accept-and-infer-next-history

setopt autopushd
setopt cdablevars

setopt extendedhistory
setopt extendedglob

setopt histexpiredupsfirst
setopt histignorealldups
setopt histignoredups
setopt histreduceblanks
setopt histsavenodups

setopt ignoreeof

setopt menucomplete

setopt nobanghist
setopt nolistambiguous
setopt nolistbeep

setopt pushdminus

# The following lines were added by compinstall
autoload -U compinit
compinit

zstyle ':completion:*' auto-description ''''specify: %d''''
zstyle ':completion:*' completer _oldlist _complete _match
zstyle ':completion:*' format ''''Completing %d''''
zstyle ':completion:*' group-name ''
zstyle ':completion:*' list-colors ${(s.:.)LS_COLORS}
zstyle ':completion:*' list-prompt '%SAt %p: Hit TAB for more, or the 
character
to insert%s'
zstyle ':completion:*' matcher-list '' 'm:{a-z}={A-Z}' 'm:{a-z}={A-Z} r:|
[._-]=*
* r:|=**'
zstyle ':completion:*' match-original both
zstyle ':completion:*' menu select=0
zstyle ':completion:*' verbose true
zstyle :compinstall filename '/home/bor/.zshrc'
# End of lines added by compinstall


zstyle ':completion:*' list-rows-first true
zstyle ':completion:*:paths' accept-exact true

regards

-andrey

Re: Strange _values completion on accept-and-menu-complete and menu selection

[Bart Schaefer]

On Sat, 11 Dec 2004, Andrey Borzenkov wrote:

> Apparently it does not work any more for _values; I presume it did work 
> once because _urpmi completion includes _values usage in question.

Looking at the code, I can't see any way it ever worked.  There's been no 
interesting change anywhere near or beyond (in the function call sense) 
the point where complist.c handles accept-and-menu-complete.

The problem is that accept_last() is called, which eventually always calls 
iremovesuffix(' ', 1).  If "compadd -R" had been used, the suffix function
would be called and could subvert the behavior of iremovesuffix(), but as
it stands the suffix is always zapped.

The deeper problem seems to be that the complist module doesn't really 
know anything about compvalues; e.g., as far as complist is concerned it's 
selecting among shell words, not from a set of values that are part of a 
word.  That it comes as close as it does to working is serendipitous.

A fix seems to be something in the direction of this pseudo-patch (do NOT
apply it as-is) to compresult.c, but I don't know how to get the boolean
value for "we are not completing values from compvalues" at this location.

@@ -1241,16 +1241,19 @@
 	lastbrbeg->str[l + 1] = '\0';
     } else {
 	int l;
+	int rem = /* We are not completing values from compvalues */;
 
 	cs = minfo.pos + minfo.len + minfo.insc;
-	iremovesuffix(' ', 1);
+	if (rem)
+	  iremovesuffix(' ', 1);
 	l = cs;
 	cs = minfo.pos + minfo.len + minfo.insc - (*(minfo.cur))->qisl;
 	if (cs < l)
 	    foredel(l - cs);
 	else if (cs > ll)
 	    cs = ll;
-	inststrlen(" ", 1, 1);
+	if (rem)
+	  inststrlen(" ", 1, 1);
 	minfo.insc = minfo.len = 0;
 	minfo.pos = cs;
 	minfo.we = 1;

Re: Strange _values completion on accept-and-menu-complete and menu selection

[Andrey Borzenkov]

On Sunday 12 December 2004 00:05, Bart Schaefer wrote:

> A fix seems to be something in the direction of this pseudo-patch (do NOT
> apply it as-is) to compresult.c, but I don't know how to get the boolean
> value for "we are not completing values from compvalues" at this location.
>

I already got a prototype patch for it when I accidentally hit a-a-i-n-h and 
voila - it did what I expected from a-a-m-c (except that a-a-m-c stays on the 
same word while a-a-i-n-h starts from scratch - because with a-a-i-n-h it 
actually starts new completion every time).

I attach prototype patch - it adds compstate element that tells accept_last to 
skip suffix removal. It can be used in more general case than just _values - 
what I am not sure how and when this is to be set. I.e. using this patch 
following function now "correctly" works for a-a m-c:

function _foo () {
        compstate[stayinword]=1
        compset -P '*,'
        compadd -qS , ${(k)compstate}
}

compdef _foo foo

regards

-andrey

Index: Src/Zle/complete.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/complete.c,v
retrieving revision 1.27
diff -u -p -r1.27 complete.c
--- Src/Zle/complete.c  7 Dec 2004 16:55:11 -0000       1.27
+++ Src/Zle/complete.c  12 Dec 2004 16:13:03 -0000
@@ -38,7 +38,8 @@ zlong compcurrent,
       complistmax;
 /**/
 zlong complistlines,
-      compignored;
+      compignored,
+      compstayinword;

 /**/
 mod_export
@@ -1018,6 +1019,7 @@ static struct compparam compkparams[] =
     { "list_lines", PM_INTEGER | PM_READONLY, NULL, GSU(listlines_gsu) },
     { "all_quotes", PM_SCALAR | PM_READONLY, VAL(compqstack), NULL },
     { "ignored", PM_INTEGER | PM_READONLY, VAL(compignored), NULL },
+    { "stayinword", PM_INTEGER, VAL(compstayinword), NULL },
     { NULL, 0, NULL, NULL }
 };

Index: Src/Zle/compresult.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/compresult.c,v
retrieving revision 1.52
diff -u -p -r1.52 compresult.c
--- Src/Zle/compresult.c        12 Jul 2004 10:05:52 -0000      1.52
+++ Src/Zle/compresult.c        12 Dec 2004 16:13:04 -0000
@@ -1243,14 +1243,16 @@ accept_last(void)
        int l;

        cs = minfo.pos + minfo.len + minfo.insc;
-       iremovesuffix(' ', 1);
+       if (!compstayinword)
+           iremovesuffix(' ', 1);
        l = cs;
        cs = minfo.pos + minfo.len + minfo.insc - (*(minfo.cur))->qisl;
        if (cs < l)
            foredel(l - cs);
        else if (cs > ll)
            cs = ll;
-       inststrlen(" ", 1, 1);
+       if (!compstayinword)
+           inststrlen(" ", 1, 1);
        minfo.insc = minfo.len = 0;
        minfo.pos = cs;
        minfo.we = 1;

Re: Strange _values completion on accept-and-menu-complete and menu selection

[Bart Schaefer]

On Sun, 12 Dec 2004, Andrey Borzenkov wrote:

> I attach prototype patch - it adds compstate element that tells 
> accept_last to skip suffix removal. It can be used in more general case 
> than just _values - what I am not sure how and when this is to be set. 

I wonder if perhaps another state name would be better.  "Stay in word" is 
a directive, whereas everything else in compstate is, well, state data.

What if it were compstate[compound_word] and the value is the separator
character to insert?  (Or perhaps two characters, corresponding to
"compvalues -s" and "compvalues -S" in that order.)

Normally compstate[compound_word] would be unset, but "compvalues -i" 
would set it based on the parse.

Re: Strange _values completion on accept-and-menu-complete and menu selection

[Andrey Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 1638 bytes --]

On Sunday 12 December 2004 20:51, Bart Schaefer wrote:
> On Sun, 12 Dec 2004, Andrey Borzenkov wrote:
> > I attach prototype patch - it adds compstate element that tells
> > accept_last to skip suffix removal. It can be used in more general case
> > than just _values - what I am not sure how and when this is to be set.
>
> I wonder if perhaps another state name would be better.  "Stay in word" is
> a directive, whereas everything else in compstate is, well, state data.
>
> What if it were compstate[compound_word] and the value is the separator
> character to insert?  (Or perhaps two characters, corresponding to
> "compvalues -s" and "compvalues -S" in that order.)
>
> Normally compstate[compound_word] would be unset, but "compvalues -i"
> would set it based on the parse.

I still believe this is more general and should not be limited to _values 
only. Also -S does not seem to be needed here.

Here is updated patch (which also fixes initialization problem). It makes 
parameter compound_word as scalar but currently tests only for empty value 
(is it correct way to test for it BTW?) Setting compstate[compound_word] to 
any string in completion function will stop accept_last from removing suffix 
(actually, altering string in any way). This also changes _values to set it 
as appropriate.

This still has problems in completing nested calls to _values (should it 
work?) Try this example:

function _foo_val () {
        _values -s : "subvalues" baz bad bam
}
function _foo () {
        _values -s , -S = "test values" \
                foo \
                "bar:bar values:_foo_val"
}
compdef _foo foo

regards

-andrey

[-- Attachment #2: compound_word.diff --]
[-- Type: text/x-diff, Size: 4531 bytes --]

Index: Completion/Base/Utility/_values
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Base/Utility/_values,v
retrieving revision 1.8
diff -u -p -r1.8 _values
--- Completion/Base/Utility/_values	16 Apr 2002 07:55:49 -0000	1.8
+++ Completion/Base/Utility/_values	12 Dec 2004 20:44:45 -0000
@@ -10,10 +10,13 @@ zparseopts -D -E -a garbage C=usecc O:=s
 if compvalues -i "$@"; then
 
   local noargs args opts descr action expl sep argsep subc test='*'
-  local oldcontext="$curcontext"
+  local oldcontext="$curcontext" compound=$compstate[compound]
 
   compvalues -S argsep
-  compvalues -s sep && [[ -n "$sep" ]] && test="[^${(q)sep}]#"
+  compvalues -s sep && [[ -n "$sep" ]] && { 
+    test="[^${(q)sep}]#"
+    compstate[compound_word]=yes
+  }
 
   if ! compvalues -D descr action; then
 
@@ -148,6 +151,7 @@ if compvalues -i "$@"; then
     fi
   fi
 
+  compstate[compound]=$compound
   curcontext="$oldcontext"
 
   [[ nm -ne "$compstate[nmatches]" ]]
Index: Src/Zle/comp.h
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/comp.h,v
retrieving revision 1.14
diff -u -p -r1.14 comp.h
--- Src/Zle/comp.h	21 May 2002 08:07:51 -0000	1.14
+++ Src/Zle/comp.h	12 Dec 2004 20:44:46 -0000
@@ -375,9 +375,11 @@ typedef void (*CLPrintFunc)(Cmgroup, Cma
 #define CP_QUOTES      (1 << CPN_QUOTES)
 #define CPN_IGNORED    25
 #define CP_IGNORED     (1 << CPN_IGNORED)
+#define CPN_COMPOUND   26
+#define CP_COMPOUND    (1 << CPN_COMPOUND)
 
-#define CP_KEYPARAMS   26
-#define CP_ALLKEYS     ((unsigned int) 0x3ffffff)
+#define CP_KEYPARAMS   27
+#define CP_ALLKEYS     ((unsigned int) 0x7ffffff)
 
 /* Hooks. */
 
Index: Src/Zle/compcore.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/compcore.c,v
retrieving revision 1.68
diff -u -p -r1.68 compcore.c
--- Src/Zle/compcore.c	5 Nov 2004 16:20:24 -0000	1.68
+++ Src/Zle/compcore.c	12 Dec 2004 20:44:49 -0000
@@ -764,6 +764,8 @@ callcompfunc(char *s, char *fn)
 	    compoldlist = compoldins = "";
 	compoldlist = ztrdup(compoldlist);
 	compoldins = ztrdup(compoldins);
+	zsfree(compcompound);
+	compcompound = ztrdup("");
 
 	incompfunc = 1;
 	startparamscope();
Index: Src/Zle/complete.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/complete.c,v
retrieving revision 1.27
diff -u -p -r1.27 complete.c
--- Src/Zle/complete.c	7 Dec 2004 16:55:11 -0000	1.27
+++ Src/Zle/complete.c	12 Dec 2004 20:44:49 -0000
@@ -70,7 +70,8 @@ char *compiprefix,
      *comptoend,
      *compoldlist,
      *compoldins,
-     *compvared;
+     *compvared,
+     *compcompound;
 
 /**/
 Param *comprpms, *compkpms;
@@ -1018,6 +1019,7 @@ static struct compparam compkparams[] = 
     { "list_lines", PM_INTEGER | PM_READONLY, NULL, GSU(listlines_gsu) },
     { "all_quotes", PM_SCALAR | PM_READONLY, VAL(compqstack), NULL },
     { "ignored", PM_INTEGER | PM_READONLY, VAL(compignored), NULL },
+    { "compound_word", PM_SCALAR, VAL(compcompound), NULL },
     { NULL, 0, NULL, NULL }
 };
 
@@ -1428,7 +1430,7 @@ setup_(UNUSED(Module m))
 	compquoting = comprestore = complist = compinsert =
 	compexact = compexactstr = comppatmatch = comppatinsert =
 	complastprompt = comptoend = compoldlist = compoldins =
-	compvared = compqstack = NULL;
+	compvared = compqstack = compcompound = NULL;
     complastprefix = ztrdup("");
     complastsuffix = ztrdup("");
     complistmax = 0;
@@ -1508,6 +1510,7 @@ finish_(UNUSED(Module m))
     zsfree(compoldlist);
     zsfree(compoldins);
     zsfree(compvared);
+    zsfree(compcompound);
 
     hascompmod = 0;
 
Index: Src/Zle/compresult.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/compresult.c,v
retrieving revision 1.52
diff -u -p -r1.52 compresult.c
--- Src/Zle/compresult.c	12 Jul 2004 10:05:52 -0000	1.52
+++ Src/Zle/compresult.c	12 Dec 2004 20:44:49 -0000
@@ -1243,14 +1243,16 @@ accept_last(void)
 	int l;
 
 	cs = minfo.pos + minfo.len + minfo.insc;
-	iremovesuffix(' ', 1);
+	if (!compcompound || strlen(compcompound) == 0)
+	    iremovesuffix(' ', 1);
 	l = cs;
 	cs = minfo.pos + minfo.len + minfo.insc - (*(minfo.cur))->qisl;
 	if (cs < l)
 	    foredel(l - cs);
 	else if (cs > ll)
 	    cs = ll;
-	inststrlen(" ", 1, 1);
+	if (!compcompound || strlen(compcompound) == 0)
+	    inststrlen(" ", 1, 1);
 	minfo.insc = minfo.len = 0;
 	minfo.pos = cs;
 	minfo.we = 1;

Re: Strange _values completion on accept-and-menu-complete and menu selection

[Bart Schaefer]

On Mon, 13 Dec 2004, Andrey Borzenkov wrote:

> > Normally compstate[compound_word] would be unset, but "compvalues -i"
> > would set it based on the parse.
> 
> I still believe this is more general and should not be limited to _values 

Sure; compstate[compound_word] doesn't have to be read-only, any function
that needs it could set it.  It just seems correct and convenient to have
compvalues set it automatically.

> Here is updated patch (which also fixes initialization problem).

There's one line of shell code where you have compstate[compound] but want
compstate[compound_word].  No need to repost, but fix before commit.

> This still has problems in completing nested calls to _values (should it 
> work?)

I don't have time to debug carefully just now, but I suspect without any 
real evidence that compstate[compound_word] is becoming unset on return 
from the nested call and therefore is wrong for the subsequent selections
for the outer call.

Re: Strange _values completion on accept-and-menu-complete and menu selection

[Peter Stephenson]

Bart Schaefer wrote:
> > This still has problems in completing nested calls to _values (should it 
> > work?)
> 
> I don't have time to debug carefully just now, but I suspect without any 
> real evidence that compstate[compound_word] is becoming unset on return 
> from the nested call and therefore is wrong for the subsequent selections
> for the outer call.

I'm not looking at the particular problem in any detail, but I believe
this is standard behaviour for completion variables to make it easier to
chain completions without saving and restoring inconvenient amounts of
state.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070


**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

This footnote also confirms that this email message has been swept by
MIMEsweeper for the presence of computer viruses.

www.mimesweeper.com
**********************************************************************

Re: Strange _values completion on accept-and-menu-complete and menu selection

[Andrey Borzenkov]

On Monday 13 December 2004 00:44, Bart Schaefer wrote:
> On Mon, 13 Dec 2004, Andrey Borzenkov wrote:
> > > Normally compstate[compound_word] would be unset, but "compvalues -i"
> > > would set it based on the parse.
> >
> > I still believe this is more general and should not be limited to _values
>
> Sure; compstate[compound_word] doesn't have to be read-only, any function
> that needs it could set it.  It just seems correct and convenient to have
> compvalues set it automatically.
>
> > Here is updated patch (which also fixes initialization problem).
>
> There's one line of shell code where you have compstate[compound] but want
> compstate[compound_word].  No need to repost, but fix before commit.
>

it is far from being suitable for commit (and I was off most of the time).

> > This still has problems in completing nested calls to _values (should it
> > work?)
>
> I don't have time to debug carefully just now, but I suspect without any
> real evidence that compstate[compound_word] is becoming unset on return
> from the nested call and therefore is wrong for the subsequent selections
> for the outer call.

it is more general problem. When match is inserted it has all parts - ignored 
prefix, prefix, etc insterted. In our case we actually have as iprefix 
everything up to separator - and get everything up to separator reinserted. 
It happens without any nested calls too:

{pts/0}% foo foo,TAB
{pts/0}% foo foo,bazzz,
Completing test values
barr   bazzz

and hitting a-a-m-c

{pts/0}% foo foo,bazzz,foo,barr

It is the same problem as completion in braces. Braces are treated as special 
case inside completion code unfortunately. Hopefully it is possile to 
generalize it.

-andrey