DateTimeFormatterPreferences #5842
Conversation
sffc
requested review from
Manishearth,
zbraniecki and
a team
as code owners
| @@ -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 { | |||
There was a problem hiding this comment.
Should I rename this function?
There was a problem hiding this comment.
@Manishearth What do you think about the name of this function? (a constructor for AnyCalendar)
Note that there are also constructors from AnyCalendarKind.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
We already have
#[cfg(feature = "compiled_data")]
pub const fn new(kind: AnyCalendarKind) -> SelfThere was a problem hiding this comment.
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)
There was a problem hiding this comment.
I made #5843 to discuss this before 2.0 (but after 2.0 Beta) and unblock this PR
| /// Constructs a [`DataLocale`] for this [`DataMarkerInfo`]. | ||
| pub fn make_locale( |
There was a problem hiding this comment.
Notice the new function in icu_provider. OK?
| @@ -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 { | |||
| AnyCalendarPreferences, | ||
| { | ||
| /// The user's preferred calendar system. | ||
| calendar: CalendarAlgorithm |
There was a problem hiding this comment.
I guess I'll stick with the UTS 35 language: "calendar algorithm"
https://unicode.org/reports/tr35/tr35.html#Key_And_Type_Definitions_
#1317