Builtin strftime and TZ assignments

Builtin strftime and TZ assignments

[Bart Schaefer]

On Feb 21, 10:23am, Bart Schaefer wrote:
} Subject: Re: Convert UTC time to local time using strftime and zsh/datetime
}
} On Feb 20,  9:02pm, Kurtis Rader wrote:
} }
} } TZ=UTC strftime -r "%Y-%m-%d %H:%M:%S +0000" "2015-02-11 16:42:30 +0000"
} 
} The next question is whether the strftime builtin should implicitly
} perform "local -x TZ" to make Kurtis' example do the expected thing.

So the patch below does this if we want it.  It also has the side-effect
that the builtin strftime respects an otherwise not-exported $TZ, which
may or may not be desirable.

However, I'm hoping someone (PWS?) can explain why the extra pm->level
bump is necessary to get the parameter to be removed from the environment
upon endparamscope().


diff --git a/Src/Modules/datetime.c b/Src/Modules/datetime.c
index 00ebd2b..15edfc8 100644
--- a/Src/Modules/datetime.c
+++ b/Src/Modules/datetime.c
@@ -94,7 +94,7 @@ reverse_strftime(char *nam, char **argv, char *scalar, int quiet)
 }
 
 static int
-bin_strftime(char *nam, char **argv, Options ops, UNUSED(int func))
+output_strftime(char *nam, char **argv, Options ops, UNUSED(int func))
 {
     int bufsize, x;
     char *endptr = NULL, *scalar = NULL, *buffer;
@@ -145,6 +145,27 @@ bin_strftime(char *nam, char **argv, Options ops, UNUSED(int func))
     return 0;
 }
 
+static int
+bin_strftime(char *nam, char **argv, Options ops, int func)
+{
+    int result = 1;
+    char *tz = getsparam("TZ");
+
+    startparamscope();
+    if (tz && *tz) {
+	Param pm = createparam("TZ", PM_LOCAL|PM_SCALAR|PM_EXPORTED);
+	if (pm) {
+	    /* Why is this needed inside startparamscope()? */
+	    pm->level = locallevel + 1; /* copied from Zle/zle_params.c */
+	}
+	setsparam("TZ", ztrdup(tz));
+    }
+    result = output_strftime(nam, argv, ops, func);
+    endparamscope();
+
+    return result;
+}
+
 static zlong
 getcurrentsecs(UNUSED(Param pm))
 {

-- 
Barton E. Schaefer

Re: Builtin strftime and TZ assignments

[Bart Schaefer]

On Feb 21, 12:45pm, Bart Schaefer wrote:
}
} } The next question is whether the strftime builtin should implicitly
} } perform "local -x TZ" to make Kurtis' example do the expected thing.

Regardless, the documentation is wrong about timezone handling.  Here
is an update.

diff --git a/Doc/Zsh/mod_datetime.yo b/Doc/Zsh/mod_datetime.yo
index 6190676..27bc781 100644
--- a/Doc/Zsh/mod_datetime.yo
+++ b/Doc/Zsh/mod_datetime.yo
@@ -8,25 +8,46 @@ findex(strftime)
 cindex(date string, printing)
 xitem(tt(strftime) [ tt(-s) var(scalar) ] var(format) var(epochtime) )
 item(tt(strftime) tt(-r) [ tt(-q) ] [ tt(-s) var(scalar) ] var(format) var(timestring) )(
-Output the date denoted by var(epochtime) in the var(format)
-specified.
+Output the date denoted by var(epochtime) in the var(format) specified.
+See manref(strftime)(3) for details.  The zsh extensions described in
+ifzman(the section EXPANSION OF PROMPT SEQUENCES in zmanref(zshmisc))\
+ifnzman(noderef(Prompt Expansion)) are also available.
 
-With the option tt(-r) (reverse), use the format var(format) to parse the
-input string var(timestring) and output the number of seconds since the
-epoch at which the time occurred.  If no timezone is parsed, the current
-timezone is used; other parameters are set to zero if not present.  If
-var(timestring) does not match var(format) the command returns status 1; it
-will additionally print an error message unless the option tt(-q) (quiet)
-is given.  If var(timestring) matches var(format) but not all characters in
-var(timestring) were used, the conversion succeeds; however, a warning is
-issued unless the option tt(-q) is given.  The matching is implemented by
-the system function tt(strptime); see manref(strptime)(3).  This means that
-zsh format extensions are not available, however for reverse lookup they
-are not required.  If the function is not implemented, the command returns
-status 2 and (unless tt(-q) is given) prints a message.
+startitem()
+item(tt(-q))(
+Run quietly; suppress printing of all error messages described below.
+Errors for invalid var(epochtime) values are always printed.
+)
+item(tt(-r))(
+With the option tt(-r) (reverse), use var(format) to parse the input
+string var(timestring) and output the number of seconds since the epoch at
+which the time occurred.  The parsing is implemented by the system
+function tt(strptime); see manref(strptime)(3).  This means that zsh
+format extensions are not available, but for reverse lookup they are not
+required.
+
+In most implementations of tt(strftime) any timezone in the
+var(timestring) is ignored and the local timezone declared by the tt(TZ)
+environment variable is used; other parameters are set to zero if not
+present.
+
+If var(timestring) does not match var(format) the command returns status 1
+and prints an error message.  If var(timestring) matches var(format) but
+not all characters in var(timestring) were used, the conversion succeeds
+but also prints an error message.
+
+If either of the system functions tt(strptime) or tt(mktime) is not
+available, status 2 is returned and an error message is printed.
+)
+item(tt(-s) var(scalar))(
+Assign the date string (or epoch time in seconds if tt(-r) is given) to
+var(scalar) instead of printing it.
+)
+enditem()
 
-If tt(-s) var(scalar) is given, assign the date string (or epoch time
-in seconds if tt(-r) is given) to var(scalar) instead of printing it.
+Note that depending on the system's declared integral time type,
+tt(strftime) may produce incorrect results for epoch times greater than
+2147483647 which corresponds to 2038-01-19 03:14:07 +0000.
 )
 enditem()
 

