-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Surprising result from %s format #24
Comments
primary fix was to make the datetime UTC however, this exposed an oddity (maybe a bug?) in Time::Piece: - see Dual-Life/Time-Piece#24 for a full explanation so now we have our own `strftime` implementation which is not just a pass-through - now, for us, `->strftime("%s")` is always equivalent to `->epoch` (note that my use of look-behind means you have to have at least Perl 5.005) (but that doesn't seem like too much of a hardship)
Someone added a test case that was failing because of this :) What I am doing right now is just parsing out the %s before hand and replacing it with an epoch that is calculated on class instantiation. There was also a problem within strptime. The epoch was only passed to gmtime (in C) and never tp localtime. I fixed that as well. I think..still testing though. On my local american machine:
And on a machine configured to UTC and using a German locale:
user@e643:~/Time-Piece$ |
Should be fixed in 1.31_03 |
as long as we demand Time::Piece v1.32 or higher - cf Dual-Life/Time-Piece#24
Okay, I figure I've got about a 50/50 shot of this one being considered a bug. :-)
Now, the evidence that this isn't a bug is this:
So, on the one hand,
Time::Piece::strftime
is just doing the same thing thatPOSIX::strftime
is. On the other hand, it's a pretty non-intuitive result. :-)The problem, of course, is that the underlying
strftime
is just receiving a structure of the individual time values, and has no concept of whether it's local time or UTC. But, as a counterpoint, a Time::Piece object does know whether it's local or UTC, so I think there's a reasonable argument to be made thatTime::Piece::strftime
ought to be smarter thanPOSIX::strftime
.So, one option is to pre-handle
%s
in a format string similar to how it currently does for%z
and%Z
:It's a fairly simple change, but of course it does constitute a definite change in behavior. I can't imagine anyone who would actually be counting on the existing behavior, but maybe there is someone somewhere.
So another alternative would be just to document the weirdness, perhaps in the "CAVEATS" section of the POD. (We could add a new subsection for
strftime
and also mention #23, even.)If either of these seems like a good idea to anyone, I'm happy to submit a patch for whichever gains the concensus.
The text was updated successfully, but these errors were encountered: