Add BoundLocaleDataProvider and some impls for it#7621
Add BoundLocaleDataProvider and some impls for it#7621sffc wants to merge 9 commits intounicode-org:mainfrom
Conversation
|
In case it helps, here's a summary of what you can do after this PR: You can write code like let formatter = HelloWorldAttributeFormatter::try_new_with_buffer_provider(
&provider,
locale!("en").into()
).unwrap();
assert_writeable_eq!(formatter.format("reverse").unwrap(), "Olleh Dlrow");
assert_writeable_eq!(formatter.format_for_prefix("rev").unwrap(), "Olleh Dlrow");Please observe that the data marker attribute is given in the format function, not in the constructor. To make this work, we save a small slice of the data provider structure inside of the HelloWorldAttributeFormatter, which has a generic parameter that varies based on the provider type, powered by some new traits. This PR adds impls of the new traits in the BlobDataProvider and HelloWorldProvider universes. We will need to add them to baked data, and probably FsDataProvider. |
robertbastian
left a comment
robertbastian
left a comment
There was a problem hiding this comment.
As far as I remember the display names design, it doesn't involve the formatter holding on to the provider. I don't think this approach gives nice APIs, as the formatters will need to be generic in the providers. This also requires a lot of new traits and types. Overall I'm very negative on this.
provider/core/src/buf/serde.rs
Outdated
| /// - `deserialize_bincode_1` | ||
| /// | ||
| /// ✨ *Enabled with the `serde` Cargo feature.* | ||
| pub fn new(inner: &'a P) -> Self { |
There was a problem hiding this comment.
the canonical way to wrap a provider in a DeserializingBufferProvider is to call .as_deserializing(). we use this in a lot of docs, and I don't think we should introduce an alternative API that does the same
| use alloc::boxed::Box; | ||
| use icu_provider::buf::BufferFormat; | ||
| use icu_provider::prelude::*; | ||
| use icu_provider::unstable::BindLocale; |
There was a problem hiding this comment.
if you start depending on unstable APIs, do we need to use a ~ dep?
| /// assert_writeable_eq!(fmt.format("reverse").unwrap(), "Olleh Dlrow"); | ||
| /// ``` | ||
| #[derive(Debug)] | ||
| pub struct HelloWorldAttributeFormatter<P: BoundLocaleDataProvider<HelloWorldV1>> { |
There was a problem hiding this comment.
is this a bad example, or does this mean every formatter using this pattern needs to be generic in a provider?
|
The use case is when you need to load a list of many different data marker attributes. If you load each one in sequence from a provider, you need to re-read the locale (and marker in the case of blob provider) for every lookup. What this design does is it creates an intermediate "bound locale provider" that is halfway through a data load operation. You can then more efficiently perform attribute lookups. |
|
See #7663 for benchmark results. @robertbastian |
2 participants
#3260
Please take a look at this design for a formatter that retains a piece of the data provider for runtime lookup of data marker attributes, intended for use cases such as currencies, units, and display names.
Reviewable commit-by-commit.
No AI was used when writing this, but I might use one to fix up the clippy and feature warnings in CI.
The docs are somewhat limited. Please focus on the docs tests for the end-to-end usage examples. I will add more docs before merging.