Open robertbastian opened 3 months ago
Initial thoughts:
icu_list/AndListV1Marker
DataMarker
trait directly.We could have both inlined const fields and a helper DataMarkerInfo
pub trait DataMarker: DynamicDataMarker {
const NAME: DataMarkerName;
const IS_SINGLETON: bool;
const FALLBACK_CONFIG: LocaleFallbackConfig;
}
#[non_exhaustive]
pub struct DataMarkerInfo {
pub name: DataMarkerName;
pub is_singleton: bool;
pub fallback_config: LocaleFallbackConfig;
}
impl DataMarkerInfo {
pub fn from_marker<M: DataMarker>() -> Self { /* ... */ }
}
pub trait DynamicDataProvider<M: DynamicDataMarker> { load(&self, DataMarkerInfo, DataRequest) }
But I guess if you do that you might as well just make DataMarkerInfo
a const field, as in Proposal 3.
icu::
and the ::provider::
and then snake case the rest.stringify!
macro if we keep the exact same path. It also makes them more searchable.Summary of additional discussion later:
DateTime_Year_Chinese_V1
which are both hierarchical and brief, despite not being conventional Rust names. @sffc argues that these are weird types that only appear in trait bounds so it is fine to establish a new paradigm for how we name them. "V1Marker" is already a paradigm; we're swapping one for another.icu::datetime::data::year::ChineseV1
as the identifier. @sffc is potentially okay with this compromise, since it has hierarchy and filterability, but is disappointed that it sacrifices brevity.But the "Marker" name is overloaded
to expand on this a little bit (but not to rabbithole too deep because I don't care about "Marker" strongly): I find Marker to be a good default, but it's nice when it can be replaced with a useful concept name instead. I think we may have options here.
one thing I proposed in that discussion that isn't included here was encoding the heirarchy in the trait, so you can have something like:
impl DataMarker for DaySymbolsV1Marker {
const COMPONENT = "datetime";
const SUBCOMPONENT = Some("symbols");
}
Notes from the scratch pad:
pub fn try_new_unstable<D>(
provider: &D,
locale: &DataLocale,
options: CollatorOptions
) -> Result<Self, DataError>
where
D: DataProvider<CollationSpecialPrimariesV1Marker> + DataProvider<CollationDataV1Marker> + DataProvider<CollationDiacriticsV1Marker> + DataProvider<CollationJamoV1Marker> + DataProvider<CollationMetadataV1Marker> + DataProvider<CollationReorderingV1Marker> + DataProvider<CanonicalDecompositionDataV1Marker> + DataProvider<CanonicalDecompositionTablesV1Marker> + ?Sized,
pub fn try_new_unstable<D>(
provider: &D,
locale: &DataLocale,
options: CollatorOptions
) -> Result<Self, DataError>
where
D: DataProvider<Collation_SpecialPrimaries_V1> + DataProvider<Collation_Data_V1> + DataProvider<Collation_Diacritics_V1> + DataProvider<Collation_Jamo_V1> + DataProvider<Collation_Metadata_V1> + DataProvider<Collation_Reordering_V1> + DataProvider<Collator_CanonicalDecomposition_Data_V1> + DataProvider<Collation_CanonicalDecomposition_Tables_V1> + ?Sized,
pub fn try_new_unstable<D>(
provider: &D,
locale: &DataLocale,
options: FixedDecimalFormatterOptions
) -> Result<Self, DataError>
where
D: DataProvider<DecimalSymbolsV1Marker> + ?Sized
pub fn try_new_unstable<D>(
provider: &D,
locale: &DataLocale,
options: FixedDecimalFormatterOptions
) -> Result<Self, DataError>
where
D: DataProvider<Decimal_Symbols_V1> + ?Sized
Collation_CanonicalDecomposition_Tables_V1
TimeZone_IanaMapper_ToBcp47_V2
or TimezoneIanamapperTobcp47V1Marker
TimeZone_IanaMapper_ToIana_V1
TimeZone_Metazone_Period_V1
TimezoneIanaToBcp47V1
, TimezoneBcpToIanaV1
, icu::timezone::provider::iana_mapper::ToIanaV1Marker
Current: DangiDatePatternV1Marker
+ “datetime/patterns/dangi/date@1”
Shane Proposes: DateTime_Patterns_Date_Dangi_V1
Rob Proposes: DatetimePatternsDateDangiV1Marker
, icu::datetime::provider::patterns::date::DangiV1Marker
where: P: DataProvider<patterns::date::DangiV1Marker> + DataProvider<patterns::date::GregorianV1Marker> ...
where: P: DataProvider<DateTime_Patterns_Date_Dangi_V1> + DataProvider<DateTime_Patterns_Date_Gregorian_V1> ...
icu::datetime::provider::patterns::date::DangiV1Marker
datetime/patterns/date/DangiV1Marker/en.json
datetime/patterns/date/dangi@1/en.json
Discussion:
ListV1Marker
but in datagen they need to select list/and@1
. I fixed this by accepting both formulations in datagen. But they still see the path3 in errors.Some proposals:
More discussion:
Conclusion:
LGTM: @sffc @robertbastian @Manishearth
Current architecture:
DataProvider
paths, they are only used inDynamicDataProvider
BufferProvider
/AnyProvider
are functionallyDynamicDataProvider<BufferMarker/AnyMarker>
dyn KeyedDataMarker
KeyedDataMarkers
Proposal 1: Align data marker names with data provider names
Proposal 2: Get rid of "data key" terminology
DataMarker
(new terminology)DataMarkerInfo
s on the APIProposal 3:
DataMarkerInfo.path
becomesDataMarkerInfo.name
path
is not clear (it probably originated from FS provider)DataMarker
(new terminology) in the tagged string.AndListV1Marker
on the API, they can tell datagen to generate"AndListV1Marker"
(currently they have to figure out that it's"list/and@1"
)AndListV1Marker/en.json
instead oflist/and@1/en.json
.icu_list::provider::AndListV1Marker
), FS provider can then doicu_list/AndListV1Marker/en.json
Proposal 4: Split
DataMarkerInfo
's fields (maybe)IS_SINGLETON
closer to the type system could allow further optimisationspub trait SingletonDataMarker: DataMarker
insteadNAME
s can be optimised out if nothing uses them. The hash would live inside the name, so blob provider would still use the name, and static slicing would still work. However, in non-dynamic-data-provider binaries without debug logging, the names can be removedDataKeyInfo
parameter by three parameters, so that's a downside to proposal