Re: preventing the leading space in process substitution

Re: preventing the leading space in process substitution

[Peter Stephenson]

On Wed, 10 Sep 2008 14:51:04 -0400
"Rocky Bernstein" <rocky.bernstein@gmail.com> wrote:
> Also (mentioned previously), I think the multiple function definition with a
> single body syntax should be removed when sh or bash emulation is in effect.
> Those POSIX shell variants neither support this form and having it around
> allows it to get confused causing a more obscure error message when one
> writes "typeset -a foo=(list)" which is valid in those other POSIX shell
> variants.

(Moved to zsh-workers.)

That's probably useful; it's been spotted before that multiple function
definitions of this form can be pretty confusing to zsh users, too.

Note, however, that the case you showed already gives a different error,
since the parentheses aren't empty.  In bash/ksh emulation you should get
the reasonable "typeset: foo: can't assign initial value for array".

Index: Doc/Zsh/options.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/options.yo,v
retrieving revision 1.63
diff -u -r1.63 options.yo
--- Doc/Zsh/options.yo	7 Aug 2008 16:27:15 -0000	1.63
+++ Doc/Zsh/options.yo	11 Sep 2008 11:43:08 -0000
@@ -1133,6 +1133,14 @@
 
 will restore normally handling of tt(SIGINT) after the function exits.
 )
+pindex(MULTI_FUNC_DEF)
+item(tt(MULTI_FUNC_DEF) <Z>)(
+Allow definitions of multiple functions at once in the form `tt(fn1
+fn2)var(...)tt(LPAR()RPAR())'; if the option is not set, this causes
+a parse error.  Definition of multiple functions with the tt(function)
+keyword is always allowed.  Multiple function definitions are not often
+used and can cause obscure errors.
+)
 pindex(MULTIOS)
 item(tt(MULTIOS) <Z>)(
 Perform implicit bf(tee)s or bf(cat)s when multiple
Index: Src/options.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/options.c,v
retrieving revision 1.45
diff -u -r1.45 options.c
--- Src/options.c	7 Aug 2008 16:27:15 -0000	1.45
+++ Src/options.c	11 Sep 2008 11:43:08 -0000
@@ -181,6 +181,7 @@
 			      0
 #endif
 			      },			 MULTIBYTE},
+{{NULL, "multifuncdef",	      OPT_EMULATE|OPT_ZSH},	 MULTIFUNCDEF},
 {{NULL, "multios",	      OPT_EMULATE|OPT_ZSH},	 MULTIOS},
 {{NULL, "nomatch",	      OPT_EMULATE|OPT_NONBOURNE},NOMATCH},
 {{NULL, "notify",	      OPT_ZSH},			 NOTIFY},
Index: Src/parse.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/parse.c,v
retrieving revision 1.73
diff -u -r1.73 parse.c
--- Src/parse.c	1 Sep 2008 20:18:48 -0000	1.73
+++ Src/parse.c	11 Sep 2008 11:43:08 -0000
@@ -1663,6 +1663,9 @@
 	    zlong oldlineno = lineno;
 	    int onp, so, oecssub = ecssub;
 
+	    if (!isset(MULTIFUNCDEF) && argc > 1)
+		YYERROR(oecused);
+
 	    *complex = c;
 	    lineno = 0;
 	    incmdpos = 1;
Index: Src/zsh.h
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/zsh.h,v
retrieving revision 1.145
diff -u -r1.145 zsh.h
--- Src/zsh.h	3 Sep 2008 09:08:22 -0000	1.145
+++ Src/zsh.h	11 Sep 2008 11:43:08 -0000
@@ -1882,6 +1882,7 @@
     MENUCOMPLETE,
     MONITOR,
     MULTIBYTE,
+    MULTIFUNCDEF,
     MULTIOS,
     NOMATCH,
     NOTIFY,


-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070

Re: preventing the leading space in process substitution

[Rocky Bernstein]

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

Thanks. I think this will be a big improvement.

A gentle nudge on having trap DEBUG (if not other traps as well) inherited
in subshells. ;-)

Thanks again.

On Thu, Sep 11, 2008 at 8:00 AM, Peter Stephenson <pws@csr.com> wrote:

> On Wed, 10 Sep 2008 14:51:04 -0400
> "Rocky Bernstein" <rocky.bernstein@gmail.com> wrote:
> > Also (mentioned previously), I think the multiple function definition
> with a
> > single body syntax should be removed when sh or bash emulation is in
> effect.
> > Those POSIX shell variants neither support this form and having it around
> > allows it to get confused causing a more obscure error message when one
> > writes "typeset -a foo=(list)" which is valid in those other POSIX shell
> > variants.
>
> (Moved to zsh-workers.)
>
> That's probably useful; it's been spotted before that multiple function
> definitions of this form can be pretty confusing to zsh users, too.
>
> Note, however, that the case you showed already gives a different error,
> since the parentheses aren't empty.  In bash/ksh emulation you should get
> the reasonable "typeset: foo: can't assign initial value for array".
>
> Index: Doc/Zsh/options.yo
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Doc/Zsh/options.yo,v
> retrieving revision 1.63
> diff -u -r1.63 options.yo
> --- Doc/Zsh/options.yo  7 Aug 2008 16:27:15 -0000       1.63
> +++ Doc/Zsh/options.yo  11 Sep 2008 11:43:08 -0000
> @@ -1133,6 +1133,14 @@
>
>  will restore normally handling of tt(SIGINT) after the function exits.
>  )
> +pindex(MULTI_FUNC_DEF)
> +item(tt(MULTI_FUNC_DEF) <Z>)(
> +Allow definitions of multiple functions at once in the form `tt(fn1
> +fn2)var(...)tt(LPAR()RPAR())'; if the option is not set, this causes
> +a parse error.  Definition of multiple functions with the tt(function)
> +keyword is always allowed.  Multiple function definitions are not often
> +used and can cause obscure errors.
> +)
>  pindex(MULTIOS)
>  item(tt(MULTIOS) <Z>)(
>  Perform implicit bf(tee)s or bf(cat)s when multiple
> Index: Src/options.c
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Src/options.c,v
> retrieving revision 1.45
> diff -u -r1.45 options.c
> --- Src/options.c       7 Aug 2008 16:27:15 -0000       1.45
> +++ Src/options.c       11 Sep 2008 11:43:08 -0000
> @@ -181,6 +181,7 @@
>                              0
>  #endif
>                              },                         MULTIBYTE},
> +{{NULL, "multifuncdef",              OPT_EMULATE|OPT_ZSH},
>  MULTIFUNCDEF},
>  {{NULL, "multios",           OPT_EMULATE|OPT_ZSH},      MULTIOS},
>  {{NULL, "nomatch",           OPT_EMULATE|OPT_NONBOURNE},NOMATCH},
>  {{NULL, "notify",            OPT_ZSH},                  NOTIFY},
> Index: Src/parse.c
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Src/parse.c,v
> retrieving revision 1.73
> diff -u -r1.73 parse.c
> --- Src/parse.c 1 Sep 2008 20:18:48 -0000       1.73
> +++ Src/parse.c 11 Sep 2008 11:43:08 -0000
> @@ -1663,6 +1663,9 @@
>            zlong oldlineno = lineno;
>            int onp, so, oecssub = ecssub;
>
> +           if (!isset(MULTIFUNCDEF) && argc > 1)
> +               YYERROR(oecused);
> +
>            *complex = c;
>            lineno = 0;
>            incmdpos = 1;
> Index: Src/zsh.h
> ===================================================================
> RCS file: /cvsroot/zsh/zsh/Src/zsh.h,v
> retrieving revision 1.145
> diff -u -r1.145 zsh.h
> --- Src/zsh.h   3 Sep 2008 09:08:22 -0000       1.145
> +++ Src/zsh.h   11 Sep 2008 11:43:08 -0000
> @@ -1882,6 +1882,7 @@
>     MENUCOMPLETE,
>     MONITOR,
>     MULTIBYTE,
> +    MULTIFUNCDEF,
>     MULTIOS,
>     NOMATCH,
>     NOTIFY,
>
>
> --
> Peter Stephenson <pws@csr.com>                  Software Engineer
> CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
> Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070
>

[-- Attachment #2: Type: text/html, Size: 5712 bytes --]

Re: preventing the leading space in process substitution

[Peter Stephenson]

On Thu, 11 Sep 2008 08:44:07 -0400
"Rocky Bernstein" <rocky.bernstein@gmail.com> wrote:
> A gentle nudge on having trap DEBUG (if not other traps as well) inherited
> in subshells. ;-)

They are currently reset explicitly.  I think for most traps that's to
avoid the subshell picking up a signal destined for the parent shell
(unless the trap has specifically been set in the subshell), but it's
possible Bart has detailed knowledge.

I can't see why we shouldn't keep the debug-related traps, ZERR and DEBUG,
alive in subshells.  It doesn't look like the behaviour was ever
documented.  Would anyone expect the current behaviour?  I hadn't realised
until you pointed it out.

Index: Doc/Zsh/builtins.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/builtins.yo,v
retrieving revision 1.112
diff -u -r1.112 builtins.yo
--- Doc/Zsh/builtins.yo	5 Sep 2008 09:05:22 -0000	1.112
+++ Doc/Zsh/builtins.yo	11 Sep 2008 13:01:02 -0000
@@ -1330,7 +1330,9 @@
 and the tt(trap) statement is not executed inside the body of a function,
 then the command var(arg) is executed when the shell terminates.
 