-- 
Barton E. Schaefer

Re: Builtin strftime and TZ assignments

[Peter Stephenson]

On Sat, 21 Feb 2015 12:45:46 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> However, I'm hoping someone (PWS?) can explain why the extra pm->level
> bump is necessary to get the parameter to be removed from the environment
> upon endparamscope().

I don't know.  Is it necessary?  The code in params.c that deals with
locallevel suggests it shouldn't be.

>+	    pm->level = locallevel + 1; /* copied from Zle/zle_params.c*/

In zle_params.c the parameter scope isn't supplied, instead it's
supplied by the surrounding code, so this might be caution, historical,
whatever.

pws

Re: Builtin strftime and TZ assignments

[Bart Schaefer]

On Feb 21, 10:27pm, Peter Stephenson wrote:
} Subject: Re: Builtin strftime and TZ assignments
}
} On Sat, 21 Feb 2015 12:45:46 -0800
} Bart Schaefer <schaefer@brasslantern.com> wrote:
} > However, I'm hoping someone (PWS?) can explain why the extra pm->level
} > bump is necessary to get the parameter to be removed from the environment
} > upon endparamscope().
} 
} I don't know.  Is it necessary?

Without it, starting from an unset TZ, doing

torch% TZ=UTC strftime ...

results in TZ remaining set to UTC after strftime completes.  With the
pm->level bump, TZ is back to unset again after strftime completes.

} The code in params.c that deals with
} locallevel suggests it shouldn't be.

If I skip the createparam() [necessary for PM_EXPORT] and instead just
setsparam(), then the parameter is properly removed from scope as I
would expect, but of course then strptime() doesn't work as desired.
So it has something to do with export?

I briefly considered adding an ASSPM_EXPORTED flag and changing the
behavior of assignstrvalue() in params.c, but decided against it.

Re: Builtin strftime and TZ assignments

[Peter Stephenson]

On Sat, 21 Feb 2015 14:54:41 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> Without it, starting from an unset TZ, doing
> 
> torch% TZ=UTC strftime ...
> 
> results in TZ remaining set to UTC after strftime completes.  With the
> pm->level bump, TZ is back to unset again after strftime completes.

When you say "without", do you mean without setting pm->level at all?
That wouldn't work because a level is only assigned if you "typeset" or
equivalent --- a normal assignment creates a global parameter.  But I'd
expect it to work with locallevel instead of locallevel+1.

I don't understand the difference if createparam() isn't there.  It
should always be called by any function that needs to create a parameter.

pws

Re: Builtin strftime and TZ assignments

[Bart Schaefer]

On Feb 21, 11:16pm, Peter Stephenson wrote:
} Subject: Re: Builtin strftime and TZ assignments
}
} On Sat, 21 Feb 2015 14:54:41 -0800
} Bart Schaefer <schaefer@brasslantern.com> wrote:
} > Without it, starting from an unset TZ, doing
} > 
} > torch% TZ=UTC strftime ...
} > 
} > results in TZ remaining set to UTC after strftime completes.  With the
} > pm->level bump, TZ is back to unset again after strftime completes.
} 
} When you say "without", do you mean without setting pm->level at all?

