Re: Minor expansion problem

Re: Minor expansion problem

[Peter Stephenson]

Bart Schaefer wrote:
> Expanding $(...) doesn't put
> in backslashes, but expanding `...` does.  This appears to be happening
> because the ${(e)exp} at _expand line 83 returns one word for `...` but
> an array for $(...).

Yes, it's completely reproducible just with ${(e)exp}.

> } but what I expect, i.e. no backslashes, with the _expand_word widget?
> 
> Hrm, I get backslashes in both cases.  Are you sure you're using the
> _expand_word widget (^Xe) and not the builtin expand-word (^X*) ?

You're right, I was using expand-word, which is not rebound by the
completion system.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 392070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************

PATCH: Re: Minor expansion problem

[Bart Schaefer]

On Sep 25,  5:08pm, Peter Stephenson wrote:
}
} Bart Schaefer wrote:
} > Expanding $(...) doesn't put
} > in backslashes, but expanding `...` does.  This appears to be happening
} > because the ${(e)exp} at _expand line 83 returns one word for `...` but
} > an array for $(...).
} 
} Yes, it's completely reproducible just with ${(e)exp}.

This seems to fix it (and a typo in a comment).  I'm a bit dubious of
changing mult_isarr possibly in the middle of a word, but that's what
happens in the $(...) case at line 111, so ...

Index: Src/subst.c
===================================================================
diff -c -r1.6 subst.c
--- Src/subst.c	2001/09/05 15:22:33	1.6
+++ Src/subst.c	2001/09/25 16:33:20
@@ -133,7 +133,7 @@
 		str3 = (char *)getdata(node);
 		continue;
 	    }
-	} else if ((qt = c == Qtick) || c == Tick)
+	} else if ((qt = c == Qtick) || (c == Tick ? mult_isarr = 1 : 0))
 	  comsub: {
 	    LinkList pl;
 	    char *s, *str2 = str;
@@ -1492,7 +1492,7 @@
             /* This once was executed only `if (qt) ...'. But with that
              * patterns in a expansion resulting from a ${(e)...} aren't
              * tokenized even though this function thinks they are (it thinks
-             * they are because subst_parse_string() turns Qstring tokens
+             * they are because subst_parse_str() turns Qstring tokens
              * into String tokens and for unquoted parameter expansions the
              * lexer normally does tokenize patterns inside parameter
              * expansions). */


-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

Re: PATCH: Re: Minor expansion problem

[Bart Schaefer]

On Sep 25,  4:39pm, Bart Schaefer wrote:
}
} } > Expanding $(...) doesn't put
} } > in backslashes, but expanding `...` does.  This appears to be happening
} } > because the ${(e)exp} at _expand line 83 returns one word for `...` but
} } > an array for $(...).
} 
} This seems to fix it

No, on second look, I mistyped my test case; it doesn't fix it after all.
Seems subst_parse_str() needs to turn Qtick into Tick as well as Qstring
into String.  The following goes on top of 15871.

Now I'm concerned enough about this to wait for some feedback before I
commit it.  All tests pass (including the new one below), but better safe
than sorry.

Index: Src/subst.c
===================================================================
diff -c -r1.6 subst.c
--- Src/subst.c	2001/09/05 15:22:33	1.6
+++ Src/subst.c	2001/09/25 17:00:08
@@ -133,7 +133,7 @@
 		str3 = (char *)getdata(node);
 		continue;
 	    }
-	} else if ((qt = c == Qtick) || c == Tick)
+	} else if ((qt = c == Qtick) || (c == Tick ? (mult_isarr = 1) : 0))
 	  comsub: {
 	    LinkList pl;
 	    char *s, *str2 = str;
@@ -724,9 +724,12 @@
             int qt = 0;
 
 	    for (; *s; s++)
-		if (!qt && *s == Qstring)
-		    *s = String;
-                else if (*s == Dnull)
+		if (!qt) {
+		    if (*s == Qstring)
+			*s = String;
+		    else if (*s == Qtick)
+			*s = Tick;
+                } else if (*s == Dnull)
                     qt = !qt;
 	}
 	return 0;
Index: Test/D04parameter.ztst
===================================================================
diff -c -r1.4 D04parameter.ztst
--- Test/D04parameter.ztst	2001/09/05 15:22:35	1.4
+++ Test/D04parameter.ztst	2001/09/25 16:52:53
@@ -212,10 +212,17 @@
 >split me
 >I'm yours
 
-  foo='$(print Howzat)'
-  print ${(e)foo}
+  foo='$(print Howzat usay)'
+  print -l ${(e)foo}
 0:${(e)...}
 >Howzat
+>usay
+
+  foo='`print Howzat usay`'
+  print -l ${(e)foo}
+0:Regress ${(e)...} with backticks (see zsh-workers/15871)
+>Howzat
+>usay
 
   foo='I'\''m nearly out of my mind with tedium'
   bar=foo

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

Re: PATCH: Re: Minor expansion problem

[Bart Schaefer]

On Sep 25,  5:07pm, Bart Schaefer wrote:
}
} The following goes on top of 15871.

Except that I screwed up and included one of the hunks it's supposed to
go on top of.  Sorry about that.

Re: PATCH: Re: Minor expansion problem

[Peter Stephenson]

Bart Schaefer wrote:
> Now I'm concerned enough about this to wait for some feedback before I
> commit it.  All tests pass (including the new one below), but better safe
> than sorry.

I understand subst.c nowadays about as far as it's commented, but it all
looks pretty sensible.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 392070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************

Re: PATCH: Re: Minor expansion problem

[Bart Schaefer]

On Sep 25,  6:23pm, Peter Stephenson wrote:
}
} Bart Schaefer wrote:
} > Now I'm concerned enough about this to wait for some feedback [...]
} 
} [...] looks pretty sensible.

I'll commit it to the 4.1 trunk now, and wait a bit for the 4.0 branch.

Incidentally, we should probably have a new -dev-X soon, too.

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net