[PATCH] Fix ignorebraces handling for nofork substitution

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

This was surprisingly easy from a parsing standpoint and surprisingly
difficult to restructure execution to get useful parse errors.  E.g.,
with ignorebraces, current-shell constructs must be written with a
final semicolon or newline separator like { ... ;} but if anything
follows the closing brace the parse error will be reported in that
following part rather than where you want it at the point of the
missing separator.

I think (hope?) that this is the last bit of the nofork stuff, at
least until a complete rewrite of the tempfile hack, and I will commit
and push it in the next couple of days if no one finds anything broken
or objectionable.

Opinions on whether I should squash the patches and make one commit
vs. exposing the sausage-making by doing each of the seven patches
separately?  (I would list all the X-Seq numbers in the changelog
either way.)

diff --git a/Src/lex.c b/Src/lex.c
index e7a75478b..33b17cc95 100644
--- a/Src/lex.c
+++ b/Src/lex.c
@@ -1135,7 +1135,7 @@ gettokstr(int c, int sub)
 	    c = Inpar;
 	    break;
 	case LX2_INBRACE:
-	    if (isset(IGNOREBRACES) || sub)
+	    if ((isset(IGNOREBRACES) && !cmdsubst) || sub)
 		c = '{';
 	    else {
 		if (!lexbuf.len && incmdpos) {
diff --git a/Src/subst.c b/Src/subst.c
index 4d1f8cb99..dce2380ba 100644
--- a/Src/subst.c
+++ b/Src/subst.c
@@ -1871,6 +1871,8 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags,
     char *rplyvar = NULL;
     /* Indicates ${ ... ;} */
     char *rplytmp = NULL;
+    /* How to assign to REPLY */
+    char *rplyasg = NULL;
 
     *s++ = '\0';
     /*
@@ -1957,15 +1959,21 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags,
 		     * Then fall through to the regular handling of $REPLY
 		     * to manage word splitting, expansion flags, etc.
 		     */
-		    char *fmt = "{ %s ;} >| %s; REPLY=\"$(<%s)\""; /* 28 */
+		    char *outfmt = ">| %s { %s ;}";	/* 13 */
+		    char *infmt = "REPLY=\"$(<%s)\"";	/* 15 */;
 		    if ((rplytmp = gettempname(NULL, 1))) {
 			/* Prevent shenanigans with $TMPPREFIX */
 			char *tmpfile = quotestring(rplytmp, QT_BACKSLASH);
 			char *dummy = zhalloc(strlen(cmdarg) +
-					      2 * strlen(tmpfile) +
-					      28);
-			sprintf(dummy, fmt, cmdarg, tmpfile, tmpfile);
+					      strlen(tmpfile) +
+					      13);
+			sprintf(dummy, outfmt, tmpfile, cmdarg);
 			cmdarg = dummy;
+			/* Split command and assignment to force parse
+			 * error in the right place if cmdarg is bad */
+			dummy = zhalloc(strlen(tmpfile) + 15);
+			sprintf(dummy, infmt, tmpfile);
+			rplyasg = dummy;
 		    } else {
 			/* TMPPREFIX not writable? */
 			cmdoutval = lastval;
@@ -1992,6 +2000,12 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags,
 	    untokenize(cmdarg);
 	    execstring(cmdarg, 1, 0, "cmdsubst");
 	    cmdoutval = lastval;
+	    /* In ${ ... } form, propagate error like $(...) */
+	    if (lastval && rplytmp)
+		errflag |= ERRFLAG_ERROR;
+	    else if (rplyasg) {
+		execstring(rplyasg, 1, 0, "cmdsubst");
+	    }
 	    /* "return" behaves as if in a function */
 	    if (retflag) {
 		retflag = 0;
diff --git a/Test/D10nofork.ztst b/Test/D10nofork.ztst
index 3f11f80f1..d504e8964 100644
--- a/Test/D10nofork.ztst
+++ b/Test/D10nofork.ztst
@@ -302,8 +302,8 @@ F:status of "print" should hide return
 0:local declaration, global assignment, part 2 (evaluation order)
 >NONLOCAL GLOBAL 0
 
-  : ${| fn1() { () { print -v REPLY $'Defined Function' } } }
-  print "IN${| fn2() { () { print "${:-Second }${|fn1}" } } }OUT"
+  : ${| fn1() { () { print -v REPLY $'Defined Function' ;} ;} }
+  print "IN${| fn2() { () { print "${:-Second }${|fn1}" ;} ;} }OUT"
   fn2
 0:function definition, brace nesting, quote nesting
 >INOUT
@@ -331,6 +331,26 @@ F:Fiddly here to get EOF past the test syntax
 >typeset -g wrap='REPLY in environment assignment'
 >typeset -g wrap='capture in environment assignment'
 
+  setopt ignorebraces
+0:dummy test to set option soon enough
+F:must do this before evaluating the next test block
+
+  purr ${| REPLY=${REPLY:-buried}}}
+0:ignored braces, part 1
+>buried}
+
+  purr ${ purr ${REPLY:-buried}}}
+0:ignored braces, part 2
+>buried}
+
+  purr ${ { echo nested ;} }
+0:ignored braces, part 3
+>nested
+
+  purr ${ { echo nested } }
+1:ignored braces, part 4
+?(eval):1: parse error near `}'
+
   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
@@ -338,3 +358,4 @@ F:some tests might silently break the test harness
 %clean
 
   unfunction purr purl
+  unsetopt ignorebraces