Skip to content
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

DateTimeFormatterPreferences #5842

Merged
merged 11 commits into from
Nov 20, 2024
Merged
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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ use icu::datetime::{DateTimeFormatter, NeoSkeletonLength, fieldsets::YMDT};
use icu::locale::locale;

let dtf = DateTimeFormatter::try_new(
&locale!("es").into(),
locale!("es").into(),
YMDT::long()
)
.expect("locale should be present in compiled data");
Expand Down
75 changes: 62 additions & 13 deletions components/calendar/src/any_calendar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,26 @@ use crate::roc::Roc;
use crate::{types, AsCalendar, Calendar, Date, DateDuration, DateDurationUnit, DateTime, Ref};

use icu_locale_core::extensions::unicode::{key, value, Value};
use icu_locale_core::preferences::define_preferences;
use icu_locale_core::preferences::extensions::unicode::keywords::{
CalendarAlgorithm, IslamicCalendarAlgorithm,
};
use icu_locale_core::subtags::language;
use icu_locale_core::Locale;
use icu_provider::prelude::*;

use core::fmt;

define_preferences!(
/// The prefs for datetime formatting.
[Copy]
AnyCalendarPreferences,
{
/// The user's preferred calendar system.
calendar_algorithm: CalendarAlgorithm
}
);

/// This is a calendar that encompasses all formattable calendars supported by this crate
///
/// This allows for the construction of [`Date`] objects that have their calendar known at runtime.
Expand All @@ -46,7 +60,7 @@ use core::fmt;
///
/// let locale = locale!("en-u-ca-japanese"); // English with the Japanese calendar
///
/// let calendar = AnyCalendar::new_for_locale(&locale.into());
/// let calendar = AnyCalendar::new_for_locale(locale.into());
/// let calendar = Rc::new(calendar); // Avoid cloning it each time
/// // If everything is a local reference, you may use icu::calendar::Ref instead.
///
Expand Down Expand Up @@ -748,13 +762,13 @@ impl AnyCalendar {
///
/// [📚 Help choosing a constructor](icu_provider::constructors)
#[cfg(feature = "compiled_data")]
pub fn new_for_locale(locale: &DataLocale) -> Self {
let kind = AnyCalendarKind::from_data_locale_with_fallback(locale);
pub fn new_for_locale(prefs: AnyCalendarPreferences) -> Self {
Copy link
Member Author

Choose a reason for hiding this comment

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

Should I rename this function?

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 so - just new?

Copy link
Member Author

@sffc sffc Nov 20, 2024

Choose a reason for hiding this comment

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

@Manishearth What do you think about the name of this function? (a constructor for AnyCalendar)

Note that there are also constructors from AnyCalendarKind.

Copy link
Member

Choose a reason for hiding this comment

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

I like this! Though the locale and prefs is somewhat confusing. But we're going to be reasonably clear that Preferences are a locale-derivable concept so that's fine.

new() is also fine and maaaaybe better?

Copy link
Member Author

Choose a reason for hiding this comment

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

We already have

    #[cfg(feature = "compiled_data")]
    pub const fn new(kind: AnyCalendarKind) -> Self

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with that being renamed if we need. new_for_kind(kind) vs new(prefs) or new(kind) vs new_for_prefs(prefs)/new_for_locale(prefs)

Copy link
Member Author

Choose a reason for hiding this comment

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

I made #5843 to discuss this before 2.0 (but after 2.0 Beta) and unblock this PR

let kind = AnyCalendarKind::from_prefs_with_fallback(prefs);
Self::new(kind)
}

icu_provider::gen_any_buffer_data_constructors!(
(locale) -> error: DataError,
(prefs: AnyCalendarPreferences) -> error: DataError,
functions: [
new_for_locale: skip,
try_new_for_locale_with_any_provider,
Expand All @@ -767,7 +781,7 @@ impl AnyCalendar {
#[doc = icu_provider::gen_any_buffer_unstable_docs!(UNSTABLE, Self::new_for_locale)]
pub fn try_new_for_locale_unstable<P>(
provider: &P,
locale: &DataLocale,
prefs: AnyCalendarPreferences,
) -> Result<Self, DataError>
where
P: DataProvider<crate::provider::JapaneseErasV1Marker>
Expand All @@ -778,7 +792,7 @@ impl AnyCalendar {
+ DataProvider<crate::provider::IslamicUmmAlQuraCacheV1Marker>
+ ?Sized,
{
let kind = AnyCalendarKind::from_data_locale_with_fallback(locale);
let kind = AnyCalendarKind::from_prefs_with_fallback(prefs);
Self::try_new_unstable(provider, kind)
}

Expand Down Expand Up @@ -1028,6 +1042,40 @@ impl AnyCalendarKind {
}
}

fn get_for_preferences_calendar(pcal: CalendarAlgorithm) -> Option<Self> {
match pcal {
CalendarAlgorithm::Buddhist => Some(Self::Buddhist),
CalendarAlgorithm::Chinese => Some(Self::Chinese),
CalendarAlgorithm::Coptic => Some(Self::Coptic),
CalendarAlgorithm::Dangi => Some(Self::Dangi),
CalendarAlgorithm::Ethioaa => Some(Self::EthiopianAmeteAlem),
CalendarAlgorithm::Ethiopic => Some(Self::Ethiopian),
CalendarAlgorithm::Gregory => Some(Self::Gregorian),
CalendarAlgorithm::Hebrew => Some(Self::Hebrew),
CalendarAlgorithm::Indian => Some(Self::Indian),
CalendarAlgorithm::Islamic(None) => Some(Self::IslamicObservational),
CalendarAlgorithm::Islamic(Some(islamic)) => match islamic {
IslamicCalendarAlgorithm::Umalqura => Some(Self::IslamicUmmAlQura),
IslamicCalendarAlgorithm::Tbla => Some(Self::IslamicTabular),
IslamicCalendarAlgorithm::Civil => Some(Self::IslamicCivil),
// Rgsa is not supported
IslamicCalendarAlgorithm::Rgsa => None,
_ => {
debug_assert!(false, "unknown calendar algorithm {pcal:?}");
None
}
},
CalendarAlgorithm::Iso8601 => Some(Self::Iso),
CalendarAlgorithm::Japanese => Some(Self::Japanese),
CalendarAlgorithm::Persian => Some(Self::Persian),
CalendarAlgorithm::Roc => Some(Self::Roc),
_ => {
debug_assert!(false, "unknown calendar algorithm {pcal:?}");
None
}
}
}

fn debug_name(self) -> &'static str {
match self {
AnyCalendarKind::Buddhist => Buddhist.debug_name(),
Expand Down Expand Up @@ -1062,21 +1110,22 @@ impl AnyCalendarKind {
.and_then(Self::get_for_bcp47_value)
}

/// Extract the calendar component from a [`DataLocale`]
/// Extract the calendar component from a [`AnyCalendarPreferences`]
///
/// Returns `None` if the calendar is not specified or unknown.
fn get_for_data_locale(l: &DataLocale) -> Option<Self> {
l.get_unicode_ext(&key!("ca"))
.and_then(|v| Self::get_for_bcp47_value(&v))
fn get_for_prefs(prefs: AnyCalendarPreferences) -> Option<Self> {
prefs
.calendar_algorithm
.and_then(Self::get_for_preferences_calendar)
}

// Do not make public, this will eventually need fallback
// data from the provider
fn from_data_locale_with_fallback(l: &DataLocale) -> Self {
if let Some(kind) = Self::get_for_data_locale(l) {
fn from_prefs_with_fallback(prefs: AnyCalendarPreferences) -> Self {
if let Some(kind) = Self::get_for_prefs(prefs) {
kind
} else {
let lang = l.language;
let lang = prefs.locale_prefs.language;
if lang == language!("th") {
Self::Buddhist
} else if lang == language!("sa") {
Expand Down
2 changes: 1 addition & 1 deletion components/calendar/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ pub mod week {
#[cfg(feature = "ixdtf")]
pub use crate::ixdtf::ParseError;
#[doc(no_inline)]
pub use any_calendar::{AnyCalendar, AnyCalendarKind};
pub use any_calendar::{AnyCalendar, AnyCalendarKind, AnyCalendarPreferences};
pub use calendar::Calendar;
pub use date::{AsCalendar, Date, Ref};
pub use datetime::DateTime;
Expand Down
2 changes: 1 addition & 1 deletion components/datetime/README.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion components/datetime/benches/datetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ fn datetime_benches(c: &mut Criterion) {

let dtf = {
FixedCalendarDateTimeFormatter::<Gregorian, _>::try_new(
&locale.into(),
locale.into(),
skeleton,
)
.expect("Failed to create FixedCalendarDateTimeFormatter.")
Expand Down
2 changes: 1 addition & 1 deletion components/datetime/examples/work_log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const DATES_ISO: &[(i32, u8, u8, u8, u8, u8)] = &[
];

fn main() {
let dtf = FixedCalendarDateTimeFormatter::try_new(&locale!("en").into(), YMDT::medium())
let dtf = FixedCalendarDateTimeFormatter::try_new(locale!("en").into(), YMDT::medium())
.expect("Failed to create FixedCalendarDateTimeFormatter instance.");

println!("\n====== Work Log (en) example ============");
Expand Down
6 changes: 3 additions & 3 deletions components/datetime/src/combo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use crate::{provider::neo::*, scaffold::*};
///
/// // Note: Combo type can be elided, but it is shown here for demonstration
/// let formatter = DateTimeFormatter::<Combo<ET, L>>::try_new(
/// &locale!("en-US").into(),
/// locale!("en-US").into(),
/// ET::short().hm().zone_l(),
/// )
/// .unwrap();
Expand Down Expand Up @@ -62,7 +62,7 @@ use crate::{provider::neo::*, scaffold::*};
///
/// // Note: Combo type can be elided, but it is shown here for demonstration
/// let formatter = FixedCalendarDateTimeFormatter::<_, Combo<ET, L>>::try_new(
/// &locale!("en-US").into(),
/// locale!("en-US").into(),
/// ET::short().hm().zone_l(),
/// )
/// .unwrap();
Expand Down Expand Up @@ -91,7 +91,7 @@ use crate::{provider::neo::*, scaffold::*};
///
/// // Note: Combo type can be elided, but it is shown here for demonstration
/// let formatter = DateTimeFormatter::<Combo<DateFieldSet, Vs>>::try_new(
/// &locale!("en-US").into(),
/// locale!("en-US").into(),
/// DateFieldSet::YMD(YMD::long()).zone_v(),
/// )
/// .unwrap();
Expand Down
3 changes: 1 addition & 2 deletions components/datetime/src/dynamic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,10 @@
//! }
//! }
//!
//! let locale = locale!("en-US").into();
//! let datetime = DateTime::try_new_iso(2025, 1, 15, 16, 0, 0).unwrap();
//!
//! let results = [true, false].map(get_field_set).map(|field_set| {
//! DateTimeFormatter::try_new(&locale, field_set).unwrap()
//! DateTimeFormatter::try_new(locale!("en-US").into(), field_set).unwrap()
//! }).map(|formatter| {
//! formatter.convert_and_format(&datetime).try_write_to_string().unwrap().into_owned()
//! });
Expand Down
44 changes: 20 additions & 24 deletions components/datetime/src/external_loaders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@

//! Internal traits and structs for loading data from other crates.

use icu_calendar::AnyCalendar;
use icu_calendar::{AnyCalendar, AnyCalendarPreferences};
use icu_decimal::options::FixedDecimalFormatterOptions;
use icu_decimal::FixedDecimalFormatter;
use icu_decimal::{FixedDecimalFormatter, FixedDecimalFormatterPreferences};
use icu_provider::prelude::*;

/// Trait for loading a FixedDecimalFormatter.
Expand All @@ -15,7 +15,7 @@ use icu_provider::prelude::*;
pub(crate) trait FixedDecimalFormatterLoader {
fn load(
&self,
locale: &DataLocale,
prefs: FixedDecimalFormatterPreferences,
options: FixedDecimalFormatterOptions,
) -> Result<FixedDecimalFormatter, DataError>;
}
Expand All @@ -24,7 +24,7 @@ pub(crate) trait FixedDecimalFormatterLoader {
///
/// Implemented on the provider-specific loader types in this module.
pub(crate) trait AnyCalendarLoader {
fn load(&self, locale: &DataLocale) -> Result<AnyCalendar, DataError>;
fn load(&self, prefs: AnyCalendarPreferences) -> Result<AnyCalendar, DataError>;
}

/// Loader for types from other crates using compiled data.
Expand All @@ -36,19 +36,18 @@ impl FixedDecimalFormatterLoader for ExternalLoaderCompiledData {
#[inline]
fn load(
&self,
locale: &DataLocale,
prefs: FixedDecimalFormatterPreferences,
options: FixedDecimalFormatterOptions,
) -> Result<FixedDecimalFormatter, DataError> {
let temp_loc = locale.clone().into_locale();
FixedDecimalFormatter::try_new(temp_loc.into(), options)
FixedDecimalFormatter::try_new(prefs, options)
}
}

#[cfg(feature = "compiled_data")]
impl AnyCalendarLoader for ExternalLoaderCompiledData {
#[inline]
fn load(&self, locale: &DataLocale) -> Result<AnyCalendar, DataError> {
Ok(AnyCalendar::new_for_locale(locale))
fn load(&self, prefs: AnyCalendarPreferences) -> Result<AnyCalendar, DataError> {
Ok(AnyCalendar::new_for_locale(prefs))
}
}

Expand All @@ -62,11 +61,10 @@ where
#[inline]
fn load(
&self,
locale: &DataLocale,
prefs: FixedDecimalFormatterPreferences,
options: FixedDecimalFormatterOptions,
) -> Result<FixedDecimalFormatter, DataError> {
let temp_loc = locale.clone().into_locale();
FixedDecimalFormatter::try_new_with_any_provider(self.0, temp_loc.into(), options)
FixedDecimalFormatter::try_new_with_any_provider(self.0, prefs, options)
}
}

Expand All @@ -75,8 +73,8 @@ where
P: ?Sized + AnyProvider,
{
#[inline]
fn load(&self, locale: &DataLocale) -> Result<AnyCalendar, DataError> {
AnyCalendar::try_new_for_locale_with_any_provider(self.0, locale)
fn load(&self, prefs: AnyCalendarPreferences) -> Result<AnyCalendar, DataError> {
AnyCalendar::try_new_for_locale_with_any_provider(self.0, prefs)
}
}

Expand All @@ -92,11 +90,10 @@ where
#[inline]
fn load(
&self,
locale: &DataLocale,
prefs: FixedDecimalFormatterPreferences,
options: FixedDecimalFormatterOptions,
) -> Result<FixedDecimalFormatter, DataError> {
let temp_loc = locale.clone().into_locale();
FixedDecimalFormatter::try_new_with_buffer_provider(self.0, temp_loc.into(), options)
FixedDecimalFormatter::try_new_with_buffer_provider(self.0, prefs, options)
}
}

Expand All @@ -106,8 +103,8 @@ where
P: ?Sized + BufferProvider,
{
#[inline]
fn load(&self, locale: &DataLocale) -> Result<AnyCalendar, DataError> {
AnyCalendar::try_new_for_locale_with_buffer_provider(self.0, locale)
fn load(&self, prefs: AnyCalendarPreferences) -> Result<AnyCalendar, DataError> {
AnyCalendar::try_new_for_locale_with_buffer_provider(self.0, prefs)
}
}

Expand All @@ -123,11 +120,10 @@ where
#[inline]
fn load(
&self,
locale: &DataLocale,
prefs: FixedDecimalFormatterPreferences,
options: FixedDecimalFormatterOptions,
) -> Result<FixedDecimalFormatter, DataError> {
let temp_loc = locale.clone().into_locale();
FixedDecimalFormatter::try_new_unstable(self.0, temp_loc.into(), options)
FixedDecimalFormatter::try_new_unstable(self.0, prefs, options)
}
}

Expand All @@ -142,7 +138,7 @@ where
+ ?Sized,
{
#[inline]
fn load(&self, locale: &DataLocale) -> Result<AnyCalendar, DataError> {
AnyCalendar::try_new_for_locale_unstable(self.0, locale)
fn load(&self, prefs: AnyCalendarPreferences) -> Result<AnyCalendar, DataError> {
AnyCalendar::try_new_for_locale_unstable(self.0, prefs)
}
}
Loading