PATCH: problem accepting completion in menu selection

PATCH: problem accepting completion in menu selection

[Peter Stephenson]

I've been having a problem for some time now that when I'm in menu
selection (menuselect keymap) and use return (or anything bound to
accept-line) to accept a completion: that doesn't happen, instead the
list is scrolled.  Space is the other obvious thing to use to accept the
completion and start typing a new argument, but it scrolls too, so this
is especially frustrating (it's only supposed to be special in
listscroll---as far as I can see listscroll is OK).

You can accept the completion implicitly by typing another key, such as
a self-insert character other than space.  I have absolutely no idea how
space managed to be treated specially but I think it's because within
zrefresh() list-scrolling kicks in if the list is still active, which it
shouldn't be any longer, which is the fundamental problem.  Even if you
successfully accept the completion, however, it's not doing quite the
right thing, for example you don't get always_last_prompt behaviour.  I
think it's going through a series of baroque displays that top anything
Johann Bernhard Fischer von Erlach managed in his entire illustrious
career, ending up with nothing else to do other than the normal action
of the command which caused selection to exit and which was pushed back
onto the command stack, and which finally causes selection to be no
longer active but in a slightly awkward fashion.

It's a little curious that no one else has noticed this; I assumed it
was due to some odd key binding I had but I see no evidence it is.

I haven't a clue why this started happening, but I can get it to stop by
taking special action when the completion is accepted and before
zrefresh() is called.  Since I can test for acceptance I'm hopeful this
doesn't kick in at the wrong time.

One minor addition:  allow accept-search to mean the same as accept-line
(it accepts the completion but doesn't accept the line---it just so
happens that this is the same behaviour as accept-line in this
particular case).  This is purely for consistency, it doesn't give you
any additional behaviour in this case, but it does fit in with the
normal meaning of accept-search.

Obviously I'd be interested in unwanted side effects.

Index: Doc/Zsh/mod_complist.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/mod_complist.yo,v
retrieving revision 1.26
diff -u -r1.26 mod_complist.yo
--- Doc/Zsh/mod_complist.yo	6 Aug 2008 02:24:47 -0000	1.26
+++ Doc/Zsh/mod_complist.yo	1 Jul 2009 11:18:11 -0000
@@ -287,8 +287,9 @@
 menu selection:
 
 startitem()
