[PATCH] zformat: better handle literal % in format string

[dana <dana@xxxxxxx>]

  % zformat -F REPLY '<%3%>' && print -r - $REPLY
  <%  >  # expected <%3%>
  % zformat -F REPLY '%(%.true.false)' && print -r - $REPLY
  true  # expected false or an error

this is because zformat uses an implicit %:% spec to handle literal %s
in the format string rather than accounting for them specially

this fixes it, improves some error messages, adds a bunch of tests

dana


diff --git a/Src/Modules/zutil.c b/Src/Modules/zutil.c
index e50f68ece..53b3abe72 100644
--- a/Src/Modules/zutil.c
+++ b/Src/Modules/zutil.c
@@ -844,6 +844,14 @@ static char *zformat_substring(char* instr, char **specs, char **outp,
 	    } else if (*s == '.' || testit)
 		s++;
 
+	    // next char isn't a legal spec char -- unwind, treat the sequence
+	    // literally
+	    if (!testit && (!*s || *s == '%' || *s == ')' || *s == '-' || *s == '.')) {
+		// but swallow the % if this is %% or %)
+		start += (s - start == 1 && (*s == '%' || *s == ')'));
+		s = start;
+	    }
+
 	    if (testit && (unsigned char) *s) {
 		int actval, testval, endcharl;
 
@@ -972,15 +980,12 @@ bin_zformat(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
 	    char **ap, *specs[256] = {0}, *out;
 	    int olen, oused = 0;
 
-	    specs['%'] = "%";
-	    specs[')'] = ")";
-
 	    /* Parse the specs in argv. */
 	    for (ap = args + 2; *ap; ap++) {
 		if (!ap[0][0] || ap[0][0] == '-' || ap[0][0] == '.' ||
 		    ap[0][0] == '%' || ap[0][0] == ')' ||
 		    idigit(ap[0][0]) || ap[0][1] != ':') {
-		    zwarnnam(nam, "invalid argument: %s", *ap);
+		    zwarnnam(nam, "invalid spec: %s", *ap);
 		    return 1;
 		}
 		specs[(unsigned char) ap[0][0]] = ap[0] + 2;
@@ -989,7 +994,7 @@ bin_zformat(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
 
 	    if (!zformat_substring(args[1], specs, &out, &oused, &olen, '\0',
 			presence, 0)) {
-		zwarnnam(nam, "malformed format string");
+		zwarnnam(nam, "malformed format string: %s", args[1]);
 		return 1;
 	    }
 	    out[oused] = '\0';
diff --git a/Test/V13zformat.ztst b/Test/V13zformat.ztst
index 035a0a495..545d5e615 100644
--- a/Test/V13zformat.ztst
+++ b/Test/V13zformat.ztst
@@ -89,3 +89,141 @@
 >ipsum.bar
 >bazbaz
 >\esc:ape
+
+  zformat REPLY ''
+  zformat REPLY '' x:
+1:one of -f -F -a required
+?(eval):zformat:1: not enough arguments
+?(eval):zformat:2: invalid argument: REPLY
+
+  zformat -F REPLY %B  && print -r - $REPLY
+  zformat -F REPLY %3B && print -r - $REPLY
+0:sequence with no matching spec falls through
+>%B
+>%3B
+
+  for 1 in - . 0 9; do
+    REPLY1= REPLY2=
+    zformat -F REPLY1 %$1
+    zformat -F REPLY2 %1$1
+    zformat -F REPLY3 %1%$1
+    print -r - $REPLY1 $REPLY2 $REPLY3
+  done
+0:impossible spec in format string
+>%- %1- %1%-
+>%. %1. %1%.
+>%0 %10 %1%0
+>%9 %19 %1%9
+
+  # extra char at end to avoid triggering premature eos condition
+  zformat -F REPLY '%% %3% %) %3) x'
+  print -r - $REPLY
+0:%% and %) in format string
+>% %3% ) %3) x
+
+  for 1 in % %% %%% %%%% %%%%% %%%%%%; do
+    zformat -F REPLY $1 &&
+    print -r - $REPLY
+  done
+0:more literal % in format string
+>%
+>%
+>%%
+>%%
+>%%%
+>%%%
+
+  for 1 in % \) - . 0 9 ''; do
+    zformat -F REPLY '' $1:
+  done
+1:spec with illegal char
+?(eval):zformat:2: invalid spec: %:
+?(eval):zformat:2: invalid spec: ):
+?(eval):zformat:2: invalid spec: -:
+?(eval):zformat:2: invalid spec: .:
+?(eval):zformat:2: invalid spec: 0:
+?(eval):zformat:2: invalid spec: 9:
+?(eval):zformat:2: invalid spec: :
+
+  zformat -F REPLY '' ab:
+  zformat -F REPLY '' é:
+-:spec char longer than 1 byte
+?(eval):zformat:1: invalid spec: ab:
+?(eval):zformat:2: invalid spec: \M-C\M-):
+
+  for 1 in ! $ + , : \; \\ $'\a' $'\xff'; do
+    zformat -F REPLY "<%$1>" $1:$1 &&
+    print -r - ${(V)REPLY}
+  done
+0:weird spec char
+><!>
+><$>
+><+>
+><,>
+><:>
+><;>
+><\>
+><^G>
+><\M-^?>
+
+  zformat -F REPLY '%(' &&
+  print -r - $REPLY
+0:%( at end of format string
+>%(
+
+  zformat -F REPLY '%(.'
+  zformat -F REPLY '%()'
+  zformat -F REPLY '%(..)'
+  zformat -F REPLY '<%(>'
+1:incomplete ternary expression
+?(eval):zformat:1: malformed format string: %(.
+?(eval):zformat:2: malformed format string: %()
+?(eval):zformat:3: malformed format string: %(..)
+?(eval):zformat:4: malformed format string: <%(>
+
+  for 1 in % - . 0 9; do
+    zformat -F REPLY "%($1.true.false)" &&
+    print -r - $REPLY
+  done
+1:ternary expression with impossible spec char
+>false
+?(eval):zformat:2: malformed format string: %(-.true.false)
+>false
+?(eval):zformat:2: malformed format string: %(0.true.false)
+?(eval):zformat:2: malformed format string: %(9.true.false)
+
+  for 1 in ! / : @; do
+    zformat -F REPLY "%(x${1}true${1}false)" &&
+    print -r - $REPLY
+  done
+0:ternary expression with alternate delimiters
+>false
+>false
+>false
+>false
+
+  zformat -F REPLY '%(x.%t.%f)' x:123 t:true f:false && print -r - $REPLY
+  zformat -F REPLY '%(X.%t.%f)' x:123 t:true f:false && print -r - $REPLY
+0:ternary expression returning matching spec
+>true
+>false
+
+  zformat -F REPLY '%(x.%T.%F)' x:123 && print -r - $REPLY
+  zformat -F REPLY '%(X.%T.%F)' x:123 && print -r - $REPLY
+0:ternary expression returning non-matching spec
+>%T
+>%F
+
+  zformat -F REPLY '%(x/%../%--)' x:123 && print -r - $REPLY
+  zformat -F REPLY '%(X/%../%--)' x:123 && print -r - $REPLY
+  zformat -F REPLY '%(X.%1%-.%2%-)' x:123 && print -r - $REPLY
+0:ternary expression returning impossible spec
+>%..
+>%--
+>%2%-
+
+  zformat -F REPLY '%(x.%%.%))' x:123 && print -r - $REPLY
+  zformat -F REPLY '%(X.%%.%))' x:123 && print -r - $REPLY
+0:ternary expression returning literal % or )
+>%
+>)