OOC, why this approach with wrapping DateTime.new?

[lizmat]

Putting this in lib/Foo.rakumod:

class DateTime is DateTime {
    method new(|) { dd; nextsame }
}

And then using that module:

$ raku -Ilib -MFoo -e 'dd DateTime.new(now)'
method new(DateTime: |)
method new(DateTime: |)
DateTime.new(2021,12,16,18,58,12.786556813)

shows that the altered module is called. The DateTime class that is exported, is a subclass of the original DateTime class, so everything else just works.

The advantage of this approach, is that you can localize the improved DateTime in a scope:

if $extra {
    use DateTime::Timezones;
    DateTime.new: ... ;  # improved DateTime
}
DateTim.new: ... ;  # original DateTime

But even inside the scope, you can refer to the original one with the CORE:: prefix:

$ raku -Ilib -MFoo -e 'dd DateTime.new(now); dd CORE::DateTime.new(now)'
method new(DateTime: |)
method new(DateTime: |)
DateTime.new(2021,12,16,19,3,50.5971788)
DateTime.new(2021,12,16,19,3,50.612688994)

Wouldn't this significantly simplify things?

[alabamenhu]

The advantage of this approach, is that you can localize the improved DateTime in a scope:

This is actually the main reason for using my more complicated approach. Effectively, I wanted to do

augment class DateTime {
    # all timezone stuff
}

which would ensure that enhanced timezone stuff was available globally. But augment presents the precompilation problem (or at least did, and I should check that now. before I repeat it too much). Wrapping, on the other hand, can occur at run time. Either way, the goal is to ensure that at no point, an enhanced DateTime gets downgraded. With your approach, imagine:

sub day-start(DateTime $dt) {
    DateTime.new: $dt.year, $dt.month, $dt.year, 0, 0, 0
}

use DateTime::Timezones:auth<lizmat>;
my $dt = DateTime.now; # enhanced DateTime
dd day-start $dt;      # ends up with a CORE::DateTime

This would be non-intuitive for someone who wanted to use timezones and also employ some fancy module that helps with time manipulation. Of course, while the above subroutine could have called, e.g., DateTime.clone: :0hour, :0minute, :0second and kept the upgraded class, that sub might not always be in the users control.

Down the line, I may make a DateTime::Timezones::Lexical (tbd: better name) module that allows for the more nuanced control, with the caveats that the above might bring along.

Wouldn't this significantly simplify things?

Sure, but at the loss of the global goal. I've got to play around a bit more with new-disp (aka 2021.10+) to check and see what code is still necessary and get things to play better around. If things will go as I'm hoping —and as my very early tests seemed to indicated before $day-job got crazy— the precomp issues with callsame and friends will allow me to eliminate a lot of unsightly and redundant code.

[lizmat]

I'm not sure the global goal is a good one. Also, right now, using DateTime::Timezones actually breaks DateTime.new:

$ r 'dd "2021-04-22T07:36:50+0200".DateTime'
DateTime.new(2021,4,22,7,36,50,:timezone(7200))

$ r 'use DateTime::Timezones; dd "2021-04-22T07:36:50+0200".DateTime'
DateTime.new(2021,4,22,7,36,50)

Basically, forcing me to do something like fetching the current DateTime.new at compile time, and refer to that to get the normal behaviour.

