Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions components/calendar/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,23 @@ pub struct DateFields<'a> {
pub day: Option<NonZeroU8>,
}

/// The type of year: Calendars like Chinese don't have an era and instead format with cyclic years.
/// Information about the year.
///
/// Returned by [`Date::year()`](crate::Date::year).
///
/// This enum supports calendars based on eras as well as calendars based on cycles.
#[derive(Copy, Clone, Debug, PartialEq)]
#[non_exhaustive]
pub enum YearInfo {
/// An era and a year in that era
/// Information about the year in calendars with eras.
///
/// A majority of calendars use this variant.
Era(EraYear),
/// A cyclic year, and the related ISO year
/// Information about the year in calendars with cycles.
///
/// 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)"
/// This is used by the [`LunarChinese`] calendar.
///
/// [`LunarChinese`]: crate::cal::LunarChinese
Cyclic(CyclicYear),
}

Expand Down Expand Up @@ -144,6 +151,9 @@ pub struct EraYear {
}

/// Year information for a year that is specified as a cyclic year
///
/// 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)"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is an out-struct, so it doesn't need to "pinpoint" a date

I also don't like the phrasing of "disambiguation can be done by saying things like"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are pre-existing docs that I moved. They were previously on YearInfo::Cyclic.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok but this is an "update docs" PR so let's update them

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cyclic year is insufficient to fully specify a date, and cyclic calendars typically do not use eras. In cases where a date needs to be unambiguously specified, one can do things like (...) using the related_iso field.

Copy link
Member

@robertbastian robertbastian Oct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The month is also insufficient to fully specify a date, yet we don't put a disclaimer that you need to add a different field if you're doing formatting.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This type contains the information to fully specify a year (it has a related_iso field).

What the comment is trying to explain is how to display this type "in cases where a date needs to be unambiguously specified", which I really don't think should be explained in this crate, as these cases are very locale-dependent. We also don't explain how EraYear should be displayed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's good context as to why there's a related_iso field

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then move this to the field? Link to other explanations like we do for extended year?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine by me

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine by me

#[derive(Copy, Clone, Debug, PartialEq)]
#[non_exhaustive]
pub struct CyclicYear {
Expand Down Expand Up @@ -270,6 +280,8 @@ impl fmt::Display for MonthCode {
}

/// Representation of a formattable month.
///
/// Returned by [`Date::month()`](crate::Date::month).
#[derive(Copy, Clone, Debug, PartialEq)]
#[non_exhaustive]
pub struct MonthInfo {
Expand Down
5 changes: 2 additions & 3 deletions components/datetime/src/provider/neo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -546,9 +546,8 @@ size_test!(YearNames, year_names_v1_size, 32);
#[cfg_attr(feature = "serde", derive(serde::Deserialize))]
#[yoke(prove_covariance_manually)]
pub enum YearNames<'data> {
/// This calendar has a small, fixed set of eras with numeric years, this stores the era names in chronological order.
///
/// See FormattableEra for a definition of what chronological order is in this context.
/// This calendar has a small, fixed set of eras with numeric years. Eras are stored
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep the first sentence of a doc comment as its own paragraph.

/// according to their [era index](icu_calendar::types::EraYear::era_index).
FixedEras(#[cfg_attr(feature = "serde", serde(borrow))] VarZeroVec<'data, str>),
/// This calendar has a variable set of eras with numeric years, this stores the era names mapped from
/// era code to the name.
Expand Down
34 changes: 27 additions & 7 deletions components/datetime/src/scaffold/calendar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,21 @@ use icu_time::{
DateTime, Time, TimeZoneInfo, ZonedDateTime,
};

/// A calendar that can be found in CLDR.
/// A calendar that can be formatted with CLDR data.
///
/// New implementors of this trait will likely also wish to modify `get_era_code_map()`
/// in the CLDR transformer to support any new era maps.
/// When formatting:
///
/// - The pattern is loaded from [`Self::SkeletaV1`]
/// - The era and year names are loaded from [`Self::YearNamesV1`] and accessed based on [`YearInfo`]
/// - The month name is loaded from [`Self::MonthNamesV1`] and accessed based on [`MonthInfo`]
Comment on lines +25 to +26
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is really internal, I'd rather not have users deal with it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the public interface between YearInfo/YearNames and MonthInfo/MonthNames. It ought to be documented. Users need to worry about it only if they're implementing a custom calendar.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You say public interface, I say unstable provider code.

///
/// <div class="stab unstable">
/// 🚫 This trait is sealed; it cannot be implemented by user code. If an API requests an item that implements this
/// trait, please consider using a type from the implementors listed below.
/// 🚧 This trait is considered unstable; it may change at any time, in breaking or non-breaking ways,
/// including in SemVer minor releases. Do not implement this trait in userland unless you are prepared for things to occasionally break.
/// </div>
///
/// [`YearInfo`]: icu_calendar::types::YearInfo
/// [`MonthInfo`]: icu_calendar::types::MonthInfo
pub trait CldrCalendar: UnstableSealed {
/// The data marker for loading year symbols for this calendar.
type YearNamesV1: DataMarker<DataStruct = YearNames<'static>>;
Expand Down Expand Up @@ -91,7 +97,19 @@ impl CldrCalendar for Indian {
type SkeletaV1 = DatetimePatternsDateIndianV1;
}

impl<R: hijri::Rules> CldrCalendar for Hijri<R> {
impl CldrCalendar for Hijri<hijri::UmmAlQura> {
type YearNamesV1 = DatetimeNamesYearHijriV1;
type MonthNamesV1 = DatetimeNamesMonthHijriV1;
type SkeletaV1 = DatetimePatternsDateHijriV1;
}
Comment on lines +100 to +104
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: This trait is very easy to implement in userland, easier than in 1.0. All you need to do is look at the menu of available data markers and pick the ones you want. If you have special needs, you can even mix and match stock data markers and custom data markers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nothing that involves the word data marker is easy for users in my experience


impl CldrCalendar for Hijri<hijri::TabularAlgorithm> {
type YearNamesV1 = DatetimeNamesYearHijriV1;
type MonthNamesV1 = DatetimeNamesMonthHijriV1;
type SkeletaV1 = DatetimePatternsDateHijriV1;
}

impl CldrCalendar for Hijri<hijri::AstronomicalSimulation> {
type YearNamesV1 = DatetimeNamesYearHijriV1;
type MonthNamesV1 = DatetimeNamesMonthHijriV1;
type SkeletaV1 = DatetimePatternsDateHijriV1;
Expand Down Expand Up @@ -130,7 +148,9 @@ impl UnstableSealed for Ethiopian {}
impl UnstableSealed for Gregorian {}
impl UnstableSealed for Hebrew {}
impl UnstableSealed for Indian {}
impl<R: hijri::Rules> UnstableSealed for Hijri<R> {}
impl UnstableSealed for Hijri<hijri::UmmAlQura> {}
impl UnstableSealed for Hijri<hijri::TabularAlgorithm> {}
impl UnstableSealed for Hijri<hijri::AstronomicalSimulation> {}
impl UnstableSealed for Japanese {}
impl UnstableSealed for JapaneseExtended {}
impl UnstableSealed for Persian {}
Expand Down