icu4x
icu4x copied to clipboard
unicode-org
ArithmeticDate::offset_date() doesn't handle wraparounds for months
The following code:
https://github.com/unicode-org/icu4x/blob/fcdbad1be0b92752f18ba8c63d36352f2781d5ff/components/calendar/src/calendar_arithmetic.rs#L28-L33
will immediately panic if the month offset is negative and less than the number of current months, since there will be an arithmetic overflow.
What this code needs to do is something similar to the ISO code:
https://github.com/unicode-org/icu4x/blob/fcdbad1be0b92752f18ba8c63d36352f2781d5ff/components/calendar/src/iso.rs#L242
where there's a specialty function that allows one to add or subtract months whilst wrapping years.
Note that instead of the hardcoded number 12 we must use the months_for_every_year() value. Furthermore, that value isn't guaranteed to always be constant so when we start supporting lunar calendars we will need to handle that too. I'm tracking that in https://github.com/unicode-org/icu4x/issues/2066
cc @pandusonu2 do you want to work on this? I'd prioritize the 1.0 stuff you're doing, but this is code you're generally familiar with.
The changes dont seem to be anything major, can keep it open for any interested contributor maybe? Would be a good first issue.
Not major if we're just trying to handle solar calendars, they get trickier for lunar. But yeah, makes sense! You can help mentor too :smile:
The original calendrical calculations LISP code has for each calendar implementation a pair of fixed-from-CAL and CAL-from-fixed methods.
E.g. for the Gregorian calendar:
You can also think of it as gregorian-to-fixed and gregorian-from-fixed.
In this project, the go-between is an ISO date instant, that is either identified with date_to_iso or converted from in date_from_iso.
Now, if all calendars had a fixed day counter along with the ISO date, we could use that fixed day counter first, then add / subtract a duration's years, months and days individually.
Finally, we could call the calendar's CAL-from-fixed method passing the resulting fixed day counter as argument.
Would that not be easier ?
I see a drawback, with each instantiation of a date in date_from_codes this fixed day counter has to be calculated additionally.
