Space after completion disappears

Space after completion disappears

[Michael Hwang]

When a file is tab completed, and you didn't have to select from a menu, a space is added immediately after the file. However, typing another character that denotes the end of that command--ampersand, semi-colon, vertical pipe--causes the space to be erased. I would expect the semi-colon to have this behavior, but not the ampersand or the pipe. Consider these keystrokes in a directory with a single file 'foobar':

% touch foo<TAB>;
After the tab, the line reads "touch foobar " (note the trailing space). Typing ";" after it removes the space, and you get "touch foobar;", which is desired. However...

% rm foo<TAB>&
I want to background the rm command. After hitting tab, I see "rm foobar " (trailing space again). Pressing "&" deletes the space, and you see "rm foobar&". It would be better if it left the space, showing "rm foobar &". Both "rm foobar&" and "rm foobar &" do exactly the same thing, but the second one is preferred. The pipe is the same:

% cat foo<TAB>| sed ....
Ignore the useless use of cat. After typing the pipe, you get "cat foobar| sed ...." and not "cat foobar | sed ....". The second is preferable because it is neater.

I tried to squash the bug myself, but got hopelessly confused looking through ZLE code. I'm reporting it in the hope that someone else will fix it.

Michael Hwang



      

Re: Space after completion disappears

[Bart Schaefer]

On Apr 20, 10:11pm, Michael Hwang wrote:
}
} I tried to squash the bug myself, but got hopelessly confused looking
} through ZLE code. I'm reporting it in the hope that someone else will
} fix it.

You can't squash it because it's not a bug.  It's doing that on purpose.

Re: Space after completion disappears

[Michael Hwang]

Sorry Bart, that should have been sent to the mailing list, and not directly to you. I'm repeating my message for the benefit of everyone else:

But my point is that it shouldn't, at least not for the last two cases I outlined.

Michael Hwang



----- Original Message ----
From: Bart Schaefer <schaefer@brasslantern.com>
To: zsh-workers@sunsite.dk
Sent: Tuesday, April 21, 2009 3:03:42 AM
Subject: Re: Space after completion disappears

On Apr 20, 10:11pm, Michael Hwang wrote:
}
} I tried to squash the bug myself, but got hopelessly confused looking
} through ZLE code. I'm reporting it in the hope that someone else will
} fix it.

You can't squash it because it's not a bug.  It's doing that on purpose.



      

Re: Space after completion disappears

[Peter Stephenson]

On Wed, 22 Apr 2009 08:28:56 -0700 (PDT)
Michael Hwang <nomex45@yahoo.com> wrote:
> Sorry Bart, that should have been sent to the mailing list, and not
> directly to you. I'm repeating my message for the benefit of everyone else:
> 
> But my point is that it shouldn't, at least not for the last two cases I
> outlined.

It's entirely cosmetic, but if you really feel the need to change it it's
easy to make it configurable.

I hope I've got the documentation right.  I couldn't find the current
behaviour described anywhere.

Index: Doc/Zsh/compwid.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/compwid.yo,v
retrieving revision 1.47
diff -u -r1.47 compwid.yo
--- Doc/Zsh/compwid.yo	13 Jan 2009 12:19:53 -0000	1.47
+++ Doc/Zsh/compwid.yo	22 Apr 2009 16:01:04 -0000
@@ -58,6 +58,11 @@
 texinode(Completion Special Parameters)(Completion Builtin Commands)()(Completion Widgets)
 sect(Completion Special Parameters)
 
+The parameter tt(ZLE_REMOVE_SUFFIX_CHARS) is used by the completion
+mechanism, but is not special.
+ifzman(See em(Parameters Used By The Shell) in zmanref(zshparam))\
+ifnzman(noderef(Parameters Used By The Shell)).
+
 Inside completion widgets, and any functions called from them, some
 parameters have special meaning; outside these functions they are not
 special to the shell in any way.  These parameters are used to pass
Index: Doc/Zsh/params.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/params.yo,v
retrieving revision 1.53
diff -u -r1.53 params.yo
--- Doc/Zsh/params.yo	6 Apr 2009 09:06:36 -0000	1.53
+++ Doc/Zsh/params.yo	22 Apr 2009 16:01:04 -0000
@@ -1330,4 +1330,19 @@
 The directory to search for shell startup files (.zshrc, etc),
 if not tt($HOME).
 )
