Re: PATCH: typeset -P

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

On Sat, 30 Sep 2017 13:48:51 +0100
Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx> wrote:
> On Fri, 29 Sep 2017 15:55:33 -0700
> Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
> > I'm ambivalent about this choice of option letter, though.  What about
> > something entirely unlikely to conflict, like -1 (ala "ls -1")?  Then
> > e.g. one could use the idiom "typeset -p1" to make it obvious.
> 
> Handling integer options is difficult as the option parser has special
> arrangements.  Optional integer arguments are possible, so we could
> enforce this as -p1, i.e. your idiom becomes the required syntax.  I
> think I've got this working; I'll send it later with documentation and
> tests.

This message never arrived, but it's been overtaken by events.

The chunk in Src/subst.c is unrelated apart from being required by a
test I've just added.  It ensures we don't elide empty nodes from the
list when using key / value pairs.  Slight hack here to avoid doing too
much extra processing: it'll keep empty nodes in the old-fashioned part
of mixed array assignments (forbidden for associative arrays).  Do you
know, I don't really care?  Shocking.

One other minor compatibility note: the existing typeset -p
prints a space between the parentheses of an empty array.  This
may have been required for parsing in the past, or may have
simply been to avoid special cases, I don't know. I've removed this
space for typeset -p1.

pws

diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index d6aa078..b4a7272 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -1817,10 +1817,10 @@ findex(typeset)
 cindex(parameters, setting)
 cindex(parameters, declaring)
 redef(SPACES)(0)(tt(ifztexi(NOTRANS(@ @ @ @ @ @ @ @ ))ifnztexi(        )))
-xitem(tt(typeset )[ {tt(PLUS())|tt(-)}tt(AHUaghlmprtux) ] \
-[ {tt(PLUS())|tt(-)}tt(EFLRZi) [ var(n) ] ])
+xitem(tt(typeset )[ {tt(PLUS())|tt(-)}tt(AHUaghlmrtux) ] \
+[ {tt(PLUS())|tt(-)}tt(EFLRZip) [ var(n) ] ])
 xitem(SPACES()[ tt(+) ] [ var(name)[tt(=)var(value)] ... ])
-xitem(tt(typeset )tt(-T) [ {tt(PLUS())|tt(-)}tt(Uglprux) ] [ {tt(PLUS())|tt(-)}tt(LRZ) [ var(n) ] ])
+xitem(tt(typeset )tt(-T) [ {tt(PLUS())|tt(-)}tt(Uglrux) ] [ {tt(PLUS())|tt(-)}tt(LRZp) [ var(n) ] ])
 xitem(SPACES()[ tt(+) | var(SCALAR)[tt(=)var(value)] var(array)[tt(=LPAR())var(value) ...tt(RPAR())] [ var(sep) ] ])
 item(tt(typeset) tt(-f) [ {tt(PLUS())|tt(-)}tt(TUkmtuz) ] [ tt(+) ] [ var(name) ... ])(
 Set or display attributes and values for shell parameters.
@@ -1985,6 +1985,11 @@ If the tt(-p) option is given, parameters and values are printed in the
 form of a typeset command with an assignment, regardless of other flags
 and options.  Note that the tt(-H) flag on parameters is respected; no
 value will be shown for these parameters.
+
+tt(-p) may be followed by an optional integer argument.  Currently
+only the value 1 is supported.  In this case arrays and associative
+arrays are printed with newlines beteween indented elements for
+readability.
 )
 item(tt(-T) [ var(scalar)[tt(=)var(value)] var(array)[tt(=LPAR())var(value) ...tt(RPAR())] [ var(sep) ] ])(
 This flag has a different meaning when used with tt(-f); see below.
diff --git a/Src/builtin.c b/Src/builtin.c
index f5ccf52..d39bb96 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -53,7 +53,7 @@ static struct builtin builtins[] =
     BUILTIN("cd", BINF_SKIPINVALID | BINF_SKIPDASH | BINF_DASHDASHVALID, bin_cd, 0, 2, BIN_CD, "qsPL", NULL),
     BUILTIN("chdir", BINF_SKIPINVALID | BINF_SKIPDASH | BINF_DASHDASHVALID, bin_cd, 0, 2, BIN_CD, "qsPL", NULL),
     BUILTIN("continue", BINF_PSPECIAL, bin_break, 0, 1, BIN_CONTINUE, NULL, NULL),
-    BUILTIN("declare", BINF_PLUSOPTS | BINF_MAGICEQUALS | BINF_PSPECIAL | BINF_ASSIGN, (HandlerFunc)bin_typeset, 0, -1, 0, "AE:%F:%HL:%R:%TUZ:%afghi:%klmprtuxz", NULL),
+    BUILTIN("declare", BINF_PLUSOPTS | BINF_MAGICEQUALS | BINF_PSPECIAL | BINF_ASSIGN, (HandlerFunc)bin_typeset, 0, -1, 0, "AE:%F:%HL:%R:%TUZ:%afghi:%klmp:%rtuxz", NULL),
     BUILTIN("dirs", 0, bin_dirs, 0, -1, 0, "clpv", NULL),
     BUILTIN("disable", 0, bin_enable, 0, -1, BIN_DISABLE, "afmprs", NULL),
     BUILTIN("disown", 0, bin_fg, 0, -1, BIN_DISOWN, NULL, NULL),
@@ -62,7 +62,7 @@ static struct builtin builtins[] =
     BUILTIN("enable", 0, bin_enable, 0, -1, BIN_ENABLE, "afmprs", NULL),
     BUILTIN("eval", BINF_PSPECIAL, bin_eval, 0, -1, BIN_EVAL, NULL, NULL),
     BUILTIN("exit", BINF_PSPECIAL, bin_break, 0, 1, BIN_EXIT, NULL, NULL),
-    BUILTIN("export", BINF_PLUSOPTS | BINF_MAGICEQUALS | BINF_PSPECIAL | BINF_ASSIGN, (HandlerFunc)bin_typeset, 0, -1, 0, "E:%F:%HL:%R:%TUZ:%afhi:%lprtu", "xg"),
+    BUILTIN("export", BINF_PLUSOPTS | BINF_MAGICEQUALS | BINF_PSPECIAL | BINF_ASSIGN, (HandlerFunc)bin_typeset, 0, -1, 0, "E:%F:%HL:%R:%TUZ:%afhi:%lp:%rtu", "xg"),
     BUILTIN("false", 0, bin_false, 0, -1, 0, NULL, NULL),
     /*
      * We used to behave as if the argument to -e was optional.
@@ -71,7 +71,7 @@ static struct builtin builtins[] =
      */
     BUILTIN("fc", 0, bin_fc, 0, -1, BIN_FC, "aAdDe:EfiIlLmnpPrRt:W", NULL),
     BUILTIN("fg", 0, bin_fg, 0, -1, BIN_FG, NULL, NULL),
-    BUILTIN("float", BINF_PLUSOPTS | BINF_MAGICEQUALS | BINF_PSPECIAL | BINF_ASSIGN, (HandlerFunc)bin_typeset, 0, -1, 0, "E:%F:%HL:%R:%Z:%ghlprtux", "E"),
+    BUILTIN("float", BINF_PLUSOPTS | BINF_MAGICEQUALS | BINF_PSPECIAL | BINF_ASSIGN, (HandlerFunc)bin_typeset, 0, -1, 0, "E:%F:%HL:%R:%Z:%ghlp:%rtux", "E"),
     BUILTIN("functions", BINF_PLUSOPTS, bin_functions, 0, -1, 0, "kmMstTuUWx:z", NULL),
     BUILTIN("getln", 0, bin_read, 0, -1, 0, "ecnAlE", "zr"),
     BUILTIN("getopts", 0, bin_getopts, 2, -1, 0, NULL, NULL),
@@ -82,11 +82,11 @@ static struct builtin builtins[] =
 #endif
 
     BUILTIN("history", 0, bin_fc, 0, -1, BIN_FC, "adDEfiLmnpPrt:", "l"),
-    BUILTIN("integer", BINF_PLUSOPTS | BINF_MAGICEQUALS | BINF_PSPECIAL | BINF_ASSIGN, (HandlerFunc)bin_typeset, 0, -1, 0, "HL:%R:%Z:%ghi:%lprtux", "i"),
+    BUILTIN("integer", BINF_PLUSOPTS | BINF_MAGICEQUALS | BINF_PSPECIAL | BINF_ASSIGN, (HandlerFunc)bin_typeset, 0, -1, 0, "HL:%R:%Z:%ghi:%lp:%rtux", "i"),
     BUILTIN("jobs", 0, bin_fg, 0, -1, BIN_JOBS, "dlpZrs", NULL),
     BUILTIN("kill", BINF_HANDLES_OPTS, bin_kill, 0, -1, 0, NULL, NULL),
     BUILTIN("let", 0, bin_let, 1, -1, 0, NULL, NULL),
-    BUILTIN("local", BINF_PLUSOPTS | BINF_MAGICEQUALS | BINF_PSPECIAL | BINF_ASSIGN, (HandlerFunc)bin_typeset, 0, -1, 0, "AE:%F:%HL:%R:%TUZ:%ahi:%lprtux", NULL),
+    BUILTIN("local", BINF_PLUSOPTS | BINF_MAGICEQUALS | BINF_PSPECIAL | BINF_ASSIGN, (HandlerFunc)bin_typeset, 0, -1, 0, "AE:%F:%HL:%R:%TUZ:%ahi:%lp:%rtux", NULL),
     BUILTIN("log", 0, bin_log, 0, 0, 0, NULL, NULL),
     BUILTIN("logout", 0, bin_break, 0, 1, BIN_LOGOUT, NULL, NULL),
 
@@ -120,7 +120,7 @@ static struct builtin builtins[] =
     BUILTIN("trap", BINF_PSPECIAL | BINF_HANDLES_OPTS, bin_trap, 0, -1, 0, NULL, NULL),
     BUILTIN("true", 0, bin_true, 0, -1, 0, NULL, NULL),
     BUILTIN("type", 0, bin_whence, 0, -1, 0, "ampfsSw", "v"),
-    BUILTIN("typeset", BINF_PLUSOPTS | BINF_MAGICEQUALS | BINF_PSPECIAL | BINF_ASSIGN, (HandlerFunc)bin_typeset, 0, -1, 0, "AE:%F:%HL:%R:%TUZ:%afghi:%klprtuxmz", NULL),
+    BUILTIN("typeset", BINF_PLUSOPTS | BINF_MAGICEQUALS | BINF_PSPECIAL | BINF_ASSIGN, (HandlerFunc)bin_typeset, 0, -1, 0, "AE:%F:%HL:%R:%TUZ:%afghi:%klp:%rtuxmz", NULL),
     BUILTIN("umask", 0, bin_umask, 0, 1, 0, "S", NULL),
     BUILTIN("unalias", 0, bin_unhash, 0, -1, BIN_UNALIAS, "ams", NULL),
     BUILTIN("unfunction", 0, bin_unhash, 1, -1, BIN_UNFUNCTION, "m", "f"),
@@ -2645,8 +2645,20 @@ bin_typeset(char *name, char **argv, LinkList assigns, Options ops, int func)
     queue_signals();
 
     /* Given no arguments, list whatever the options specify. */
-    if (OPT_ISSET(ops,'p'))
+    if (OPT_ISSET(ops,'p')) {
 	printflags |= PRINT_TYPESET;
+	if (OPT_HASARG(ops,'p')) {
+	    char *eptr;
+	    int pflag = (int)zstrtol(OPT_ARG(ops,'p'), &eptr, 10);
+	    if (pflag == 1 && !*eptr)
+		printflags |= PRINT_LINE;
+	    else {
+		zwarnnam(name, "bad argument to -p: %s", OPT_ARG(ops,'p'));
+		unqueue_signals();
+		return 1;
+	    }
+	}
+    }
     hasargs = *argv != NULL || (assigns && firstnode(assigns));
     if (!hasargs) {
 	if (!OPT_ISSET(ops,'p')) {
diff --git a/Src/params.c b/Src/params.c
index 3236f71..ddf3ce1 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -5682,40 +5682,60 @@ printparamvalue(Param p, int printflags)
 	/* array */
 	if (!(printflags & PRINT_KV_PAIR)) {
 	    putchar('(');
-	    putchar(' ');
+	    if (!(printflags & PRINT_LINE))
+		putchar(' ');
 	}
 	u = p->gsu.a->getfn(p);
 	if(*u) {
+	    if (printflags & PRINT_LINE) {
+		if (printflags & PRINT_KV_PAIR)
+		    printf("  ");
+		else
+		    printf("\n  ");
+	    }
 	    quotedzputs(*u++, stdout);
 	    while (*u) {
-		putchar(' ');
+		if (printflags & PRINT_LINE)
+		    printf("\n  ");
+		else
+		    putchar(' ');
 		quotedzputs(*u++, stdout);
 	    }
+	    if ((printflags & (PRINT_LINE|PRINT_KV_PAIR)) == PRINT_LINE)
+		putchar('\n');
 	}
 	if (!(printflags & PRINT_KV_PAIR)) {
-	    putchar(' ');
+	    if (!(printflags & PRINT_LINE))
+		putchar(' ');
 	    putchar(')');
 	}
 	break;
     case PM_HASHED:
 	/* association */
-	if (!(printflags & PRINT_KV_PAIR)) {
-	    putchar('(');
-	    putchar(' ');
-	}
 	{
-            HashTable ht = p->gsu.h->getfn(p);
+	    HashTable ht;
+	    int found = 0;
+	    if (!(printflags & PRINT_KV_PAIR)) {
+		putchar('(');
+		if (!(printflags & PRINT_LINE))
+		    putchar(' ');
+	    }
+            ht = p->gsu.h->getfn(p);
             if (ht)
-		scanhashtable(ht, 1, 0, PM_UNSET,
-			      ht->printnode, PRINT_KV_PAIR);
+		found = scanhashtable(ht, 1, 0, PM_UNSET,
+				      ht->printnode, PRINT_KV_PAIR |
+				      (printflags & PRINT_LINE));
+	    if (!(printflags & PRINT_KV_PAIR)) {
+		if (found && (printflags & PRINT_LINE))
+		    putchar('\n');
+		putchar(')');
+	    }
 	}
-	if (!(printflags & PRINT_KV_PAIR))
-	    putchar(')');
 	break;
     }
-    if (printflags & PRINT_KV_PAIR)
+    if ((printflags & (PRINT_KV_PAIR|PRINT_LINE)) == PRINT_KV_PAIR)
 	putchar(' ');
-    else
+    else if (!(printflags & PRINT_KV_PAIR))
 	putchar('\n');
 }
 
@@ -5809,8 +5829,11 @@ printparamnode(HashNode hn, int printflags)
 	zputs(p->node.nam, stdout);
 	putchar('\n');
     } else {
-	if (printflags & PRINT_KV_PAIR)
+	if (printflags & PRINT_KV_PAIR) {
+	    if (printflags & PRINT_LINE)
+		printf("\n  ");
 	    putchar('[');
+	}
 	quotedzputs(p->node.nam, stdout);
 	if (printflags & PRINT_KV_PAIR)
 	    printf("]=");
diff --git a/Src/subst.c b/Src/subst.c
index eef0dc7..2d3eeb2 100644
--- a/Src/subst.c
+++ b/Src/subst.c
@@ -158,7 +158,9 @@ prefork(LinkList list, int flags, int *ret_flags)
 		filesub(&cptr, flags & (PREFORK_TYPESET|PREFORK_ASSIGN));
 		setdata(node, cptr);
 	    }
-	} else if (!(flags & PREFORK_SINGLE) && !keep)
+	} else if (!(flags & PREFORK_SINGLE) &&
+		   !(*ret_flags & PREFORK_KEY_VALUE) &&
+		   !keep)
 	    uremnode(list, node);
 	if (errflag) {
 	    unqueue_signals();
diff --git a/Src/zsh.h b/Src/zsh.h
index c1138bf..24d06ba 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -2128,13 +2128,14 @@ typedef groupset *Groupset;
 #define PRINT_KV_PAIR		(1<<3)
 #define PRINT_INCLUDEVALUE	(1<<4)
 #define PRINT_TYPESET		(1<<5)
+#define PRINT_LINE	        (1<<6)
 
 /* flags for printing for the whence builtin */
-#define PRINT_WHENCE_CSH	(1<<6)
-#define PRINT_WHENCE_VERBOSE	(1<<7)
-#define PRINT_WHENCE_SIMPLE	(1<<8)
-#define PRINT_WHENCE_FUNCDEF	(1<<9)
-#define PRINT_WHENCE_WORD	(1<<10)
+#define PRINT_WHENCE_CSH	(1<<7)
+#define PRINT_WHENCE_VERBOSE	(1<<8)
+#define PRINT_WHENCE_SIMPLE	(1<<9)
+#define PRINT_WHENCE_FUNCDEF	(1<<10)
+#define PRINT_WHENCE_WORD	(1<<11)
 
 /* Return values from loop() */
 
diff --git a/Test/B02typeset.ztst b/Test/B02typeset.ztst
index 13f0d5e..996af06 100644
--- a/Test/B02typeset.ztst
+++ b/Test/B02typeset.ztst
@@ -793,3 +793,29 @@
   local -A keyvalhash=(1 one [2]=two 3 three)
 1:Mixed syntax with [key]=val not allowed for hash.
 ?(eval):1: bad [key]=value syntax for associative array
+
+  local -a myarray
+  typeset -p1 myarray
+  myarray=("&" sand '""' "" plugh)
+  typeset -p1 myarray
+0:typeset -p1 output for array
+>typeset -a myarray=()
+>typeset -a myarray=(
+>  '&'
+>  sand
+>  '""'
+>  ''
+>  plugh
+>)
+
+  local -A myhash
+  typeset -p1 myhash
+  myhash=([one]=two [three]= [four]="[]")
+  typeset -p1 myhash
+0:typeset -p1 output for associative array
+>typeset -A myhash=()
+>typeset -A myhash=(
+>  [four]='[]'
+>  [one]=two
+>  [three]=''
+>)