PATCH: Make ztrftime pass more things to strftime

PATCH: Make ztrftime pass more things to strftime

[Mikael Magnusson]

I poked at this for an hour, and this is the first version that passes
make check and doesn't segfault randomly, so it is not very well tested,
to say the least. Opinions on the approach?

---
 Src/utils.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/Src/utils.c b/Src/utils.c
index aea89c3..0e079c9 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -2887,7 +2887,7 @@ ztrftime(char *buf, int bufsize, char *fmt, struct tm *tm, long usec)
     int hr12;
 #ifdef HAVE_STRFTIME
     int decr;
-    char tmp[4];
+    char *fmtstart;
 #else
     static char *astr[] =
     {"Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"};
@@ -2903,7 +2903,11 @@ ztrftime(char *buf, int bufsize, char *fmt, struct tm *tm, long usec)
 	    int strip;
 	    int digs = 3;
 
+#ifdef HAVE_STRFTIME
+	    fmtstart =
+#endif
 	    fmt++;
+
 	    if (*fmt == '-') {
 		strip = 1;
 		fmt++;
@@ -2928,6 +2932,7 @@ ztrftime(char *buf, int bufsize, char *fmt, struct tm *tm, long usec)
 	     */
 	    if (ztrftimebuf(&bufsize, 2))
 		return -1;
+morefmt:
 	    switch (*fmt++) {
 	    case '.':
 		if (ztrftimebuf(&bufsize, digs))
@@ -3052,13 +3057,24 @@ ztrftime(char *buf, int bufsize, char *fmt, struct tm *tm, long usec)
 		if (fmt[-1] != '%')
 		    *buf++ = fmt[-1];
 #else
+	    case 'E':
+	    case 'O':
+	    case '^':
+	    case '#':
+	    case '_':
+	    case '0' ... '9':
+		goto morefmt;
 	    default:
 		/*
 		 * Remember we've already allowed for two characters
 		 * in the accounting in bufsize (but nowhere else).
 		 */
+		{
+		int size = fmt - fmtstart;
+		char *tmp = zhalloc(size + 1);
+		strncpy(tmp, fmtstart, size);
+		tmp[size] = '\0';
 		*buf = '\1';
-		sprintf(tmp, strip ? "%%-%c" : "%%%c", fmt[-1]);
 		if (!strftime(buf, bufsize + 2, tmp, tm))
 		{
 		    if (*buf) {
@@ -3070,6 +3086,7 @@ ztrftime(char *buf, int bufsize, char *fmt, struct tm *tm, long usec)
 		decr = strlen(buf);
 		buf += decr;
 		bufsize -= decr - 2;
+		}
 #endif
 		break;
 	    }
-- 
2.4.0

Re: PATCH: Make ztrftime pass more things to strftime

[Mikael Magnusson]

On Wed, Jul 8, 2015 at 1:15 AM, Mikael Magnusson <mikachu@gmail.com> wrote:
> I poked at this for an hour, and this is the first version that passes
> make check and doesn't segfault randomly, so it is not very well tested,
> to say the least. Opinions on the approach?

Hey look, it actually works too,

% print -P %D\{%x\}
2015年07月08日
% print -P %D\{%Ex\}
平成27年07月08日

I guess my little case+goto should include '-' too, or it defaults
into strftime early if it's not the first modifier.

-- 
Mikael Magnusson

Re: PATCH: Make ztrftime pass more things to strftime

[Peter Stephenson]

On Wed, 8 Jul 2015 01:21:42 +0200
Mikael-san <mikachu@gmail.com> wrote:
> Hey look, it actually works too,
> 
> % print -P %D\{%x\}
> 2015年07月08日
> % print -P %D\{%Ex\}
> 平成27年07月08日

By the way, that's also worth testing explicitly, but it should probably
go with the multibyte tests together with an extra test for availability
of the locale in question to avoid false test failures.

pws

Re: PATCH: Make ztrftime pass more things to strftime

[Mikael Magnusson]

On Wed, 8 Jul 2015 01:15:00 +0200
Mikael Magnusson <mikachu@gmail.com> wrote:
> I poked at this for an hour, and this is the first version that passes
> make check and doesn't segfault randomly, so it is not very well tested,
> to say the least. Opinions on the approach?

"Not totally doomed" (I haven't done a character-by-character survey of
the goings on).

Implementing on top of an interface like this is a real pain, since you
can't help re-implementing chunks and scratching your head about how to
pass the info you've got out back in again.  But it's worth doing now
internationalisation / internationalization / Internationalisierung /
... is considered basic.

It probably mostly needs a few tests.  print -P %D{...} will do most of
what you want.

On Wed, Jul 8, 2015 at 12:53 PM, Peter Stephenson
<p.stephenson@samsung.com> wrote:
> On Wed, 8 Jul 2015 01:21:42 +0200
> Mikael-san <mikachu@gmail.com> wrote:
haha
>> Hey look, it actually works too,
>>
>> % print -P %D\{%x\}
>> 2015年07月08日
>> % print -P %D\{%Ex\}
>> 平成27年07月08日
>
> By the way, that's also worth testing explicitly, but it should probably
> go with the multibyte tests together with an extra test for availability
> of the locale in question to avoid false test failures.
>
> pws

Okay, I got as far as
  LC_ALL=C
0:ztrftime extensions

And I'm not sure how to test anything. If the libc doesn't implement
the gnu extensions, then it will either fail or we could just skip the
test. I suppose if we don't skip, then we'll get plenty of reports of
test failures. But if we always skip if it detects no gnu extensions,
it won't ever really fail either. Unless our basic %y printing
suddenly breaks I suppose. Can you skip an individual test, or do I
need to make a new module? Same goes for the multibyte test, I don't
want to skip the whole module if there's no japanese locale, I think.
Maybe these can all go in a new module then? And only test things if
we find gnu extensions and a japanese utf8 locale.

We can only test the non-extension padding if it's the 1-9th of a
month, the second happens to be under 10, etc. I'm not sure what else
to test than the output being a number.

The existing print -P %w%W%D test already exercises some of the code
(It ends up calling ztrftime with strings like "%a %f" or so).

-- 
Mikael Magnusson

Re: PATCH: Make ztrftime pass more things to strftime

[Peter Stephenson]

There's not a particularly obvious way to skip an individual test,
though it might be time we added one (that ought to be easy).  You just
print a warning to the control fd in that case, and do any comparison
with input internally rather than relying on comparing with the sections
after the status.

You can test arbitrary dates with

%%zmodload zsh/datetime
% strftime "%Y" 0
1970

but you need to fail gracefully if zsh/datetime isn't available, too.

pws

PATCH: ztrftime: Pass everything unhandled to the system strftime()

[Mikael Magnusson]

This seems to work, I've added tests, and a comment.
See also threads 35725 and 35734.

---
 Src/utils.c           | 79 +++++++++++++++++++++++++++++++++++----------------
 Test/V09datetime.ztst | 63 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 117 insertions(+), 25 deletions(-)
 create mode 100644 Test/V09datetime.ztst

Eagerly awaiting reports of systems that handle %04y but not %#A.

diff --git a/Src/utils.c b/Src/utils.c
index 13fc96a..8ff575f 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -2883,7 +2883,7 @@ ztrftime(char *buf, int bufsize, char *fmt, struct tm *tm, long usec)
     int hr12;
 #ifdef HAVE_STRFTIME
     int decr;
-    char tmp[4];
+    char *fmtstart;
 #else
     static char *astr[] =
     {"Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"};
@@ -2899,7 +2899,11 @@ ztrftime(char *buf, int bufsize, char *fmt, struct tm *tm, long usec)
 	    int strip;
 	    int digs = 3;
 
+#ifdef HAVE_STRFTIME
+	    fmtstart =
+#endif
 	    fmt++;
+
 	    if (*fmt == '-') {
 		strip = 1;
 		fmt++;
@@ -2924,6 +2928,21 @@ ztrftime(char *buf, int bufsize, char *fmt, struct tm *tm, long usec)
 	     */
 	    if (ztrftimebuf(&bufsize, 2))
 		return -1;
+#ifdef HAVE_STRFTIME
+	    /* Our internal handling doesn't handle padding and other gnu extensions,
+	     * so here we detect them and pass over to strftime(). We don't want
+	     * to do this unconditionally though, as we have some extensions that
+	     * strftime() doesn't have (%., %f, %L and %K) */
+morefmt:
+	    if (!((fmt - fmtstart == 1) || (fmt - fmtstart == 2 && strip) || *fmt == '.')) {
+		while (*fmt && strchr("OE^#_-0123456789", *fmt))
+		    fmt++;
+		if (*fmt) {
+		    fmt++;
+		    goto strftimehandling;
+		}
+	    }
+#endif
 	    switch (*fmt++) {
 	    case '.':
 		if (ztrftimebuf(&bufsize, digs))
@@ -2939,10 +2958,10 @@ ztrftime(char *buf, int bufsize, char *fmt, struct tm *tm, long usec)
 		sprintf(buf, "%0*ld", digs, usec);
 		buf += digs;
 		break;
-	    case 'd':
-		if (tm->tm_mday > 9 || !strip)
-		    *buf++ = '0' + tm->tm_mday / 10;
-		*buf++ = '0' + tm->tm_mday % 10;
+	    case '\0':
+		/* Guard against premature end of string */
+		*buf++ = '%';
+		fmt--;
 		break;
 	    case 'f':
 		strip = 1;
@@ -2983,6 +3002,12 @@ ztrftime(char *buf, int bufsize, char *fmt, struct tm *tm, long usec)
 
 		*buf++ = '0' + (hr12 % 10);
 		break;
+#ifndef HAVE_STRFTIME
+	    case 'd':
+		if (tm->tm_mday > 9 || !strip)
+		    *buf++ = '0' + tm->tm_mday / 10;
+		*buf++ = '0' + tm->tm_mday % 10;
+		break;
 	    case 'm':
 		if (tm->tm_mon > 8 || !strip)
 		    *buf++ = '0' + (tm->tm_mon + 1) / 10;
@@ -3003,18 +3028,8 @@ ztrftime(char *buf, int bufsize, char *fmt, struct tm *tm, long usec)
 		    *buf++ = '0' + (tm->tm_year / 10) % 10;
 		*buf++ = '0' + tm->tm_year % 10;
 		break;
-	    case '\0':
-		/* Guard against premature end of string */
-		*buf++ = '%';
-		fmt--;
-		break;
-#ifndef HAVE_STRFTIME
 	    case 'Y':
 	    {
-		/*
-		 * Not worth handling this natively if
-		 * strftime has it.
-		 */
 		int year, digits, testyear;
 		year = tm->tm_year + 1900;
 		digits = 1;
@@ -3048,24 +3063,38 @@ ztrftime(char *buf, int bufsize, char *fmt, struct tm *tm, long usec)
 		if (fmt[-1] != '%')
 		    *buf++ = fmt[-1];
 #else
+	    case 'E':
+	    case 'O':
+	    case '^':
+	    case '#':
+	    case '_':
+	    case '-':
+	    case '0' ... '9':
+		goto morefmt;
+strftimehandling:
 	    default:
 		/*
 		 * Remember we've already allowed for two characters
 		 * in the accounting in bufsize (but nowhere else).
 		 */
-		*buf = '\1';
-		sprintf(tmp, strip ? "%%-%c" : "%%%c", fmt[-1]);
-		if (!strftime(buf, bufsize + 2, tmp, tm))
 		{
-		    if (*buf) {
-			buf[0] = '\0';
-			return -1;
+		    int size = fmt - fmtstart;
+		    char *tmp = zhalloc(size + 1);
+		    strncpy(tmp, fmtstart, size);
+		    tmp[size] = '\0';
+		    *buf = '\1';
+		    if (!strftime(buf, bufsize + 2, tmp, tm))
+		    {
+			if (*buf) {
+			    buf[0] = '\0';
+			    return -1;
+			}
+			return 0;
 		    }
-		    return 0;
+		    decr = strlen(buf);
+		    buf += decr;
+		    bufsize -= decr - 2;
 		}
-		decr = strlen(buf);
-		buf += decr;
-		bufsize -= decr - 2;
 #endif
 		break;
 	    }
diff --git a/Test/V09datetime.ztst b/Test/V09datetime.ztst
new file mode 100644
index 0000000..c69e31e
--- /dev/null
+++ b/Test/V09datetime.ztst
@@ -0,0 +1,63 @@
+%prep
+
+  if ! (zmodload zsh/datetime >/dev/null 2>/dev/null); then
+    ZTST_unimplemented="can't load the zsh/datetime module for testing"
+  fi
+  setopt multibyte
+  zmodload zsh/datetime
+  unset LC_ALL
+  LC_TIME=C
+  if [[ "$(strftime %04y 1)" = "0070" ]]; then
+    [[ "$(LC_TIME=ja_JP.UTF-8 strftime %OS 1)" = 一 ]] || {
+      print -u $ZTST_fd "Not testing alternate date format extensions (missing ja_JP.UTF-8 locale)"
+      skip_japanese=1
+    }
+  else
+    print -u $ZTST_fd "Skipping strftime extension tests"
+    skip_extensions=1
+  fi
+
+%test
+
+  strftime %y 0
+  strftime %Y 1000000000
+  strftime %x 1200000000
+  strftime %X 1200000001
+0:basic format specifiers
+>70
+>2001
+>01/10/08
+>22:20:01
+
+  strftime %-m_%f_%K_%L 1181000000
+  strftime %6. 0
+0:zsh extensions
+>6_5_1_1
+>000000
+
+  [[ $skip_japanese = 1 ]] && repeat 5; do echo skipped; done || (
+  LC_TIME=ja_JP.UTF-8
+  strftime %Ey 1000000000
+  strftime %Oy 1000000000
+  strftime %Ex 1000000000
+  strftime %OS 1000000000
+  strftime %03Ey 650000000
+  )
+0:alternate format extensions
+*>skipped|13
+>skipped|一
+>skipped|平成13年09月09日
+>skipped|四十
+>skipped|002
+
+  [[ $skip_extensions = 1 ]] && repeat 4; do echo skipped; done || (
+  strftime '%#A' 0
+  strftime '%^_10B' 0
+  strftime %03Ey 650000000
+  strftime %-Oe 0
+  )
+0:various extensions
+*>skipped|THURSDAY
+>skipped|   JANUARY
+>skipped|090
+>skipped|1
-- 
2.4.0

Re: PATCH: ztrftime: Pass everything unhandled to the system strftime()

[Peter Stephenson]

On Thu, 9 Jul 2015 11:58:07 +0200
Mikael Magnusson <mikachu@gmail.com> wrote:
> This seems to work, I've added tests, and a comment.

OK, the printing "skipped" trick looks like it should cover all bases.
Still, it ought to be possible to make it redundant within the test
framework...

pws

Re: PATCH: ztrftime: Pass everything unhandled to the system strftime()

[Peter Stephenson]

On Thu, 9 Jul 2015 11:17:56 +0100
Peter Stephenson <p.stephenson@samsung.com> wrote:
> On Thu, 9 Jul 2015 11:58:07 +0200
> Mikael Magnusson <mikachu@gmail.com> wrote:
> > This seems to work, I've added tests, and a comment.
> 
> OK, the printing "skipped" trick looks like it should cover all bases.
> Still, it ought to be possible to make it redundant within the test
> framework...

e.g.

pws

diff --git a/Test/A01grammar.ztst b/Test/A01grammar.ztst
index 8221735..82d9c8c 100644
--- a/Test/A01grammar.ztst
+++ b/Test/A01grammar.ztst
@@ -13,6 +13,19 @@
 #
 # Tests for `Simple Commands and Pipelines'
 #
+
+  # Test skipping early to ensure we run the remainder...
+  if [[ -n $ZTST_test_skip ]]; then
+    ZTST_skip="Test system verification for skipping"
+  else
+    print "This is standard output"
+    print "This is standard error" >&2
+    false
+  fi
+1:Test skipping if ZTST_test_skip is set
+>This is standard output
+?This is standard error
+
   echo foo | cat | sed 's/foo/bar/'
 0:Basic pipeline handling
 >bar
diff --git a/Test/C02cond.ztst b/Test/C02cond.ztst
index 02fa4d4..e9a596a 100644
--- a/Test/C02cond.ztst
+++ b/Test/C02cond.ztst
@@ -151,14 +151,11 @@
   cat $unmodified
   touch $newnewnew
   if [[ $OSTYPE == "cygwin" ]]; then
-    print -u$ZTST_fd "Warning: not testing [[ -N file ]] (not supported on Cygwin)"
-    true
+    ZTST_skip="[[ -N file ]] not supported on Cygwin"
   elif (( isnfs )); then
-    print -u$ZTST_fd "Warning: not testing [[ -N file ]] (not supported with NFS)"
-    true
+    ZTST_skip="[[ -N file ]] not supported with NFS"
   elif test -f /etc/mtab && { grep $(df . 2>/dev/null| tail -n1 | awk '{print $1}') /etc/mtab | grep -q noatime; }; then
-    print -u$ZTST_fd "Warning: not testing [[ -N file ]] (not supported with noatime file system)"
-    true
+    ZTST_skip="[[ -N file ]] not supported with noatime file system"
   else
     [[ -N $newnewnew && ! -N $unmodified ]]
   fi
diff --git a/Test/V09datetime.ztst b/Test/V09datetime.ztst
index a3b4e8c..a7ef983 100644
--- a/Test/V09datetime.ztst
+++ b/Test/V09datetime.ztst
@@ -10,11 +10,9 @@
   TZ=UTC+0
   if [[ "$(strftime %04y 1)" = "0070" ]]; then
     [[ "$(LC_TIME=ja_JP.UTF-8 strftime %OS 1)" = 一 ]] || {
-      print -u $ZTST_fd "Not testing alternate date format extensions (missing ja_JP.UTF-8 locale)"
       skip_japanese=1
     }
   else
-    print -u $ZTST_fd "Skipping strftime extension tests"
     skip_extensions=1
   fi
 
@@ -36,29 +34,37 @@
 >6_6_3_3
 >000000
 
-  [[ $skip_japanese = 1 ]] && repeat 5; do echo skipped; done || (
-  LC_TIME=ja_JP.UTF-8
-  strftime %Ey 1000000000
-  strftime %Oy 1000000000
-  strftime %Ex 1000000000
-  strftime %OS 1000000000
-  strftime %03Ey 650000000
-  )
+  if [[ $skip_japanese = 1 ]]; then
+    ZTST_skip="Japanese UTF-8 locale not supported"
+  else
+    (
+    LC_TIME=ja_JP.UTF-8
+    strftime %Ey 1000000000
+    strftime %Oy 1000000000
+    strftime %Ex 1000000000
+    strftime %OS 1000000000
+    strftime %03Ey 650000000
+    )
+  fi
 0:alternate format extensions
-*>skipped|13
->skipped|一
->skipped|平成13年09月09日
->skipped|四十
->skipped|002
+>13
+>一
+>平成13年09月09日
+>四十
+>002
 
-  [[ $skip_extensions = 1 ]] && repeat 4; do echo skipped; done || (
-  strftime '%#A' 0
-  strftime '%^_10B' 0
-  strftime %03Ey 650000000
-  strftime %-Oe 0
-  )
+  if [[ $skip_extensions = 1 ]]; then
+    ZTST_skip="strftime extensions not supported"
+  else
+    (
+      strftime '%#A' 0
+      strftime '%^_10B' 0
+      strftime %03Ey 650000000
+      strftime %-Oe 0
+    )
+  fi
 0:various extensions
-*>skipped|THURSDAY
->skipped|   JANUARY
->skipped|090
->skipped|1
+>THURSDAY
+>   JANUARY
+>090
+>1
diff --git a/Test/ztst.zsh b/Test/ztst.zsh
index 74111f6..ce89a83 100755
--- a/Test/ztst.zsh
+++ b/Test/ztst.zsh
@@ -343,6 +343,7 @@ ZTST_diff() {
 ZTST_test() {
   local last match mbegin mend found substlines
   local diff_out diff_err
+  local ZTST_skip
 
   while true; do
     rm -f $ZTST_in $ZTST_out $ZTST_err
@@ -427,6 +428,16 @@ $ZTST_curline"
 
       ZTST_execchunk <$ZTST_in >$ZTST_tout 2>$ZTST_terr
 
+      if [[ -n $ZTST_skip ]]; then
+	ZTST_verbose 0 "Test case skipped: $ZTST_skip"
+	ZTST_skip=
+	if [[ -n $last ]]; then
+	  break
+	else
+	  continue
+	fi
+      fi
+
       # First check we got the right status, if specified.
       if [[ $ZTST_xstatus != - && $ZTST_xstatus != $ZTST_status ]]; then
 	ZTST_testfailed "bad status $ZTST_status, expected $ZTST_xstatus from:

Skipping tests (was Re: PATCH: ztrftime: Pass everything ...)

[Bart Schaefer]

On Jul 10,  2:37pm, Peter Stephenson wrote:
} Subject: Re: PATCH: ztrftime: Pass everything unhandled to the system strf
}
} On Thu, 9 Jul 2015 11:17:56 +0100
} Peter Stephenson <p.stephenson@samsung.com> wrote:
} > 
} > OK, the printing "skipped" trick looks like it should cover all bases.
} > Still, it ought to be possible to make it redundant within the test
} > framework...
} 
} +  # Test skipping early to ensure we run the remainder...
} +  if [[ -n $ZTST_test_skip ]]; then
} +    ZTST_skip="Test system verification for skipping"
} +  else
} +    print "This is standard output"
} +    print "This is standard error" >&2
} +    false
} +  fi
} +1:Test skipping if ZTST_test_skip is set
} +>This is standard output
} +?This is standard error

