Refactor YearInfo to separate cyclic/Temporal/Formatting eras #5509
Conversation
Manishearth
force-pushed
the
year-refactor
branch
2 times, most recently
from
3540469 to
02def68
Compare
Manishearth
force-pushed
the
year-refactor
branch
from
02def68 to
43a4175
Compare
Manishearth
force-pushed
the
year-refactor
branch
from
9b0f89c to
5693d75
Compare
There was a problem hiding this comment.
I think the main point of splitting Month/FormattableMonth and Year/FormattableYear is that the month codes and era codes are in different namespaces.
For example:
- Hebrew month code is "M06" for Temporal but "M06L" for data
- Islamic era code is the Islamic calendar name for Temporal but could be anything for data
I wonder if maybe the right design is, rather than splitting Year/FormattableYear, to just make a struct with a field for both namespaces of codes?
components/calendar/src/types.rs
Outdated
| pub related_iso: Option<i32>, | ||
| /// Knowing the cyclic year is typically not enough to pinpoint a date, however cyclic calendars | ||
| /// don't typically use eras, so disambiguation can be done by saying things like "Year 甲辰 (2024)" | ||
| Cyclic(NonZeroU8, i32), |
There was a problem hiding this comment.
Can you explain why the cyclic year is NonZeroU8? Is it just because Chinese and Dangi count from 1 to 60?
There was a problem hiding this comment.
It's what it was before.
People don't really use zeroes in calendar systems.
components/calendar/src/types.rs
Outdated
| } | ||
| } | ||
| } | ||
|
|
||
| impl FormattableYear { | ||
| /// Construct a new Year given an era and number | ||
| pub fn new_era(extended_year: i32, era: TinyStr16, number: i32) -> Self { |
There was a problem hiding this comment.
Nit: I think this should probably take Era not TinyStr16
There was a problem hiding this comment.
This mostly gets us slightly cleaner ctor calls everywhere
I think there were some other reasons too, things were diverging over time. I'm not opposed to keeping these merged, but perhaps we can discuss this in more depth later today. |
Manishearth
force-pushed
the
year-refactor
branch
from
d411e61 to
c58ae78
Compare
Manishearth
changed the title
Manishearth
force-pushed
the
year-refactor
branch
from
159bfaa to
5abf6a4
Compare
components/calendar/src/ethiopian.rs
Outdated
| }); | ||
| } | ||
| 1 - year | ||
| } else if era.0 == tinystr!(16, "mundi") || era.0 == tinystr!(16, "ethiopicaa") { |
There was a problem hiding this comment.
This isn't what the spec says
There was a problem hiding this comment.
Updated, but the spec should also be fixed, then.
There was a problem hiding this comment.
tc39/proposal-intl-era-monthcode#19 , seems like there already was a PR to fix this
There was a problem hiding this comment.
Yes, I thought that PR was merged long ago but no one ever hit the button.
Manishearth
force-pushed
the
year-refactor
branch
from
6e7a842 to
1ab0166
Compare
Part of #3821
This produces a more precise YearInfo that produces multiple era types, and separates out cyclic from non-cyclic. Consumers of this data get to be more specific about the fallback behavior they want when asking for eras from cyclic calendars (etc).