Re: [PATCH?] Nofork and removing newlines

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Thu, Mar 14, 2024 at 3:15 PM Oliver Kiddle <opk@xxxxxxx> wrote:
>
> Bart Schaefer wrote:
> > I hesitate in suggesting this, but ... is there any existing case in
> > which "${{" is valid?  If not, I think I can change ${|var|...} to be
> > ${{var}...} without too much violence (except to the doc, bleah).
>
> [...] I marginally prefer ${{var}...}
> Certainly if it does involve much violence, what we currently have is
> working.

It was slightly more violent than I expected, and consequently there
is probably some room for optimization, but the attached has it
working (minus Doc update as yet).

Following workers/52635 the extra "TEST COMPLETE" test in D10 is not
really needed any more.

diff --git a/Src/lex.c b/Src/lex.c
index 31b130b07..700af2da1 100644
--- a/Src/lex.c
+++ b/Src/lex.c
@@ -1423,7 +1423,7 @@ gettokstr(int c, int sub)
 	if (lexstop)
 	    break;
 	if (!cmdsubst && in_brace_param && act == LX2_STRING &&
-	    (c == '|' || c == Bar || inblank(c))) {
+	    (c == '|' || c == Bar || c == '{' || c == Inbrace || inblank(c))) {
 	    cmdsubst = in_brace_param;
 	    cmdpush(CS_CURSH);
 	} else if (in_pattern == 2 && c != '/')
diff --git a/Src/subst.c b/Src/subst.c
index 9d20a2d0e..3764ed786 100644
--- a/Src/subst.c
+++ b/Src/subst.c
@@ -1898,11 +1898,10 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags,
      */
     if (c == Inbrace) {
 	/* The command string to be run by ${|...;} */
-	char *cmdarg = NULL;
+	char *cmdarg = NULL, *endvar = NULL, inchar = *++s;
 	size_t slen = 0;
 	int trim = (!EMULATION(EMULATE_ZSH)) ? 2 : !qt;
 	inbrace = 1;
-	s++;
 
         /* Short-path for the nofork command substitution ${|cmd;}
 	 * See other comments about kludges for why this is here.
@@ -1913,43 +1912,74 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags,
          * should not be part of command substitution in any case.
          * Use ${(U)${|cmd;}} as you would for ${(U)$(cmd;)}.
 	 */
-	if (*s == '|' || *s == Bar || inblank(*s)) {
+	if (inchar == '|' || inchar == Bar || inblank(inchar)) {
 	    char *outbracep = s;
 	    char sav = *s;
 	    *s = Inbrace;
 	    if (skipparens(Inbrace, Outbrace, &outbracep) == 0) {
 		slen = outbracep - s - 1;
 		if ((*s = sav) != Bar) {
+		    /* This tokenize() is important */
 		    sav = *outbracep;
 		    *outbracep = '\0';
 		    tokenize(s);
 		    *outbracep = sav;
 		}
 	    }
+	} else if (inchar == '{' || inchar == Inbrace) {
+	    char *outbracep;
+	    *s = Inbrace;
+
+	    if ((outbracep = itype_end(s+1, INAMESPC, 0))) {
+		if (*outbracep == Inbrack &&
+		    (outbracep = parse_subscript(++outbracep, 1, ']')))
+		    ++outbracep;
+	    }
+	    /* True for valid substitution, or we messed up in lex.c */
+	    if (outbracep && *outbracep == Outbrace) {
+		char outchar = inchar == Inbrace ? Outbrace : '}';
+		endvar = outbracep++;
+
+		/* Reached the first close brace, find the last */
+		*endvar = '|';	/* Almost anything but braces/brackets */
+		outbracep = s;
+		if (skipparens(Inbrace, outchar, &outbracep) == 0)
+		    *endvar = Outbrace;
+		else {	/* Never happens? */
+		    *endvar = outchar;
+		    outbracep = endvar + 1;
+		}
+		slen = outbracep - s - 1;
+		if (inchar != Inbrace) {
+		    char sav = *outbracep;
+		    *outbracep = '\0';
+		    tokenize(s);
+		    *outbracep = sav;
+		    outbracep[-1] = Outbrace;
+		}
+	    } else {
+		zerr("bad substitution");
+		return NULL;
+	    }
 	}
 	if (slen > 1) {
 	    char *outbracep = s + slen;
 	    if (*outbracep == Outbrace) {
-		if ((rplyvar = itype_end(s+1, INAMESPC, 0))) {
-		    if (*rplyvar == Inbrack &&
-			(rplyvar = parse_subscript(++rplyvar, 1, ']')))
-			++rplyvar;
-		}
-		if (rplyvar == s+1 && *rplyvar == Bar) {
-		    /* Is ${||...} a subtitution error or a syntax error?
+		if (endvar == s+1 && !inblank(*endvar)) {
+		    /* Is ${{}...} a substitution error or a syntax error?
 		    zerr("bad substitution");
 		    return NULL;
 		    */
 		    rplyvar = NULL;
 		}
-		if (rplyvar && *rplyvar == Bar) {
-		    cmdarg = dupstrpfx(rplyvar+1, outbracep-rplyvar-1);
-		    rplyvar = dupstrpfx(s+1,rplyvar-s-1);
+		if (endvar && *endvar == Outbrace) {
+		    cmdarg = dupstrpfx(endvar+1, outbracep-endvar-1);
+		    rplyvar = dupstrpfx(s+1,endvar-s-1);
 		} else {
 		    cmdarg = dupstrpfx(s+1, outbracep-s-1);
 		    rplyvar = "REPLY";
 		}
-		if (inblank(*s)) {
+		if (inblank(inchar)) {
 		    /*
 		     * Admittedly a hack.  Take advantage of the enforced
 		     * locality of REPLY and the semantics of $(<file) to
diff --git a/Test/D10nofork.ztst b/Test/D10nofork.ztst
index fc6b84613..0616cf9e9 100644
--- a/Test/D10nofork.ztst
+++ b/Test/D10nofork.ztst
@@ -14,6 +14,28 @@
 0:Basic substitution and REPLY scoping
 >INNER OUTER
 
+  reply=(x OUTER x)
+  purl ${{reply}reply=(\{ INNER \})} $reply
+0:Basic substitution, brace quoting, and array result
+>{
+>INNER
+>}
+>{
+>INNER
+>}
+
+  () {
+    setopt localoptions ignorebraces
+    purl ${{reply} reply=({ INNER })} $reply
+  }
+0:Basic substitution, ignorebraces, and array result
+>{
+>INNER
+>}
+>{
+>INNER
+>}
+
   purr ${| REPLY=first}:${| REPLY=second}:$REPLY
 0:re-scoping of REPLY in one statement
 >first:second:OUTER
@@ -229,7 +251,7 @@ F:Why not use this error in the previous case as well?
 >26
 
   unset reply
-  purl ${|reply| reply=(1 2 ${| REPLY=3 } 4) }
+  purl ${{reply} reply=(1 2 ${| REPLY=3 } 4) }
   typeset -p reply
 0:array behavior with global assignment
 >1
@@ -315,7 +337,7 @@ F:status of "print" should hide return
 
   unset zz
   outer=GLOBAL
-  purr "${|zz|
+  purr "${{zz}
    local outer=LOCAL
    zz=NONLOCAL
   } $outer $?"
@@ -453,6 +475,7 @@ F:must do this before evaluating the next test block
 1:ignored braces, part 4
 ?(eval):3: parse error near `}'
 
+  unsetopt ignorebraces
   # "break" blocks function calls in outer loop
   # Could use print, but that might get fixed
   repeat 3 do purr ${
@@ -467,11 +490,6 @@ F:must do this before evaluating the next test block
 ?1
 ?2
 
-  print -u $ZTST_fd ${ZTST_testname}: TEST COMPLETE
-0:make sure we got to the end
-F:some tests might silently break the test harness
-
 %clean
 
   unfunction purr purl
-  unsetopt ignorebraces
diff --git a/Test/V10private.ztst b/Test/V10private.ztst
index ed51316f3..26004a2dc 100644
--- a/Test/V10private.ztst
+++ b/Test/V10private.ztst
@@ -497,7 +497,7 @@ F:Better if caught in checkclobberparam() but exec.c doesn't know scope
  () {
    private z=outer
    print ${(t)z} $z
-   print ${| REPLY=${|z| z=nofork} }
+   print ${| REPLY=${{z} z=nofork} }
    print ${(t)z} $z
  }
 0:nofork may write to private in calling function
@@ -518,9 +518,9 @@ F:Better if caught in checkclobberparam() but exec.c doesn't know scope
  () {
    private z=outer
    print ${(t)z} $z
-   print ${|z|
+   print ${{z}
      private q
-     z=${|q| q=nofork}
+     z=${{q} q=nofork}
    }
    print ${(t)z} $z
  }
@@ -533,7 +533,7 @@ F:Better if caught in checkclobberparam() but exec.c doesn't know scope
    print ${|
      () { REPLY="{$q}" }
    }
-   print ${|q|
+   print ${{q}
      () { q=nofork }
    }
  }