Yes; I assumed incorrectly that passing PM_LOCAL in the flags would cause
createparam() to assign pm->level = locallevel, but obviously I should
have explicitly checked that.  (Why does it not?)

} That wouldn't work because a level is only assigned if you "typeset" or
} equivalent --- a normal assignment creates a global parameter.  But I'd
} expect it to work with locallevel instead of locallevel+1.

It probably does, then, and I was just confused.

} I don't understand the difference if createparam() isn't there.  It
} should always be called by any function that needs to create a parameter.

If you just call setsparam("TZ", ...), it will eventually go through
createparam() and the right thing happens with locallevel, but there's
no way to tell setsparam() [which is just a macro for assignsparam()] to
create the parameter already exported.

Also I now suspect my patch breaks strftime -s, I didn't test that.

Re: Builtin strftime and TZ assignments

[Bart Schaefer]

On Feb 21,  4:39pm, Bart Schaefer wrote:
}
} On Feb 21, 11:16pm, Peter Stephenson wrote:
} }
} } That wouldn't work because a level is only assigned if you "typeset" or
} } equivalent --- a normal assignment creates a global parameter.  But I'd
} } expect it to work with locallevel instead of locallevel+1.

It does in fact work with locallevel.

} } I don't understand the difference if createparam() isn't there.  It
} } should always be called by any function that needs to create a parameter.
} 
} If you just call setsparam("TZ", ...), it will eventually go through
} createparam() and the right thing happens with locallevel

Apparently I was confused about that too -- it's the use of the TZ=UTC
prefix which creates and removes again the parameter, not the use of
the parameter scope in bin_strftime().

} Also I now suspect my patch breaks strftime -s, I didn't test that.

Which it does not.  Except for one special case:  "strftime -s TZ ..."
doesn't change the value of TZ.

Here's a revised patch.  Question:  How does one know when to metafy() the
value passed to setsparam()?  It doesn't appear that getsparam() performs
an unmetafy(), so I presume I can pass directly back to setsparam().


diff --git a/Src/Modules/datetime.c b/Src/Modules/datetime.c
index 00ebd2b..63a04dc 100644
--- a/Src/Modules/datetime.c
+++ b/Src/Modules/datetime.c
@@ -94,7 +94,7 @@ reverse_strftime(char *nam, char **argv, char *scalar, int quiet)
 }
 
 static int
-bin_strftime(char *nam, char **argv, Options ops, UNUSED(int func))
+output_strftime(char *nam, char **argv, Options ops, UNUSED(int func))
 {
     int bufsize, x;
     char *endptr = NULL, *scalar = NULL, *buffer;
@@ -145,6 +145,25 @@ bin_strftime(char *nam, char **argv, Options ops, UNUSED(int func))
     return 0;
 }
 
+static int
+bin_strftime(char *nam, char **argv, Options ops, int func)
+{
+    int result = 1;
+    char *tz = getsparam("TZ");
+
+    startparamscope();
+    if (tz && *tz) {
+	Param pm = createparam("TZ", PM_LOCAL|PM_SCALAR|PM_EXPORTED);
+	if (pm)
+	    pm->level = locallevel; /* because createparam() doesn't */
+	setsparam("TZ", ztrdup(tz));
+    }
+    result = output_strftime(nam, argv, ops, func);
+    endparamscope();
+
+    return result;
+}
+
 static zlong
 getcurrentsecs(UNUSED(Param pm))
 {

Re: Builtin strftime and TZ assignments

[Peter Stephenson]

On Sat, 21 Feb 2015 16:39:22 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> Yes; I assumed incorrectly that passing PM_LOCAL in the flags would cause
> createparam() to assign pm->level = locallevel, but obviously I should
> have explicitly checked that.  (Why does it not?)

Up to now the assignment of local level has been associated with the
"local" command etc., which is what the user sees when declaring the
parameter local, rather than the internal flag.  To decide which is
better you'd have to survey all the uses and see which way gave you the
best match.

I think the PM_LOCAL flag (or maybe some equivalent predecessor) came
about because we were caught between the fact that the "typeset" family
usually made things local and the fact that "export" traditionally
didn't, so we needed an extra way of signalling the difference.  Then
along came the -g option and we needed a way of signalling that, too.
The level was the primary way of signalling whether a parameter needed
removing at the end of the scope and the flag was an additional
convenience.

All this logic got piled into bin_typeset() rather than the core
parameter code, and as I was saying a couple of weeks ago the split
doesn't work very well for many of the internal uses of parameter logic.
Logically, I would guess you don't actually need both: level 0 should
mean global, but the level doesn't appear in the interface to the core
parameter code.

pws