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.

Sign up for GitHub

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

Jump to bottom

Add field set options constructor #5556

Merged
merged 9 commits into from
Sep 19, 2024
Merged

Add field set options constructor #5556

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
955c5f1
Internal refactor: create RawNeoOptions
sffc Sep 18, 2024
0e7a43a
Switch yes/no to yes-or-absent in marker macro
sffc Sep 18, 2024
dd9ff1a
Proof-of-concept field set options working
sffc Sep 18, 2024
566bc1a
features
sffc Sep 18, 2024
5bdf407
Features, refactor
sffc Sep 18, 2024
8ef9d88
Refactor
sffc Sep 19, 2024
62fd36c
Add time option bag impls
sffc Sep 19, 2024
ed6d177
diplomat-coverage
sffc Sep 19, 2024
b6f0349
internal docs
sffc Sep 19, 2024
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
138 changes: 120 additions & 18 deletions components/datetime/src/neo.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,20 @@ pub struct NeoOptions<R: DateTimeMarkers> {
pub fractional_second_digits: R::FractionalSecondDigitsOption,
}

impl<R> From<NeoOptions<R>> for RawNeoOptions
where
R: DateTimeMarkers,
{
fn from(options: NeoOptions<R>) -> Self {
Self {
length: options.length.into(),
alignment: options.alignment.into(),
era_display: options.era_display.into(),
fractional_second_digits: options.fractional_second_digits.into(),
}
}
}

impl<R> From<NeoSkeletonLength> for NeoOptions<R>
where
R: DateTimeMarkers,
Expand Down Expand Up @@ -214,6 +228,28 @@ where
}
}

impl RawNeoOptions {
#[cfg(feature = "compiled_data")]
pub(crate) fn from_field_set<FSet>(field_set: FSet) -> Self
where
FSet: DateTimeMarkers,
FSet: NeoGetField<FSet::LengthOption>,
FSet: NeoGetField<FSet::AlignmentOption>,
FSet: NeoGetField<FSet::EraDisplayOption>,
FSet: NeoGetField<FSet::FractionalSecondDigitsOption>,
{
Self {
length: NeoGetField::<FSet::LengthOption>::get_field(&field_set).into(),
alignment: NeoGetField::<FSet::AlignmentOption>::get_field(&field_set).into(),
era_display: NeoGetField::<FSet::EraDisplayOption>::get_field(&field_set).into(),
fractional_second_digits: NeoGetField::<FSet::FractionalSecondDigitsOption>::get_field(
&field_set,
)
.into(),
}
}
}

size_test!(TypedNeoFormatter<icu_calendar::Gregorian, crate::neo_marker::NeoYearMonthDayMarker>, typed_neo_year_month_day_formatter_size, 504);

/// [`TypedNeoFormatter`] is a formatter capable of formatting dates and/or times from
Expand All @@ -229,6 +265,78 @@ pub struct TypedNeoFormatter<C: CldrCalendar, R: DateTimeNamesMarker> {
_calendar: PhantomData<C>,
}

impl<C: CldrCalendar, FSet: DateTimeMarkers + HasConstComponents> TypedNeoFormatter<C, FSet>
where
FSet::D: TypedDateDataMarkers<C>,
FSet::T: TimeMarkers,
FSet::Z: ZoneMarkers,
FSet: NeoGetField<FSet::LengthOption>,
FSet: NeoGetField<FSet::AlignmentOption>,
FSet: NeoGetField<FSet::EraDisplayOption>,
FSet: NeoGetField<FSet::FractionalSecondDigitsOption>,
{
/// Creates a new [`TypedNeoFormatter`] from compiled data with
/// datetime components specified at build time.
///
/// Use this constructor for optimal data size and memory use
/// if you know the required datetime components at build time.
/// If you do not know the datetime components until runtime,
/// use a `with_components` constructor.
///
/// # Examples
///
/// Basic usage:
///
/// ```
/// use icu::calendar::Date;
/// use icu::calendar::Gregorian;
/// use icu::datetime::neo::TypedNeoFormatter;
/// use icu::datetime::neo_marker::NeoYearMonthDayMarker;
/// use icu::datetime::neo_skeleton::NeoSkeletonLength;
/// use icu::locale::locale;
/// use writeable::assert_try_writeable_eq;
///
/// let formatter =
/// TypedNeoFormatter::try_new2(
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer if you just replaced the current constructors, then the docs would actually be a diff to what we have now. I have no idea what we have now.

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 there are still some tests that need fixing.

I'm quite open to doing a gradual migration here. We have to take a holistic look at this crate in the end anyway.

/// &locale!("es-MX").into(),
/// NeoYearMonthDayMarker::with_length(NeoSkeletonLength::Long),
/// )
/// .unwrap();
///
/// assert_try_writeable_eq!(
/// formatter.format(&Date::try_new_gregorian_date(2023, 12, 20).unwrap()),
/// "20 de diciembre de 2023"
/// );
/// ```
#[cfg(feature = "compiled_data")]
pub fn try_new2(locale: &DataLocale, options: FSet) -> Result<Self, LoadError>
Copy link
Member

Choose a reason for hiding this comment

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

issue (nb): don't like this name, but we can revisit later

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 intend to replace the old try_new with the new signature

Copy link
Member

Choose a reason for hiding this comment

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

sgtm

where
crate::provider::Baked: Sized
// Date formatting markers
+ DataProvider<<FSet::D as TypedDateDataMarkers<C>>::YearNamesV1Marker>
+ DataProvider<<FSet::D as TypedDateDataMarkers<C>>::MonthNamesV1Marker>
+ DataProvider<<FSet::D as TypedDateDataMarkers<C>>::DateSkeletonPatternsV1Marker>
+ DataProvider<<FSet::D as TypedDateDataMarkers<C>>::WeekdayNamesV1Marker>
+ DataProvider<<FSet::T as TimeMarkers>::DayPeriodNamesV1Marker>
+ DataProvider<<FSet::T as TimeMarkers>::TimeSkeletonPatternsV1Marker>
+ DataProvider<<FSet::Z as ZoneMarkers>::EssentialsV1Marker>
+ DataProvider<<FSet::Z as ZoneMarkers>::ExemplarCitiesV1Marker>
+ DataProvider<<FSet::Z as ZoneMarkers>::GenericLongV1Marker>
+ DataProvider<<FSet::Z as ZoneMarkers>::GenericShortV1Marker>
+ DataProvider<<FSet::Z as ZoneMarkers>::SpecificLongV1Marker>
+ DataProvider<<FSet::Z as ZoneMarkers>::SpecificShortV1Marker>
+ DataProvider<FSet::GluePatternV1Marker>,
{
Self::try_new_internal(
&crate::provider::Baked,
&ExternalLoaderCompiledData,
locale,
FSet::COMPONENTS,
RawNeoOptions::from_field_set(options),
)
}
}

impl<C: CldrCalendar, R: DateTimeMarkers + HasConstComponents> TypedNeoFormatter<C, R>
where
R::D: TypedDateDataMarkers<C>,
Expand Down Expand Up @@ -292,7 +400,7 @@ where
&ExternalLoaderCompiledData,
locale,
R::COMPONENTS,
options,
options.into(),
)
}

Expand Down Expand Up @@ -337,7 +445,7 @@ where
&ExternalLoaderUnstable(provider),
locale,
R::COMPONENTS,
options,
options.into(),
)
}
}
Expand Down Expand Up @@ -459,7 +567,7 @@ where
&ExternalLoaderCompiledData,
locale,
components.into(),
options,
options.into(),
)
}

Expand Down Expand Up @@ -505,7 +613,7 @@ where
&ExternalLoaderUnstable(provider),
locale,
components.into(),
options,
options.into(),
)
}
}
Expand All @@ -521,7 +629,7 @@ where
loader: &L,
locale: &DataLocale,
components: NeoComponents,
options: NeoOptions<R>,
options: RawNeoOptions,
) -> Result<Self, LoadError>
where
P: ?Sized
Expand Down Expand Up @@ -550,11 +658,8 @@ where
&<R::T as TimeMarkers>::TimeSkeletonPatternsV1Marker::bind(provider),
&R::GluePatternV1Marker::bind(provider),
locale,
options.length.into(),
components,
options.alignment.into(),
options.era_display.into(),
options.fractional_second_digits.into(),
options,
hour_cycle,
)
.map_err(LoadError::Data)?;
Expand Down Expand Up @@ -790,7 +895,7 @@ where
&ExternalLoaderCompiledData,
locale,
R::COMPONENTS,
options,
options.into(),
)
}

Expand Down Expand Up @@ -890,7 +995,7 @@ where
&ExternalLoaderUnstable(provider),
locale,
R::COMPONENTS,
options,
options.into(),
)
}
}
Expand Down Expand Up @@ -1055,7 +1160,7 @@ where
&ExternalLoaderCompiledData,
locale,
components.into(),
options,
options.into(),
)
}

Expand Down Expand Up @@ -1156,7 +1261,7 @@ where
&ExternalLoaderUnstable(provider),
locale,
components.into(),
options,
options.into(),
)
}
}
Expand All @@ -1172,7 +1277,7 @@ where
loader: &L,
locale: &DataLocale,
components: NeoComponents,
options: NeoOptions<R>,
options: RawNeoOptions,
) -> Result<Self, LoadError>
where
P: ?Sized
Expand Down Expand Up @@ -1251,11 +1356,8 @@ where
&<R::T as TimeMarkers>::TimeSkeletonPatternsV1Marker::bind(provider),
&R::GluePatternV1Marker::bind(provider),
locale,
options.length.into(),
components,
options.alignment.into(),
options.era_display.into(),
options.fractional_second_digits.into(),
options,
hour_cycle,
)
.map_err(LoadError::Data)?;
Expand Down
Loading