Re: nohistsubstpattern and :s//

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

There's a patch near the bottom of this, which needs as many other
eyeballs on it as possible because it's changing some really old code.

On Jul 5, 11:30am, Peter Stephenson wrote:
} Subject: Re: nohistsubstpattern and :s//
}
} On Sat, 4 Jul 2015 17:01:47 -0700
} Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
} > Peter, this is yours from way back in workers/22934 -- any memory left
} > from 2006?
} 
} I've forgotten the option completely, but I'd be very surprised if it
} was deliberate.  I'd at least have stuck in a snotty note saying this
} was for caompatibility.  It doesn't look at all useful.

Not sure which "it" doesn't look useful, but here's a deeper look at a
few things -- first, the change to scalar expansion:

torch% a="aaa bab cac" b=d; echo ${a:gs/a/${b}/}

This goes into subst() in hist.c with ${b} already metafied, and comes
out again looking like

    ${b}${b}${b} b${b}b c${b}c

with all the ${b} still metafied.  I think this is exactly as expected.

#0  subst (strptr=0xbfee4ca4, in=0x82d0e00 "a", out=0x82d0e10 "\205\217b\220", 
    gbal=1) at ../../zsh-5.0/Src/hist.c:2125
#1  0x080c5a4c in modify (str=0xbfee4ca4, ptr=0xbfee4ce4)
    at ../../zsh-5.0/Src/subst.c:4210
#2  0x080c32d5 in paramsubst (l=0xb7d67758, n=0xb7d67770, str=0xbfee4d58, 
    qt=0, pf_flags=0) at ../../zsh-5.0/Src/subst.c:3237
#3  0x080bd8ae in stringsubst (list=0xb7d67758, node=0xb7d67770, pf_flags=0, 
    asssub=0) at ../../zsh-5.0/Src/subst.c:236
#4  0x080bd05e in prefork (list=0xb7d67758, flags=0)
    at ../../zsh-5.0/Src/subst.c:77

At this point stringsubst() [frame #3] keeps looping until all of the
metafied ${b} have been replaced by their expansions.

Further, if I then try a straight history substitution rather than use
a history modifier in a parameter substitution:

torch% !:0:gs/a/${b}/:p
${b}="${b}${b}${b} b${b}b c${b}c"

Here the ${b} is not metafied on the way in to subst(), because we're
doing the replacement very early in the lexing of the command line.

So the expansion of ${b} isn't really related to hist_subst_pattern at
all, it must merely be approximately coincident with it in time.  I
have no idea where to go next with this one, the hist_subst_pattern
test case relies on it behaving this way -- so maybe we should just
document it (in history section 14.1.4 under modifiers, or in Rule #7
for parameter expansion, or where)?

One more comment on this at the end of this message.


Now let's look at 

torch% a=(aaa bab cac) b=d; echo $a:gs/a/${b}/

#0  subst (strptr=0xb7d67850, in=0x82cfe28 "a", out=0x82d8e88 "\205\217b\220", 
    gbal=1) at ../../zsh-5.0/Src/hist.c:2125
#1  0x080c5a4c in modify (str=0xb7d67850, ptr=0xbfee4bb4)
    at ../../zsh-5.0/Src/subst.c:4210
#2  0x080c3354 in paramsubst (l=0xb7d67810, n=0xb7d67828, str=0xbfee4d58, 
    qt=0, pf_flags=0) at ../../zsh-5.0/Src/subst.c:3246
#3  0x080bd8ae in stringsubst (list=0xb7d67810, node=0xb7d67828, pf_flags=0, 
    asssub=0) at ../../zsh-5.0/Src/subst.c:236
#4  0x080bd05e in prefork (list=0xb7d67810, flags=0)
    at ../../zsh-5.0/Src/subst.c:77

Note modify() is being called from a different place, the loop at 3244
that applies it to each element of the array.

Passed down to stringsubst() was "node" pointing at the first element
of the array, but returning from stringsubst() to paramsubst() we have
*str pointing into the last node of the array, and assign "node" there
to that last node.  Then we continue looping until the ${b} in "c${b}c"
is expanded, but we have thus skipped over the first two elements of
the array.

This in turn all comes from the "if (isarr)" block at line 3652, where
we record "Linknode on = n" at line 3657 but conditionally restore n at
3838 based on "if (eval)".

With the patch below, removing that conditional restore, all tests still
pass and the array version of :s// works like the scalar version:

torch% a=(aaa bab cac) b=d; echo $a:gs/a/${b}/
ddd bdb cdc

The patch simply removes that conditional restore and does it always,
but restores both the node and the string pointer instead of only the
node.  There's a little more discussion after the patch.


diff --git a/Src/subst.c b/Src/subst.c
index 81d34d2..021d234 100644
--- a/Src/subst.c
+++ b/Src/subst.c
@@ -3834,8 +3834,14 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags)
 		y = dupstring(nulstring);
 	    insertlinknode(l, n, (void *) y), incnode(n);
 	}
-	if (eval)
-	    n = on;
+	/* This used to omit restoring of *str and instead test
+	 *   if (eval)
+	 *       n = on;
+	 * but that causes strange behavior of history modifiers when
+	 * applied across all values of an array.  What is magic about
+	 * eval here that *str seemed not to need restoring?
+	 */
+	*str = getdata(n = on);
     } else {
 	/*
 	 * Scalar value.  Handle last minute transformations
diff --git a/Test/D04parameter.ztst b/Test/D04parameter.ztst
index d06a73a..0a9e253 100644
--- a/Test/D04parameter.ztst
+++ b/Test/D04parameter.ztst
@@ -1711,3 +1711,12 @@
 0:Avoid confusion after overloaded characters in braceless substitution in sh
 >13
 >0-1
+
+  a="aaa bab cac"
+  b=d
+  echo $a:gs/a/${b}/
+  a=(aaa bab cac)
+  echo $a:gs/a/${b}/
+0:History modifier works the same for scalar and array substitution
+>ddd bdb cdc
+>ddd bdb cdc



Parameter substitution also occurs in the replacement part when the
modifier is applied as a glob qualfier, in case that matters.  In this
case the replacement happens during parsing of the qualifier rather
than after the substitution:

torch% echo *a*
config.modules.local config.status Makefile stamp-h
torch% b=1
torch% print *a*(:s/a/$[b++]/) $b
config.modules.loc1l config.st1tus M1kefile st1mp-h 2
torch% a=(*a*)
torch% print ${a:s/a/$[b++]/} $b
config.modules.loc1l config.st2tus M3kefile st4mp-h 5

So we have three different behaviors of the same substitution depending
on the context.

-- 
Barton E. Schaefer