projectfluent / fluent-rs

Rust implementation of Project Fluent
https://projectfluent.org
Apache License 2.0
1.07k stars 96 forks source link

Custom `FluentValue` types #146

Closed zbraniecki closed 4 years ago

zbraniecki commented 4 years ago

At the moment, fluent-bundle accepts arguments to format_pattern that must be of type FluentValue. FluentValue is an enum that can handle 4 different types - Number, String, Error and None.

Internally, None and Error are used in error scenarios, but from the variable point of view, we really only accept String and Number. In JS world, we also accept DateTime type.

What's also important, is that in JS world we accept Number and we format it using Intl.NumberFormat, but in Rust case we can't really do that, so all that we get out of Number is that it matches and compares to FTL AST Numbers for selectors.

The final piece needed for https://bugzilla.mozilla.org/show_bug.cgi?id=1560038 is to hook Intl.DateTimeFormat and Intl.NumberFormat into fluent-bundle-rs and that requires changes to how we treat those FluentValue types.

In particular, I think we should investigate switching them to be traits, so that customers (in this case, Gecko) can implement their own traits that will provide formatting capabilities using Intl APIs.

@stasm - thoughts?

Pike commented 4 years ago

I think traits make sense.

In the scenario of format-to-parts, stas and I also entertained the idea of having DOM nodes be FluentValue-wrapped things.

Can we design this trait such that it'd be usable if we had a format-to-parts API that was an iterator over FluentValues?

stasm commented 4 years ago

Agreed, a trait sounds like a good choice for this.

zbraniecki commented 4 years ago

I talked to @Manishearth and he suggested that we don't switch the default types to FluentType trait because dynamic dispatching is quite costly.

Instead, we'll add a new FluentValue variant - External(Box<dyn FluentTrait>) and use it to add DateTime etc.