* Inconsistency with SHWORDSPLIT and leading spaces
@ 2015-11-06 10:54 Christian Neukirchen
2015-11-06 17:00 ` Peter Stephenson
0 siblings, 1 reply; 18+ messages in thread
From: Christian Neukirchen @ 2015-11-06 10:54 UTC (permalink / raw)
To: zsh-workers
Hi,
I noticed the following different behavior trying to use zsh as /bin/sh:
juno ~% dash -c 'x=foo; echo x${x:+ $x}'
x foo
juno ~% bash -c 'x=foo; echo x${x:+ $x}'
x foo
juno ~% mksh -c 'x=foo; echo x${x:+ $x}'
x foo
juno ~% ksh -c 'x=foo; echo x${x:+ $x}'
x foo
juno ~% zsh -c 'x=foo; echo x${x:+ $x}'
x foo
juno ~% zsh -c 'emulate -L sh; x=foo; echo x${x:+ $x}'
xfoo
juno ~% zsh -c 'emulate -L ksh; x=foo; echo x${x:+ $x}'
xfoo
juno ~% zsh -c 'setopt shwordsplit; x=foo; echo x${x:+ $x}'
xfoo
zsh 5.1.1 (x86_64-unknown-linux-gnu)
zsh-5.1.1-0-g68405f3
It's not clear to me whether this is *actually* POSIX-nonconformant, but
it's at least surprising compared to the other POSIX-compatible shells.
Thanks,
--
Christian Neukirchen <chneukirchen@gmail.com> http://chneukirchen.org
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: Inconsistency with SHWORDSPLIT and leading spaces 2015-11-06 10:54 Inconsistency with SHWORDSPLIT and leading spaces Christian Neukirchen @ 2015-11-06 17:00 ` Peter Stephenson 2015-11-07 17:42 ` Peter Stephenson 0 siblings, 1 reply; 18+ messages in thread From: Peter Stephenson @ 2015-11-06 17:00 UTC (permalink / raw) To: zsh-workers On Fri, 6 Nov 2015 11:54:49 +0100 Christian Neukirchen <chneukirchen@gmail.com> wrote: > juno ~% zsh -c 'setopt shwordsplit; x=foo; echo x${x:+ $x}' > xfoo This is unlikely to be deliberate --- we should do *something* with the space, not pretend it's not there. This makes already insane code even worse, but it doesn't seem to have broken anything in the tests. Possible breakage is I haven't spotted some case where we use multsub() with word splitting but we need to reset the knock-on splitting (ws_at_start) for some reason similar to transforming the result into a length (though I've picked that particular one up). pws diff --git a/Src/subst.c b/Src/subst.c index 021d234..5289121 100644 --- a/Src/subst.c +++ b/Src/subst.c @@ -430,11 +430,16 @@ singsub(char **s) * set to 1. Otherwise, *isarr is set to 0, and the result is put into *s, * with any necessary joining of multiple elements using sep (which can be * NULL to use IFS). The return value is true iff the expansion resulted - * in an empty list. */ + * in an empty list. + * + * *ws_at_start is set to 1 if the string had whitespace at thes start + * that should cause word splitting against any preceeding string. + */ /**/ static int -multsub(char **s, int pf_flags, char ***a, int *isarr, char *sep) +multsub(char **s, int pf_flags, char ***a, int *isarr, char *sep, + int *ws_at_start) { int l; char **r, **p, *x = *s; @@ -450,6 +455,7 @@ multsub(char **s, int pf_flags, char ***a, int *isarr, char *sep) l++; if (!iwsep(STOUC(c))) break; + *ws_at_start = 1; } } @@ -1717,6 +1723,14 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) * This is for compatibility. */ int horrible_offset_hack = 0; + /* + * Signal back from multsub: with something like + * x${:- $foo} + * with word-splitting active we need to split on that leading + * whitespace. However, if there's no "x" the whitespace is + * simply removed. + */ + int ws_at_start = 0; *s++ = '\0'; /* @@ -2265,7 +2279,8 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) * remove the aspar test and extract a value from an array, if * necessary, when we handle (P) lower down. */ - if (multsub(&val, 0, (aspar ? NULL : &aval), &isarr, NULL) && quoted) { + if (multsub(&val, 0, (aspar ? NULL : &aval), &isarr, NULL, + &ws_at_start) && quoted) { /* Empty quoted string --- treat as null string, not elided */ isarr = -1; aval = (char **) hcalloc(sizeof(char *)); @@ -2736,7 +2751,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) split_flags = PREFORK_NOSHWORDSPLIT; } multsub(&val, split_flags, (aspar ? NULL : &aval), - &isarr, NULL); + &isarr, NULL, &ws_at_start); copied = 1; spbreak = 0; /* Leave globsubst on if forced */ @@ -2765,13 +2780,14 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) * behavior on caller choice of PREFORK_SHWORDSPLIT. */ multsub(&val, spbreak ? PREFORK_SINGLE : PREFORK_NOSHWORDSPLIT, - NULL, &isarr, NULL); + NULL, &isarr, NULL, &ws_at_start); } else { if (spbreak) split_flags = PREFORK_SPLIT|PREFORK_SHWORDSPLIT; else split_flags = PREFORK_NOSHWORDSPLIT; - multsub(&val, split_flags, &aval, &isarr, NULL); + multsub(&val, split_flags, &aval, &isarr, NULL, + &ws_at_start); spbreak = 0; } if (arrasg) { @@ -3303,6 +3319,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) } if (haserr || errflag) return NULL; + ws_at_start = 0; } /* * This handles taking a length with ${#foo} and variations. @@ -3341,6 +3358,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) sprintf(buf, "%ld", len); val = dupstring(buf); isarr = 0; + ws_at_start = 0; } /* At this point we make sure that our arrayness has affected the * arrayness of the linked list. Then, we can turn our value into @@ -3370,6 +3388,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) if (isarr) { val = sepjoin(aval, sep, 1); isarr = 0; + ws_at_start = 0; } if (!ssub && (spbreak || spsep)) { aval = sepsplit(val, spsep, 0, 1); @@ -3649,6 +3668,15 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) * equivalent and only locally decide if we need to treat it * as a scalar.) */ + + /* + * If a multsub result had whitespace at the start and we're + * splitting and there's a previous string, now's the time to do so. + */ + if (ws_at_start && aptr > ostr) { + insertlinknode(l, n, dupstrpfx(ostr, aptr - ostr)), incnode(n); + ostr = aptr; + } if (isarr) { char *x; char *y; diff --git a/Test/D04parameter.ztst b/Test/D04parameter.ztst index cb7079c..59c14a4 100644 --- a/Test/D04parameter.ztst +++ b/Test/D04parameter.ztst @@ -1744,3 +1744,57 @@ >3_4_5_6 >6 >1_2_3_4_5_6 + + (setopt shwordsplit + do_test() { + print $#: "$@" + } + foo=bar + foo2="bar bar" + do_test ${:- foo} + do_test ${:- foo bar} + do_test ${:- $foo} + do_test ${:- $foo2} + do_test x${:- foo} + do_test x${:- foo bar} + do_test x${:- $foo} + do_test x${:- $foo2} + do_test x${foo:+ $foo} + ) +0:We Love SH_WORD_SPLIT Day celebrated with space at start of internal subst +>1: foo +>2: foo bar +>1: bar +>2: bar bar +>2: x foo +>3: x foo bar +>2: x bar +>3: x bar bar +>2: x bar + + (unsetopt shwordsplit # default, for clarity + do_test() { + print $#: "$@" + } + foo=bar + foo2="bar bar" + do_test ${:- foo} + do_test ${:- foo bar} + do_test ${:- $foo} + do_test ${:- $foo2} + do_test x${:- foo} + do_test x${:- foo bar} + do_test x${:- $foo} + do_test x${:- $foo2} + do_test x${foo:+ $foo} + ) +0:We Love NO_SH_WORD_SPLIT Even More Day celebrated as sanity check +>1: foo +>1: foo bar +>1: bar +>1: bar bar +>1: x foo +>1: x foo bar +>1: x bar +>1: x bar bar +>1: x bar ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Inconsistency with SHWORDSPLIT and leading spaces 2015-11-06 17:00 ` Peter Stephenson @ 2015-11-07 17:42 ` Peter Stephenson 2015-11-07 19:43 ` Bart Schaefer 0 siblings, 1 reply; 18+ messages in thread From: Peter Stephenson @ 2015-11-07 17:42 UTC (permalink / raw) To: Peter Stephenson; +Cc: zsh-workers OK, I know what you're all thinking now. "What about whitespace at the end of an internal parameter argument subject to word-splitting? Doesn't that need handling, too? Isn't this, in fact, the most burning question of our day?" Grmph. This is essentially equivalent. I haven't tested RC_EXPAND_PARAM (see under "soul-destroying") but the following is generic enough it ought to be OK. pws diff --git a/Src/subst.c b/Src/subst.c index 5289121..4d04fa7 100644 --- a/Src/subst.c +++ b/Src/subst.c @@ -420,6 +420,22 @@ singsub(char **s) DPUTS(nonempty(&foo), "BUG: singsub() produced more than one word!"); } +/* + * Bit flags passed back from multsub() to paramsubst(). + */ +enum { + /* + * Set if the string had whitespace at the start + * that should cause word splitting against any preceeding string. + */ + WS_AT_START = 1, + /* + * Set if the string had whitespace at the end + * that should cause word splitting against any following string. + */ + WS_AT_END = 2 +}; + /* Perform substitution on a single word, *s. Unlike with singsub(), the * result can be more than one word. If split is non-zero, the string is * first word-split using IFS, but only for non-quoted "whitespace" (as @@ -432,14 +448,13 @@ singsub(char **s) * NULL to use IFS). The return value is true iff the expansion resulted * in an empty list. * - * *ws_at_start is set to 1 if the string had whitespace at thes start - * that should cause word splitting against any preceeding string. + * *ws_at_start is set to bits in the enum above as neeed. */ /**/ static int multsub(char **s, int pf_flags, char ***a, int *isarr, char *sep, - int *ws_at_start) + int *ws_sub) { int l; char **r, **p, *x = *s; @@ -455,7 +470,7 @@ multsub(char **s, int pf_flags, char ***a, int *isarr, char *sep, l++; if (!iwsep(STOUC(c))) break; - *ws_at_start = 1; + *ws_sub |= WS_AT_START; } } @@ -487,8 +502,10 @@ multsub(char **s, int pf_flags, char ***a, int *isarr, char *sep, if (!WC_ZISTYPE(c, ISEP)) break; } - if (!*x) + if (!*x) { + *ws_sub |= WS_AT_END; break; + } insertlinknode(&foo, n, (void *)x), incnode(n); } } @@ -1730,7 +1747,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) * whitespace. However, if there's no "x" the whitespace is * simply removed. */ - int ws_at_start = 0; + int ws_sub = 0; *s++ = '\0'; /* @@ -2280,7 +2297,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) * necessary, when we handle (P) lower down. */ if (multsub(&val, 0, (aspar ? NULL : &aval), &isarr, NULL, - &ws_at_start) && quoted) { + &ws_sub) && quoted) { /* Empty quoted string --- treat as null string, not elided */ isarr = -1; aval = (char **) hcalloc(sizeof(char *)); @@ -2751,7 +2768,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) split_flags = PREFORK_NOSHWORDSPLIT; } multsub(&val, split_flags, (aspar ? NULL : &aval), - &isarr, NULL, &ws_at_start); + &isarr, NULL, &ws_sub); copied = 1; spbreak = 0; /* Leave globsubst on if forced */ @@ -2780,14 +2797,14 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) * behavior on caller choice of PREFORK_SHWORDSPLIT. */ multsub(&val, spbreak ? PREFORK_SINGLE : PREFORK_NOSHWORDSPLIT, - NULL, &isarr, NULL, &ws_at_start); + NULL, &isarr, NULL, &ws_sub); } else { if (spbreak) split_flags = PREFORK_SPLIT|PREFORK_SHWORDSPLIT; else split_flags = PREFORK_NOSHWORDSPLIT; multsub(&val, split_flags, &aval, &isarr, NULL, - &ws_at_start); + &ws_sub); spbreak = 0; } if (arrasg) { @@ -3319,7 +3336,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) } if (haserr || errflag) return NULL; - ws_at_start = 0; + ws_sub = 0; } /* * This handles taking a length with ${#foo} and variations. @@ -3358,7 +3375,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) sprintf(buf, "%ld", len); val = dupstring(buf); isarr = 0; - ws_at_start = 0; + ws_sub = 0; } /* At this point we make sure that our arrayness has affected the * arrayness of the linked list. Then, we can turn our value into @@ -3388,7 +3405,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) if (isarr) { val = sepjoin(aval, sep, 1); isarr = 0; - ws_at_start = 0; + ws_sub = 0; } if (!ssub && (spbreak || spsep)) { aval = sepsplit(val, spsep, 0, 1); @@ -3673,10 +3690,14 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) * If a multsub result had whitespace at the start and we're * splitting and there's a previous string, now's the time to do so. */ - if (ws_at_start && aptr > ostr) { + if ((ws_sub & WS_AT_START) && aptr > ostr) { insertlinknode(l, n, dupstrpfx(ostr, aptr - ostr)), incnode(n); ostr = aptr; } + if ((ws_sub & WS_AT_END) && *fstr) { + insertlinknode(l, n, dupstring(fstr)); /* appended, no incnode */ + *fstr = '\0'; + } if (isarr) { char *x; char *y; diff --git a/Test/D04parameter.ztst b/Test/D04parameter.ztst index 59c14a4..694b613 100644 --- a/Test/D04parameter.ztst +++ b/Test/D04parameter.ztst @@ -1751,26 +1751,26 @@ } foo=bar foo2="bar bar" - do_test ${:- foo} - do_test ${:- foo bar} - do_test ${:- $foo} - do_test ${:- $foo2} - do_test x${:- foo} - do_test x${:- foo bar} - do_test x${:- $foo} - do_test x${:- $foo2} - do_test x${foo:+ $foo} + do_test ${:- foo } + do_test ${:- foo bar } + do_test ${:- $foo } + do_test ${:- $foo2 } + do_test x${:- foo }y + do_test x${:- foo bar }y + do_test x${:- $foo }y + do_test x${:- $foo2 }y + do_test x${foo:+ $foo }y ) 0:We Love SH_WORD_SPLIT Day celebrated with space at start of internal subst >1: foo >2: foo bar >1: bar >2: bar bar ->2: x foo ->3: x foo bar ->2: x bar ->3: x bar bar ->2: x bar +>3: x foo y +>4: x foo bar y +>3: x bar y +>4: x bar bar y +>3: x bar y (unsetopt shwordsplit # default, for clarity do_test() { @@ -1778,23 +1778,23 @@ } foo=bar foo2="bar bar" - do_test ${:- foo} - do_test ${:- foo bar} - do_test ${:- $foo} - do_test ${:- $foo2} - do_test x${:- foo} - do_test x${:- foo bar} - do_test x${:- $foo} - do_test x${:- $foo2} - do_test x${foo:+ $foo} + do_test ${:- foo } + do_test ${:- foo bar } + do_test ${:- $foo } + do_test ${:- $foo2 } + do_test x${:- foo }y + do_test x${:- foo bar }y + do_test x${:- $foo }y + do_test x${:- $foo2 }y + do_test x${foo:+ $foo }y ) 0:We Love NO_SH_WORD_SPLIT Even More Day celebrated as sanity check ->1: foo ->1: foo bar ->1: bar ->1: bar bar ->1: x foo ->1: x foo bar ->1: x bar ->1: x bar bar ->1: x bar +>1: foo +>1: foo bar +>1: bar +>1: bar bar +>1: x foo y +>1: x foo bar y +>1: x bar y +>1: x bar bar y +>1: x bar y ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Inconsistency with SHWORDSPLIT and leading spaces 2015-11-07 17:42 ` Peter Stephenson @ 2015-11-07 19:43 ` Bart Schaefer 2015-11-08 18:18 ` Peter Stephenson 0 siblings, 1 reply; 18+ messages in thread From: Bart Schaefer @ 2015-11-07 19:43 UTC (permalink / raw) To: zsh-workers On Nov 7, 5:42pm, Peter Stephenson wrote: } } +/* } + * Bit flags passed back from multsub() to paramsubst(). } + */ Maybe now that we're doing this, we could have a bit flag passed in the other (and back?) direction for whether the (P) flag is in effect, and thereby fix the ${${(P)hash}[index]} thing? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Inconsistency with SHWORDSPLIT and leading spaces 2015-11-07 19:43 ` Bart Schaefer @ 2015-11-08 18:18 ` Peter Stephenson 2015-11-08 18:55 ` Bart Schaefer 2015-11-11 17:49 ` PATCH: nested ${(P)} (formerly SHWORDSPLIT and leading spaces) Peter Stephenson 0 siblings, 2 replies; 18+ messages in thread From: Peter Stephenson @ 2015-11-08 18:18 UTC (permalink / raw) To: zsh-workers On Sat, 7 Nov 2015 11:43:14 -0800 Bart Schaefer <schaefer@brasslantern.com> wrote: > On Nov 7, 5:42pm, Peter Stephenson wrote: > } > } +/* > } + * Bit flags passed back from multsub() to paramsubst(). > } + */ > > Maybe now that we're doing this, we could have a bit flag passed in the > other (and back?) direction for whether the (P) flag is in effect, and > thereby fix the ${${(P)hash}[index]} thing? It's a bit more complicated than that, though maybe doable. If: - we spot on the way down this is a multsub when we reached the nested paramsubst() (we could do that with a pf_flags bit) - we return a name of a parameter and a flag saying what we're doing on noticing this - we pass this up to the to paramsubst() regardless of what else is going on in the prefork() and the multsub() --- this is the bit I a little scared of, but (P) is so explicit in what it's doing maybe that's OK - in the upper paramsubst we use the flag to retrieve a Value based on the return, i.e. it now really is just a name for use up above as you might have been entitled to think it always was --- this is surely doable but I bet it's messier than you'd expect does that do it? pws ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Inconsistency with SHWORDSPLIT and leading spaces 2015-11-08 18:18 ` Peter Stephenson @ 2015-11-08 18:55 ` Bart Schaefer 2015-11-09 6:03 ` Bart Schaefer 2015-11-11 17:49 ` PATCH: nested ${(P)} (formerly SHWORDSPLIT and leading spaces) Peter Stephenson 1 sibling, 1 reply; 18+ messages in thread From: Bart Schaefer @ 2015-11-08 18:55 UTC (permalink / raw) To: zsh-workers On Nov 8, 6:18pm, Peter Stephenson wrote: } } does that do it? Yes, that's a more fully enumerated description of the vague notion I had when I made the suggestion. The next step would be to add a nameref parameter flag for declaring via typedef that "(P) is implied whenever this parameter is used" ... ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Inconsistency with SHWORDSPLIT and leading spaces 2015-11-08 18:55 ` Bart Schaefer @ 2015-11-09 6:03 ` Bart Schaefer 0 siblings, 0 replies; 18+ messages in thread From: Bart Schaefer @ 2015-11-09 6:03 UTC (permalink / raw) To: zsh-workers On Nov 8, 10:55am, Bart Schaefer wrote: } } The next step would be to add a nameref parameter flag for declaring } via typedef that "(P) is implied whenever this parameter is used" ... It occurs to me that this would have to keep track of the locallevel at which the nameref was created, so as to link to the parameter at the correct scope. ^ permalink raw reply [flat|nested] 18+ messages in thread
* PATCH: nested ${(P)} (formerly SHWORDSPLIT and leading spaces) 2015-11-08 18:18 ` Peter Stephenson 2015-11-08 18:55 ` Bart Schaefer @ 2015-11-11 17:49 ` Peter Stephenson 2015-11-11 18:13 ` Bart Schaefer 2015-11-11 21:55 ` Peter Stephenson 1 sibling, 2 replies; 18+ messages in thread From: Peter Stephenson @ 2015-11-11 17:49 UTC (permalink / raw) To: zsh-workers On Sun, 8 Nov 2015 18:18:33 +0000 Peter Stephenson <p.w.stephenson@ntlworld.com> wrote: > - we spot on the way down this is a multsub when we reached the nested > paramsubst() (we could do that with a pf_flags bit) > > - we return a name of a parameter and a flag saying what we're doing on > noticing this > > - we pass this up to the to paramsubst() regardless of what else is going > on in the prefork() and the multsub() --- this is the bit I a little scared > of, but (P) is so explicit in what it's doing maybe that's OK > > - in the upper paramsubst we use the flag to retrieve a Value based on > the return, i.e. it now really is just a name for use up above as you > might have been entitled to think it always was --- this is surely doable > but I bet it's messier than you'd expect This isn't obviously broken, yet. You can explore while I think about tests. To get this work, I simply welded the parameter name from downbelow together with the remainder of the expression in the outer substitution. It seems pretty much guaranteed to work, though it also seems somehow unsatisfying. In theory this should clearly separate transformations that happen on the name you're going to pass up, and those on the result after name lookup. Hmmm... should I sanity check that the the substituted name is an identifier, or is it a feature that % array=(one two three) % word='array[2]' % print ${${(P)word}[2]} w works? If the latter, is there any sanity checking I can do? pws diff --git a/Doc/Zsh/expn.yo b/Doc/Zsh/expn.yo index 20e0c8d..4c373d1 100644 --- a/Doc/Zsh/expn.yo +++ b/Doc/Zsh/expn.yo @@ -1033,7 +1033,16 @@ var(name) used in this fashion. If used with a nested parameter or command substitution, the result of that will be taken as a parameter name in the same way. For example, if you have `tt(foo=bar)' and `tt(bar=baz)', the strings tt(${(P)foo}), -tt(${(P)${foo}}), and tt(${(P)$(echo bar)}) will be expanded to `tt(baz)'. +tt(${(P)${foo}}), and tt(${(P)$(echo bar)}) will be expanded to +`tt(baz)'. + +Likewise, if the reference is itself nested, the expression with the +flag is treated as if it were directly replaced by the parameter name. +It is an error if this nested substitution produces an array with more +than one word. For example, if `tt(name=assoc)' where the parameter +tt(assoc) is an associative array, then +`tt(${${(P)name}[elt]})' refers to the element of the associative +subscripted `tt(elt)'. ) item(tt(q))( Quote characters that are special to the shell in the resulting words with diff --git a/Src/Zle/compctl.c b/Src/Zle/compctl.c index bac533e..8381867 100644 --- a/Src/Zle/compctl.c +++ b/Src/Zle/compctl.c @@ -2116,7 +2116,7 @@ getreal(char *str) noerrs = 1; addlinknode(l, dupstring(str)); - prefork(l, 0); + prefork(l, 0, NULL); noerrs = ne; if (!errflag && nonempty(l) && ((char *) peekfirst(l)) && ((char *) peekfirst(l))[0]) @@ -3728,7 +3728,7 @@ makecomplistflags(Compctl cc, char *s, int incmd, int compadd) errflag &= ~ERRFLAG_ERROR; zcontext_restore(); /* Fine, now do full expansion. */ - prefork(foo, 0); + prefork(foo, 0, NULL); if (!errflag) { globlist(foo, 0); if (!errflag) diff --git a/Src/Zle/zle_tricky.c b/Src/Zle/zle_tricky.c index e26f663..4e68549 100644 --- a/Src/Zle/zle_tricky.c +++ b/Src/Zle/zle_tricky.c @@ -2223,7 +2223,7 @@ doexpansion(char *s, int lst, int olst, int explincmd) else if (*ts == '\'') *ts = Snull; addlinknode(vl, ss); - prefork(vl, 0); + prefork(vl, 0, NULL); if (errflag) goto end; if (lst == COMP_LIST_EXPAND || lst == COMP_EXPAND) { diff --git a/Src/cond.c b/Src/cond.c index df90656..c5ab65e 100644 --- a/Src/cond.c +++ b/Src/cond.c @@ -43,7 +43,7 @@ static void cond_subst(char **strp, int glob_ok) checkglobqual(*strp, strlen(*strp), 1, NULL)) { LinkList args = newlinklist(); addlinknode(args, *strp); - prefork(args, 0); + prefork(args, 0, NULL); while (!errflag && args && nonempty(args) && has_token((char *)peekfirst(args))) zglob(args, firstnode(args), 0); diff --git a/Src/exec.c b/Src/exec.c index f0d1d2f..c0ee527 100644 --- a/Src/exec.c +++ b/Src/exec.c @@ -2290,7 +2290,7 @@ addvars(Estate state, Wordcode pc, int addflags) if (vl && htok) { prefork(vl, (isstr ? (PREFORK_SINGLE|PREFORK_ASSIGN) : - PREFORK_ASSIGN)); + PREFORK_ASSIGN), NULL); if (errflag) { state->pc = opc; return; @@ -2416,7 +2416,7 @@ void execsubst(LinkList strs) { if (strs) { - prefork(strs, esprefork); + prefork(strs, esprefork, NULL); if (esglob && !errflag) { LinkList ostrs = strs; globlist(strs, 0); @@ -2721,7 +2721,7 @@ execcmd(Estate state, int input, int output, int how, int last1) /* Do prefork substitutions */ esprefork = (assign || isset(MAGICEQUALSUBST)) ? PREFORK_TYPESET : 0; if (args && htok) - prefork(args, esprefork); + prefork(args, esprefork, NULL); if (type == WC_SIMPLE || type == WC_TYPESET) { int unglobbed = 0; @@ -3558,7 +3558,7 @@ execcmd(Estate state, int input, int output, int how, int last1) */ /* Unused dummy value for name */ (void)ecgetstr(state, EC_DUPTOK, &htok); - prefork(&svl, PREFORK_TYPESET); + prefork(&svl, PREFORK_TYPESET, NULL); if (errflag) { state->pc = opc; break; @@ -3584,7 +3584,7 @@ execcmd(Estate state, int input, int output, int how, int last1) } continue; } - prefork(&svl, PREFORK_SINGLE); + prefork(&svl, PREFORK_SINGLE, NULL); name = empty(&svl) ? "" : (char *)getdata(firstnode(&svl)); } @@ -3600,7 +3600,9 @@ execcmd(Estate state, int input, int output, int how, int last1) } else { if (htok) { init_list1(svl, val); - prefork(&svl, PREFORK_SINGLE|PREFORK_ASSIGN); + prefork(&svl, + PREFORK_SINGLE|PREFORK_ASSIGN, + NULL); if (errflag) { state->pc = opc; break; @@ -3622,7 +3624,7 @@ execcmd(Estate state, int input, int output, int how, int last1) EC_DUPTOK, &htok); if (asg->value.array) { - prefork(asg->value.array, PREFORK_ASSIGN); + prefork(asg->value.array, PREFORK_ASSIGN, NULL); if (errflag) { state->pc = opc; break; diff --git a/Src/glob.c b/Src/glob.c index 51ffeb5..94b3f62 100644 --- a/Src/glob.c +++ b/Src/glob.c @@ -2093,7 +2093,7 @@ xpandredir(struct redir *fn, LinkList redirtab) /* Stick the name in a list... */ init_list1(fake, fn->name); /* ...which undergoes all the usual shell expansions */ - prefork(&fake, isset(MULTIOS) ? 0 : PREFORK_SINGLE); + prefork(&fake, isset(MULTIOS) ? 0 : PREFORK_SINGLE, NULL); /* Globbing is only done for multios. */ if (!errflag && isset(MULTIOS)) globlist(&fake, 0); diff --git a/Src/subst.c b/Src/subst.c index febdc9b..f3a4ad4 100644 --- a/Src/subst.c +++ b/Src/subst.c @@ -44,15 +44,23 @@ char nulstring[] = {Nularg, '\0'}; * - Brace expansion * - Tilde and equals substitution * - * PREFORK_* flags are defined in zsh.h + * "flag"s contains PREFORK_* flags, defined in zsh.h. + * + * "ret_flags" is used to return values from nested parameter + * substitions. It may be NULL in which case PREFORK_SUBEXP + * must not appear in flags; any return value from below + * will be discarded. */ /**/ mod_export void -prefork(LinkList list, int flags) +prefork(LinkList list, int flags, int *ret_flags) { LinkNode node, stop = 0; int keep = 0, asssub = (flags & PREFORK_TYPESET) && isset(KSHTYPESET); + int ret_flags_local = 0; + if (!ret_flags) + ret_flags = &ret_flags_local; /* will be discarded */ queue_signals(); for (node = firstnode(list); node; incnode(node)) { @@ -75,10 +83,8 @@ prefork(LinkList list, int flags) setdata(node, cptr); } if (!(node = stringsubst(list, node, - flags & (PREFORK_SINGLE|PREFORK_SPLIT| - PREFORK_SHWORDSPLIT| - PREFORK_NOSHWORDSPLIT), - asssub))) { + flags & ~(PREFORK_TYPESET|PREFORK_ASSIGN), + ret_flags, asssub))) { unqueue_signals(); return; } @@ -149,7 +155,8 @@ stringsubstquote(char *strstart, char **pstrdpos) /**/ static LinkNode -stringsubst(LinkList list, LinkNode node, int pf_flags, int asssub) +stringsubst(LinkList list, LinkNode node, int pf_flags, int *ret_flags, + int asssub) { int qt; char *str3 = (char *)getdata(node); @@ -235,7 +242,8 @@ stringsubst(LinkList list, LinkNode node, int pf_flags, int asssub) pf_flags |= PREFORK_SHWORDSPLIT; node = paramsubst( list, node, &str, qt, - pf_flags & (PREFORK_SINGLE|PREFORK_SHWORDSPLIT)); + pf_flags & (PREFORK_SINGLE|PREFORK_SHWORDSPLIT| + PREFORK_SUBEXP), ret_flags); if (errflag || !node) return NULL; str3 = (char *)getdata(node); @@ -413,29 +421,13 @@ singsub(char **s) init_list1(foo, *s); - prefork(&foo, PREFORK_SINGLE); + prefork(&foo, PREFORK_SINGLE, NULL); if (errflag) return; *s = (char *) ugetnode(&foo); DPUTS(nonempty(&foo), "BUG: singsub() produced more than one word!"); } -/* - * Bit flags passed back from multsub() to paramsubst(). - */ -enum { - /* - * Set if the string had whitespace at the start - * that should cause word splitting against any preceeding string. - */ - WS_AT_START = 1, - /* - * Set if the string had whitespace at the end - * that should cause word splitting against any following string. - */ - WS_AT_END = 2 -}; - /* Perform substitution on a single word, *s. Unlike with singsub(), the * result can be more than one word. If split is non-zero, the string is * first word-split using IFS, but only for non-quoted "whitespace" (as @@ -448,13 +440,13 @@ enum { * NULL to use IFS). The return value is true iff the expansion resulted * in an empty list. * - * *ws_at_start is set to bits in the enum above as neeed. + * *ms_flags is set to bits in the enum above as neeed. */ /**/ static int multsub(char **s, int pf_flags, char ***a, int *isarr, char *sep, - int *ws_sub) + int *ms_flags) { int l; char **r, **p, *x = *s; @@ -470,7 +462,7 @@ multsub(char **s, int pf_flags, char ***a, int *isarr, char *sep, l++; if (!iwsep(STOUC(c))) break; - *ws_sub |= WS_AT_START; + *ms_flags |= MULTSUB_WS_AT_START; } } @@ -503,7 +495,7 @@ multsub(char **s, int pf_flags, char ***a, int *isarr, char *sep, break; } if (!*x) { - *ws_sub |= WS_AT_END; + *ms_flags |= MULTSUB_WS_AT_END; break; } insertlinknode(&foo, n, (void *)x), incnode(n); @@ -532,7 +524,7 @@ multsub(char **s, int pf_flags, char ***a, int *isarr, char *sep, } } - prefork(&foo, pf_flags); + prefork(&foo, pf_flags, ms_flags); if (errflag) { if (isarr) *isarr = 0; @@ -1517,7 +1509,8 @@ check_colon_subscript(char *str, char **endp) /**/ static LinkNode -paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) +paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags, + int *ret_flags) { char *aptr = *str, c, cc; char *s = aptr, *fstr, *idbeg, *idend, *ostr = (char *) getdata(n); @@ -1747,7 +1740,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) * whitespace. However, if there's no "x" the whitespace is * simply removed. */ - int ws_sub = 0; + int ms_flags = 0; *s++ = '\0'; /* @@ -2296,8 +2289,8 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) * remove the aspar test and extract a value from an array, if * necessary, when we handle (P) lower down. */ - if (multsub(&val, 0, (aspar ? NULL : &aval), &isarr, NULL, - &ws_sub) && quoted) { + if (multsub(&val, PREFORK_SUBEXP, (aspar ? NULL : &aval), &isarr, NULL, + &ms_flags) && quoted) { /* Empty quoted string --- treat as null string, not elided */ isarr = -1; aval = (char **) hcalloc(sizeof(char *)); @@ -2311,6 +2304,28 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) */ while (inull(*s)) s++; + if (ms_flags & MULTSUB_PARAM_NAME) { + /* + * Downbelow has told us this is a parameter name, e.g. + * ${${(P)name}...}. We're going to behave as if + * we have exactly that name followed by the rest of + * the parameter for subscripting etc. + * + * See below for where we set the flag in the nested + * substitution. + */ + if (isarr) { + if (aval[1]) { + zerr("parameter name reference used with array"); + return NULL; + } + val = aval[0]; + isarr = 0; + } + s = dyncat(val, s); + /* Now behave po-faced as if it was always like that... */ + subexp = aspar = 0; + } v = (Value) NULL; } else if (aspar) { /* @@ -2328,13 +2343,24 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) } else vunset = 1; } + if (aspar && (pf_flags & PREFORK_SUBEXP)) { + /* + * This is the inner handling for the case referred to above + * where we have something like ${${(P)name}...}. + * + * Treat this as as a normal value here; all transformations on + * result are in outer instance. + */ + aspar = 0; + *ret_flags |= MULTSUB_PARAM_NAME; + } /* * We need to retrieve a value either if we haven't already * got it from a subexpression, or if the processing so * far has just yielded us a parameter name to be processed * with (P). */ - if (!subexp || aspar) { + else if (!subexp || aspar) { char *ov = val; /* @@ -2768,7 +2794,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) split_flags = PREFORK_NOSHWORDSPLIT; } multsub(&val, split_flags, (aspar ? NULL : &aval), - &isarr, NULL, &ws_sub); + &isarr, NULL, &ms_flags); copied = 1; spbreak = 0; /* Leave globsubst on if forced */ @@ -2797,14 +2823,14 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) * behavior on caller choice of PREFORK_SHWORDSPLIT. */ multsub(&val, spbreak ? PREFORK_SINGLE : PREFORK_NOSHWORDSPLIT, - NULL, &isarr, NULL, &ws_sub); + NULL, &isarr, NULL, &ms_flags); } else { if (spbreak) split_flags = PREFORK_SPLIT|PREFORK_SHWORDSPLIT; else split_flags = PREFORK_NOSHWORDSPLIT; multsub(&val, split_flags, &aval, &isarr, NULL, - &ws_sub); + &ms_flags); spbreak = 0; } if (arrasg) { @@ -3336,7 +3362,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) } if (haserr || errflag) return NULL; - ws_sub = 0; + ms_flags = 0; } /* * This handles taking a length with ${#foo} and variations. @@ -3375,7 +3401,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) sprintf(buf, "%ld", len); val = dupstring(buf); isarr = 0; - ws_sub = 0; + ms_flags = 0; } /* At this point we make sure that our arrayness has affected the * arrayness of the linked list. Then, we can turn our value into @@ -3405,7 +3431,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) if (isarr) { val = sepjoin(aval, sep, 1); isarr = 0; - ws_sub = 0; + ms_flags = 0; } if (!ssub && (spbreak || spsep)) { aval = sepsplit(val, spsep, 0, 1); @@ -3690,12 +3716,12 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) * If a multsub result had whitespace at the start and we're * splitting and there's a previous string, now's the time to do so. */ - if ((ws_sub & WS_AT_START) && aptr > ostr) { + if ((ms_flags & MULTSUB_WS_AT_START) && aptr > ostr) { insertlinknode(l, n, dupstrpfx(ostr, aptr - ostr)), incnode(n); ostr = aptr; } /* Likewise at the end */ - if ((ws_sub & WS_AT_END) && *fstr) { + if ((ms_flags & MULTSUB_WS_AT_END) && *fstr) { insertlinknode(l, n, dupstring(fstr)); /* appended, no incnode */ *fstr = '\0'; } @@ -3777,7 +3803,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags) *--fstr = Marker; init_list1(tl, fstr); - if (!eval && !stringsubst(&tl, firstnode(&tl), ssub, 0)) + if (!eval && !stringsubst(&tl, firstnode(&tl), ssub, ret_flags, 0)) return NULL; *str = aptr; tn = firstnode(&tl); diff --git a/Src/zsh.h b/Src/zsh.h index a6f0397..d3bfcef 100644 --- a/Src/zsh.h +++ b/Src/zsh.h @@ -1866,18 +1866,45 @@ enum { }; /* Flags as the second argument to prefork */ -/* argument handled like typeset foo=bar */ -#define PREFORK_TYPESET 0x01 -/* argument handled like the RHS of foo=bar */ -#define PREFORK_ASSIGN 0x02 -/* single word substitution */ -#define PREFORK_SINGLE 0x04 -/* explicitly split nested substitution */ -#define PREFORK_SPLIT 0x08 -/* SHWORDSPLIT in parameter expn */ -#define PREFORK_SHWORDSPLIT 0x10 -/* SHWORDSPLIT forced off in nested subst */ -#define PREFORK_NOSHWORDSPLIT 0x20 +enum { + /* argument handled like typeset foo=bar */ + PREFORK_TYPESET = 0x01, + /* argument handled like the RHS of foo=bar */ + PREFORK_ASSIGN = 0x02, + /* single word substitution */ + PREFORK_SINGLE = 0x04, + /* explicitly split nested substitution */ + PREFORK_SPLIT = 0x08, + /* SHWORDSPLIT in parameter expn */ + PREFORK_SHWORDSPLIT = 0x10, + /* SHWORDSPLIT forced off in nested subst */ + PREFORK_NOSHWORDSPLIT = 0x20, + /* Prefork is part of a parameter subexpression */ + PREFORK_SUBEXP = 0x40 +}; + +/* + * Bit flags passed back from multsub() to paramsubst(). + * Some flags go from a nested parmsubst() through the enclosing + * stringsubst() and prefork(). + */ +enum { + /* + * Set if the string had whitespace at the start + * that should cause word splitting against any preceeding string. + */ + MULTSUB_WS_AT_START = 1, + /* + * Set if the string had whitespace at the end + * that should cause word splitting against any following string. + */ + MULTSUB_WS_AT_END = 2, + /* + * Set by nested paramsubst() to indicate the return + * value is a parameter name, rather than a value. + */ + MULTSUB_PARAM_NAME = 4 +}; /* * Structure for adding parameters in a module. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: PATCH: nested ${(P)} (formerly SHWORDSPLIT and leading spaces) 2015-11-11 17:49 ` PATCH: nested ${(P)} (formerly SHWORDSPLIT and leading spaces) Peter Stephenson @ 2015-11-11 18:13 ` Bart Schaefer 2015-11-11 21:55 ` Peter Stephenson 1 sibling, 0 replies; 18+ messages in thread From: Bart Schaefer @ 2015-11-11 18:13 UTC (permalink / raw) To: Peter Stephenson; +Cc: Zsh hackers list [-- Attachment #1: Type: text/plain, Size: 623 bytes --] On Nov 11, 2015 9:59 AM, "Peter Stephenson" <p.stephenson@samsung.com> wrote: > > This isn't obviously broken, yet. You can explore while I think about > tests. Nifty. > Hmmm... should I sanity check that the the substituted name is an > identifier, or is it a feature that > > % array=(one two three) > % word='array[2]' > % print ${${(P)word}[2]} > w > > works? I've used that trick before, though the context was with nested (P) not working. I guess I'd leave it as a feature ... essentially if the value would be legal as the left side of an assignment, it should be OK here. Check ${(P)thing::=newvalue} too ... ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: PATCH: nested ${(P)} (formerly SHWORDSPLIT and leading spaces) 2015-11-11 17:49 ` PATCH: nested ${(P)} (formerly SHWORDSPLIT and leading spaces) Peter Stephenson 2015-11-11 18:13 ` Bart Schaefer @ 2015-11-11 21:55 ` Peter Stephenson 2015-11-12 9:46 ` Peter Stephenson 1 sibling, 1 reply; 18+ messages in thread From: Peter Stephenson @ 2015-11-11 21:55 UTC (permalink / raw) To: Peter Stephenson; +Cc: zsh-workers On Wed, 11 Nov 2015 17:49:11 +0000 Peter Stephenson <p.stephenson@samsung.com> wrote: > tests. With a little more tweaking, nested references work naturally, if "naturally" is the word. See final test. pws diff --git a/Src/subst.c b/Src/subst.c index f3a4ad4..c1369b5 100644 --- a/Src/subst.c +++ b/Src/subst.c @@ -2315,7 +2315,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags, * substitution. */ if (isarr) { - if (aval[1]) { + if (aval[0] && aval[1]) { zerr("parameter name reference used with array"); return NULL; } @@ -2324,7 +2324,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags, } s = dyncat(val, s); /* Now behave po-faced as if it was always like that... */ - subexp = aspar = 0; + subexp = 0; } v = (Value) NULL; } else if (aspar) { @@ -2360,7 +2360,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags, * far has just yielded us a parameter name to be processed * with (P). */ - else if (!subexp || aspar) { + if (!subexp || aspar) { char *ov = val; /* diff --git a/Test/D04parameter.ztst b/Test/D04parameter.ztst index 694b613..6f325d2 100644 --- a/Test/D04parameter.ztst +++ b/Test/D04parameter.ztst @@ -1798,3 +1798,52 @@ >1: x bar y >1: x bar bar y >1: x bar y + + testfn() { + local scalar=obfuscation + local -a array=(alpha bravo charlie delta echo foxtrot) + local -A assoc=(one eins two zwei three drei four vier) + local name subscript + for name subscript in scalar 3 array 5 assoc three; do + print ${${(P)name}[$subscript]} + done + } + testfn +0:${(P)...} with normal subscripting +>f +>echo +>drei + + testfn() { + local s1=foo s2=bar + local -a val=(s1) + print ${${(P)val}[1,3]} + val=(s1 s2) + print ${${(P)val}[1,3]} + } + testfn +1:${(P)...} with array as name +>foo +?testfn:5: parameter name reference used with array + + testfn() { + local -A assoc=(one buckle two show three knock four door) + local name='assoc[two]' + print ${${(P)name}[2,3]} + } + testfn +0:${(P)...} with internal subscripting +>ho + + testfn() { + local one=two + local two=three + local three=four + local -a four=(all these worlds belong to foo) + print ${${(P)${(P)${(P)one}}}} + print ${${(P)${(P)${(P)one}}}[3]} + } + testfn +0:nested parameter name references +>all these worlds belong to foo +>worlds ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: PATCH: nested ${(P)} (formerly SHWORDSPLIT and leading spaces) 2015-11-11 21:55 ` Peter Stephenson @ 2015-11-12 9:46 ` Peter Stephenson 2015-11-12 14:19 ` Peter Stephenson 0 siblings, 1 reply; 18+ messages in thread From: Peter Stephenson @ 2015-11-12 9:46 UTC (permalink / raw) To: zsh-workers On Wed, 11 Nov 2015 21:55:41 +0000 Peter Stephenson <p.w.stephenson@ntlworld.com> wrote: > With a little more tweaking, nested references work naturally, if > "naturally" is the word. See final test. >... > + testfn() { > + local one=two > + local two=three > + local three=four > + local -a four=(all these worlds belong to foo) > + print ${${(P)${(P)${(P)one}}}} > + print ${${(P)${(P)${(P)one}}}[3]} > + } Well, one thing that isn't natural is that when you're relying on the new logic you need that extra ${...} wrapper even in the first of the two cases to trigger the recursive (P). Normally a surrounding level with no flags, modifications or subscripts has no effect. You don't need that for one (P) at the top level, ${(P)...}, because that's going a different path through the code. Either I should work out how to fix that or document the difference. I can't imagine anyone is going to lose sleep. pws ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: PATCH: nested ${(P)} (formerly SHWORDSPLIT and leading spaces) 2015-11-12 9:46 ` Peter Stephenson @ 2015-11-12 14:19 ` Peter Stephenson 2015-11-13 8:17 ` Jun T. 0 siblings, 1 reply; 18+ messages in thread From: Peter Stephenson @ 2015-11-12 14:19 UTC (permalink / raw) To: zsh-workers On Thu, 12 Nov 2015 09:46:28 +0000 Peter Stephenson <p.stephenson@samsung.com> wrote: > > + print ${${(P)${(P)${(P)one}}}} > > Well, one thing that isn't natural is that when you're relying on the > new logic you need that extra ${...} wrapper even in the first of the > two cases to trigger the recursive (P). Seems to work via the obvious fix. diff --git a/Src/subst.c b/Src/subst.c index c1369b5..b7f8338 100644 --- a/Src/subst.c +++ b/Src/subst.c @@ -1741,6 +1741,12 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags, * simply removed. */ int ms_flags = 0; + /* + * We need to do an extra fetch to honour the (P) flag. + * Complicated by the use of subexpressions that may have + * nested (P) flags. + */ + int fetch_needed; *s++ = '\0'; /* @@ -2325,9 +2331,18 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags, s = dyncat(val, s); /* Now behave po-faced as if it was always like that... */ subexp = 0; - } + /* + * If this is a (P) (first test) and at the top level + * (second test) we can't rely on the caller fetching + * the result from the pending aspar. So do it below. + */ + fetch_needed = aspar && !(pf_flags & PREFORK_SUBEXP); + } else + fetch_needed = 0; /* any initial aspar fetch already done */ v = (Value) NULL; - } else if (aspar) { + } else + fetch_needed = aspar; /* aspar fetch still needed */ + if (fetch_needed) { /* * No subexpression, but in any case the value is going * to give us the name of a parameter on which we do diff --git a/Test/D04parameter.ztst b/Test/D04parameter.ztst index 6f325d2..210c0d8 100644 --- a/Test/D04parameter.ztst +++ b/Test/D04parameter.ztst @@ -1840,7 +1840,7 @@ local two=three local three=four local -a four=(all these worlds belong to foo) - print ${${(P)${(P)${(P)one}}}} + print ${(P)${(P)${(P)one}}} print ${${(P)${(P)${(P)one}}}[3]} } testfn ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: PATCH: nested ${(P)} (formerly SHWORDSPLIT and leading spaces) 2015-11-12 14:19 ` Peter Stephenson @ 2015-11-13 8:17 ` Jun T. 2015-11-13 15:07 ` Jun T. 0 siblings, 1 reply; 18+ messages in thread From: Jun T. @ 2015-11-13 8:17 UTC (permalink / raw) To: zsh-workers Since the commit 830d54e629e8e12eb5a219a65a013876662e7b3e I can't complete git subcommand: % git <TAB> _git_commands:186: bad substitution It is failing at _git:5246 ${${(Pr.COLUMNS-4.)cmdtype/(#s)(#m)[^:]##:/${(r.len.)MATCH[1,-2]} $sep }%% #} A simpler example which fails is: % array=() % name=array % echo ${${(Pr.6.)name}} zsh: bad substitution but ${${(Pr.5.)name}} works. It seems 5 is $#name. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: PATCH: nested ${(P)} (formerly SHWORDSPLIT and leading spaces) 2015-11-13 8:17 ` Jun T. @ 2015-11-13 15:07 ` Jun T. 2015-11-14 1:33 ` Bart Schaefer 0 siblings, 1 reply; 18+ messages in thread From: Jun T. @ 2015-11-13 15:07 UTC (permalink / raw) To: zsh-workers > but ${${(Pr.5.)name}} works. It doesn't say 'bad substitution' but doesn't work as expected. % array=(a b) % arra=xxx % name=array % echo ${${(Pr.6.)name}} zsh: bad substitution % echo ${${(Pr.5.)name}} a b % echo ${${(Pr.4.)name}} xxx % echo ${(r.5.)${(P)name}}X a X b X It seems ${${(Pr.n.)name}} is behaving like ${(P)${(r.n.)name}}. Instead, the expected behavior may be ${(r.n.)${(P)name}}. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: PATCH: nested ${(P)} (formerly SHWORDSPLIT and leading spaces) 2015-11-13 15:07 ` Jun T. @ 2015-11-14 1:33 ` Bart Schaefer 2015-11-14 9:45 ` Peter Stephenson 0 siblings, 1 reply; 18+ messages in thread From: Bart Schaefer @ 2015-11-14 1:33 UTC (permalink / raw) To: zsh-workers On Nov 14, 12:07am, Jun T. wrote: } } It seems ${${(Pr.n.)name}} is behaving like ${(P)${(r.n.)name}}. } Instead, the expected behavior may be ${(r.n.)${(P)name}}. Yes, that definitely seems to be the issue. The "Rules" say that (P) is applied at step 4, whereas padding should not occur until step 22. The flags (r), (l), and (j) are all applying in unexpected order with respect to (P), but double-quote joining, (s), (f), and shwordsplit are applied in the expected order, as also are (U), (L), (u), (o), (0), and rcexpandparam. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: PATCH: nested ${(P)} (formerly SHWORDSPLIT and leading spaces) 2015-11-14 1:33 ` Bart Schaefer @ 2015-11-14 9:45 ` Peter Stephenson 2015-11-14 18:51 ` Bart Schaefer 0 siblings, 1 reply; 18+ messages in thread From: Peter Stephenson @ 2015-11-14 9:45 UTC (permalink / raw) To: Bart Schaefer; +Cc: zsh-workers On 14 Nov 2015 01:33, Bart Schaefer <schaefer@brasslantern.com> wrote: > > On Nov 14, 12:07am, Jun T. wrote: > } > } It seems ${${(Pr.n.)name}} is behaving like ${(P)${(r.n.)name}}. > } Instead, the expected behavior may be ${(r.n.)${(P)name}}. > > Yes, that definitely seems to be the issue. (On my mobile at the moment so lacking in linr breaks.) Sorry, I obviously didn't explain the change properly. The new behaviour is both expected and necessary to have the clean separation between levels of nesting: if a (P) occurs inside multiple braces, the immediately surrounding level deals with the parameter name, and the outer level deals with the value. That's the only way to implement this in a workable fashion. If the old kludge where it's mixed together is needed for compatibility, we'll need to go back to the old state (and leave it permanently - it is not compatible with a properly nested form of the (P) flag). Or, of course, we can update the documentation, or, yuk, use a different flag and deprecate the old one. pws ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: PATCH: nested ${(P)} (formerly SHWORDSPLIT and leading spaces) 2015-11-14 9:45 ` Peter Stephenson @ 2015-11-14 18:51 ` Bart Schaefer 0 siblings, 0 replies; 18+ messages in thread From: Bart Schaefer @ 2015-11-14 18:51 UTC (permalink / raw) To: zsh-workers On Nov 14, 9:45am, Peter Stephenson wrote: } } Sorry, I obviously didn't explain the change properly. The new } behaviour is both expected and necessary to have the clean separation } between levels of nesting: if a (P) occurs inside multiple braces, } the immediately surrounding level deals with the parameter name, } and the outer level deals with the value. I think it's more subtle than that. It occurs only when: 1. there is an outer ${...} around the ${(P)...} expansion 2. the (P) is combined with one of (l), (r), or (j). If there's only one level, ${(Pr.N.)...} et al. work as expected. So I suspect that the specific code path(s) used by padding and joining are failing to turn off the flag bit that indicates the value has been fetched already, causing the outer ${ } to unnecessarily attempt again to fetch the value. Or something like that. -- Barton E. Schaefer ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <0faa0ee3-303a-4cb5-a270-d3d1787accf1@email.android.com>]
* Re: PATCH: nested ${(P)} (formerly SHWORDSPLIT and leading spaces) [not found] <0faa0ee3-303a-4cb5-a270-d3d1787accf1@email.android.com> @ 2015-11-15 5:14 ` Bart Schaefer 0 siblings, 0 replies; 18+ messages in thread From: Bart Schaefer @ 2015-11-15 5:14 UTC (permalink / raw) To: zsh-workers [PWS mis-addressed his original message to "zsh-hackers" so I've included it in its entirety below.] On Nov 14, 10:31pm, Peter Stephenson wrote: } } On 14 Nov 2015 18:51, Bart Schaefer <schaefer@brasslantern.com> wrote: } > } > 1. there is an outer ${...} around the ${(P)...} expansion } > 2. the (P) is combined with one of (l), (r), or (j). } > } > If there's only one level, ${(Pr.N.)...} et al. work as expected. } } Yes, that's also deliberate. As there is no outer layer here, the code } executed is identical to before (the only alternative would be yet a } third type of behaviour; we can0x02BCt fake up a whole surrounding } layer) . If the (P) is surrounded by a an outer layer, the effect } mentioned before hsppens: the inner layer refers to the name, and } the outer to the substituted value. As I already said, if we are } separating the effects of layers to get a sensible effect with e. g. } subscripts, there is no question of flags in the inner layer having an } effect on the value. It is not just a minor side effect. } } pws OK ... but this means we definitely need some updates to the "Rules" section. For example, rule #2 says that effect of "typeset -u" should happen before the effects of (P) at rule #4. This appears to have been incorrect at least as far back as zsh 4.2 -- the name deref'd by (P) is the raw value of the variable, though whether the internal flags might change the raw value is different in 4.2 -- so given: torch% ARRAY=(foo bar) torch% array=(one two) torch% name=array torch% typeset -u name torch% print $name ARRAY In zsh-5.1.1: % print ${(P)name} one two % print ${${(P)name}} one two With zsh-5.1.1-160-gd5ba08a, nesting the (P) makes -u take effect: torch% print ${(P)name} one two torch% print ${${(P)name}} foo bar Aside: It's also worth remembering that the LRZul "internal flags" have never worked for arrays; you can set them and they're remembered but they have no effect on the individual array elements. Subscripting #3 applies before (P) #4 as described, even when (P) is within a nested substitution, but applies after #2 when there is no (P) involved, so we somehow have to explain how the order of 2/3/4 changes when a nested (P) is introduced. Here's a stab at it (and also a fix for _git, which appears to be the only contributed function that relied on the former behavior). diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git index 3dfd604..614185e 100644 --- a/Completion/Unix/Command/_git +++ b/Completion/Unix/Command/_git @@ -5244,7 +5244,7 @@ _git_commands () { for cmdtype in aliases $cmdtypes; do local -a ${cmdtype}_d (( $#disp )) && set -A ${cmdtype}_d \ - ${${(Pr.COLUMNS-4.)cmdtype/(#s)(#m)[^:]##:/${(r.len.)MATCH[1,-2]} $sep }%% #} + ${${(r.COLUMNS-4.)${(P)cmdtype}/(#s)(#m)[^:]##:/${(r.len.)MATCH[1,-2]} $sep }%% #} alts+=( "${cmdtype//_/-}:${${cmdtype//_/ }%%(e|)s}:compadd ${(e)disp} -a ${cmdtype}_m" ) done diff --git a/Doc/Zsh/expn.yo b/Doc/Zsh/expn.yo index 4c373d1..6f08d7d 100644 --- a/Doc/Zsh/expn.yo +++ b/Doc/Zsh/expn.yo @@ -1380,9 +1380,13 @@ outermost. The flags are not propagated up to enclosing substitutions; the nested substitution will return either a scalar or an array as determined by the flags, possibly adjusted for quoting. All the following steps take place where applicable at all levels of substitution. -Note that, unless the `tt((P))' flag is present, the flags and any subscripts -apply directly to the value of the nested substitution; for example, the -expansion tt(${${foo}}) behaves exactly the same as tt(${foo}). + +Note that, unless the `tt((P))' flag is present, the flags and any +subscripts apply directly to the value of the nested substitution; for +example, the expansion tt(${${foo}}) behaves exactly the same as +tt(${foo}). When the `tt((P))' flag is present in a nested substitution, +the other substitution rules are applied to the value em(before) it is +interpreted as a name, so tt(${${(P)foo}}) may differ from tt(${(P)foo}). At each nested level of substitution, the substituted words undergo all forms of single-word substitution (i.e. not filename generation), including @@ -1400,6 +1404,12 @@ in particular the tt(L), tt(R), tt(Z), tt(u) and tt(l) flags for padding and capitalization, are applied directly to the parameter value. Note these flags are options to the command, e.g. `tt(typeset -Z)'; they are not the same as the flags used within parameter substitutions. + +At the outermost level of substitution, the `tt((P))' flag ignores these +transformations and uses the unmodified value of the parameter as the name +to be replaced. This is usually the desired behavior because padding may +make the value syntactically illegal as a parameter name, but if +capitalization changes are desired, use the tt(${${(P)foo}}) form. ) item(tt(3.) em(Parameter subscripting))( If the value is a raw parameter reference with a subscript, such as @@ -1413,8 +1423,10 @@ original array). Any number of subscripts may appear. Flags such as tt((k)) and tt((v)) which alter the result of subscripting are applied. ) item(tt(4.) em(Parameter name replacement))( -The effect of any tt((P)) flag, which treats the value so far as a -parameter name and replaces it with the corresponding value, is applied. +At the outermost level of nesting only, the effect of any tt((P)) flag, +which treats the value so far as a parameter name and replaces it with the +corresponding value, is applied. This replacement occurs later if the +tt((P)) flag appears in a nested substitution. ) item(tt(5.) em(Double-quoted joining))( If the value after this process is an array, and the substitution @@ -1534,6 +1546,11 @@ Strictly speaking, the removal happens later as the same happens with other forms of substitution; the point to note here is simply that it occurs after any of the above parameter operations. ) +item(tt(25.) em(Parameter name replacement))( +If the `tt((P))' flag is present and this has not yet been done, the value +so far is looked up as a parameter name. Errors may occur if the value is +neither a valid identifier nor an identifier plus subscript expression. +) enditem() subsect(Examples) ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2015-11-15 5:14 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-06 10:54 Inconsistency with SHWORDSPLIT and leading spaces Christian Neukirchen
2015-11-06 17:00 ` Peter Stephenson
2015-11-07 17:42 ` Peter Stephenson
2015-11-07 19:43 ` Bart Schaefer
2015-11-08 18:18 ` Peter Stephenson
2015-11-08 18:55 ` Bart Schaefer
2015-11-09 6:03 ` Bart Schaefer
2015-11-11 17:49 ` PATCH: nested ${(P)} (formerly SHWORDSPLIT and leading spaces) Peter Stephenson
2015-11-11 18:13 ` Bart Schaefer
2015-11-11 21:55 ` Peter Stephenson
2015-11-12 9:46 ` Peter Stephenson
2015-11-12 14:19 ` Peter Stephenson
2015-11-13 8:17 ` Jun T.
2015-11-13 15:07 ` Jun T.
2015-11-14 1:33 ` Bart Schaefer
2015-11-14 9:45 ` Peter Stephenson
2015-11-14 18:51 ` Bart Schaefer
[not found] <0faa0ee3-303a-4cb5-a270-d3d1787accf1@email.android.com>
2015-11-15 5:14 ` Bart Schaefer
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).