Skip to content

Commit

Permalink
Use alternate test for DST change
Browse files Browse the repository at this point in the history
As AlexDaniel pointed out, what if there is more than an hour between
calls?  Fixed by taking the absolute hour value since epoch and compare
against that.
  • Loading branch information
lizmat committed Nov 2, 2020
1 parent 151fd31 commit 89211e2
Showing 1 changed file with 7 additions and 6 deletions.
13 changes: 7 additions & 6 deletions src/core.c/DateTime.pm6
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ my class DateTime does Dateish {
# $dt.utc.local.utc is equivalent to $dt.utc. Otherwise,
# DST-induced ambiguity could ruin our day.

my int $last-minute-seen = 60;
my int $last-hour-seen;
my int $TZ-was-set-explicitly;
my int $TZ-offset;
sub get-local-timezone-offset() {
Expand All @@ -17,12 +17,13 @@ my class DateTime does Dateish {

# not set explicitly
else {
my int $utc = nqp::time_i;
my $lt := nqp::decodelocaltime($utc);
my int $utc = nqp::time_i;
my int $hour = nqp::div_i($utc,3600);

This comment has been minimized.

Copy link
@AlexDaniel

AlexDaniel Nov 2, 2020

Contributor

Thanks! But I still don't understand it. By the way, explaining the behavior in the comments is also a way to notice and avoid bugs (see Rubber duck debugging).

So, nqp::time_i is the amount of seconds from something… I'll need to check to see what it is. Doesn't matter for now. So you divide that by 3600 and get hours, and the ≠ logic seems to be right to get it to trigger every hour.

The amount of seconds of time_i is synced with the amount of seconds of DateTime.now, so that's good. However, timezones are not limited to hours. For example, UTC+5:30 in India?

Now, given that this code is ticking strictly hourly from an arbitrary point, doesn't it mean that the check in fractional timezones is going to misfire or not fire at the right time? Maybe it's OK, in which case yeah, some comments can be very helpful.

This comment has been minimized.

Copy link
@lizmat

lizmat Nov 2, 2020

Author Contributor

Good points. I guess we should trust the system. I just realized that nqp::decodelocaltime actually reports whether we are in DST or not. So we only need to track changes in there.

So implemented with ede52fb02e


# first time, or possible DST change
if nqp::atpos_i($lt,1) < $last-minute-seen {
$last-minute-seen = nqp::atpos_i($lt,1);
if nqp::isne_i($hour,$last-hour-seen) {
my $lt := nqp::decodelocaltime($utc);
$last-hour-seen = $hour;

# algorithm from Claus Tøndering
my int $a = (14 - nqp::atpos_i($lt,4)) div 12;
Expand All @@ -33,7 +34,7 @@ my class DateTime does Dateish {
$TZ-offset = (
($jd - 2440588) * 86400
+ nqp::atpos_i($lt,2) * 3600
+ $last-minute-seen * 60
+ nqp::atpos_i($lt,1) * 60
+ nqp::atpos_i($lt,0)
) - $utc
}
Expand Down

0 comments on commit 89211e2

Please sign in to comment.