OK, so the way we actually test THIS is e.g.

    ZTST_test_skip=skip make check TESTNUM=A01

And that seems to correctly print "Test system verification for skipping"
followed by actually running all the other tests in the script.  Great!

Given that we can now skip individual test cases as well as entire test
scripts, should we start counting the individual test cases to report
the number of successful/failed tests, rather than counting only the
number of scripts that ran without any failures?

Re: Skipping tests (was Re: PATCH: ztrftime: Pass everything ...)

[Peter Stephenson]

On Fri, 10 Jul 2015 09:18:21 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> Given that we can now skip individual test cases as well as entire test
> scripts, should we start counting the individual test cases to report
> the number of successful/failed tests, rather than counting only the
> number of scripts that ran without any failures?

Probably, but it's not trivial since currently the bit that does the
counting is in a separate executable, which is how we protect
different scripts from one another. We need a new way of signalling e.g.
a temporary file. I think we should avoid pipes --- we have enough
trouble with those without making the test system run in one.

pws

Re: PATCH: ztrftime: Pass everything unhandled to the system strftime()

[Daniel Shahaf]

Peter Stephenson wrote on Fri, Jul 10, 2015 at 14:37:08 +0100:
> +++ b/Test/C02cond.ztst
> @@ -151,14 +151,11 @@
>    cat $unmodified
>    touch $newnewnew
>    if [[ $OSTYPE == "cygwin" ]]; then
> -    print -u$ZTST_fd "Warning: not testing [[ -N file ]] (not supported on Cygwin)"
> -    true
> +    ZTST_skip="[[ -N file ]] not supported on Cygwin"

This skip mechanism has the unusual property that the remainder of the
test is still run, but its output is discarded.

Perhaps ZTST_skip should be converted to a statement?  e.g.,
    ZTST_skip "[[ -N file ]] not supported on Cygwin"
where ZTST_skip is defined such that the remainder of the test case
isn't executed.

And for the sake of explicitness, this should be mentioned in B01cd.ztst
along with the other test harness features.

Cheers,

Daniel

ZTST_skip (was Re: PATCH: ztrftime ...)

[Bart Schaefer]

On Jul 10,  6:31pm, Daniel Shahaf wrote:
}
} Perhaps ZTST_skip should be converted to a statement?  e.g.,
}     ZTST_skip "[[ -N file ]] not supported on Cygwin"
} where ZTST_skip is defined such that the remainder of the test case
} isn't executed.

Not really possible; each test chunk is run by "eval" inside an anonymous
function scope, so there's really no way to break out of it early without
the entire test script being aborted.

Given that one already needs some kind of test to decide whether to set a
value for ZTST_skip, it seems reasonable to ask that the remainder of the
test chunk be in an "else" branch of whatever test that is.

Re: PATCH: ztrftime: Pass everything unhandled to the system strftime()

[Jun T.]

The output of the test depends on the timezone in use;
we may need something like TZ=UTC+0 in the preamble of the test.

Re: PATCH: ztrftime: Pass everything unhandled to the system strftime()

[Jun T.]

V09datetime fails on Mac OS X, because the OS does not
support "%04y" nor "%Ey" but only skip_extensions is set.

mac% strftime %04y 1
4y
mac% LC_TIME=ja_JP.UTF-8 strftime %OS 1
01

I think these two skip_* need be set independently.


diff --git a/Test/V09datetime.ztst b/Test/V09datetime.ztst
index a7ef983..475adde 100644
--- a/Test/V09datetime.ztst
+++ b/Test/V09datetime.ztst
@@ -8,13 +8,8 @@
   unset LC_ALL
   LC_TIME=C
   TZ=UTC+0
-  if [[ "$(strftime %04y 1)" = "0070" ]]; then
-    [[ "$(LC_TIME=ja_JP.UTF-8 strftime %OS 1)" = 一 ]] || {
-      skip_japanese=1
-    }
-  else
-    skip_extensions=1
-  fi
+  [[ "$(strftime %04y 1)" = "0070" ]] || skip_extensions=1
+  [[ "$(LC_TIME=ja_JP.UTF-8 strftime %OS 1)" = 一 ]] || skip_japanese=1
 
 %test
 

Re: PATCH: ztrftime: Pass everything unhandled to the system strftime()

[Mikael Magnusson]

On Fri, Jul 10, 2015 at 4:53 PM, Jun T. <takimoto-j@kba.biglobe.ne.jp> wrote:
> V09datetime fails on Mac OS X, because the OS does not
> support "%04y" nor "%Ey" but only skip_extensions is set.
>
> mac% strftime %04y 1
> 4y
> mac% LC_TIME=ja_JP.UTF-8 strftime %OS 1
> 01
>
> I think these two skip_* need be set independently.
>
>
> diff --git a/Test/V09datetime.ztst b/Test/V09datetime.ztst
> index a7ef983..475adde 100644
> --- a/Test/V09datetime.ztst
> +++ b/Test/V09datetime.ztst
> @@ -8,13 +8,8 @@
>    unset LC_ALL
>    LC_TIME=C
>    TZ=UTC+0
> -  if [[ "$(strftime %04y 1)" = "0070" ]]; then
> -    [[ "$(LC_TIME=ja_JP.UTF-8 strftime %OS 1)" = 一 ]] || {
> -      skip_japanese=1
> -    }
> -  else
> -    skip_extensions=1
> -  fi
> +  [[ "$(strftime %04y 1)" = "0070" ]] || skip_extensions=1
> +  [[ "$(LC_TIME=ja_JP.UTF-8 strftime %OS 1)" = 一 ]] || skip_japanese=1
>
>  %test

