Drop the bare plural keys and point translators at Weblate - #506
Conversation
Twenty-six i18next plural families kept a bare key (e.g. "activeTasks") next to their _zero/_one/_two/_few/_many/_other siblings. i18next only ever reads the bare key when no suffixed form is present, so once a family has suffixes the bare key is text nobody renders, and Weblate was reading it as a 27th string that duplicates one of the variants (364 flagged strings = 26 families x 14 locales). Checked every t() call site for the 26 families (grep across src/renderer, no dynamic/template-literal keys involved): all of them pass a count in the options object, so every bare key could go without changing behavior. Removed the bare key from all 14 locale files, and trimmed drafted.json of the review entries that pointed at the now-gone keys. i18n-parity.test.ts checked keys with a literal `key in enUS` lookup, which no longer finds a plural family once its bare key is gone; added resolveTranslationValue() to helpers.ts so both parity checks resolve a key through its suffixed siblings the way i18next does at runtime. Added no-bare-plural-keys.test.ts, which fails if a bare key ever reappears beside its suffixed forms in any locale file, naming the key and the file. Also found, but left alone as out of scope: en-US.json, fr-FR.json and ru-RU.json each carry the whole "server mods" key block duplicated verbatim twice in a row. Harmless (JSON.parse just keeps the last value) but worth a follow-up cleanup on its own.
Weblate's free hosting for libre projects asks that the project say it uses Weblate. Note in the README's translation section, and at the top of the translation guide, that translation now happens on Weblate and that the pull-request guide is still there for anyone who prefers that route.
Translation statusen-US is the source and carries 874 keys.
The status page is out of date. Refresh it with Drafted values are the machine-drafted ones still waiting for a native review, listed per locale in |
Zaldaryon
left a comment
Zaldaryon
left a comment
There was a problem hiding this comment.
Changes requested. The locale data is structurally consistent, but this removal exposes two runtime regressions and the new parity guard misses a plural-call contract.
…e the scan Dropping the bare keys left two paths with nothing to fall back on. Four families (errorCount, warningCount, activeTasks, newNotifications) carried only _one and _other, which is all English selects between. Nine locales select more: i18next picked _few for a Russian count of 2, found nothing, and rendered the English sentence. The forms those languages need are now there, derived from what Intl.PluralRules resolves for each locale rather than from a hand-kept list. The mod count on an Installation read an undefined _modsCount until ConfigProvider's delayed scan filled it in, two and a half seconds into every session. That used to interpolate into the bare key as a blank number; with the bare key gone it selected no form at all and showed the raw key. The three call sites now count 0 until something has counted, which is what a freshly created Installation already shows. The parity guard resolved a plural family through whichever suffixed sibling it found first, so it could not tell a call that passes a count from one that does not. It now requires a count at every plural-family call site, requires every family to be reached with one somewhere, and holds every locale to the categories its own language selects. Refs #496.
|
All three addressed in 5147fa4. The plural fallback was wider than the three locales named: every locale whose Intl.PluralRules asks for more than one/other had it, so _few and _many now cover the four families in nine locales. A runtime test drives each locale through i18next with no fallbackLng, which is what makes the miss visible instead of silently English. The mod count now reads _modsCount ?? 0 at the three call sites, matching what a freshly created Installation already shows, with a DOM test on both components before the scan runs. The parity guard now requires a count at every plural-family call site and requires every family to be reached with one. Your areYouSureDeleteSelected example fails it by name. Ready for another look. |
Zaldaryon
left a comment
Zaldaryon
left a comment
There was a problem hiding this comment.
All three previously requested items are verified on 5147fa4.
The plural categories derived from Intl.PluralRules cover the required forms across all nine multi-category locales. The runtime test without fallbackLng confirms that count-dependent translations such as errorCount resolve in the target language rather than falling back to English.
The fallback for _modsCount ?? 0 in InstallationsDropdownMenu and ListInstallations prevents rendering raw translation keys while the initial mod scan is pending, backed by the new DOM tests.
The static parity checks now enforce that plural families receive a count argument at all call sites and cover each locale's grammatical categories. Local test runs and CI checks are green.
2 participants
The duplicate
26 plural families kept a bare key next to their
_zero/_one/_two/_few/_many/_othersiblings. i18next only falls back to the bare key when no suffixed form exists, so once a family has suffixes the bare key renders nothing: it is text nobody sees, and Weblate read it as a 27th string carrying the same text as one of the variants. 26 families x 14 locales = the 364 duplicate strings Weblate flagged.Example,
components.activityCenter.activeTasksinen-US.json:Before:
After:
Call-site check
Grepped every
t("<key>")use of the 26 families acrosssrc/renderer(no dynamic or template-literal keys were involved for any of them). Every call site passes acountin the options object, so removing the bare key changes nothing at runtime. Nothing had to be kept.While checking, found that
en-US.json,fr-FR.jsonandru-RU.jsoneach carry the whole "server mods" key block (serverModsTitlethroughserverModsRemoveError) duplicated verbatim, back to back, insidefeatures.mods. Harmless today (JSON.parsejust keeps the last value of a repeated key) but unrelated to this issue, so left alone here and flagged separately.The new test
tests/i18n/no-bare-plural-keys.test.tswalks every locale file and fails if any key sits beside one of its own cardinal suffixes, naming the offending key and file.tests/i18n/i18n-parity.test.tsused a literalkey in enUSlookup, which stops finding a plural family once its bare key is gone. AddedresolveTranslationValue()totests/i18n/helpers.tsso both parity checks (key exists, interpolation object passed when needed) resolve a key through its suffixed siblings the way i18next does at runtime, instead of asserting on the now-removed bare key.drafted.jsonalso had review entries pointing at the removed bare keys; trimmed those out.README and guide
Weblate's free hosting for libre projects asks that the project say it uses Weblate. Added, in the README's translation section and at the top of the translation guide, that translation now happens on Weblate (https://hosted.weblate.org/projects/riftlauncher/) where anyone with a GitHub account can help, and that the guide stays for anyone who prefers a pull request.
Gate
npm run typecheck: cleannpm run lint:ci: 0 errors (14 pre-existing warnings, unrelated)npm run format:check: cleannpm run test:coverage: 240 test files passed, 4452 tests passed, 2 skipped. Coverage: statements 94.58%, branches 91.02%, functions 95.22%, lines 96.26%.Refs #496.
Review round 1
Locale fallback for counts a language treats separately
errorCount,warningCount,activeTasksandnewNotificationscarried only_oneand_other, which is all en-US selects between. With the bare key gone, i18next selected_fewfor a Russian count of 2, found nothing in ru-RU, and rendered the English sentence.This turned out to be wider than the three locales named: every locale whose
Intl.PluralRulesasks for more thanone/otherwas affected, so nine were, not three._fewand_manyare now defined for all four families in be-BY, es-ES, fr-FR, it-IT, pl-PL, pt-BR, pt-PT, ru-RU and uk-UA (52 forms, e.g.src/renderer/src/locales/ru-RU.json:796)._manyrepeats_otherthroughout: genitive plural in the Slavic locales, and in the Romance ones the category only fires on round millions, where the wording does not change. The new drafted forms are listed indrafted.jsonbeside the siblings it already tracked.Test:
tests/i18n/plural-runtime.test.ts:74sweeps every locale through a real i18next built with nofallbackLng, so a form the language selects but the file lacks returns the key instead of quietly becoming English, and:97pins the count of 2 you reproduced.tests/i18n/i18n-parity.test.ts:189is the static twin, holding every locale to the categoriesIntl.PluralRulesresolves for it rather than to a hand-kept list.Undefined
_modsCountduring the delayed scanConfirmed:
t("features.mods.modsCount", { count: undefined })returns the literal key, and every Installation loaded from disk is in that state until ConfigProvider's 2.5 second scan lands.Took the defined-count option. The three call sites now read
_modsCount ?? 0:src/renderer/src/features/installations/components/InstallationsDropdownMenu.tsx:65and:97, andsrc/renderer/src/features/installations/pages/ListInstallations.tsx:161, where it also replaces anas numbercast that was asserting the field is always present. 0 until something has counted is whatadapters/create.tsalready stamps on a freshly created Installation, so the pre-scan window now reads the same either way. The bare key rendered a blank number here, never a real one.Test:
tests/renderer-dom/installationsModsCount.test.tsx:54and:68render both real components with an Installation that has no_modsCount.The count contract the guard missed
Right that
resolveTranslationValueanswers only "does this key resolve to anything", and that a family whose_zeroform carries no placeholder would slip past the interpolation check on top of that. Enforced the count argument rather than inspecting every variant, since a plural family that is called without a count is broken regardless of what its variants contain.collectTranslationCallsnow reads the arguments at()call passes and records whether they namecount(tests/i18n/helpers.ts:128and:172). On the back of that,tests/i18n/i18n-parity.test.ts:62fails any call site whose key is a plural family and passes no count, and:75fails any family no call site reaches with one, so a family cannot go dark behind a dynamic key the scan cannot see either.Checked against your example: removing the count from
areYouSureDeleteSelectedinDeleteModDialog.tsxfails the new check by name. All 26 families pass a count today, so both guards are green on current code.One consequence worth flagging: fr-FR now carries
errorCount_many, which en-US has no reason to define, so the "no key en-US does not have" check would have read it as an orphan. It now compares plural families rather than raw keys (tests/i18n/i18n-parity.test.ts:246).Gate
npm run typecheck: cleannpm run lint:ci: 0 errors (14 pre-existing warnings, unrelated)npm run format:check: cleannpm run test:coverage: 242 test files passed, 4461 tests passed, 2 skipped. Coverage: statements 94.58%, branches 91.02%, functions 95.22%, lines 96.26%.Each fix was reverted on its own to confirm the test covering it goes red, then restored.