Bug in { completion's comma removal

Bug in { completion's comma removal

[Wayne Davison]

I haven't tried to trace this down yet, but I noticed a bug in recent
multibyte-enabled zsh versions (including CVS):

  % touch foo faa
  % ls f{oo,a<TAB>       # pressing TAB results in:
  % ls f{oo,aa,_         # (note: _ is the cursor) OK so far...
  % ls f{oo,aa,}_        # typing } does not remove the comma

This bug does not appear in the latest code if I build it without
MULTIBYTE_SUPPORT enabled.

..wayne..

Re: Bug in { completion's comma removal

[Mikael Magnusson]

On 1/6/06, Wayne Davison <wayned@users.sourceforge.net> wrote:
> I haven't tried to trace this down yet, but I noticed a bug in recent
> multibyte-enabled zsh versions (including CVS):
>
>   % touch foo faa
>   % ls f{oo,a<TAB>       # pressing TAB results in:
>   % ls f{oo,aa,_         # (note: _ is the cursor) OK so far...
>   % ls f{oo,aa,}_        # typing } does not remove the comma
>
> This bug does not appear in the latest code if I build it without
> MULTIBYTE_SUPPORT enabled.
>
> ..wayne..

While we're talking about {}, it also doesn't work so well on
filenames with spaces and there's no way (that i've figured out) to
escape a comma inside the braces.

--
Mikael Magnusson

Re: Bug in { completion's comma removal

[Wayne Davison]

On Fri, Jan 06, 2006 at 09:07:19PM +0100, Mikael Magnusson wrote:
> While we're talking about {}, it also doesn't work so well on
> filenames with spaces

I haven't seen any problems with spaces as long as you put a backslash
in front of each one (which is what the completion of a filename with
spaces will automatically add).  This is to be expected because a
command-line of "f{oo bar,oo-baz}" (without the quotes) is really two
args (neither one valid in syntax), but adding a backslash before the
space makes it a single arg that will expand into two, "foo\ bar" and
"foo-baz".

> and there's no way (that i've figured out) to escape a comma inside
> the braces.

Yes, this is a problem when completing and expanding in zle.  It does
appear to work fine in the shell itself:

    % echo ba{r\,1,z\,2}
    bar,1 baz,2

However, when completing a filename with a comma inside an open {, zsh
does not add a backslash before the comma, and it strips any existing
backslash before a comma (e.g. pressing <TAB> on the above term would
incorrectly expand the line into "echo bar ba1 baz ba2").

..wayne..

Re: Bug in { completion's comma removal

[Mikael Magnusson]

On 1/6/06, Wayne Davison <wayned@users.sourceforge.net> wrote:
> On Fri, Jan 06, 2006 at 09:07:19PM +0100, Mikael Magnusson wrote:
> > While we're talking about {}, it also doesn't work so well on
> > filenames with spaces
>
> I haven't seen any problems with spaces as long as you put a backslash
> in front of each one (which is what the completion of a filename with
> spaces will automatically add).  This is to be expected because a
> command-line of "f{oo bar,oo-baz}" (without the quotes) is really two
> args (neither one valid in syntax), but adding a backslash before the
> space makes it a single arg that will expand into two, "foo\ bar" and
> "foo-baz".

Hm, it appears that it doesn't happen with zsh -f, so I'll have to
poke around a bit in my settings before I can say anything useful...

> > and there's no way (that i've figured out) to escape a comma inside
> > the braces.
>
> Yes, this is a problem when completing and expanding in zle.  It does
> appear to work fine in the shell itself:
>
>     % echo ba{r\,1,z\,2}
>     bar,1 baz,2
>
> However, when completing a filename with a comma inside an open {, zsh
> does not add a backslash before the comma, and it strips any existing
> backslash before a comma (e.g. pressing <TAB> on the above term would
> incorrectly expand the line into "echo bar ba1 baz ba2").
>
> ..wayne..
>


--
Mikael Magnusson

Re: Bug in { completion's comma removal

[Wayne Davison]

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

On Fri, Jan 06, 2006 at 09:07:19PM +0100, Mikael Magnusson wrote:
> While we're talking about {} [...] there's no way (that i've figured
> out) to escape a comma inside the braces.

I've looked at this, and I'm thinking that the easiest thing to do is to
sometimes mark a comma with ISPECIAL, just like {, }, space, etc.  Since
user-typed commas inside a brace are already transformed into "Comma"
(0x97), this ensures that completing filenames that contain a comma get
a backslash prefixed, and that a user-typed "\," sequence does not lose
the backslash.  I'll attach a patch that works, but I'd appreciate some
input if someone would care to take a look at what I've done.

Two caveats about my patch:

 - When I turn on the ISPECIAL bit for ',', I also set a flag that makes
   sure that all calls to inittyptab() continue to set ISPECIAL for the
   comma.  Without this, a comma's specialness was quickly lost.

 - Expansion of { ... } sequences by the completion system are still not
   working right if there are backslash-escaped commas and/or closing
   braces.  (The shell handles "echo {foo\,bar,foo\,baz,bar\}foo}"
   properly, just not TAB expansion.)

..wayne..

[-- Attachment #2: comma.patch --]
[-- Type: text/plain, Size: 1582 bytes --]

--- Src/utils.c	6 Feb 2006 11:57:07 -0000	1.116
+++ Src/utils.c	11 Feb 2006 03:29:36 -0000
@@ -2515,6 +2515,8 @@ equalsplit(char *s, char **t)
     return 0;
 }
 
+static int specialcomma;
+
 /* the ztypes table */
 
 /**/
@@ -2614,10 +2614,22 @@ inittyptab(void)
     }
     for (s = SPECCHARS; *s; s++)
 	typtab[STOUC(*s)] |= ISPECIAL;
+    if (specialcomma)
+	typtab[STOUC(',')] |= ISPECIAL;
     if (isset(BANGHIST) && bangchar && interact && isset(SHINSTDIN))
 	typtab[bangchar] |= ISPECIAL;
 }
 
+/**/
+void
+makecommaspecial(int yesno)
+{
+    if ((specialcomma = yesno) != 0)
+	typtab[STOUC(',')] |= ISPECIAL;
+    else
+	typtab[STOUC(',')] &= ~ISPECIAL;
+}
+
 
 /**/
 #ifdef MULTIBYTE_SUPPORT
--- Src/Zle/zle_tricky.c	12 Jan 2006 00:51:53 -0000	1.64
+++ Src/Zle/zle_tricky.c	11 Feb 2006 03:29:38 -0000
@@ -586,6 +586,7 @@ docomplete(int lst)
     }
     active = 1;
     comprecursive = 0;
+    makecommaspecial(0);
     if (undoing)
 	setlastline();
 
@@ -662,6 +663,7 @@ docomplete(int lst)
 	    unmetafy_line();
 	    zsfree(s);
 	    active = 0;
+	    makecommaspecial(0);
 	    return 1;
 	}
 	ocs = zlemetacs;
@@ -852,6 +854,7 @@ docomplete(int lst)
     unmetafy_line();
 
     active = 0;
+    makecommaspecial(0);
     return dat[1];
 }
 
@@ -1593,6 +1596,7 @@ get_comp_string(void)
 			p = tp - 1;
 			continue;
 		    }
+		    makecommaspecial(1);
 		    if (bbeg) {
 			Brinfo new;
 			int len = bend - bbeg;
@@ -1639,6 +1643,7 @@ get_comp_string(void)
 			continue;
 		    }
 		    cant = 1;
+		    makecommaspecial(1);
 		    break;
 		}
 		if (p == curs) {

Re: Bug in { completion's comma removal

[Peter Stephenson]

Wayne Davison wrote:
> On Fri, Jan 06, 2006 at 09:07:19PM +0100, Mikael Magnusson wrote:
> > While we're talking about {} [...] there's no way (that i've figured
> > out) to escape a comma inside the braces.
> 
> I've looked at this, and I'm thinking that the easiest thing to do is to
> sometimes mark a comma with ISPECIAL, just like {, }, space, etc.  Since
> user-typed commas inside a brace are already transformed into "Comma"
> (0x97), this ensures that completing filenames that contain a comma get
> a backslash prefixed, and that a user-typed "\," sequence does not lose
> the backslash.  I'll attach a patch that works, but I'd appreciate some
> input if someone would care to take a look at what I've done.

It's a bit of a hack, but that's hardly a big change around here...

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


To access the latest news from CSR copy this link into a web browser:  http://www.csr.com/email_sig.php

Re: Bug in { completion's comma removal

[Wayne Davison]

On Fri, Jan 06, 2006 at 10:00:19AM -0800, Wayne Davison wrote:
> I haven't tried to trace this down yet

OK, I finally looked into this.  This turns out to be as simple as
iremovesuffix() using memchr() on a wide-character string instead of
wmemchr().  I created a ZS_memchr() define in zle.h and changed the
two places that called memchr() to use it, and all is now well.  This
also fixed the removal of other suffixes too, such as the removal of
an auto-added space when ';' is typed.

..wayne..