For reference, I'm trying to integrate DateTime::Timezones into IRC::Log::Textual. The problem is that older versions of the Textual IRC client logged only [HH:MM:SS], whereas newer have a full [YYYY-MM-DDTHH:MM:SS+TZTZ] DateTime compatible string. Textual logs in daily logs, but separated at midnight local time. For IRC::Log, I would like to have logs separated at midnight UTC. So the older logs with just [HH:MM:SS] need some magic to convert to UTC, and I hoped that DateTime::Timezones would be the ticket. So far, that didn't turn out to be the case :-(

[lizmat]

Re your example:

sub day-start(DateTime $dt) {
    DateTime.new: $dt.year, $dt.month, $dt.year, 0, 0, 0
}

If the author had written that as:

sub day-start(DateTime $dt) {
    $dt.WHAT.new: $dt.year, $dt.month, $dt.year, 0, 0, 0
}

this would not be an issue.

[alabamenhu]

If `DateTime.new` is broken that's... not intended, and a bug. I will try to look into this very quickly.

…

On Thu, Dec 16, 2021, 18:21 Elizabeth Mattijsen ***@***.***> wrote: I'm not sure the global goal is a good one. Also, right now, using DateTime::Timezones actually breaks DateTime.new: $ r 'dd "2021-04-22T07:36:50+0200".DateTime' DateTime.new(2021,4,22,7,36,50,:timezone(7200)) $ r 'use DateTime::Timezones; dd "2021-04-22T07:36:50+0200".DateTime' DateTime.new(2021,4,22,7,36,50) Basically, forcing me to do something like fetching the current DateTime.new at compile time, and refer to that to get the normal behaviour. For reference, I'm trying to integrate DateTime::Timezones into IRC::Log::Textual. The problem is that older versions of the Textual IRC client logged only [HH:MM:SS], whereas newer have a full [YYYY-MM-DDTHH:MM:SS+TZTZ] DateTime compatible string. Textual logs in daily logs, but separated at midnight *local* time. For IRC::Log, I would like to have logs separated at midnight UTC. So the older logs with just [HH:MM:SS] need some magic to convert to UTC, and I hoped that DateTime::Timezones would be the ticket. So far, that didn't turn out to be the case :-( — Reply to this email directly, view it on GitHub <#4 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFBLGIYVV4U2U46WE7VVF5TURJX53ANCNFSM5KHCJVJA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you commented.Message ID: ***@***.***>

[lizmat]

Now wrt to unintentional downgrading: I've spent quite some time making DateTime subclassable. If there's some "leakage" somewhere, that's a bug.

class A is DateTime { method foo() { "foo" } }
dd A.now.later(:1hour).foo;  # foo

Even if that class is also called DateTime:

class DateTime is DateTime { method foo() { "foo" } }
dd DateTime.now.later(:1hour).foo;  # foo

[lizmat]

Sure, but at the loss of the global goal. I've got to play around a bit more with new-disp (aka 2021.10+) to check and see what code is still necessary and get things to play better around.

I think the goal of making a better global DateTime is admirable, but then maybe it should have a different name. Maybe a class DateTimeZone is DateTime { ... } would be better, so that everybody knows they're dealing with an improved version?

[lizmat]

isn't this just what you need?

class DateTime is DateTime {
    method new(:$timezone, |c) {
        $timezone
          ?? $timezone ~~ Int
            ?? callwith(|c, :$timezone)
            !! (callwith |c).in-timezone($timezone)
          !! nextsame()
    }

    method in-timezone($timezone) {
        $timezone ~~ Int
          ?? nextsame()
          !! callwith(42)   # magic happens here
    }
}

dd DateTime.new(2021,12,17,15,1,3,:timezone<Europe/Amsterdam>);
# DateTime.new(2021,12,17,15,1,45,:timezone(42))

[alabamenhu]

It's likely with `new-disp` an approach like this will work. On old-disp, there were a lot of issues with `callsame` and `nextwith`, which is the whole reason I was processing the arguments manually, rather than passing to `CORE`. (This is on my docket for today, BTW)

…

On Fri, Dec 17, 2021, 09:20 Elizabeth Mattijsen ***@***.***> wrote: isn't this just what you need? class DateTime is DateTime { method new(:$timezone, |c) { $timezone ?? $timezone ~~ Int ?? callwith(|c, :$timezone) !! (callwith |c).in-timezone($timezone) !! (nextwith |c) } method in-timezone($timezone) { $timezone ~~ Int ?? nextsame() !! callwith(42) # magic happens here } } dd DateTime.new(2021,12,17,15,1,3,:timezone<Europe/Amsterdam>);# DateTime.new(2021,12,17,15,1,45,:timezone(42)) — Reply to this email directly, view it on GitHub <#4 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFBLGI46R56F2UNZKWQXJCTURNBKVANCNFSM5KHCJVJA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you commented.Message ID: ***@***.***>