+vindex(ZLE_REMOVE_SUFFIX_CHARS)
+item(tt(ZLE_REMOVE_SUFFIX_CHARS))(
+This parameter is used by the line editor.  In certain circumstances
+suffixes (typically space or slash) added by the completion system
+will be removed automatically, either because the next editing command
+was not an insertable character, or because the character was marked
+as requiring the suffix to be removed.  If this variable is set
+it contains the set of characters which will cause the suffix to be
+removed.  The default value is equivalent to
+
+example(ZLE_REMOVE_SUFFIX_CHARS=$' \t\n;&|')
+
+Note that certain completions may override this default set of characters
+in specific cases.
+)
 enditem()
Index: Src/Zle/zle_misc.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_misc.c,v
retrieving revision 1.57
diff -u -r1.57 zle_misc.c
--- Src/Zle/zle_misc.c	28 Jan 2009 09:32:01 -0000	1.57
+++ Src/Zle/zle_misc.c	22 Apr 2009 16:01:04 -0000
@@ -1278,8 +1278,21 @@
 mod_export void
 makesuffix(int n)
 {
-    addsuffix(SUFTYP_POSSTR, ZWS(" \t\n;&|"), 6, n);
+    char *suffixchars;
+    ZLE_STRING_T suffixstr;
+    int slen, alloclen;
+
+    if (!(suffixchars = getsparam("ZLE_REMOVE_SUFFIX_CHARS")))
+	suffixchars = " \t\n;&|";
+
+    /* string needs to be writable... I've been regretting this for years.. */
+    suffixchars = ztrdup(suffixchars);
+
+    suffixstr = stringaszleline(suffixchars, 0, &slen, &alloclen, NULL);
+    addsuffix(SUFTYP_POSSTR, suffixstr, slen, n);
     suffixnoinslen = n;
+    zfree(suffixstr, alloclen);
+    zsfree(suffixchars);
 }
 
 /* Set up suffix for parameter names: the last n characters are a suffix *

Re: Space after completion disappears

[Greg Klanderman]

>>>>> Peter Stephenson <pws@csr.com> writes:

> It's entirely cosmetic, but if you really feel the need to change it it's
> easy to make it configurable.

Hmm, your change does what he wants if the suffix is a space, but for
example if the suffix is a '/' I'm not sure.  I suspect he really
just wants the suffix to always be removed, but have a space inserted
first if the character about to be inserted is non-whitespace.

greg

Re: Space after completion disappears

[Peter Stephenson]

Greg Klanderman wrote:
> 
> >>>>> Peter Stephenson <pws@csr.com> writes:
> 
> > It's entirely cosmetic, but if you really feel the need to change it it's
> > easy to make it configurable.
> 
> Hmm, your change does what he wants if the suffix is a space, but for
> example if the suffix is a '/' I'm not sure.  I suspect he really
> just wants the suffix to always be removed, but have a space inserted
> first if the character about to be inserted is non-whitespace.

I see what you mean---if you end up with "Modules/" and type "&", then
(with & removed from the remove-suffix list) you get "Modules/&" instead
of "Modules&" but you really want "Modules &".  However, that's a new
feature.  I think it would be doable (and probably not difficult) by
adding an additional variable (which would take precedence so you didn't
have to remove the character from the old mechanism) and a new SUFTYP
and changing makesuffix() and iremovesuffix(), but it's beyond the
capability of simple local tweaks.

-- 
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: Space after completion disappears

[Peter Stephenson]

On Thu, 23 Apr 2009 10:31:50 +0100
Peter Stephenson <pws@csr.com> wrote:
> Greg Klanderman wrote:
> > 
> > >>>>> Peter Stephenson <pws@csr.com> writes:
> > 
> > > It's entirely cosmetic, but if you really feel the need to change it it's
> > > easy to make it configurable.
> > 
> > Hmm, your change does what he wants if the suffix is a space, but for
> > example if the suffix is a '/' I'm not sure.  I suspect he really
> > just wants the suffix to always be removed, but have a space inserted
> > first if the character about to be inserted is non-whitespace.
> 
> I see what you mean---if you end up with "Modules/" and type "&", then
> (with & removed from the remove-suffix list) you get "Modules/&" instead
> of "Modules&" but you really want "Modules &".  However, that's a new
> feature.  I think it would be doable (and probably not difficult) by
> adding an additional variable (which would take precedence so you didn't
> have to remove the character from the old mechanism) and a new SUFTYP
> and changing makesuffix() and iremovesuffix(), but it's beyond the
> capability of simple local tweaks.

Here it is.

The answer to "why doesn't it do that already" is "because every single
time we hardwire something into the completion system we run up against
problems somehwere no matter how much you argue till your blue in the face
that it should always work your way".

We don't have the controls for causing a space to be added in non-default
cases yet.

Index: Doc/Zsh/compwid.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/compwid.yo,v
retrieving revision 1.47
diff -u -r1.47 compwid.yo
--- Doc/Zsh/compwid.yo	13 Jan 2009 12:19:53 -0000	1.47
+++ Doc/Zsh/compwid.yo	23 Apr 2009 10:38:48 -0000
@@ -58,6 +58,11 @@
 texinode(Completion Special Parameters)(Completion Builtin Commands)()(Completion Widgets)
 sect(Completion Special Parameters)
 
+The parameters tt(ZLE_REMOVE_SUFFIX_CHARS) and tt(ZLE_SPACE_SUFFIX_CHARS)
+are used by the completion mechanism, but are not special.
+ifzman(See em(Parameters Used By The Shell) in zmanref(zshparam))\
+ifnzman(noderef(Parameters Used By The Shell)).
+
 Inside completion widgets, and any functions called from them, some
 parameters have special meaning; outside these functions they are not
 special to the shell in any way.  These parameters are used to pass
Index: Doc/Zsh/params.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/params.yo,v
retrieving revision 1.53
diff -u -r1.53 params.yo
--- Doc/Zsh/params.yo	6 Apr 2009 09:06:36 -0000	1.53
+++ Doc/Zsh/params.yo	23 Apr 2009 10:38:48 -0000
@@ -1330,4 +1330,46 @@
 The directory to search for shell startup files (.zshrc, etc),
 if not tt($HOME).
 )
+vindex(ZLE_REMOVE_SUFFIX_CHARS)
+vindex(ZLE_SPACE_SUFFIX_CHARS)
+xitem(tt(ZLE_REMOVE_SUFFIX_CHARS))
+item(tt(ZLE_SPACE_SUFFIX_CHARS))(
+These parameters are used by the line editor.  In certain circumstances
+suffixes (typically space or slash) added by the completion system
+will be removed automatically, either because the next editing command
+was not an insertable character, or because the character was marked
+as requiring the suffix to be removed.
+
+These variables can contain the sets of characters that will cause the
+suffix to be removed.  If tt(ZLE_REMOVE_SUFFIX_CHARS) is set, those
+characters will cause the suffix to be removed; if
+tt(ZLE_SPACE_SUFFIX_CHARS) is set, those characters will cause the
+suffix to be removed and replaced by a space.
+
+The default value is equivalent to:
+
+example(ZLE_REMOVE_SUFFIX_CHARS=$' \t\n;&|')
+
+If tt(ZLE_REMOVE_SUFFIX_CHARS) is set but is empty, no characters have this
+behaviour.  tt(ZLE_SPACE_SUFFIX_CHARS) takes precedence, so that the
+following:
+
+example(ZLE_SPACE_SUFFIX_CHARS=$'&|')
+
+causes the characters `tt(&)' and `tt(|)' to remove the suffix but to
+replace it with a space.
+
+To illustrate the difference, suppose that the option tt(AUTO_REMOVE_SLASH)
+is in effect and the directory tt(DIR) has just been completed, with an
+appended tt(/), following which the user types `tt(&)'.  The default result
+is `tt(DIR&)'.  With tt(ZLE_REMOVE_SUFFIX_CHARS) set em(not) to include
+`tt(&)' the result is `tt(DIR/&)'.  With tt(ZLE_SPACE_SUFFIX_CHARS) set to
+include `tt(&)' the result is `tt(DIR &)'.
+
+Note that certain completions may provide their own suffix removal
+or replacement behaviour which overrides the values described here.
+See the completion system documentation in
+ifzman(zmanref(zshcompsys))\
+ifnzman(noderef(Completion System)).
+)
 enditem()
Index: Src/Zle/compresult.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/compresult.c,v
retrieving revision 1.77
diff -u -r1.77 compresult.c
--- Src/Zle/compresult.c	13 Jan 2009 12:19:54 -0000	1.77
+++ Src/Zle/compresult.c	23 Apr 2009 10:38:48 -0000
@@ -1011,7 +1011,7 @@
 		    stringaszleline(m->suf, 0, &len, NULL, NULL);
 		makesuffixstr(m->remf, m->rems, len);
 		if (len == 1)
-		    addsuffix(SUFTYP_POSSTR, wsuf, 1, 1);
+		    addsuffix(SUFTYP_POSSTR, 0, wsuf, 1, 1);
 		free(wsuf);
 	    }
 	}
@@ -1104,7 +1104,7 @@
 			makesuffixstr(m->remf, m->rems, 1);
 		    else if (isset(AUTOREMOVESLASH)) {
 			makesuffix(1);
-			addsuffix(SUFTYP_POSSTR, ZWS("/"), 1, 1);
+			addsuffix(SUFTYP_POSSTR, 0, ZWS("/"), 1, 1);
 		    }
 		}
 	    }
@@ -1119,7 +1119,7 @@
 	    /* If a suffix was added, and is removable, let *
 	     * `,' and `}' remove it.                       */
 	    if (isset(AUTOPARAMKEYS))
