PATCH: bug in ${(z)...} with newlines

PATCH: bug in ${(z)...} with newlines

[Peter Stephenson]

This must be a bug, mustn't it?

% foo=$'one\ntwo'
% print ${(z)foo}
one ; two

As far as splitting arguments goes, the newline is just whitespace.

Index: Src/hist.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/hist.c,v
retrieving revision 1.98
diff -p -u -r1.98 hist.c
--- Src/hist.c	22 Mar 2010 23:20:11 -0000	1.98
+++ Src/hist.c	9 Jun 2010 13:05:20 -0000
@@ -2876,7 +2876,7 @@ bufferwords(LinkList list, char *buf, in
 		sprintf(b, "%d%s", tokfd, tokstrings[tok]);
 		addlinknode(list, dupstring(b));
 		num++;
-	    } else if (tok != NEWLIN) {
+	    } else if (tok != NEWLIN && tok != SEPER) {
 		addlinknode(list, dupstring(tokstrings[tok]));
 		num++;
 	    }
Index: Test/D04parameter.ztst
===================================================================
RCS file: /cvsroot/zsh/zsh/Test/D04parameter.ztst,v
retrieving revision 1.43
diff -p -u -r1.43 D04parameter.ztst
--- Test/D04parameter.ztst	12 May 2010 10:17:58 -0000	1.43
+++ Test/D04parameter.ztst	9 Jun 2010 13:05:20 -0000
@@ -389,6 +389,14 @@
 >)
 >ten( more
 
+  foo=$'first line\nsecond line'
+  print -l ${(z)foo}
+0:${(z)...} with newlines
+>first
+>line
+>second
+>line
+
   psvar=(dog)
   setopt promptsubst
   foo='It shouldn'\''t $(happen) to a %1v.'

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Re: PATCH: bug in ${(z)...} with newlines

[Bart Schaefer]

On Wed, Jun 9, 2010 at 6:07 AM, Peter Stephenson <pws@csr.com> wrote:
> This must be a bug, mustn't it?
>
> % foo=$'one\ntwo'
> % print ${(z)foo}
> one ; two

I'm pretty sure that's intentional, so that when the newline is
removed by splitting, the join is still a semantically equivalent
shell expression.  Otherwise you can't do stuff like this:

foo=(${(q)${(z)foo}})
bar=time
for word in $foo
do if [[ $word == '\;' ]]
   then bar+=($word time)
   else bar+=($word)
   fi
done
eval $bar

In fact if newline is not replaced with semicolon by (z), I think it
becomes impossible to find command words by searching through the
resulting array.

> As far as splitting arguments goes, the newline is just whitespace.

Yes, but (z) is really for parsing, not merely splitting.

Re: PATCH: bug in ${(z)...} with newlines

[Bart Schaefer]

On Wed, Jun 9, 2010 at 7:34 AM, Bart Schaefer <schaefer@brasslantern.com> wrote:
> foo=(${(q)${(z)foo}})
> bar=time

Er, that should be bar=(time) of course.

> for word in $foo
> do if [[ $word == '\;' ]]
>   then bar+=($word time)
>   else bar+=($word)
>   fi
> done

Re: PATCH: bug in ${(z)...} with newlines

[Peter Stephenson]

On Wed, 9 Jun 2010 07:34:47 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Wed, Jun 9, 2010 at 6:07 AM, Peter Stephenson <pws@csr.com> wrote:
> > This must be a bug, mustn't it?
> >
> > % foo=$'one\ntwo'
> > % print ${(z)foo}
> > one ; two
> 
> I'm pretty sure that's intentional, so that when the newline is
> removed by splitting, the join is still a semantically equivalent
> shell expression.  Otherwise you can't do stuff like this:
>...
> > As far as splitting arguments goes, the newline is just whitespace.
> 
> Yes, but (z) is really for parsing, not merely splitting.

Hmm.  I've only ever used (z) for splitting words (which is why it was 'z',
since 's' was used), which is my memory of why it was introduced.  Now
you're telling me it's for something other than what it says in the manual,
which just says parsing is to find the words, and I can't use it without
jumping through hoops.  Nor can I think of a case where I would use it to
find lines of code.

I can certainly believe the completion system, which uses the same code
underneath, might be relying on undocumented 'intentional' features.

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Re: PATCH: bug in ${(z)...} with newlines

[Bart Schaefer]

Sorry to have left this sit so long, I had relatives in town for my
son's high school graduation and wasn't really keeping up with all my
email.

