search

JoeStrout / miniscript

source code of both C# and C++ implementations of the MiniScript scripting language
MIT License
272 stars 63 forks source link

[C++] `dateTime.hour(date)` is off by one hour #159

Open kseistrup opened 1 month ago

kseistrup commented 1 month ago

In the following I expected dateTime.hour(now) to return 16, instead I got 17:

» miniscript
MiniScript 
Command-Line (Linux) v1.3; language v1.6.2 (Jul 13 2024)
> import "dateTime"
> now = dateTime.now
> now
2024-07-13 16:51:19
> dateTime.hour(now)
17
kseistrup commented 1 month ago

Oh, dateTime.hour() expects a .nowVal, not a .now…

marcgurevitx commented 1 month ago

.hour() calls _dateStr() which should handle both of those...

@kseistrup : Is your area affected by "daylight savings"? If so, maybe this issue should be reopened.

kseistrup commented 1 month ago

@marcgurevitx

Is your area affected by "daylight savings"?

Yes, I'm in Europe/Copenhagen: currently CEST (UTC+2), normally CET (UTC+1).

I am completely new to miniscript (after it was mentioned on lobste.rs) and wrote a short script in miniscript:

https://codeberg.org/kas/qtime/src/branch/master/miniscript/qtime.ms

It works with .nowVal, but not with .now. I will reopen this issue then.

edit: typo

marcgurevitx commented 1 month ago

@kseistrup Can you edit the source and add

dateTime.tm_isdst = -1;

right before this call to mktime() in DateTimeUtils.cpp, line 307 ?

It fixes the issue on my machine when I "pretend" that I'm in Copenhagen. Want to be sure that for the real Copenhagen it works as well.

kseistrup commented 1 month ago

@marcgurevitx

After adding that line dateTime.hour(date) gives the right answer both when date is a .now and a .nowVal.

Using faketime, it also seems to work for dates when Denmark has switched back to normal time. This goes well with what mktime(3) says abouit the tm_isdst field, but I'm unsure how reliable results obtained with faketime is in this context.

For now the addition seems to have solved the issue.

Thanks :pray: