-
Notifications
You must be signed in to change notification settings - Fork 224
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
Allow other "dates of calendar reform" in Date#strptime #389
Allow other "dates of calendar reform" in Date#strptime #389
Conversation
Timecop is incompatible with the newest patch version of Psych (4.0.5) because of this change: https://github.com/ruby/psych/compare/v4.0.4..v4.0.5#diff-6a459e056cadf37665f54005bd2dde09d9ba8e66c9807eb0dc67145f9b841771L66-R66 Timecop only allows strptime to be called with the default of Date::ITALY as the fourth argument. This fourth argument is the "day of calendar reform" for leap years. Dates pre-reform are Julian calendar dates, which have leap years every 4 years. Dates after reform are Gregorian calendar dates, which have more complicated leap year logic (every 4 years, except if the year is divisible by 100 and not by 400). https://en.wikipedia.org/wiki/Gregorian_calendar Psych starts calling strptime with Date::GREGORIAN as of 4.0.5 (I'm not sure why, but it's a legal use of the standard Ruby API and so it seems like Timecop shouldn't break it). This PR updates strptime_without_mock_date to allow passing in other values for the fourth argument by passing them along to date initialization.
|
||
d = Date._strptime(str, fmt) | ||
now = Time.now.to_date | ||
year = d[:year] || now.year | ||
mon = d[:mon] || now.mon | ||
if d.keys == [:year] | ||
Date.new(year) | ||
Date.new(year, 1, 1, start) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't love hardcoding the 1s here or repeating all the occurrences of start
but this seemed safer than refactoring the method.
Also reproduced with using psych >= 3.3.3 because changes was back-ported to psych: 3.x in ruby/psych#573 |
Please merge and release :) |
I was looking at this yesterday. Meant to update but I got sidetracked, thanks for bringing this up and making a PR. Tested locally and it works. Not sure why our GitHub actions didn't run all tests. Maybe a first time contributor restriction?? |
Thanks @joshuacronemeyer! Appreciate you taking the time to look into this. |
You're welcome @scpike. Hey @johnnyshields we released. Thanks for the friendly nudge. |
Timecop is incompatible with the newest patch version of Psych (4.0.5) because of this change.
Timecop only allows Date#strptime to be called with the default of Date::ITALY as the fourth argument. This fourth argument is the "day of calendar reform" for leap years. Dates pre-reform are Julian calendar dates, which have leap years every 4 years. Dates after reform are Gregorian calendar dates, which have more complicated leap year logic (every 4 years, except if the year is divisible by 100 and not by 400). https://en.wikipedia.org/wiki/Gregorian_calendar
Psych starts calling strptime with Date::GREGORIAN as of 4.0.5 (I'm not sure why, but it's a legal use of the standard Ruby API and so it seems like Timecop shouldn't break it).
This PR updates strptime_without_mock_date to allow passing in other values for the fourth argument by passing them along to date initialization.