On Wed, Jun 9, 2010 at 8:54 AM, Peter Stephenson
<Peter.Stephenson@csr.com> wrote:
> On Wed, 9 Jun 2010 07:34:47 -0700
> Bart Schaefer <schaefer@brasslantern.com> wrote:
>> On Wed, Jun 9, 2010 at 6:07 AM, Peter Stephenson <pws@csr.com> wrote:
>> > This must be a bug, mustn't it?
>> >
>> > % foo=$'one\ntwo'
>> > % print ${(z)foo}
>> > one ; two
>>
>> Yes, but (z) is really for parsing, not merely splitting.
>
> Hmm.  I've only ever used (z) for splitting words (which is why it was 'z',
> since 's' was used), which is my memory of why it was introduced.  Now
> you're telling me it's for something other than what it says in the manual
> which just says parsing is to find the words, and I can't use it without
> jumping through hoops.  Nor can I think of a case where I would use it to
> find lines of code.

I did a dive into the list archive and the addition of (z) originated
with this remark by Felix Rosencrantz (zsh-workers/10951):

-> I wasn't sure how to break up the values of $history to find the start
-> of commands, and to break up along word boundaries.  I was thinking
-> that we might need C code support for that.

Sven proposed something unworkable involving $historywords, and I replied with:

+> I think having a way to chop a string into shell words -- something like
+> what you did for copy-prev-shell-word -- would be more effective.

This eventually resulted in Sven posting the patch for (z).

Now, this was all in the context of the _history completer which may
very well by now be doing its thing in an entirely different way -- I
don't find any use of (z) in _history or indeed anywhere in
Completion/**/*(.) [scanning with egrep] that conflicts with your
proposed change.  I just want to make sure the ramifications have been
considered.

> I can certainly believe the completion system, which uses the same code
> underneath, might be relying on undocumented 'intentional' features.

I find calls to bufferwords() in Zle/{zle_hist,zle_misc}.c and subst.c
but not in completion (oddly, as a comment indicates that's where it
originally came from).  The only place your change may have a
detectable effect is in the implementation of copyprevshellword() in
zle_misc.

Re: PATCH: bug in ${(z)...} with newlines

[Bart Schaefer]

On Wed, Jun 16, 2010 at 3:06 PM, Bart Schaefer
<schaefer@brasslantern.com> wrote:
> I don't find any use of (z) in _history or indeed anywhere in
> Completion/**/*(.) [scanning with egrep] that conflicts with your
> proposed change.  I just want to make sure the ramifications have been
> considered.

BTW there are uses of (z) in Functions/Zle/ that may very well be
relying on the last ten years of behavior, including in
match-word-context and match-words-by-style and even which-command.

Re: PATCH: bug in ${(z)...} with newlines

[Peter Stephenson]

On Wed, 16 Jun 2010 15:13:29 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:

> On Wed, Jun 16, 2010 at 3:06 PM, Bart Schaefer
> <schaefer@brasslantern.com> wrote:
> > I don't find any use of (z) in _history or indeed anywhere in
> > Completion/**/*(.) [scanning with egrep] that conflicts with your
> > proposed change.  I just want to make sure the ramifications have
> > been considered.

Thanks for checking...
 
> BTW there are uses of (z) in Functions/Zle/ that may very well be
> relying on the last ten years of behavior, including in
> match-word-context and match-words-by-style and even which-command.

Several of those are mine, which definitely don't rely on getting something
other than split arguments.  I've been assuming up to now that as long
as the words can be parsed (there are odd cases with unquoted
parentheses and the like) there should be no gotchas, and in particular
that if you've input quoted words separated by white space you should
just get the words back (hence the current thread).

I suspect it would be safest at the least to ensure bufferwords() behaves
the same for calls from ZLE; trying to sanity check that is a whole
different ballgame from looking to see whether shell code has made
assumptions about undocumented behaviour.

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom

Re: PATCH: bug in ${(z)...} with newlines

[Bart Schaefer]

On Thu, Jun 17, 2010 at 4:50 AM, Peter Stephenson
<Peter.Stephenson@csr.com> wrote:
> On Wed, 16 Jun 2010 15:13:29 -0700
> Bart Schaefer <schaefer@brasslantern.com> wrote:
>> BTW there are uses of (z) in Functions/Zle/ that may very well be
>> relying on the last ten years of behavior, including in
>> match-word-context and match-words-by-style and even which-command.
>
> Several of those are mine, which definitely don't rely on getting something
> other than split arguments.  I've been assuming up to now that as long
> as the words can be parsed (there are odd cases with unquoted
> parentheses and the like) there should be no gotchas, and in particular
> that if you've input quoted words separated by white space you should
> just get the words back (hence the current thread).

There are a bunch of other potential gotchas with (z) from that
standpoint, such as special treatment of redirection operators.  If
what you really want is purely quotation parsing, it may be better to
add something new rather than modify (z).

Also it is helpful to have something that can distinguish a newline
that would be parsed as a command separator from one that
shell-syntactically is merely whitespace.  That was Felix's original
problem all those years ago.