-
Notifications
You must be signed in to change notification settings - Fork 227
Remove generic CldrCalendar impl for Hijri and add update docs #7005
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
base: main
Are you sure you want to change the base?
Changes from 4 commits
3bb5417
e939d12
89efadf
daf0da8
6486d68
c28484f
2268368
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,16 +43,19 @@ 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 in the Chinese and Dangi lunisolar calendars. | ||
Cyclic(CyclicYear), | ||
} | ||
|
||
|
@@ -144,6 +147,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)" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are pre-existing docs that I moved. They were previously on There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fine by me There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 { | ||
|
@@ -269,7 +275,7 @@ impl fmt::Display for MonthCode { | |
} | ||
} | ||
|
||
/// Representation of a formattable month. | ||
/// Representation of a formattable month. Returned by [`Date::month()`](crate::Date::month) | ||
sffc marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
#[derive(Copy, Clone, Debug, PartialEq)] | ||
#[non_exhaustive] | ||
pub struct MonthInfo { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -548,7 +548,7 @@ size_test!(YearNames, year_names_v1_size, 32); | |
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. | ||
/// See [`EraYear`](icu_calendar::types::EraYear) for a definition of what chronological order is in this context. | ||
|
||
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. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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>>; | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
|
@@ -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 {} | ||
|
Uh oh!
There was an error while loading. Please reload this page.