-		addsuffix(SUFTYP_POSSTR, ZWS(",}"), 2, suffixnoinslen);
+		addsuffix(SUFTYP_POSSTR, 0, ZWS(",}"), 2, suffixnoinslen);
 	} else if (!menucmp) {
 	    /*{{*/
 	    /* Otherwise, add a `,' suffix, and let `}' remove it. */
@@ -1129,7 +1129,7 @@
 	    minfo.insc++;
 	    makesuffix(1);
 	    if ((!menucmp || minfo.we) && isset(AUTOPARAMKEYS))
-		addsuffix(SUFTYP_POSSTR, ZWS(",}"), 2, 1);
+		addsuffix(SUFTYP_POSSTR, 0, ZWS(",}"), 2, 1);
 	}
     } else if (!havesuff && (!(m->flags & CMF_FILE) || !sr)) {
 	/* If we didn't add a suffix, add a space, unless we are *
Index: Src/Zle/zle.h
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle.h,v
retrieving revision 1.40
diff -u -r1.40 zle.h
--- Src/Zle/zle.h	20 Apr 2008 21:19:52 -0000	1.40
+++ Src/Zle/zle.h	23 Apr 2009 10:38:48 -0000
@@ -380,6 +380,10 @@
     SUFTYP_NEGRNG		/* Range of characters not to match */
 };
 
