PATCH: problem accepting completion in menu selection

[Peter Stephenson <pws@xxxxxxx>]

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@xxxxxxx>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070