-
Notifications
You must be signed in to change notification settings - Fork 149
-
Notifications
You must be signed in to change notification settings - Fork 149
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
CalendarFieldDescriptors: Require eraYear to be alway positive? #2900
Comments
@sffc How does ICU4X handle far-past years in calendars which have positive-counting (not inverse) eras as their first era? Does it refuse to create dates before the first year of those eras? If not, then |
Looking at the docs for the Ethiopian calendar:
What is the Or should it throw because the world hadn't been created yet so time didn't exist? 😄 |
The ISO date Output:
use icu::calendar::{Calendar, Date};
use icu::calendar::ethiopian::{Ethiopian, EthiopianEraStyle};
fn main() {
let iso = Date::try_new_iso_date(-6000, 1, 1).unwrap();
let styles = [
EthiopianEraStyle::AmeteMihret,
EthiopianEraStyle::AmeteAlem,
];
for style in styles {
let cal = Ethiopian::new_with_era_style(style);
let date = cal.date_from_iso(iso);
let era = cal.year(&date).era.0;
println!(
"{}: {}-{}-{} {}",
format!("{style:?}"),
cal.year(&date).number,
cal.month(&date).ordinal,
cal.day_of_month(&date).0,
format!("{era:?}"),
);
}
} |
Such years are usually nonsense. But, we've been working for a long time that all ISO dates are representable in all calendars, and calendars just need to do something reasonable (the definition of "reasonable" is a topic for another thread, #2869). |
This only applies to calendars with a single era, right? CalendarDateEraYear requires to return |
For some additional discussion on Ethiopian, please see tc39/proposal-intl-era-monthcode#4. I think we probably want to support negative eraYear with mixed eras, definitely on input and probably on output, though I could be convinced otherwise if we can work out Ethiopian. What is the advantage to adding this restriction, though? |
Can you explain "mixed eras"? I'm not sure what you mean and I don't want to misinterpret your comment.
Adding this restriction avoids having to define how to interpret inputs like "-100 BCE" or "-123 Reiwa". |
It seems to me the issue isn't so much negative eraYears as much as it is eras whose eraYear increase with earlier times and decrease as time moves forward. So BCE would be a prime candidate as So I think it's more about:
Now in my personal view, any calendar should be able to handle those cases smoothly and I'd be surprised if ICU4X didn't do this well already. |
Meeting, 2024-09-19: We don't want to add this restriction. So in the Gregorian calendar, an input of -5 CE should resolve to 6 BCE, and an input of -5 BCE should resolve to 6 CE. I'm pretty sure this needs test262 coverage. We do want to avoid outputting negative |
CalendarFieldDescriptors:
Should
eraYear
requireToPositiveIntegerWithTruncation
instead ofToIntegerWithTruncation
?The current SpiderMonkey implementations uses ICU4X, which doesn't allow negative era years. The consensus from #1231 (comment) matches my understanding:
But the implementation notes from #1231 (comment) suddenly state that negative era years are allowed:
Maybe this different implementation is because the polyfill accepts
eraYear
as an equivalent toyear
(#2870)?I'd prefer to restrict signed years for algorithmic use to the
year
property anderaYear
to count up from the start of the era.The polyfill allows negative
eraYear
inputs in some cases. I'm not sure about the exact rules, but I guess the polyfill allows negativeeraYear
for all eras but the very first era. For example this works in the polyfill:Whereas this throws:
The text was updated successfully, but these errors were encountered: