🚧 ICU-23424 Initial implementation of Builder::setFormattingLocale()

[srl295]

Draft… API proposal, etc forthcoming.

• plus tests

• Adds Builder& Builder::setFormattingLocale(const Locale& locale); as tech preview

• Propagate the formatting locale (equal to the regular locale normally)

Checklist

• Required: Issue filed: ICU-23424
• Required: The PR title must be prefixed with a JIRA Issue number. Example: "ICU-NNNNN Fix xyz"
• Required: Each commit message must be prefixed with a JIRA Issue number. Example: "ICU-NNNNN Fix xyz"
• Issue accepted (done by Technical Committee after discussion)
• Tests included, if applicable
• API docs and/or User Guide docs changed or added, if applicable
• Approver: Feel free to merge on my behalf

[srl295]

This test will FAIL on the existing code if set to the "en" locale (because it won't get the Polish plural buckets).

And the date format will be wrong (will be in Polish) if the formatting locale wasn't set to "en". 4.07.1776

[catamorphism]

Indentation

[catamorphism]

As of #3536 there are no factories anymore and formatters and selectors aren't distinguished, so I think this comment needs updating? Perhaps it just applies to built-in functions?

[srl295]

I think you're right, this was from pre-3536

[catamorphism]

Indentation

[catamorphism]

I find it confusing to refer to the two locales as "locale" and "formatting locale" -- perhaps rename "locale" to indicate what it does in contrast with the formatting locale?

[srl295]

OK. Will need to address Mihai's comment also.

[moconor]

perhaps selectionLocale and formattingLocale ?

[mihnita]

I've been thinking about this in the last few days.
And I don't think this is actually a good solution for the use case.

This works for the formatters and plural selectors in MF1, that we know relatively well.

But in MF2 the placeholders are not necessary formatters, and the selectors can be anything.
So my comparison with that Java Locale.Category (DISPLAY and FORMAT) is incorrect.
We said the plural selection depends on the language of the string loaded, so it depends on the DISPLAY locale.
But other selectors (standard or custom) might now.

And for example the placeholders can be used for grammar agreement (we have a prototype). And the locale for that should be the language of the message, so DISPLAY locale. Although it is not a selector, and by the logic of this PR it would use the FORMAT locale.

TLDR: I think this instrument is too blunt.

[srl295]

I've been thinking about this in the last few days.

And I don't think this is actually a good solution for the use case.

OK,

This works for the formatters and plural selectors in MF1, that we know relatively well.

But in MF2 the placeholders are not necessary formatters, and the selectors can be anything.

So my comparison with that Java Locale.Category (DISPLAY and FORMAT) is incorrect.

We said the plural selection depends on the language of the string loaded, so it depends on the DISPLAY locale.

But other selectors (standard or custom) might now.

And for example the placeholders can be used for grammar agreement (we have a prototype). And the locale for that should be the language of the message, so DISPLAY locale. Although it is not a selector, and by the logic of this PR it would use the FORMAT locale.

TLDR: I think this instrument is too blunt.

OK. Do you have suggestions on how to (shall we say) slice it up to be more precise?

The best parallel to existing APIs i can think of is something like adoptFormats in the MF1 implementation, where a custom ICU formatter object is dropped down into the message.

[mihnita]

OK. Do you have suggestions on how to (shall we say) slice it up to be more precise?

The best parallel to existing APIs i can think of is something like adoptFormats in the MF1 implementation, where a custom ICU formatter object is dropped down into the message.

Yes, but adoptFormats is overriding the whole function.

I think that the best option is still a locale in the placeholder / selector itself, in the message.

There is no ideal option, really.

This should be discussed in a TC design proposal, all options, with pros and cons.

For example if I say "yes, users want en-US dates, but only in the en-IN, fr-CA, ro, and zh.
Overrides in the code means that we litter the code with something like

   final static Set LOCALES_THAT_WANT_EN_US = [ ...... ]
   if getDefault locale in LOCALES_THAT_WANT_EN_US:
       messageFormatterBuilder.setFunctionOverride("date", myOwnthing);

Also note that the current function overrides are not global.
You need a custom function registry where you register all custom functions, then pass that registry to the formatter.

static final MFFunctionRegistry REGISTRY =
    MFFunctionRegistry.builder()
        .setFunction("date", new CustomDateFunctionFactory())
        .build();
MessageFormatter.Builder builder = ... .setFunctionRegistry(REGISTRY);
MessageFormatter mf = builder.build();

So you have to do that every time you format a message.
You might make the REGISTRY global and reuse it, but you still have to set it on the builder every time.
That can be thousands of calls.

So in the end you the end up with a wrapper instead using MessageFormatter directly.
And in the wrapper you do all the registry dance.

Changing messages is a lot easier. It can be done algorithmically, at build time, on the resource bundles (whatever that is).

Changing messages to use {$foo :custom_date ...} still requires the same registry dance in all calls.

Taking allowing a locale on all functions ({$foo :date locale:en_US}) means no code changes, no registry customization. And can be done algorithmically, and can be done only on the language resources you want, and not on others.

There was a reason why we had that as a proposal...

---

I think we should list all the options, with pros and cons, and present to the TC.
Not rush ahead with an implementation (unless you consider that to be prototyping, to see how it works.

[srl295]

Hi, thanks @mihnita for the constructive feedback!

OK. Do you have suggestions on how to (shall we say) slice it up to be more precise?

The best parallel to existing APIs i can think of is something like adoptFormats in the MF1 implementation, where a custom ICU formatter object is dropped down into the message.

Yes, but adoptFormats is overriding the whole function.

Actually, you can override the format for a specific placeholder. (In MF1)

I think that the best option is still a locale in the placeholder / selector itself, in the message.

There is no ideal option, really.

:locale isn't part of the spec, anymore, though.

This should be discussed in a TC design proposal, all options, with pros and cons.

Yes, as I noted I will get a design proposal to the TC. I'm collecting unofficial feedback to start.

For example if I say "yes, users want en-US dates, but only in the en-IN, fr-CA, ro, and zh.

Overrides in the code means that we litter the code…

Okay, but I wasn't asking to only override in some locales.

But if I did, I could do the following in user code:

if (locale == 'zh') builder.setFormattingLocale('en-US');

And still only the single API is needed.

   messageFormatterBuilder.setFunctionOverride("date", myOwnthing);

Also note that the current function overrides are not global.

You need a custom function registry where you register all custom functions, then pass that registry to the formatter.

static final MFFunctionRegistry REGISTRY =

    MFFunctionRegistry.builder()

        .setFunction("date", new CustomDateFunctionFactory())

        .build();

MessageFormatter.Builder builder = ... .setFunctionRegistry(REGISTRY);

MessageFormatter mf = builder.build();

So you have to do that every time you format a message.

You might make the REGISTRY global and reuse it, but you still have to set it on the builder every time.

That can be thousands of calls.

So in the end you the end up with a wrapper instead using MessageFormatter directly.

And in the wrapper you do all the registry dance.

OK- are you saying that you support that overriding?I had thought that was exactly what you disagreed with in MFWG. Maybe I am misunderstanding and you are just saying the above for the sake of argument.

Changing messages is a lot easier. It can be done algorithmically, at build time, on the resource bundles (whatever that is).

But to what, though? Not to :locale=en since that doesn't exist anymore?

Changing messages to use {$foo :custom_date ...} still requires the same registry dance in all calls.

Are the builders cloneable? If so then a builder that's all set except for the message, could be used.

It does requiring knowing which :x needs to change to :custom_x just as knowing which :y needs to change to :y :locale=en

Taking allowing a locale on all functions ({$foo :date locale:en_US}) means no code changes, no registry customization. And can be done algorithmically, and can be done only on the language resources you want, and not on others.

There was a reason why we had that as a proposal...

There's the rub..

And I know it was because of ICU4X's optimization angle that it was removed.

Here's an idea:

• what if builder.setLocales(['ro','pl', 'en-US']) took a list instead of a singleton - that is, the MF2 Context was an ordered list.
• And then, what if :locale could be used, but had to be exactly one of the locales set in setLocales()? Maybe that would even satisfy the icu4x optimization situation.

I think we should list all the options, with pros and cons, and present to the TC.

Sure. Would you be able to help expand on the use cases you have beyond this 'blunt instrument'? One of the pros of this PR as written is that it is simple for the client in this use case.

Not rush ahead with an implementation (unless you consider that to be prototyping, to see how it works.

Hm - no rush here, this is a draft PR that doesn't even have an API proposal out. This PR does, however, represent just how we solved the issue, which is time honored way to start a request. I have confidence that the TC is not overly swayed by the presence of an implementation.

To @catamorphism point the present implementation doesn't explicitly list what the new function applies to.