Implement strftime

[x-hgg-x]

Implement strftime.

[lopopolo]

Thanks for working on this @x-hgg-x! I'm excited to see where this goes.

Just to clarify, is your intent to match the behavior of all the time specifiers found in https://github.com/artichoke/strftime-ruby/blob/trunk/vendor/ruby-3.1.2/strftime.c?

[x-hgg-x]

Yes, I use the specification at https://ruby-doc.org/core-3.1.2/Time.html#method-i-strftime, with verification using the interpreter.

[lopopolo]

Awesome thank you!

[x-hgg-x]

Potential inconsistency with Ruby 3.1.2:

Time.new(2021,10,21,1,1,1,'Z').wday # 7 (Sunday)
Time.at(Time.new(2021,10,21,1,1,1,'Z'), in: 'Z').wday # 4 (Thursday)

[x-hgg-x]

It seems that many specifiers are invalid when the time zone is UTC:

Time.new(2019,12,31,12,1,1,'Z').strftime("%G %g %V %U %W %u %w %j %a %A") # "2019 19 00 00 00 7 6 001 ? ?"
Time.new(2019,12,31,12,1,1,'A').strftime("%G %g %V %U %W %u %w %j %a %A") # "2020 20 01 52 52 2 2 365 Tue Tuesday"

[lopopolo]

It seems that many specifiers are invalid when the time zone is UTC:

Time.new(2019,12,31,12,1,1,'Z').strftime("%G %g %V %U %W %u %w %j %a %A") # "2019 19 00 00 00 7 6 001 ? ?"
Time.new(2019,12,31,12,1,1,'A').strftime("%G %g %V %U %W %u %w %j %a %A") # "2020 20 01 52 52 2 2 365 Tue Tuesday"

yea something weird is going on there:

[3.1.2] > Time.new(2019,12,31,12,1,1,'Z').asctime
=> "? Dec 31 12:01:01 2019"
[3.1.2] > Time.new(2019,12,31,12,1,1,'A').asctime
=> "Tue Dec 31 12:01:01 2019"

[lopopolo]

Potential inconsistency with Ruby 3.1.2:

Time.new(2021,10,21,1,1,1,'Z').wday # 7 (Sunday)
Time.at(Time.new(2021,10,21,1,1,1,'Z'), in: 'Z').wday # 4 (Thursday)

Adding some more examples of this weirdness:

[3.1.2] > Time.new(2021,10,21,1,1,1,'Z').asctime
=> "? Oct 21 01:01:01 2021"
[3.1.2] > Time.at(Time.new(2021,10,21,1,1,1,'Z'), in: 'Z').asctime
=> "Thu Oct 21 01:01:01 2021"
[3.1.2] > Time.new(2021,10,21,1,1,1, in: 'Z').asctime
=> "? Oct 21 01:01:01 2021"
[3.1.2] > Time.at(Time.new(2021,10,21,1,1,1,in: 'Z'), in: 'Z').asctime
=> "Thu Oct 21 01:01:01 2021"

[lopopolo]

yea something weird is going on there

I'm taking a guess here but I bet Time::new manually populates the tm struct from time.h whereas Time::at actually does timezone resolution.

Regardless, I think what spinoso-time and tz-rs + tzdb are doing is more correct and I think it's too much complexity to emulate MRI's (broken) behavior.

[lopopolo]

this is looking great so far!

[x-hgg-x]

The doc of spinoso_time::tzrs::Time::day_of_year() is inconsistent with its implementation:
https://github.com/artichoke/artichoke/blob/b3284a42fb0d175da1a822bda73bd300ddab72c3/spinoso-time/src/time/tzrs/parts.rs#L511-L533.

tz::datetime::DateTime returns a year day in [0, 365], but the spinoso_time doc say it should be in 1..366 (1..=366 is more correct I think).

[lopopolo]

The doc of spinoso_time::tzrs::Time::day_of_year() is inconsistent with its implementation: https://github.com/artichoke/artichoke/blob/b3284a42fb0d175da1a822bda73bd300ddab72c3/spinoso-time/src/time/tzrs/parts.rs#L511-L533.

tz::datetime::DateTime returns a year day in [0, 365], but the spinoso_time doc say it should be in 1..366 (1..=366 is more correct I think).

Bug and PR to fix this:

• The doc of spinoso_time::tzrs::Time::day_of_year() is inconsistent with its implementation artichoke#2072
• Fix tzrs::Time::day_of_year to be 1-indexed artichoke#2073

[lopopolo]

Fix is merged in artichoke/artichoke@17e469b.

[lopopolo]

Thank you so much for the PR @x-hgg-x.

Sorry for the boatload of comments, but this code is tricky and I want to make sure I grok the implementation.

I'd love to grant you commit rights to this repository if that's something you'd be open to. Let me know what you'd like me to do there.

I'll hold off on any more reviewing until you move the PR out of draft to "ready for review".

Thanks again!

[lopopolo]

tweaks to get the docs to build without std feature.

[lopopolo]

I can prep the release after the tests pass and this gets merged. ooc, given that a trait is part of the public API, should we release as 1.0.0?

[x-hgg-x]

I think the API is pretty complete at this point, so this should be ok.

[lopopolo]

I have a couple of questions. Apart from the suspicious calculation of size_limit, this looks good to go.

Once all of the conversations are resolved, I'll merge this PR.

Huge thank you @x-hgg-x. This is some amazing work.

[lopopolo]

This looks great. Now that #15 is merged, the MSRV check should pass. Please rebase and then I'll merge.

[lopopolo]

🎉

@x-hgg-x thank you again so much. I'll make you a maintainer here later today when I'm done with work.

There are a few changes I'd like to try and make, e.g. removing the bitflags dep. I'll tag you as a reviewer.

Hopefully we can push out a 1.0 by the end of this week or next.