Review the fluent (i18n) implementation

[Omikhleia]

Further the issues I encountered in #1474...

The Fluent implementation is real weird...

1. It has potential side-effects...

   

   • Line 68: message is nil is there was no localization found. Boom?
   • Line 57: So if a "locale" was set via options, it's now enforced for the rest of the document outside that command... Eek?
     • Additionally, This SILE.fluent:set_locale() thing really annoys me. It means the Fluent thing is state-full. It ought to be state-less, and SILE.fluent:get_message(key, locale) should be the way to go, IMHO.

2. Let's checks every other call at SILE.fluent:set_locale()

   • In \font: likewise, the font change might be transient (if we had content, i.e. we are in a temporary settings push/pop state): the previous local ought to be restored... Or this thing be state-less, again...
   • Likewise in \ftl...
   • Hard to say for sure in SILE.languageSupport.languages.loadLanguage() but this have some smell...

3. The Fluent library implementation is even weirder... It uses Penlight classes, which are already broken to some extent (Er... Do I have to quote the package owner here?), but that could be okay... Er, wait, all the metatable access are weird, and this is typically why one would want to use an OOP framework: to avoid those... Even after trying to fix (1) and (2) above, I couldn't make sense of the results seen on the SILE manual with Localization for as many languages as possible #1474...

I would suggest:

• Rewriting this thing to be state-less, with the locale passed to commands needing it (e.g. get_message) rather than use some "global".
• Revisit the core fluent implementation and stop trying to do more than what Penlight proposes... The _patch_init and self:catch things all have heavy code smell (IMHO) and are hardly understandable, therefore maintainable... for something, after all, which in the end resolves to some (key, value) patterns...

[alerque]

Yes, aware of scope leakage here. Right now it's basically required to reset the local before using any strings. This probably needs to be moved to a per-language instantiation and all calls to strings should use the instance connected to the language setting.

This is closely related to #1367.

[alerque]

The Fluent implementation is real weird...

Sure. Still true.

The Lua implementation of Project Fluent was my own from the start. It tries to present the upstream ProjectFluent paradigm in a Lua idiomatic way. The major variable scope leakage I know about has been addressed, but there may be more issues of course.

Since I started with a Lua implementation things have gotten even crazier as I'm now the upstream maintainer for the Rust implementation (used in Firefox and many many other places). I've also contributed to the upcoming Message Format 2 spec. Localization is a bugbear.

Line 68: message is nil is there was no localization found. Boom?

warn() + output nil and continue was chosen as preferable to error(). For SILE I still think this makes sense, although personally I'm starting to want a "treat warnings as errors" mode. Which of course can be cobbled together easily, but a CLI flag wouldn't break my heart.

Line 57: So if a "locale" was set via options, it's now enforced for the rest of the document outside that command... Eek?

No, it's enforced on the Fluent "bundle". Any direct access the user tries to make on the Fluent global would be affected by this, but no SILE functions should be. In the future when we get a settings stack that separates language settings per typesetter we should initialize a bundle for each typesetter.

Given that this is set every time the SILE function runs it will always

Additionally, This SILE.fluent:set_locale() thing really annoys me. It means the Fluent thing is state-full. It ought to be state-less, and SILE.fluent:get_message(key, locale) should be the way to go, IMHO.

The Fluent bundle is state-full. The SILE function we're in here is also state-full, but with a different scope. Hence we're resetting the Fluent state to the SILE state every time this function is called.

We're creating a wrapper here: we're wrapping an API that doesn't fit very naturally with SILE in some functions that do fit fairly nicely inside SILE.

Let's checks every other call at SILE.fluent:set_locale()

This has been done. Did I miss something?

In \font: likewise, the font change might be transient (if we had content, i.e. we are in a temporary settings push/pop state): the previous local ought to be restored... Or this thing be state-less, again...

Again who cares, we're resetting the bundle state every time we call it to match whatever the current SILE state is.

Hard to say for sure in SILE.languageSupport.languages.loadLanguage() but this have some smell...

Yes, the language loader stuff stinks a bit right now. It isn't just Fluent though, the whole thing as a mess and I can't get around to fixing it until 15 is out the door.

The Fluent library implementation is even weirder...

Sure, but if there is any way to make it both idiomatic Lua and follow he upstream Project Fluent model well I'd love to hear about it. The fluent-lua repo would be a better place for any issues with how that API works though.

I would suggest:

Rewriting this thing to be state-less, with the locale passed to commands needing it (e.g. get_message) rather than use some "global".

We're providing state-less functions that wrap an inherently state-full API. The global state is in the API we're using (and the model it is meant to implement) not our own. get_message() is not SILE's function, the higher level SILE functions already work as you described, no?

Revisit the core fluent implementation and stop trying to do more than what Penlight proposes... The _patch_init and self:catch things all have heavy code smell (IMHO) and are hardly understandable, therefore maintainable... for something, after all, which in the end resolves to some (key, value) patterns...

self:catch is provided by Penlight, and allows us to make the somewhat Penlight idiomatic usage more generic. Given:

hello = Hello { $name }!
foo = bar
    .attr = baz

The somewhat Penlight object model usage that looks like this:

print(bundle:format("foo"))
print(bundle:format("foo.attr"))
print(bundle:format("hello", { name = "World" }))

...could also be used as more generally Lua idioms that look like this:

print(bundle["foo"])
print(bundle.foo.attr)
print(bundle.hello({ name = "World" }))

Since lots of things in SILE are modeled around Penlight the first set of usages probably makes a lot more sense here, but for folks in many other Lua scopes with different models the later approach may be preferred. Hence fluent-lua provides both.

[alerque]

If there is anything actionable for the SILE wrapper functions I'm quite happy to review them, but the last couple times I've read this issue I didn't come up with any. Please do re-open if I'm missing something.