-item(tt(accept-line))(
-accepts the current match and leaves menu selection
+item(tt(accept-line), tt(accept-search))(
+accept the current match and leave menu selection (but do
+not cause the command line to be accepted)
 )
 item(tt(send-break))(
 leaves menu selection and restores the previous contents of the
Index: Src/Zle/complist.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/complist.c,v
retrieving revision 1.119
diff -u -r1.119 complist.c
--- Src/Zle/complist.c	20 Jan 2009 10:42:08 -0000	1.119
+++ Src/Zle/complist.c	1 Jul 2009 11:18:11 -0000
@@ -2572,7 +2572,7 @@
                               cmd != Th(z_selfinsertunmeta)))) {
 	    ungetkeycmd();
 	    break;
-	} else if (cmd == Th(z_acceptline)) {
+	} else if (cmd == Th(z_acceptline) || cmd == Th(z_acceptsearch)) {
             if (mode == MM_FSEARCH || mode == MM_BSEARCH) {
                 mode = 0;
                 continue;
@@ -3316,7 +3316,23 @@
 	mlbeg = -1;
 	showinglist = ((validlist && !nolist) ? -2 : 0);
 	onlyexpl = oe;
-	if (!smatches)
+	if (acc && listshown) {
+	    /*
+	     * Clear the list without spending sixteen weeks of
+	     * redrawing it in slightly different states first.
+	     * The following seems to work.  I'm not sure what
+	     * the difference is between listshown and showinglist,
+	     * but listshown looks like the traditional thing to
+	     * check for in this file at least.
+	     *
+	     * showinglist has a normally undocumented value of 1,
+	     * and an extra-specially undocumented value of -2, which
+	     * seems to be a force---it appears we need to kick it out
+	     * of that state, though it worries me that in some places
+	     * the code actually forces it back into that state.
+	     */
+	    clearlist = listshown = showinglist = 1;
+	} else if (!smatches)
 	    clearlist = listshown = 1;
 	zrefresh();
     }


-- 
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

Re: PATCH: problem accepting completion in menu selection

[Mikael Magnusson]

2009/7/1 Peter Stephenson <pws@csr.com>:
> I've been having a problem for some time now that when I'm in menu
> selection (menuselect keymap) and use return (or anything bound to
> accept-line) to accept a completion: that doesn't happen, instead the
> list is scrolled.  Space is the other obvious thing to use to accept the
> completion and start typing a new argument, but it scrolls too, so this
> is especially frustrating (it's only supposed to be special in
> listscroll---as far as I can see listscroll is OK).
>
> You can accept the completion implicitly by typing another key, such as
> a self-insert character other than space.  I have absolutely no idea how
> space managed to be treated specially but I think it's because within
> zrefresh() list-scrolling kicks in if the list is still active, which it
> shouldn't be any longer, which is the fundamental problem.  Even if you
> successfully accept the completion, however, it's not doing quite the
> right thing, for example you don't get always_last_prompt behaviour.  I
> think it's going through a series of baroque displays that top anything
> Johann Bernhard Fischer von Erlach managed in his entire illustrious
> career, ending up with nothing else to do other than the normal action
> of the command which caused selection to exit and which was pushed back
> onto the command stack, and which finally causes selection to be no
> longer active but in a slightly awkward fashion.
>
> It's a little curious that no one else has noticed this; I assumed it
> was due to some odd key binding I had but I see no evidence it is.

I think I've noticed what you're talking about, I have menu selection
for `kill` pid completion, but I usually just press ctrl-c to abort
the list mode after i press enter to accept. I'll keep an eye out for
regressions.

-- 
Mikael Magnusson

Re: PATCH: problem accepting completion in menu selection

[Andrey Borzenkov]

[-- Attachment #1: Type: Text/Plain, Size: 679 bytes --]


Correction - it is not actually menu selection but rather display of 
completion list which is different specie as far as I know. I did not 
test whether I get the same with menu selection off though.

On Wednesday 01 of July 2009 15:31:14 Peter Stephenson wrote:
>
> It's a little curious that no one else has noticed this; I assumed it
> was due to some odd key binding I had but I see no evidence it is.
>

Well, I am a bit annoyed by it for quite some time, but

- I very rarely get overly long listings to become really annoyed
- I assumed it worked as intended :) at least after reading complist 
documentation.

So at least I do confirm this behaviour.

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: PATCH: problem accepting completion in menu selection

[Peter Stephenson]

Andrey Borzenkov wrote:
> Correction - it is not actually menu selection but rather display of
> completion list which is different specie as far as I know. I did not
> test whether I get the same with menu selection off though.

It's behaviour in the zsh/complist module, yes, but there are two things
it can do: show a list without selection (using the listscroll keymap),
or select from a list (using the menuselect keymap).  I haven't changed
the former: space and return still scroll there.  They were doing so in
selection, too, which they shouldn't.

I should, however, consistent with the previous patch, have made
accept-search stop the listing without doing anything else.  Then you
can

  bindkey -M listscroll '\r' accept-search

if you want Enter to exit the list instead of scrolling.

Index: Doc/Zsh/mod_complist.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/mod_complist.yo,v
retrieving revision 1.27
diff -u -r1.27 mod_complist.yo
--- Doc/Zsh/mod_complist.yo	1 Jul 2009 13:41:24 -0000	1.27
+++ Doc/Zsh/mod_complist.yo	1 Jul 2009 16:19:57 -0000
@@ -193,6 +193,9 @@
 item(tt(expand-or-complete-prefix), tt(menu-complete-or-expand))(
 scrolls forward one screenful
 )
+item(tt(accept-search))(
+stop listing but take no other action
+)
 enditem()
 
 Every other character stops listing and immediately processes the key
Index: Src/Zle/complist.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/complist.c,v
retrieving revision 1.120
diff -u -r1.120 complist.c
--- Src/Zle/complist.c	1 Jul 2009 13:41:25 -0000	1.120
+++ Src/Zle/complist.c	1 Jul 2009 16:19:57 -0000
@@ -974,6 +974,8 @@
 		   !strcmp(cmd->nam, "menu-complete") ||
 	     !strcmp(cmd->nam, "menu-expand-or-complete"))
 	mrestlines = lines - 1;
+    else if (cmd == Th(z_acceptsearch))
+	ret = 1;
     else {
 	ungetkeycmd();
 	ret = 1;



-- 
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