Minor bug(s) with NO_MULTI_FUNC_DEF

Minor bug(s) with NO_MULTI_FUNC_DEF

[Bart Schaefer]

Even with that option set, "functions" outputs multifuncdef syntax.

% setopt nomultifuncdef
% foo() {
function foo1 foo2 foo3 { echo $0 }
function> }
% functions foo
foo () {
    foo1 foo2 foo3 () {
        echo $0
    }
}

As an aside, something like this:

TRAP{HUP,INT,QUIT,TERM} () { print -u2 Got $1 }

bypasses the nomultifuncdef syntax check and successfully defines all
four functions.

Re: Minor bug(s) with NO_MULTI_FUNC_DEF

[Peter Stephenson]

> On 30 May 2022 at 21:31 Bart Schaefer <schaefer@brasslantern.com> wrote:
> Even with that option set, "functions" outputs multifuncdef syntax.
> 
> % setopt nomultifuncdef
> % foo() {
> function foo1 foo2 foo3 { echo $0 }
> function> }
> % functions foo
> foo () {
>     foo1 foo2 foo3 () {
>         echo $0
>     }
> }

So the question is, is it time to output all functions (except anonymous)
using the other syntax, which is the one we recommend anyway?  Updating
the tests as below suggests this is basically OK; I suppose we have to
think up interesting edge cases.  I don't see a good argument for
doing it differently depending on the option setting.

From my point, actually, it's a step in the right direction --- I
just tried

eval "$(functions)"

before and after the change; before it gave me

zsh: defining function based on alias `d'
zsh: parse error near `()'

whereas after it's silent.  I presume the latter error is a consequence
of the previous warning, but I haven't looked further. Of course,
silent doesn't mean the same as completely functional.

Strictly, we can print anonymous functions the other way, too, but it
somehow seems less natural, as well as reasonable to have some visual
discriminator, so I've left them as "()" here.

> As an aside, something like this:
> 
> TRAP{HUP,INT,QUIT,TERM} () { print -u2 Got $1 }
> 
> bypasses the nomultifuncdef syntax check and successfully defines all
> four functions.

We could probably document our way out here.  It is a single command
line word in origin so the intention must be the obvious one.

pws

diff --git a/Src/hashtable.c b/Src/hashtable.c
index bb165505e..74e838829 100644
--- a/Src/hashtable.c
+++ b/Src/hashtable.c
@@ -942,10 +942,11 @@ printshfuncnode(HashNode hn, int printflags)
 	putchar('\n');
 	return;
     }
- 
+
+    printf("function ");
     quotedzputs(f->node.nam, stdout);
     if (f->funcdef || f->node.flags & PM_UNDEFINED) {
-	printf(" () {\n");
+	printf(" {\n");
 	zoutputtab(stdout);
 	if (f->node.flags & PM_UNDEFINED) {
 	    printf("%c undefined\n", hashchar);
@@ -983,7 +984,7 @@ printshfuncnode(HashNode hn, int printflags)
 	}
 	printf("\n}");
     } else {
-	printf(" () { }");
+	printf(" { }");
     }
     if (f->redir) {
 	t = getpermtext(f->redir, NULL, 1);
diff --git a/Src/text.c b/Src/text.c
index 5cd7685fd..89de27ea5 100644
--- a/Src/text.c
+++ b/Src/text.c
@@ -578,11 +578,16 @@ gettext2(Estate state)
 		Wordcode end = p + WC_FUNCDEF_SKIP(code);
 		int nargs = *state->pc++;
 
-		taddlist(state, nargs);
-		if (nargs)
+		if (nargs) {
+		    taddstr("function ");
+		    taddlist(state, nargs);
 		    taddstr(" ");
+		} else {
+		    /* Anonymous function */
+		    taddstr("() ");
+		}
 		if (tjob) {
-		    taddstr("() { ... }");
+		    taddstr("{ ... }");
 		    state->pc = end;
 		    if (!nargs) {
 			/*
@@ -594,7 +599,7 @@ gettext2(Estate state)
 		    }
 		    stack = 1;
 		} else {
-		    taddstr("() {");
+		    taddstr("{");
 		    tindent++;
 		    taddnl(1);
 		    n = tpush(code, 1);
diff --git a/Test/A01grammar.ztst b/Test/A01grammar.ztst
index 0312fe94e..719a43c3d 100644
--- a/Test/A01grammar.ztst
+++ b/Test/A01grammar.ztst
@@ -80,7 +80,7 @@
   functions -x3 fn
   fn
 0:End of sublist containing ! with no command
->fn () {
+>function fn {
 >   : && !
 >   :
 >}
@@ -95,7 +95,7 @@
   functions -x2 fn
   fn
 0:exclamation marks without following commands
->fn () {
+>function fn {
 >  ! {
 >    !
 >  } && ! (
@@ -473,19 +473,19 @@
   fn3() { ( echo foo; ) }
   functions fn1 fn2 fn3
 0:Output of syntactic structures with and without always blocks
->fn1 () {
+>function fn1 {
 >	{
 >		echo foo
 >	}
 >}
->fn2 () {
+>function fn2 {
 >	{
 >		echo foo
 >	} always {
 >		echo bar
 >	}
 >}
->fn3 () {
+>function fn3 {
 >	(
 >		echo foo
 >	)
@@ -776,7 +776,7 @@ F:Note that the behaviour of 'exit' inside try-list inside a function is unspeci
   fn abecedinarian
   fn xylophone)
 0: case word handling in sh emulation (SH_GLOB parentheses)
->fn () {
+>function fn {
 >	case $1 in
 >		(one | two | three) print Matched $1 ;;
 >		(fo* | fi* | si*) print Pattern matched $1 ;;
@@ -846,7 +846,7 @@ F:Note that the behaviour of 'exit' inside try-list inside a function is unspeci
   which fn
   fn
 0:Long case with parsed alternatives turned back into text
->fn () {
+>function fn {
 >	typeset ac_file="the else branch" 
 >	case $ac_file in
 >		(*.$ac_ext | *.xcoff | *.tds | *.d | *.pdb | *.xSYM | *.bb | *.bbg | *.map | *.inf | *.dSYM | *.o | *.obj)  ;;
diff --git a/Test/A04redirect.ztst b/Test/A04redirect.ztst
index 17f6dfa29..855c44782 100644
--- a/Test/A04redirect.ztst
+++ b/Test/A04redirect.ztst
@@ -230,7 +230,7 @@
   eval $'fn-varid() { print {\x18}<<0 }'
   { which -x2 fn-varid; fn-varid } | tr $'\x18' '?'
 0:Regression test for off-by-one in varid check
->fn-varid () {
+>function fn-varid {
 >  print {?} <<0
 >0
 >}
@@ -575,7 +575,7 @@
 
   which redirfn
 0:text output of function with redirections
->redirfn () {
+>function redirfn {
 >	local var
 >	read var
 >	print I want to tell you about $var
@@ -653,7 +653,7 @@
   fn-two-heres
   print $functions[fn-two-heres]
 0:Two here-documents in a line are shown correctly.
->fn-two-heres () {
+>function fn-two-heres {
 >  cat <<x <<y
 >foo
 >x
@@ -679,7 +679,7 @@
   which fn-here-pipe
 0:Combination of HERE-document and |&
 >FOO
->fn-here-pipe () {
+>function fn-here-pipe {
 >	cat <<HERE 2>&1 | cat
 >FOO
 >HERE
diff --git a/Test/A05execution.ztst b/Test/A05execution.ztst
index d95ee363c..22c84aaec 100644
--- a/Test/A05execution.ztst
+++ b/Test/A05execution.ztst
@@ -272,7 +272,7 @@ F:anonymous function, and a descriptor leak when backgrounding a pipeline
 0:
 >No output yet
 >Autoloaded ksh style
->autoload_redir () {
+>function autoload_redir {
 >	print Autoloaded ksh style
 >} > autoload.log
 
diff --git a/Test/B02typeset.ztst b/Test/B02typeset.ztst
index 8b3988151..7ec43adee 100644
--- a/Test/B02typeset.ztst
+++ b/Test/B02typeset.ztst
@@ -797,10 +797,10 @@
   fn2() { typeset assignfirst=(why not); }
   which -x2 fn2
 0:text output from typeset
->fn () {
+>function fn {
 >  typeset foo bar thing=this stuff=(that other) more=woevva 
 >}
->fn2 () {
+>function fn2 {
 >  typeset assignfirst=(why not) 
 >}
 
diff --git a/Test/C02cond.ztst b/Test/C02cond.ztst
index 4366b4142..502365d05 100644
--- a/Test/C02cond.ztst
+++ b/Test/C02cond.ztst
@@ -355,7 +355,7 @@ F:scenario if you encounter it.
   }
   which crashme
 0:Regression test for examining code with regular expression match
->crashme () {
+>function crashme {
 >	if [[ $1 =~ ^http:* ]]
 >	then
 >		url=${1#*=} 
@@ -424,7 +424,7 @@ F:scenario if you encounter it.
   fn() { [[ 'a' == 'b' || 'b' = 'c' || 'c' != 'd' ]] }
   which -x2 fn
 0: = and == appear as input
->fn () {
+>function fn {
 >  [[ 'a' == 'b' || 'b' = 'c' || 'c' != 'd' ]]
 >}
 
diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst
index f120809a7..6483494d3 100644
--- a/Test/C03traps.ztst
+++ b/Test/C03traps.ztst
@@ -101,13 +101,13 @@
   }
   fn1
 0: Nested TRAPINT, not triggered
->TRAPINT () {
+>function TRAPINT {
 >	print INT1
 >}
->TRAPINT () {
+>function TRAPINT {
 >	print INT2
 >}
->TRAPINT () {
+>function TRAPINT {
 >	print INT1
 >}
 
@@ -391,10 +391,10 @@
    fn2
    fn1')
 0:POSIX_TRAPS option
->TRAPEXIT () {
+>function TRAPEXIT {
 >	print Exited
 >}
->TRAPEXIT () {
+>function TRAPEXIT {
 >	print No, really exited
 >}
 >No, really exited
diff --git a/Test/C04funcdef.ztst b/Test/C04funcdef.ztst
index af469c527..2f8ee1fe3 100644
--- a/Test/C04funcdef.ztst
+++ b/Test/C04funcdef.ztst
@@ -259,7 +259,7 @@
   }
   functions fn
 0:Text representation of anonymous function with arguments
->fn () {
+>function fn {
 >	() {
 >		print Anonymous function 1 $*
 >	} with args
@@ -321,7 +321,7 @@
  barexpansion() { print This is the correct output.; }
  funcwithalias
 0:Alias expanded in command substitution does not appear expanded in text
->funcwithalias () {
+>function funcwithalias {
 >	echo $(fooalias)
 >}
 >This is the correct output.
@@ -348,7 +348,7 @@
   )
 0:autoload containing eval
 >oops was successfully autoloaded
->oops () {
+>function oops {
 >  print oops was successfully autoloaded
 >}
 
diff --git a/Test/C05debug.ztst b/Test/C05debug.ztst
index 9a8df1dad..3a428cba0 100644
--- a/Test/C05debug.ztst
+++ b/Test/C05debug.ztst
@@ -137,7 +137,7 @@
 >6: 'x=y '
 >7: 'print $x'
 >y
->8: 'fn2 () {
+>8: 'function fn2 {
 >	echo wow
 >}'
 >9: 'fn2'
diff --git a/Test/E02xtrace.ztst b/Test/E02xtrace.ztst
index 56bc20f1a..59cdfdc5b 100644
--- a/Test/E02xtrace.ztst
+++ b/Test/E02xtrace.ztst
@@ -164,19 +164,19 @@
     "
   done
 0:a function that redefines itself preserves tracing
->f () {
+>function f {
 >	# traced
 >	echo inner
 >}
->foo-bar () {
+>function foo-bar {
 >	# traced
 >	echo inner
 >}
->$'\M-c\M-\C-C\M-\C-L' () {
+>function $'\M-c\M-\C-C\M-\C-L' {
 >	# traced
 >	echo inner
 >}
->$'ba\C-@z' () {
+>function $'ba\C-@z' {
 >	# traced
 >	echo inner
 >}
@@ -192,7 +192,7 @@
  functions f
  f
 0:define traced function: named function
->f () {
+>function f {
 >	# traced
 >	echo traced named function
 >}

Re: Minor bug(s) with NO_MULTI_FUNC_DEF

[Bart Schaefer]

On Tue, May 31, 2022 at 2:06 AM Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> So the question is, is it time to output all functions (except anonymous)
> using the other syntax, which is the one we recommend anyway?

In the case of multiple names, definitely.  In the case of a single name ...

> I don't see a good argument for
> doing it differently depending on the option setting.

... do we need to worry about sh / POSIX compatibility?

Re: Minor bug(s) with NO_MULTI_FUNC_DEF

[Bart Schaefer]

On Tue, May 31, 2022 at 10:07 AM Bart Schaefer
<schaefer@brasslantern.com> wrote:
>
> On Tue, May 31, 2022 at 2:06 AM Peter Stephenson
> <p.w.stephenson@ntlworld.com> wrote:
> > I don't see a good argument for
> > doing it differently depending on the option setting.
>
> ... do we need to worry about sh / POSIX compatibility?

To be more precise ... we still don't need to do it differently based
on the option, but we might need to retain the "name()" format when
it's not affected by the option.

Re: Minor bug(s) with NO_MULTI_FUNC_DEF

[Daniel Shahaf]

Peter Stephenson wrote on Tue, May 31, 2022 at 10:03:17 +0100:
> On 30 May 2022 at 21:31 Bart Schaefer <schaefer@brasslantern.com> wrote:
> > As an aside, something like this:
> > 
> > TRAP{HUP,INT,QUIT,TERM} () { print -u2 Got $1 }
> > 
> > bypasses the nomultifuncdef syntax check and successfully defines all
> > four functions.
> 
> We could probably document our way out here.  It is a single command
> line word in origin so the intention must be the obvious one.

Devil's advocate: letting this syntax bypass the NO_MULTI_FUNC_DEF check
would break the invariant that «foo bar baz qux» and «foo {bar,baz} qux»
are always equivalent.

So, if we document it, I'd rather it were documented as "This may change
in the future" than as a feature.

Cheers,

Daniel

> 
> diff --git a/Src/hashtable.c b/Src/hashtable.c
> index bb165505e..74e838829 100644
> --- a/Src/hashtable.c
> +++ b/Src/hashtable.c
> @@ -942,10 +942,11 @@ printshfuncnode(HashNode hn, int printflags)
>  	putchar('\n');
>  	return;
>      }
> - 
> +
> +    printf("function ");
>      quotedzputs(f->node.nam, stdout);
>      if (f->funcdef || f->node.flags & PM_UNDEFINED) {
> -	printf(" () {\n");
> +	printf(" {\n");
>  	zoutputtab(stdout);
>  	if (f->node.flags & PM_UNDEFINED) {
>  	    printf("%c undefined\n", hashchar);
> @@ -983,7 +984,7 @@ printshfuncnode(HashNode hn, int printflags)
>  	}
>  	printf("\n}");
>      } else {
> -	printf(" () { }");
> +	printf(" { }");
>      }
>      if (f->redir) {
>  	t = getpermtext(f->redir, NULL, 1);
> diff --git a/Src/text.c b/Src/text.c
> index 5cd7685fd..89de27ea5 100644
> --- a/Src/text.c
> +++ b/Src/text.c
> @@ -578,11 +578,16 @@ gettext2(Estate state)
>  		Wordcode end = p + WC_FUNCDEF_SKIP(code);
>  		int nargs = *state->pc++;
>  
> -		taddlist(state, nargs);
> -		if (nargs)
> +		if (nargs) {
> +		    taddstr("function ");
> +		    taddlist(state, nargs);
>  		    taddstr(" ");
> +		} else {
> +		    /* Anonymous function */
> +		    taddstr("() ");
> +		}
>  		if (tjob) {
> -		    taddstr("() { ... }");
> +		    taddstr("{ ... }");
>  		    state->pc = end;
>  		    if (!nargs) {
>  			/*
> @@ -594,7 +599,7 @@ gettext2(Estate state)
>  		    }
>  		    stack = 1;
>  		} else {
> -		    taddstr("() {");
> +		    taddstr("{");
>  		    tindent++;
>  		    taddnl(1);
>  		    n = tpush(code, 1);
> diff --git a/Test/A01grammar.ztst b/Test/A01grammar.ztst
> index 0312fe94e..719a43c3d 100644
> --- a/Test/A01grammar.ztst
> +++ b/Test/A01grammar.ztst
> @@ -80,7 +80,7 @@
>    functions -x3 fn
>    fn
>  0:End of sublist containing ! with no command
> ->fn () {
> +>function fn {
>  >   : && !
>  >   :
>  >}
> @@ -95,7 +95,7 @@
>    functions -x2 fn
>    fn
>  0:exclamation marks without following commands
> ->fn () {
> +>function fn {
>  >  ! {
>  >    !
>  >  } && ! (
> @@ -473,19 +473,19 @@
>    fn3() { ( echo foo; ) }
>    functions fn1 fn2 fn3
>  0:Output of syntactic structures with and without always blocks
> ->fn1 () {
> +>function fn1 {
>  >	{
>  >		echo foo
>  >	}
>  >}
> ->fn2 () {
> +>function fn2 {
>  >	{
>  >		echo foo
>  >	} always {
>  >		echo bar
>  >	}
>  >}
> ->fn3 () {
> +>function fn3 {
>  >	(
>  >		echo foo
>  >	)
> @@ -776,7 +776,7 @@ F:Note that the behaviour of 'exit' inside try-list inside a function is unspeci
>    fn abecedinarian
>    fn xylophone)
>  0: case word handling in sh emulation (SH_GLOB parentheses)
> ->fn () {
> +>function fn {
>  >	case $1 in
>  >		(one | two | three) print Matched $1 ;;
>  >		(fo* | fi* | si*) print Pattern matched $1 ;;
> @@ -846,7 +846,7 @@ F:Note that the behaviour of 'exit' inside try-list inside a function is unspeci
>    which fn
>    fn
>  0:Long case with parsed alternatives turned back into text
> ->fn () {
> +>function fn {
>  >	typeset ac_file="the else branch" 
>  >	case $ac_file in
>  >		(*.$ac_ext | *.xcoff | *.tds | *.d | *.pdb | *.xSYM | *.bb | *.bbg | *.map | *.inf | *.dSYM | *.o | *.obj)  ;;
> diff --git a/Test/A04redirect.ztst b/Test/A04redirect.ztst
> index 17f6dfa29..855c44782 100644
> --- a/Test/A04redirect.ztst
> +++ b/Test/A04redirect.ztst
> @@ -230,7 +230,7 @@
>    eval $'fn-varid() { print {\x18}<<0 }'
>    { which -x2 fn-varid; fn-varid } | tr $'\x18' '?'
>  0:Regression test for off-by-one in varid check
> ->fn-varid () {
> +>function fn-varid {
>  >  print {?} <<0
>  >0
>  >}
> @@ -575,7 +575,7 @@
>  
>    which redirfn
>  0:text output of function with redirections
> ->redirfn () {
> +>function redirfn {
>  >	local var
>  >	read var
>  >	print I want to tell you about $var
> @@ -653,7 +653,7 @@
>    fn-two-heres
>    print $functions[fn-two-heres]
>  0:Two here-documents in a line are shown correctly.
> ->fn-two-heres () {
> +>function fn-two-heres {
>  >  cat <<x <<y
>  >foo
>  >x
> @@ -679,7 +679,7 @@
>    which fn-here-pipe
>  0:Combination of HERE-document and |&
>  >FOO
> ->fn-here-pipe () {
> +>function fn-here-pipe {
>  >	cat <<HERE 2>&1 | cat
>  >FOO
>  >HERE
> diff --git a/Test/A05execution.ztst b/Test/A05execution.ztst
> index d95ee363c..22c84aaec 100644
> --- a/Test/A05execution.ztst
> +++ b/Test/A05execution.ztst
> @@ -272,7 +272,7 @@ F:anonymous function, and a descriptor leak when backgrounding a pipeline
>  0:
>  >No output yet
>  >Autoloaded ksh style
> ->autoload_redir () {
> +>function autoload_redir {
>  >	print Autoloaded ksh style
>  >} > autoload.log
>  
> diff --git a/Test/B02typeset.ztst b/Test/B02typeset.ztst
> index 8b3988151..7ec43adee 100644
> --- a/Test/B02typeset.ztst
> +++ b/Test/B02typeset.ztst
> @@ -797,10 +797,10 @@
>    fn2() { typeset assignfirst=(why not); }
>    which -x2 fn2
>  0:text output from typeset
> ->fn () {
> +>function fn {
>  >  typeset foo bar thing=this stuff=(that other) more=woevva 
>  >}
> ->fn2 () {
> +>function fn2 {
>  >  typeset assignfirst=(why not) 
>  >}
>  
> diff --git a/Test/C02cond.ztst b/Test/C02cond.ztst
> index 4366b4142..502365d05 100644
> --- a/Test/C02cond.ztst
> +++ b/Test/C02cond.ztst
> @@ -355,7 +355,7 @@ F:scenario if you encounter it.
>    }
>    which crashme
>  0:Regression test for examining code with regular expression match
> ->crashme () {
> +>function crashme {
>  >	if [[ $1 =~ ^http:* ]]
>  >	then
>  >		url=${1#*=} 
> @@ -424,7 +424,7 @@ F:scenario if you encounter it.
>    fn() { [[ 'a' == 'b' || 'b' = 'c' || 'c' != 'd' ]] }
>    which -x2 fn
>  0: = and == appear as input
> ->fn () {
> +>function fn {
>  >  [[ 'a' == 'b' || 'b' = 'c' || 'c' != 'd' ]]
>  >}
>  
> diff --git a/Test/C03traps.ztst b/Test/C03traps.ztst
> index f120809a7..6483494d3 100644
> --- a/Test/C03traps.ztst
> +++ b/Test/C03traps.ztst
> @@ -101,13 +101,13 @@
>    }
>    fn1
>  0: Nested TRAPINT, not triggered
> ->TRAPINT () {
> +>function TRAPINT {
>  >	print INT1
>  >}
> ->TRAPINT () {
> +>function TRAPINT {
>  >	print INT2
>  >}
> ->TRAPINT () {
> +>function TRAPINT {
>  >	print INT1
>  >}
>  
> @@ -391,10 +391,10 @@
>     fn2
>     fn1')
>  0:POSIX_TRAPS option
> ->TRAPEXIT () {
> +>function TRAPEXIT {
>  >	print Exited
>  >}
> ->TRAPEXIT () {
> +>function TRAPEXIT {
>  >	print No, really exited
>  >}
>  >No, really exited
> diff --git a/Test/C04funcdef.ztst b/Test/C04funcdef.ztst
> index af469c527..2f8ee1fe3 100644
> --- a/Test/C04funcdef.ztst
> +++ b/Test/C04funcdef.ztst
> @@ -259,7 +259,7 @@
>    }
>    functions fn
>  0:Text representation of anonymous function with arguments
> ->fn () {
> +>function fn {
>  >	() {
>  >		print Anonymous function 1 $*
>  >	} with args
> @@ -321,7 +321,7 @@
>   barexpansion() { print This is the correct output.; }
>   funcwithalias
>  0:Alias expanded in command substitution does not appear expanded in text
> ->funcwithalias () {
> +>function funcwithalias {
>  >	echo $(fooalias)
>  >}
>  >This is the correct output.
> @@ -348,7 +348,7 @@
>    )
>  0:autoload containing eval
>  >oops was successfully autoloaded
> ->oops () {
> +>function oops {
>  >  print oops was successfully autoloaded
>  >}
>  
> diff --git a/Test/C05debug.ztst b/Test/C05debug.ztst
> index 9a8df1dad..3a428cba0 100644
> --- a/Test/C05debug.ztst
> +++ b/Test/C05debug.ztst
> @@ -137,7 +137,7 @@
>  >6: 'x=y '
>  >7: 'print $x'
>  >y
> ->8: 'fn2 () {
> +>8: 'function fn2 {
>  >	echo wow
>  >}'
>  >9: 'fn2'
> diff --git a/Test/E02xtrace.ztst b/Test/E02xtrace.ztst
> index 56bc20f1a..59cdfdc5b 100644
> --- a/Test/E02xtrace.ztst
> +++ b/Test/E02xtrace.ztst
> @@ -164,19 +164,19 @@
>      "
>    done
>  0:a function that redefines itself preserves tracing
> ->f () {
> +>function f {
>  >	# traced
>  >	echo inner
>  >}
> ->foo-bar () {
> +>function foo-bar {
>  >	# traced
>  >	echo inner
>  >}
> ->$'\M-c\M-\C-C\M-\C-L' () {
> +>function $'\M-c\M-\C-C\M-\C-L' {
>  >	# traced
>  >	echo inner
>  >}
> ->$'ba\C-@z' () {
> +>function $'ba\C-@z' {
>  >	# traced
>  >	echo inner
>  >}
> @@ -192,7 +192,7 @@
>   functions f
>   f
>  0:define traced function: named function
> ->f () {
> +>function f {
>  >	# traced
>  >	echo traced named function
>  >}
> 

Re: Minor bug(s) with NO_MULTI_FUNC_DEF

[Bart Schaefer]

On Thu, Jun 2, 2022 at 3:47 AM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> Devil's advocate: letting this syntax bypass the NO_MULTI_FUNC_DEF check
> would break the invariant that «foo bar baz qux» and «foo {bar,baz} qux»
> are always equivalent.

We're not considering «foo {bar,baz} qux» here, rather
«foo{bar,baz}qux» (the spacing is important).  The latter results in
«foobarqux foobazqux».

> So, if we document it, I'd rather it were documented as "This may change
> in the future" than as a feature.

I don't have an opinion about that, but as long as the check is done
by counting parser words it's not going to change.

Re: Minor bug(s) with NO_MULTI_FUNC_DEF

[Peter Stephenson]

> On 31 May 2022 at 18:14 Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Tue, May 31, 2022 at 10:07 AM Bart Schaefer
> <schaefer@brasslantern.com> wrote:
> >
> > On Tue, May 31, 2022 at 2:06 AM Peter Stephenson
> > <p.w.stephenson@ntlworld.com> wrote:
> > > I don't see a good argument for
> > > doing it differently depending on the option setting.
> >
> > ... do we need to worry about sh / POSIX compatibility?
> 
> To be more precise ... we still don't need to do it differently based
> on the option, but we might need to retain the "name()" format when
> it's not affected by the option.

Hmm, this is a real issue.  As NO_MULTI_FUNC_DEF is in effect
for sh emulation, we could in fact tie it to the option.  But your
suggestion has a much lower effect on output of compiled shell code, so that
could be the way to go.

pws

Re: Minor bug(s) with NO_MULTI_FUNC_DEF

[Peter Stephenson]

> On 06 June 2022 at 09:45 Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:> 
> > To be more precise ... we still don't need to do it differently based
> > on the option, but we might need to retain the "name()" format when
> > it's not affected by the option.
> 
> Hmm, this is a real issue.  As NO_MULTI_FUNC_DEF is in effect
> for sh emulation, we could in fact tie it to the option.  But your
> suggestion has a much lower effect on output of compiled shell code, so that
> could be the way to go.

This looks easy.

pws

diff --git a/Doc/Zsh/options.yo b/Doc/Zsh/options.yo
index 5e673eb5c..bf73664c9 100644
--- a/Doc/Zsh/options.yo
+++ b/Doc/Zsh/options.yo
@@ -1884,6 +1884,11 @@ 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.
+
+Note that no error is raised if multiple functions are defined as a
+result of a set of names that were originally read as a single word on
+the command line, for example `tt(TRAP{INT,QUIT})'.  Although there are
+no plans to change this behaviour at present, it is not guaranteed.
 )
 pindex(MULTIOS)
 pindex(NO_MULTIOS)
diff --git a/Src/text.c b/Src/text.c
index 5cd7685fd..56127c457 100644
--- a/Src/text.c
+++ b/Src/text.c
@@ -578,11 +578,16 @@ gettext2(Estate state)
 		Wordcode end = p + WC_FUNCDEF_SKIP(code);
 		int nargs = *state->pc++;
 
+		if (nargs > 1)
+		    taddstr("function ");
 		taddlist(state, nargs);
 		if (nargs)
 		    taddstr(" ");
 		if (tjob) {
-		    taddstr("() { ... }");
+		    if (nargs > 1)
+			taddstr("{ ... }");
+		    else
+			taddstr("() { ... }");
 		    state->pc = end;
 		    if (!nargs) {
 			/*
@@ -594,7 +599,10 @@ gettext2(Estate state)
 		    }
 		    stack = 1;
 		} else {
-		    taddstr("() {");
+		    if (nargs > 1)
+			taddstr("{");
+		    else
+			taddstr("() {");
 		    tindent++;
 		    taddnl(1);
 		    n = tpush(code, 1);
diff --git a/Test/C04funcdef.ztst b/Test/C04funcdef.ztst
index af469c527..b8509b25c 100644
--- a/Test/C04funcdef.ztst
+++ b/Test/C04funcdef.ztst
@@ -53,6 +53,26 @@
 >b: redirection
 >a: redirection
 
+  define_multiple() {
+    fn1 fn2 fn3() {
+      print This is $0
+    }
+  }
+  which -x2 define_multiple
+  define_multiple
+  fn1
+  fn2
+  fn3
+0: Safe output of multiple function definitions
+>define_multiple () {
+>  function fn1 fn2 fn3 {
+>    print This is $0
+>  }
+>}
+>This is fn1
+>This is fn2
+>This is fn3
+
   functions -M m1
   m1() { (( $# )) }
   print $(( m1() ))