search

HowardHinnant / date

A date and time library based on the C++11/14/17 <chrono> header
Other
3.07k stars 669 forks source link

Any interest in `iso_week::year::is_leap()`? #784

Closed DavisVaughan closed 1 year ago

DavisVaughan commented 1 year ago

I tweaked the iso_week.h calendar to expose year::is_leap() directly, and then reuse it when determining year_lastweek::weeknum(). Would you be interested in having this upstream? It looks like the other calendars (like julian.h) have this function.

Implemented here: https://github.com/r-lib/tzdb/pull/27

I used it here https://github.com/r-lib/clock/pull/333, where I have calendar_leap_year() which takes an arbitrary calendar and reports on whether or not the year is a leap year, if the calendar supports such a thing. Having is_leap() made it slightly more straightforward to implement.

HowardHinnant commented 1 year ago

This looks great, I'm sold!

DavisVaughan commented 1 year ago

Ok great I'll do a PR

For historical purposes, I also found this old commit that removed an iso_week::year::is_leap() method, but that one looks to have been treating the iso year in the same way as a Gregorian year, so that wasn't right. Likely a copy paste error when making the iso calendar? Anyways, just recording the finding for future readers: https://github.com/HowardHinnant/date/commit/323e2fa5fd7786e40014b02937a404ceb4c225d3

HowardHinnant commented 1 year ago

Yeah, I don't remember the details, but it likely got in there due to copy/paste from date::year.