[PATCH]: Revamped (P) expansion flag

[PATCH]: Revamped (P) expansion flag

[Michael Hwang]

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

Hello all,

In my opinion, the current behavior of the P expansion flag is not
intuitive, and possibly even buggy. For instance:

% FOO='hello!'
% REF='FOO BAR'
% print ${(P)${REF}}
hello!

In this case, ${(P)${REF}} should expand to nothing, because 'FOO BAR'
is not a proper parameter name. However, the current logic takes only
as much as makes sense, and ignores the rest. This creates confusion
with arrays:

% ARRAY=(FOO CLUE SHOE)
% FOO=zsh
% CLUE=is
% SHOE=awesome
% print ${(P)${ARRAY}}
zsh

One would think that each element of the reference ARRAY would
replaced by the value of the variable in that element. However, zsh
will just take the value of FOO, as it's the longest string that makes
sense as a variable name.

On the more buggy-ish side:

% STRING='zsh is awesome!'
% STR='zsh sucks... :-/'
% REF=STRING
% print ${(P)REF[1,3]}
zsh sucks... :-/

As ${(P)REF} is really ${(P)${REF}}, one would think that that
${(P)REF[1,3]} would be expanded as ${(P)${REF}[1,3]}. But as you can
see, it is instead expanded as ${(P)${REF[1,3]}}.

This patch fixes all these problems. The concept of "subexpression" no
longer applies with the P flag. Instead, one should consider whether
or not the "inside" expression (I call it a "reference") expands to
more than one word.

Notably:
1.) A reference can now be a mix of plain text and expansions. For
example, ${(P)${FOO}_BAR}.
2.) A reference can be quoted. Note that ${(P)"REF"} is now possible,
and expands as if it were ${(P)"${REF}"}.
3.) If the reference expands to more than one word, then each element
will be expanded to take on the value of that variable. (See below.)

It is simplest with quotes:

% FOO='zsh.org'
% REF=FOO
% print ${(P)"REF"}
zsh.org
% print ${(P)"${REF}"}
zsh.org

Fairly straight forward. But notice what happens when our reference
expands to a non-valid parameter name:

% FOO='merry xmas!'
% REF='FOO FOO'
% print ${(P)"${REF}"}

% print ${(P)"REF"}


Now try mixing (in quotes):

% FOOBAR='buy champagne for new year'
% REF=FOO
% print ${(P)"${REF}BAR"}
buy champagne for new year

The expansion takes on the value of the variable name that the insides
expand to.

Without quotes, array references are possible.

% REFS=(A B C)
% A=1
% B=2
% C=3
% print -l -- ${(P)${REFS}}
1
2
3

It is also possible to have an array reference with an element that
refers to an array parameter:

% REFS=(A B C)
% A=1
% B=(2 two)
% C=3
% print -l -- ${(P)${REFS}}
1
2
two
3

And finally, mixing, non-quoted.

% REFS=(FOO CLUE SHOE)
% FOOBAR=zsh
% CLUEBAR=is
% SHOEBAR=awesome
% print ${(P)${^REFS}BAR}
zsh is awesome

Note that ${(P)${REFS}BAR} would just expand to "awesome", as
${REFS}BAR expands to the words "FOO" "CLUE" "SHOEBAR", with FOO and
CLUE not being set.

Please test these changes. paramsubst() took a long time to
understand, so I'm sure that I've introduced a bug or two.

Michael Hwang

Re: [PATCH]: Revamped (P) expansion flag

[Michael Hwang]

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

Sorry guys,

Frank has just informed me that the patch didn't come through. So I'm
reattaching it, providing a link (http://gist.github.com/264099), and
providing a repo (git://github.com/Nomexous/zsh.git) where the changes
can be retrieved from the "pflag" branch.

Michael Hwang

On Sat, Dec 26, 2009 at 5:48 PM, Michael Hwang
<michael.a.hwang@gmail.com> wrote:
> Hello all,
>
> In my opinion, the current behavior of the P expansion flag is not
> intuitive, and possibly even buggy. For instance:
>
> % FOO='hello!'
> % REF='FOO BAR'
> % print ${(P)${REF}}
> hello!
>
> In this case, ${(P)${REF}} should expand to nothing, because 'FOO BAR'
> is not a proper parameter name. However, the current logic takes only
> as much as makes sense, and ignores the rest. This creates confusion
> with arrays:
>
> % ARRAY=(FOO CLUE SHOE)
> % FOO=zsh
> % CLUE=is
> % SHOE=awesome
> % print ${(P)${ARRAY}}
> zsh
>
> One would think that each element of the reference ARRAY would
> replaced by the value of the variable in that element. However, zsh
> will just take the value of FOO, as it's the longest string that makes
> sense as a variable name.
>
> On the more buggy-ish side:
>
> % STRING='zsh is awesome!'
> % STR='zsh sucks... :-/'
> % REF=STRING
> % print ${(P)REF[1,3]}
> zsh sucks... :-/
>
> As ${(P)REF} is really ${(P)${REF}}, one would think that that
> ${(P)REF[1,3]} would be expanded as ${(P)${REF}[1,3]}. But as you can
> see, it is instead expanded as ${(P)${REF[1,3]}}.
>
> This patch fixes all these problems. The concept of "subexpression" no
> longer applies with the P flag. Instead, one should consider whether
> or not the "inside" expression (I call it a "reference") expands to
> more than one word.
>
> Notably:
> 1.) A reference can now be a mix of plain text and expansions. For
> example, ${(P)${FOO}_BAR}.
> 2.) A reference can be quoted. Note that ${(P)"REF"} is now possible,
> and expands as if it were ${(P)"${REF}"}.
> 3.) If the reference expands to more than one word, then each element
> will be expanded to take on the value of that variable. (See below.)
>
> It is simplest with quotes:
>
> % FOO='zsh.org'
> % REF=FOO
> % print ${(P)"REF"}
> zsh.org
> % print ${(P)"${REF}"}
> zsh.org
>
> Fairly straight forward. But notice what happens when our reference
> expands to a non-valid parameter name:
>
> % FOO='merry xmas!'
> % REF='FOO FOO'
> % print ${(P)"${REF}"}
>
> % print ${(P)"REF"}
>
>
> Now try mixing (in quotes):
>
> % FOOBAR='buy champagne for new year'
> % REF=FOO
> % print ${(P)"${REF}BAR"}
> buy champagne for new year
>
> The expansion takes on the value of the variable name that the insides
> expand to.
>
> Without quotes, array references are possible.
>
> % REFS=(A B C)
> % A=1
> % B=2
> % C=3
> % print -l -- ${(P)${REFS}}
> 1
> 2
> 3
>
> It is also possible to have an array reference with an element that
> refers to an array parameter:
>
> % REFS=(A B C)
> % A=1
> % B=(2 two)
> % C=3
> % print -l -- ${(P)${REFS}}
> 1
> 2
> two
> 3
>
> And finally, mixing, non-quoted.
>
> % REFS=(FOO CLUE SHOE)
> % FOOBAR=zsh
> % CLUEBAR=is
> % SHOEBAR=awesome
> % print ${(P)${^REFS}BAR}
> zsh is awesome
>
> Note that ${(P)${REFS}BAR} would just expand to "awesome", as
> ${REFS}BAR expands to the words "FOO" "CLUE" "SHOEBAR", with FOO and
> CLUE not being set.
>
> Please test these changes. paramsubst() took a long time to
> understand, so I'm sure that I've introduced a bug or two.
>
> Michael Hwang
>

Re: [PATCH]: Revamped (P) expansion flag

[Michael Hwang]

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

Here's an update, to be applied on top of the above patch. This
enables the k and v expansion flags to be used along side the P flag.

Attachment, Gist (http://gist.github.com/264465), and branch "pflag"
(git://github.com/Nomexous/zsh.git).

Michael Hwang

On Sat, Dec 26, 2009 at 7:59 PM, Michael Hwang
<michael.a.hwang@gmail.com> wrote:
> Sorry guys,
>
> Frank has just informed me that the patch didn't come through. So I'm
> reattaching it, providing a link (http://gist.github.com/264099), and
> providing a repo (git://github.com/Nomexous/zsh.git) where the changes
> can be retrieved from the "pflag" branch.
>
> Michael Hwang
>
> On Sat, Dec 26, 2009 at 5:48 PM, Michael Hwang
> <michael.a.hwang@gmail.com> wrote:
>> Hello all,
>>
>> In my opinion, the current behavior of the P expansion flag is not
>> intuitive, and possibly even buggy. For instance:
>>
>> % FOO='hello!'
>> % REF='FOO BAR'
>> % print ${(P)${REF}}
>> hello!
>>
>> In this case, ${(P)${REF}} should expand to nothing, because 'FOO BAR'
>> is not a proper parameter name. However, the current logic takes only
>> as much as makes sense, and ignores the rest. This creates confusion
>> with arrays:
>>
>> % ARRAY=(FOO CLUE SHOE)
>> % FOO=zsh
>> % CLUE=is
>> % SHOE=awesome
>> % print ${(P)${ARRAY}}
>> zsh
>>
>> One would think that each element of the reference ARRAY would
>> replaced by the value of the variable in that element. However, zsh
>> will just take the value of FOO, as it's the longest string that makes
>> sense as a variable name.
>>
>> On the more buggy-ish side:
>>
>> % STRING='zsh is awesome!'
>> % STR='zsh sucks... :-/'
>> % REF=STRING
>> % print ${(P)REF[1,3]}
>> zsh sucks... :-/
>>
>> As ${(P)REF} is really ${(P)${REF}}, one would think that that
>> ${(P)REF[1,3]} would be expanded as ${(P)${REF}[1,3]}. But as you can
>> see, it is instead expanded as ${(P)${REF[1,3]}}.
>>
>> This patch fixes all these problems. The concept of "subexpression" no
>> longer applies with the P flag. Instead, one should consider whether
>> or not the "inside" expression (I call it a "reference") expands to
>> more than one word.
>>
>> Notably:
>> 1.) A reference can now be a mix of plain text and expansions. For
>> example, ${(P)${FOO}_BAR}.
>> 2.) A reference can be quoted. Note that ${(P)"REF"} is now possible,
>> and expands as if it were ${(P)"${REF}"}.
>> 3.) If the reference expands to more than one word, then each element
>> will be expanded to take on the value of that variable. (See below.)
>>
>> It is simplest with quotes:
>>
>> % FOO='zsh.org'
>> % REF=FOO
>> % print ${(P)"REF"}
>> zsh.org
>> % print ${(P)"${REF}"}
>> zsh.org
>>
>> Fairly straight forward. But notice what happens when our reference
>> expands to a non-valid parameter name:
>>
>> % FOO='merry xmas!'
>> % REF='FOO FOO'
>> % print ${(P)"${REF}"}
>>
>> % print ${(P)"REF"}
>>
>>
>> Now try mixing (in quotes):
>>
>> % FOOBAR='buy champagne for new year'
>> % REF=FOO
>> % print ${(P)"${REF}BAR"}
>> buy champagne for new year
>>
>> The expansion takes on the value of the variable name that the insides
>> expand to.
>>
>> Without quotes, array references are possible.
>>
>> % REFS=(A B C)
>> % A=1
>> % B=2
>> % C=3
>> % print -l -- ${(P)${REFS}}
>> 1
>> 2
>> 3
>>
>> It is also possible to have an array reference with an element that
>> refers to an array parameter:
>>
>> % REFS=(A B C)
>> % A=1
>> % B=(2 two)
>> % C=3
>> % print -l -- ${(P)${REFS}}
>> 1
>> 2
>> two
>> 3
>>
>> And finally, mixing, non-quoted.
>>
>> % REFS=(FOO CLUE SHOE)
>> % FOOBAR=zsh
>> % CLUEBAR=is
>> % SHOEBAR=awesome
>> % print ${(P)${^REFS}BAR}
>> zsh is awesome
>>
>> Note that ${(P)${REFS}BAR} would just expand to "awesome", as
>> ${REFS}BAR expands to the words "FOO" "CLUE" "SHOEBAR", with FOO and
>> CLUE not being set.
>>
>> Please test these changes. paramsubst() took a long time to
>> understand, so I'm sure that I've introduced a bug or two.
>>
>> Michael Hwang
>>
>

Re: [PATCH]: Revamped (P) expansion flag

[Peter Stephenson]

On Sat, 26 Dec 2009 17:48:07 -0500
Michael Hwang <michael.a.hwang@gmail.com> wrote:
> Please test these changes. paramsubst() took a long time to
> understand, so I'm sure that I've introduced a bug or two.

With the two patches I know about (the second a single line), the test
suite now fails at this test in D06subscript.ztst:


  qqq=lower
  QQQ=upper
  print ${(P)A[one\"two\"three\"quotes]}
  print "${(P)A[$A[(i)one\"two\"three\"quotes]]}"
0:Keys with double quotes and the (P) expansion flag
>lower
>upper


i.e. if you execute the first four lines you expect status zero and
the output "lower" and "upper" which you don't get.  This is testing
associative array lookup.  At this point you have:


typeset -A A
typeset "A[one\"two\"three\"quotes]"=QQQ
typeset 'A[one\"two\"three\"quotes]'=qqq


-- 
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]: Revamped (P) expansion flag

[Michael Hwang]

On Tue, Jan 5, 2010 at 12:20 PM, Peter Stephenson <pws@csr.com> wrote:
> With the two patches I know about (the second a single line), the test
> suite now fails at this test in D06subscript.ztst:

On Sat, Dec 26, 2009 at 5:48 PM, Michael Hwang
<michael.a.hwang@gmail.com> wrote:
> On the more buggy-ish side:
>
> % STRING='zsh is awesome!'
> % STR='zsh sucks... :-/'
> % REF=STRING
> % print ${(P)REF[1,3]}
> zsh sucks... :-/
>
> As ${(P)REF} is really ${(P)${REF}}, one would think that that
> ${(P)REF[1,3]} would be expanded as ${(P)${REF}[1,3]}. But as you can
> see, it is instead expanded as ${(P)${REF[1,3]}}.

I believe that the behavior that test relies on should be considered a
bug. What do you think?

Michael Hwang

Re: [PATCH]: Revamped (P) expansion flag

[Peter Stephenson]

On Tue, 5 Jan 2010 17:52:47 -0500
Michael Hwang <michael.a.hwang@gmail.com> wrote:
> On Tue, Jan 5, 2010 at 12:20 PM, Peter Stephenson <pws@csr.com> wrote:
> > With the two patches I know about (the second a single line), the test
> > suite now fails at this test in D06subscript.ztst:
> 
> On Sat, Dec 26, 2009 at 5:48 PM, Michael Hwang
> <michael.a.hwang@gmail.com> wrote:
> > On the more buggy-ish side:
> >
> > % STRING='zsh is awesome!'
> > % STR='zsh sucks... :-/'
> > % REF=STRING
> > % print ${(P)REF[1,3]}
> > zsh sucks... :-/
> >
> > As ${(P)REF} is really ${(P)${REF}}, one would think that that
> > ${(P)REF[1,3]} would be expanded as ${(P)${REF}[1,3]}. But as you can
> > see, it is instead expanded as ${(P)${REF[1,3]}}.
> 
> I believe that the behavior that test relies on should be considered a
> bug. What do you think?

The test isn't very logical without the $.  If I put them into the test,

  qqq=lower
  QQQ=upper
  print ${(P)${A[one\"two\"three\"quotes]}}
  print "${(P)${A[$A[(i)one\"two\"three\"quotes]]}}"
0:Keys with double quotes and the (P) expansion flag
>lower
>upper

it still fails, however:  the first output is now "lower" as before,
but the second output is "lower" too whereas before (even in this form of
the test) it was "upper".  Something has changed in the way quoting is
done.  There may be a logical explanation for this.

-- 
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]: Revamped (P) expansion flag

[Bart Schaefer]

On Jan 5,  5:52pm, Michael Hwang wrote:
} Subject: Re: [PATCH]: Revamped (P) expansion flag
}
} > As ${(P)REF} is really ${(P)${REF}}, one would think that that
} > ${(P)REF[1,3]} would be expanded as ${(P)${REF}[1,3]}. But as you can
} > see, it is instead expanded as ${(P)${REF[1,3]}}.
} 
} I believe that the behavior that test relies on should be considered a
} bug. What do you think?

I think you haven't read manual section 14.3.2 (subsection "Rules" under
"Parameter Expansion").  The only value manipulations that are applied
before the subscripts are computed are those that are properties of the
parameter itself, such as internal padding and capitalization.

To change this for any one expansion-time flag such as (P) would be a
serious departure from long-standing semantics.  It's not a bug.

Re: [PATCH]: Revamped (P) expansion flag

[Michael Hwang]

I made a mistake with the last patch. This remedies that problem.

I'm using git send-email, so the patch should finally show up with the email.
But as always, the changes can be pulled from the pflag branch of my GitHub
repo (git://github.com/Nomexous/zsh.git).

Michael Hwang