-tt(ZERR), tt(DEBUG), and tt(EXIT) traps are not executed inside other traps.
+tt(ZERR), tt(DEBUG), and tt(EXIT) traps are not executed inside other
+traps.  tt(ZERR) and tt(DEBUG) traps are kept within subshells, while
+other traps are reset.
 
 Note that traps defined with the tt(trap) builtin are slightly different
 from those defined as `tt(TRAP)var(NAL) () { ... }', as the latter have
Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.149
diff -u -r1.149 exec.c
--- Src/exec.c	10 Sep 2008 15:57:17 -0000	1.149
+++ Src/exec.c	11 Sep 2008 13:01:02 -0000
@@ -869,7 +869,8 @@
 
     if (!(flags & ESUB_KEEPTRAP))
 	for (sig = 0; sig < VSIGCOUNT; sig++)
-	    if (!(sigtrapped[sig] & ZSIG_FUNC))
+	    if (!(sigtrapped[sig] & ZSIG_FUNC) &&
+		sig != SIGDEBUG && sig != SIGZERR)
 		unsettrap(sig);
     monitor = isset(MONITOR);
     if (flags & ESUB_NOMONITOR)


-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070

Re: preventing the leading space in process substitution

[Bart Schaefer]

On Sep 11,  2:05pm, Peter Stephenson wrote:
}
} They are currently reset explicitly.  I think for most traps that's to
} avoid the subshell picking up a signal destined for the parent shell
} (unless the trap has specifically been set in the subshell), but it's
} possible Bart has detailed knowledge.

Not this time, I'm afraid.  I'd expect ignored signals (trap '' ...)
to be preserved in subshells, but for other signals passing through
to the parent is as good a guess as any.
 
} I can't see why we shouldn't keep the debug-related traps, ZERR and
} DEBUG, alive in subshells.

I can't think of any reason either, unless maybe there are cases where
it could result in the trap being executed in both the subshell and
the parent.

Re: preventing the leading space in process substitution

[Peter Stephenson]

On Thu, 11 Sep 2008 07:35:31 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> } I can't see why we shouldn't keep the debug-related traps, ZERR and
> } DEBUG, alive in subshells.
> 
> I can't think of any reason either, unless maybe there are cases where
> it could result in the trap being executed in both the subshell and
> the parent.

Shouldn't normally happen, but I suppose we'll just have to see.  I've
committed the patch.

That's to say, "(echo foo)" causes a DEBUG trap for the (...) itself, and
then a DEBUG trap for any contents inside the subshell according to the
normal rules for a list of commands, but I think that's exactly what we
want (it's the same as for {...}).

I note that $ZSH_DEBUG_CMD has strange (overlarge) indentation for the
elements of (...)  after the opening parenthesis.  I think that needs
revisiting.  I don't quite understand why the default indentation is 1
instead of 0; obviously that's right for outputting the bodies of
functions, but the code has many other uses.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070

Re: preventing the leading space in process substitution

[Rocky Bernstein]

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

On Thu, Sep 11, 2008 at 10:54 AM, Peter Stephenson <pws@csr.com> wrote:

> On Thu, 11 Sep 2008 07:35:31 -0700
> Bart Schaefer <schaefer@brasslantern.com> wrote:
> > } I can't see why we shouldn't keep the debug-related traps, ZERR and
> > } DEBUG, alive in subshells.
> >
> > I can't think of any reason either, unless maybe there are cases where
> > it could result in the trap being executed in both the subshell and
> > the parent.
>
> Shouldn't normally happen, but I suppose we'll just have to see.  I've
> committed the patch.
>
> That's to say, "(echo foo)" causes a DEBUG trap for the (...) itself, and
> then a DEBUG trap for any contents inside the subshell according to the
> normal rules for a list of commands, but I think that's exactly what we
> want (it's the same as for {...}).


Seems to do exactly the right thing. Thanks!



>
> I note that $ZSH_DEBUG_CMD has strange (overlarge) indentation for the
> elements of (...)  after the opening parenthesis.  I think that needs
> revisiting.  I don't quite understand why the default indentation is 1
> instead of 0; obviously that's right for outputting the bodies of
> functions, but the code has many other uses.
>
> --
> Peter Stephenson <pws@csr.com>                  Software Engineer
> CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
> Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070
>

[-- Attachment #2: Type: text/html, Size: 2283 bytes --]

PATCH: indent in code output (was Re: preventing the leading space ...)

[Peter Stephenson]

On Thu, 11 Sep 2008 15:54:16 +0100
Peter Stephenson <pws@csr.com> wrote:
> I note that $ZSH_DEBUG_CMD has strange (overlarge) indentation for the
> elements of (...)  after the opening parenthesis.  I think that needs
> revisiting.  I don't quite understand why the default indentation is 1
> instead of 0; obviously that's right for outputting the bodies of
> functions, but the code has many other uses.

I think it's simply never been right for uses other than outputting
functions for the reason that usually in other case the indentation doesn't
show up.  I can't believe

% (trap '(echo foo); echo bar' DEBUG; trap) 
foo
bar
trap -- '(
		echo foo
	)
	echo bar' DEBUG

was ever the intention.  (It's certainly not the case that the code is
indented to match the code before the opening quote, that's not taken into
account.)  This makes the output code

trap -- '(
	echo foo
)
echo bar' DEBUG

and fixes other pieces of code (including $ZSH_DEBUG_CMD) similarly.

Index: Src/builtin.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/builtin.c,v
retrieving revision 1.204
diff -u -r1.204 builtin.c
--- Src/builtin.c	3 Sep 2008 09:08:22 -0000	1.204
+++ Src/builtin.c	11 Sep 2008 17:05:32 -0000
@@ -5725,7 +5725,7 @@
 		if (!siglists[sig])
 		    printf("trap -- '' %s\n", name);
 		else {
-		    s = getpermtext(siglists[sig], NULL);
+		    s = getpermtext(siglists[sig], NULL, 0);
 		    printf("trap -- ");
 		    quotedzputs(s, stdout);
 		    printf(" %s\n", name);
Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.150
diff -u -r1.150 exec.c
--- Src/exec.c	11 Sep 2008 14:47:33 -0000	1.150
+++ Src/exec.c	11 Sep 2008 17:05:32 -0000
@@ -1084,7 +1084,7 @@
 	    noerrexit = 1;
 	    if (ltype & Z_SIMPLE) /* skip the line number */
 		pc2++;
-	    pm = setsparam("ZSH_DEBUG_CMD", getpermtext(state->prog, pc2));
+	    pm = setsparam("ZSH_DEBUG_CMD", getpermtext(state->prog, pc2, 0));
 
 	    exiting = donetrap;
 	    ret = lastval;
Index: Src/hashtable.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/hashtable.c,v
retrieving revision 1.28
diff -u -r1.28 hashtable.c
--- Src/hashtable.c	11 Aug 2008 19:22:54 -0000	1.28
+++ Src/hashtable.c	11 Sep 2008 17:05:32 -0000
@@ -887,7 +887,7 @@
 	if (f->node.flags & PM_UNDEFINED)
 	    printf("%c undefined\n\t", hashchar);
 	else
-	    t = getpermtext(f->funcdef, NULL);
+	    t = getpermtext(f->funcdef, NULL, 1);
 	if (f->node.flags & PM_TAGGED)
 	    printf("%c traced\n\t", hashchar);
 	if (!t) {
Index: Src/init.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/init.c,v
retrieving revision 1.95
diff -u -r1.95 init.c
--- Src/init.c	3 Sep 2008 09:08:22 -0000	1.95
+++ Src/init.c	11 Sep 2008 17:05:32 -0000
@@ -168,7 +168,7 @@
 		else
 		    addlinknode(args, "");
 		addlinknode(args, dupstring(getjobtext(prog, NULL)));
-		addlinknode(args, cmdstr = getpermtext(prog, NULL));
+		addlinknode(args, cmdstr = getpermtext(prog, NULL, 0));
 
 		callhookfunc("preexec", args, 1, NULL);
 
Index: Src/text.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/text.c,v
retrieving revision 1.23
diff -u -r1.23 text.c
--- Src/text.c	5 Sep 2008 09:05:23 -0000	1.23
+++ Src/text.c	11 Sep 2008 17:05:32 -0000
@@ -118,7 +118,7 @@
 
 /**/
 mod_export char *
-getpermtext(Eprog prog, Wordcode c)
+getpermtext(Eprog prog, Wordcode c, int start_indent)
 {
     struct estate s;
 
@@ -131,6 +131,7 @@
     s.pc = c;
     s.strs = prog->strs;
 
+    tindent = start_indent;
     tnewlins = 1;
     tbuf = (char *)zalloc(tsiz = 32);
     tptr = tbuf;
@@ -162,6 +163,7 @@
     s.pc = c;
     s.strs = prog->strs;
 
+    tindent = 0;
     tnewlins = 0;
     tbuf = NULL;
     tptr = jbuf;
@@ -245,16 +247,6 @@
     int stack = 0;
     wordcode code;
 
-    /*
-     * Hack for parsing "simple" format of function definitions.
-     * In this case there is no surrounding context so the initial
-     * indent should be zero.
-     */
-    if (wc_code(*state->pc) == WC_FUNCDEF)
-	tindent = 0;
-    else
-	tindent = 1;
-
     while (1) {
 	if (stack) {
 	    if (!(s = tstack))
Index: Src/Modules/parameter.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Modules/parameter.c,v
retrieving revision 1.48
diff -u -r1.48 parameter.c
--- Src/Modules/parameter.c	3 Sep 2008 09:08:22 -0000	1.48
+++ Src/Modules/parameter.c	11 Sep 2008 17:05:32 -0000
@@ -390,7 +390,7 @@
 				((shf->node.flags & PM_TAGGED) ? "Ut" : "U") :
 				((shf->node.flags & PM_TAGGED) ? "t" : "")));
 	} else {
-	    char *t = getpermtext(shf->funcdef, NULL), *n, *h;
+	    char *t = getpermtext(shf->funcdef, NULL, 1), *n, *h;
 
 	    if (shf->funcdef->flags & EF_RUN) {
 		n = nicedupstring(name);
@@ -455,7 +455,8 @@
 				    ((shf->node.flags & PM_TAGGED) ? "Ut" : "U") :
 				    ((shf->node.flags & PM_TAGGED) ? "t" : "")));
 		    } else {
-			char *t = getpermtext(((Shfunc) hn)->funcdef, NULL), *n;
+			char *t = getpermtext(((Shfunc) hn)->funcdef, NULL, 1);
+			char *n;
 
 			if (((Shfunc) hn)->funcdef->flags & EF_RUN) {
 			    n = nicedupstring(hn->nam);

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070