+/* Additional flags to suffixes */
+enum suffixflags {
+    SUFFLAGS_SPACE = 0x0001	/* Add a space when removing suffix */
+};
 
 #ifdef MULTIBYTE_SUPPORT
 /*
Index: Src/Zle/zle_misc.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_misc.c,v
retrieving revision 1.57
diff -u -r1.57 zle_misc.c
--- Src/Zle/zle_misc.c	28 Jan 2009 09:32:01 -0000	1.57
+++ Src/Zle/zle_misc.c	23 Apr 2009 10:38:49 -0000
@@ -1232,6 +1232,7 @@
 struct suffixset {
     struct suffixset *next;	/* Next in the list */
     int tp;			/* The SUFTYP_* from enum suffixtype */
+    int flags;			/* Some of SUFFLAGS_* */
     ZLE_STRING_T chars;		/* Set of characters to match (or not) */
     int lenstr;			/* Length of chars */
     int lensuf;			/* Length of suffix */
@@ -1255,13 +1256,14 @@
 
 /**/
 mod_export void
-addsuffix(int tp, ZLE_STRING_T chars, int lenstr, int lensuf)
+addsuffix(int tp, int flags, ZLE_STRING_T chars, int lenstr, int lensuf)
 {
     struct suffixset *newsuf = zalloc(sizeof(struct suffixset));
     newsuf->next = suffixlist;
     suffixlist = newsuf;
 
     newsuf->tp = tp;
+    newsuf->flags = flags;
     if (lenstr) {
 	newsuf->chars = zalloc(lenstr*sizeof(ZLE_CHAR_T));
 	ZS_memcpy(newsuf->chars, chars, lenstr);
@@ -1271,6 +1273,24 @@
     newsuf->lensuf = lensuf;
 }
 
+
+/* Same as addsuffix, but from metafied string */
+
+/**/
+mod_export void
+addsuffixstring(int tp, int flags, char *chars, int lensuf)
+{
+    int slen, alloclen;
+    ZLE_STRING_T suffixstr;
+
+    /* string needs to be writable... I've been regretting this for years.. */
+    chars = ztrdup(chars);
+    suffixstr = stringaszleline(chars, 0, &slen, &alloclen, NULL);
+    addsuffix(tp, flags, suffixstr, slen, lensuf);
+    zfree(suffixstr, alloclen);
+    zsfree(chars);
+}
+
 /* Set up suffix: the last n characters are a suffix that should be *
  * removed in the usual word end conditions.                        */
 
@@ -1278,7 +1298,17 @@
 mod_export void
 makesuffix(int n)
 {
-    addsuffix(SUFTYP_POSSTR, ZWS(" \t\n;&|"), 6, n);
+    char *suffixchars;
+
+    if (!(suffixchars = getsparam("ZLE_REMOVE_SUFFIX_CHARS")))
+	suffixchars = " \t\n;&|";
+
+    addsuffixstring(SUFTYP_POSSTR, 0, suffixchars, n);
+
+    /* Do this second so it takes precedence */
+    if ((suffixchars = getsparam("ZLE_SPACE_SUFFIX_CHARS")) && *suffixchars)
+	addsuffixstring(SUFTYP_POSSTR, SUFFLAGS_SPACE, suffixchars, n);
+
     suffixnoinslen = n;
 }
 
@@ -1301,7 +1331,7 @@
 	    lenstr += 6;
     }
     if (lenstr)
