fix: i18n.interpolate tolerates leftover % and arg-count mismatch#7980
fix: i18n.interpolate tolerates leftover % and arg-count mismatch#7980keithharvey wants to merge 1 commit into
Conversation
After named (%{name}) and formatted (%<name>.fmt) interpolation, the trailing
string.format(str, unpack(vars)) errored whenever a translation left a literal
% or had a different number of positional specs than vars. Run that pass only
when real format specs remain (ignoring %%), pass one arg per spec, and
pcall-guard it so a mismatch falls back to the un-formatted string.
|
See https://github.com/beyond-all-reason/Beyond-All-Reason/pull/7962/changes#r3424568782 for the upstream branch that also contains this fix. |
|
Got an example input that currently fails? |
resource_transfer_comms is a good one: I build a dynamic hash from a classifier and then pass it uniformly to translate. Some translations only consume a subset of those keys, so today any unused variable (or a literal % in the string) blows up string.format. |
I think you're just not meant to pass anything you won't consume. This is to guard against missing key errors (e.g. catching Probably best for @WatchTheFort to say which design he prefers. |
|
How can you tell the difference between a literal % and one that the developer meant to be part of an escape sequence but messed up? |
|
Being able to put more entries in the parameters table than interpolated variables specified in the translation entry seems pretty reasonable. No reason it should crash if you pass more data than it consumes. |
You can't. A lone %x is inherently ambiguous. The current code treats every leftover % as a programmer error and crashes. The PR flips the assumption to "probably literal" and renders it raw. The pcall fallback means whether the % was an intended literal or a fat-fingered escape, you get the same graceful outcome (raw string shown) instead of a runtime error. I think this is a better default behavior than forcing every translator to escape, but happy to call that out explicitly if the lost typo-crash worries you. |
3 participants
What it does
Make the trailing positional
string.formatpass ininterpolate()optional and non-fatal: run it only when the string still has real positional specs (%d,%s, … — ignoring escaped%%), pass one argument per spec, andpcall-guard it so a spec/argument mismatch falls back to the un-formatted string instead of erroring.Why
TLDR: callers can pass named variables freely without i18n blowing up.
string.format(str, unpack(vars))ran unconditionally after%{name}interpolation, so a leftover literal%(e.g.50% off) or a positional spec/arg mismatch threw an error — even when a translation only used named variables. That made it unsafe to pass a variable a given string doesn't consume.Intermediate systems need this. In another branch, gui_chat sits between my code and i18n and infers a field's color from the translation variable key names — fragile, and my keys are resource-agnostic now (
amountSendable, notenergyAmount). I wanted to pass it aresourceTypehint, but any extra variable exploded translation. This unblocks that.Note
This is NOT fixed in the upstream kikito I18n, that's sad.