Oops, yeah, I think I intended to check both of the parameters in the
second test case, then forgot. The idea is that only skip_japanese
can't be set, so I wrote the logic to enforce that. I'll commit this
as well to not print a misleading skip message if both are set.

@@ -34,7 +29,9 @@
 >6_6_3_3
 >000000

-  if [[ $skip_japanese = 1 ]]; then
+  if [[ $skip_extensions = 1 ]]; then
+    ZTST_skip="strftime extensions not supported"
+  elif [[ $skip_japanese = 1 ]]; then
     ZTST_skip="Japanese UTF-8 locale not supported"
   else
     (


-- 
Mikael Magnusson

Re: PATCH: ztrftime: Pass everything unhandled to the system strftime()

[Jun T.]

I wrote:
> Is it possible to pass the entire format string to strftime()
> if HAVE_STRFTIME is defined?

but this was not a good idea. On cygwin, V09datetime.ztst fails as

*** 1,2 ****
! 6_6_3_3
  000000
--- 1,2 ----
!
  000000
Test ./V09datetime.ztst failed: output differs from expected as shown above for:
  strftime %-m_%f_%K_%L 1181100000
  strftime %6. 0
Was testing: zsh extensions
./V09datetime.ztst: test failed.

strftime(3) on cygwin is from newlib (not gnu libc) and does not support
any of gnu extensions including the %- modifier. But man zshmisc says:

    The GNU extension that a `-' between the % and the format  char-
    acter  causes  a leading zero or space to be stripped is handled
    directly by the shell for the format characters d, f, H,  k,  l,
    m, M, S and y; any other format characters are provided to strf-
    time() with any leading `-', present, so the handling is  system
    dependent.  Further GNU extensions are not supported at present.

so %-m etc. needs be supported also on cygwin? The simplest fix would be
to move the #ifndef HAVE_STRFTIME back to the original position:

diff --git a/Src/utils.c b/Src/utils.c
index f7aaaed..236661a 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -3006,7 +3006,6 @@ morefmt:
 
 		*buf++ = '0' + (hr12 % 10);
 		break;
-#ifndef HAVE_STRFTIME
 	    case 'd':
 		if (tm->tm_mday > 9 || !strip)
 		    *buf++ = '0' + tm->tm_mday / 10;
@@ -3032,6 +3031,7 @@ morefmt:
 		    *buf++ = '0' + (tm->tm_year / 10) % 10;
 		*buf++ = '0' + tm->tm_year % 10;
 		break;
+#ifndef HAVE_STRFTIME
 	    case 'Y':
 	    {
 		int year, digits, testyear;



BTW, the last part of the above man page
"Further GNU extensions are not supported at present."
needs be updated?

Re: PATCH: ztrftime: Pass everything unhandled to the system strftime()

[Mikael Magnusson]

On Mon, Jul 27, 2015 at 1:56 PM, Jun T. <takimoto-j@kba.biglobe.ne.jp> wrote:
> I wrote:
>> Is it possible to pass the entire format string to strftime()
>> if HAVE_STRFTIME is defined?
>
> but this was not a good idea. On cygwin, V09datetime.ztst fails as
>
> *** 1,2 ****
> ! 6_6_3_3
>   000000
> --- 1,2 ----
> !
>   000000
> Test ./V09datetime.ztst failed: output differs from expected as shown above for:
>   strftime %-m_%f_%K_%L 1181100000
>   strftime %6. 0
> Was testing: zsh extensions
> ./V09datetime.ztst: test failed.
>
> strftime(3) on cygwin is from newlib (not gnu libc) and does not support
> any of gnu extensions including the %- modifier. But man zshmisc says:
>
>     The GNU extension that a `-' between the % and the format  char-
>     acter  causes  a leading zero or space to be stripped is handled
>     directly by the shell for the format characters d, f, H,  k,  l,
>     m, M, S and y; any other format characters are provided to strf-
>     time() with any leading `-', present, so the handling is  system
>     dependent.  Further GNU extensions are not supported at present.

Oops, but it's nice that writing the tests paid off :). ACK on the
patch from me.

> so %-m etc. needs be supported also on cygwin? The simplest fix would be
> to move the #ifndef HAVE_STRFTIME back to the original position:
>
> diff --git a/Src/utils.c b/Src/utils.c
> index f7aaaed..236661a 100644
> --- a/Src/utils.c
> +++ b/Src/utils.c
> @@ -3006,7 +3006,6 @@ morefmt:
>
>                 *buf++ = '0' + (hr12 % 10);
>                 break;
> -#ifndef HAVE_STRFTIME
>             case 'd':
>                 if (tm->tm_mday > 9 || !strip)
>                     *buf++ = '0' + tm->tm_mday / 10;
> @@ -3032,6 +3031,7 @@ morefmt:
>                     *buf++ = '0' + (tm->tm_year / 10) % 10;
>                 *buf++ = '0' + tm->tm_year % 10;
>                 break;
> +#ifndef HAVE_STRFTIME
>             case 'Y':
>             {
>                 int year, digits, testyear;
>
>
>
> BTW, the last part of the above man page
> "Further GNU extensions are not supported at present."
> needs be updated?

I guess it could say something along the lines of
"Further GNU (or other) extensions are not supported, but will work if
the system libc supports them."
?

-- 
Mikael Magnusson

Re: PATCH: ztrftime: Pass everything unhandled to the system strftime()

[Jun T.]

2015/07/27 22:31, Mikael Magnusson <mikachu@gmail.com> wrote:

> I guess it could say something along the lines of
> "Further GNU (or other) extensions are not supported, but will work if
> the system libc supports them."

yes, or 
"Further GNU (or other) extensions are also passed to the strftime()
and will work if the system supports them." ?

# I'm not a native English writer so please update as you think it
# appropriate.

Re: PATCH: ztrftime: Pass everything unhandled to the system strftime()

[Jun T.]

2015/07/27 22:31, Mikael Magnusson <mikachu@gmail.com> wrote:
> "Further GNU (or other) extensions

An example of "other" extensions which works on Mac OS X:

mac% print -P %D{%+}
Tue Jul 28 01:57:22 JST 2015

Re: PATCH: ztrftime: Pass everything unhandled to the system strftime()

[Oliver Kiddle]

On 9 Jul, Mikael Magnusson wrote:
> +	    case '0' ... '9':

This range is a gcc extension to C. So this fails to build on Solaris
with the Oracle compiler.

Oliver

Re: PATCH: ztrftime: Pass everything unhandled to the system strftime()

[Peter Stephenson]

On Tue, 1 Sep 2015 15:53:25 +0200
Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> On 9 Jul, Mikael Magnusson wrote:
> > +	    case '0' ... '9':
> 
> This range is a gcc extension to C. So this fails to build on Solaris
> with the Oracle compiler.

There ought to be a way of getting warnings about extensions (without
changing the dialect of C which has too many side effects).

pws

PATCH: Avoid gcc case syntax

[Mikael Magnusson]

---
 Src/utils.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Src/utils.c b/Src/utils.c
index 47567ff..fa64b82 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -3092,7 +3092,8 @@ morefmt:
 	    case '#':
 	    case '_':
 	    case '-':
-	    case '0' ... '9':
+	    case '0': case '1': case '2': case '3': case '4':
+	    case '5': case '6': case '7': case '8': case '9':
 		goto morefmt;
 strftimehandling:
 	    default:
-- 
2.5.0

Re: PATCH: Make ztrftime pass more things to strftime

[Jun T.]

2015/07/08 08:21, Mikael Magnusson <mikachu@gmail.com> wrote:
> % print -P %D\{%x\}
> 2015年07月08日
> % print -P %D\{%Ex\}
> 平成27年07月08日

It works for %Ex but not for %Ey.
ztrftime() does not send %Ey to strftime() but ignores E and
handles %y by itself.

% date +%y
15
% date +%Ey
27
% print -P '%D{%y %Ey}'
15 15

Is it possible to pass the entire format string to strftime()
if HAVE_STRFTIME is defined?

Re: PATCH: Make ztrftime pass more things to strftime

[Mikael Magnusson]

On Wed, Jul 8, 2015 at 4:03 PM, Jun T. <takimoto-j@kba.biglobe.ne.jp> wrote:
>
> 2015/07/08 08:21, Mikael Magnusson <mikachu@gmail.com> wrote:
>> % print -P %D\{%x\}
>> 2015年07月08日
>> % print -P %D\{%Ex\}
>> 平成27年07月08日
>
> It works for %Ex but not for %Ey.
> ztrftime() does not send %Ey to strftime() but ignores E and
> handles %y by itself.
>
> % date +%y
> 15
> % date +%Ey
> 27
> % print -P '%D{%y %Ey}'
> 15 15
>
> Is it possible to pass the entire format string to strftime()
> if HAVE_STRFTIME is defined?

With my latest patch, I can now do this,
% print -P '%D{%y %Ey %f}'
15 27 9
% date +'%y %Ey %f'
15 27 %f

If anyone wants me to make %3f work, I humbly ask them to keep this to
themselves.

Bonus, weird stuff strftime() does:
% date +%014EA #zeropadding the unchanged format string
00000000%014EA
% date +%020Ey #padding Ey is fine
00000000000000000027
% date +%020Oy #but not Oy
十五
% date +%_20Oe #not any O* it seems
九
% date +%020Oe #applies to english locale too
 9

If you find any problems, please check that it doesn't happen with
date with the same format string.

-- 
Mikael Magnusson

Re: PATCH: Make ztrftime pass more things to strftime

[Peter Stephenson]

On Wed, 8 Jul 2015 01:15:00 +0200
Mikael Magnusson <mikachu@gmail.com> wrote:
> I poked at this for an hour, and this is the first version that passes
> make check and doesn't segfault randomly, so it is not very well tested,
> to say the least. Opinions on the approach?

"Not totally doomed" (I haven't done a character-by-character survey of
the goings on).

Implementing on top of an interface like this is a real pain, since you
can't help re-implementing chunks and scratching your head about how to
pass the info you've got out back in again.  But it's worth doing now
internationalisation / internationalization / Internationalisierung /
... is considered basic.

It probably mostly needs a few tests.  print -P %D{...} will do most of
what you want.

pws