-	addsuffix(SUFTYP_POSSTR, charstr, lenstr, n);
+	addsuffix(SUFTYP_POSSTR, 0, charstr, lenstr, n);
 }
 
 /* Set up suffix given a string containing the characters on which to   *
@@ -1344,11 +1374,11 @@
 		ZLE_CHAR_T str[2];
 
 		if (wptr > lasts)
-		    addsuffix(inv ? SUFTYP_NEGSTR : SUFTYP_POSSTR,
+		    addsuffix(inv ? SUFTYP_NEGSTR : SUFTYP_POSSTR, 0,
 			      lasts, wptr - lasts, n);
 		str[0] = *wptr;
 		str[1] = wptr[2];
-		addsuffix(inv ? SUFTYP_NEGRNG : SUFTYP_POSRNG,
+		addsuffix(inv ? SUFTYP_NEGRNG : SUFTYP_POSRNG, 0,
 			  str, 2, n);
 
 		wptr += 3;
@@ -1360,7 +1390,7 @@
 	    }
 	}
 	if (wptr > lasts)
-	    addsuffix(inv ? SUFTYP_NEGSTR : SUFTYP_POSSTR,
+	    addsuffix(inv ? SUFTYP_NEGSTR : SUFTYP_POSSTR, 0,
 		      lasts, wptr - lasts, n);
 	free(ws);
     } else
@@ -1410,7 +1440,7 @@
 	zsfree(suffixfunc);
 	suffixfunc = NULL;
     } else {
-	int sl = 0;
+	int sl = 0, flags = 0;
 	struct suffixset *ss;
 
 	if (c == NO_INSERT_CHAR) {
@@ -1463,8 +1493,10 @@
 		    }
 		    break;
 		}
-		if (found)
+		if (found) {
+		    flags = ss->flags;
 		    break;
+		}
 	    }
 
 	    if (!found)
@@ -1472,12 +1504,17 @@
 	}
 	if (sl) {
 	    /* must be shifting wide character lengths */
-	    if (zlemetaline != NULL) {
-		unmetafy_line();
-		backdel(sl, CUT_RAW);
-		metafy_line();
-	    } else
-		backdel(sl, CUT_RAW);
+	    backdel(sl, CUT_RAW);
+	    if (flags & SUFFLAGS_SPACE)
+	    {
+		/* Add a space and advance over it */
+		spaceinline(1);
+		if (zlemetaline) {
+		    zlemetaline[zlemetacs++] = ' ';
+		} else {
+		    zleline[zlecs++] = ZWC(' ');
+		}
+	    }
 	    if (!keep)
 		invalidatelist();
 	}


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