Re: Minor bug(s) with NO_MULTI_FUNC_DEF

[Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>]

> On 30 May 2022 at 21:31 Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> 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
 >}