feat(i18n): add 15 locales and refactor tool helpers to use i18n - #308
feat(i18n): add 15 locales and refactor tool helpers to use i18n#308Rwanbt wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Findings
-
[Minor] Regression: Grep without pattern falls back to raw tool name -
src/renderer/components/message/toolHelpers.tsx:47
Whengrepis called without apattern, the new code returnsname(the raw tool name, e.g. "Grep" or "grep") instead of a localized string. The old code returned'Grep'. This means the label is not translated and inconsistent with other tool labels.
Suggested fix:- if (nameLower === 'grep') return inp.pattern ? t('toolLabels.grepPattern', { pattern: String(inp.pattern) }) : name; + if (nameLower === 'grep') return inp.pattern ? t('toolLabels.grepPattern', { pattern: String(inp.pattern) }) : t('toolLabels.grep');
And add
"grep": "Grep"to thetoolLabelsnamespace inen.jsonandzh.json(and all other locales). -
[Minor] Arabic locale added without RTL layout support -
src/renderer/i18n/locales/ar.json(new file)
The app now supports Arabic, but the UI does not applydir="rtl"when Arabic is selected. All text and layout will remain left-to-right, causing misalignment and poor UX. This is a pre-existing limitation but is now exposed.
Suggested fix: Insrc/renderer/i18n/config.tsor a root component, detect RTL languages (e.g.ar) and setdocument.diraccordingly.
Example:const rtlLangs = ['ar', 'he', 'fa']; i18n.on('languageChanged', (lng) => { document.dir = rtlLangs.includes(lng) ? 'rtl' : 'ltr'; });
-
[Minor] Bundle size increase from static locale imports -
src/renderer/i18n/config.ts:4-21
All 15 new locale files (~50 KB each) are statically imported, adding ~750 KB to the initial bundle. This increases load time for all users, even those who only use English.
Suggested fix: Implement lazy loading for non-default locales using dynamicimport(). The PR plan acknowledges this as out of scope, but it is a noteworthy concern. -
[Minor] No unit tests for i18n tool labels -
src/renderer/components/message/toolHelpers.tsx
ThegetToolLabelfunction now depends on atfunction, but there are no tests covering the different tool name branches or the new localization path. This increases the risk of regressions.
Suggested fix: Add tests insrc/tests/that mockTFunctionand verify the correct translation keys are called for each tool type. -
[Nit] Missing trailing newline in edited files -
src/renderer/i18n/config.ts,src/renderer/components/settings/SettingsGeneral.tsx
These files no longer end with a newline character. While not functionally breaking, it violates POSIX conventions and may cause noise ingit diff.
Suggested fix: Add a final newline.
Summary
Review mode: initial
Review policy: advisory — the check reflects automation health/completion only; it does not approve the PR or resolve findings.
The PR successfully adds 15 new locale files, refactors tool helpers to use i18n, and updates the language selector. The core changes are well-structured and follow the project conventions. The main concerns are a regression in the grep label fallback (minor), lack of RTL support for Arabic (minor), increased bundle size (minor), and missing test coverage (minor). The included check_locale.js and check_placeholders.js scripts provide valuable validation, but they are not run in CI. Consider integrating them into the workflow.
Testing
Not run (automation). Manual verification of at least five locales and a tool-call interaction is recommended before merging.
Open Cowork Bot
|
🔄 PR mis à jour — Rwanbt:dev réécrit via cherry-pick sur upstream/dev (ff4328d). Stratégie exécutée : branche \dev-resynced\ créée depuis \upstream/dev\ puis \git cherry-pick d013b55\ (anciens commits fusionnés en un seul). Conflits résolus :
PR allégé pour review facile :
Vérifications : \ sc\ clean (1 erreur pré-existante non liée sur \src/main/config/config-store.ts:369), \�ite build\ OK en 17.55s. À noter : les 15 locales contiennent encore \memory.*\ (88 clés) qui n'existe plus dans votre \en.json\ refactorisé. i18next ignore silencieusement les clés orphelines, donc harmless. À votre discrétion de regénérer avec le nouveau \en.json\ ou de conserver tel quel. |
|
📦 Commit mis à jour — \ |
Add 15 new locale files (ar, bs, da, de, es, fr, ja, ko, nb, pl, pt-BR, ru, th, tr, zh-TW) and i18n infrastructure to support multilingual UI. Applied via cherry-pick from feat/i18n-15-locales onto a fresh upstream/dev base (ff4328d). Conflicts on upstream-deleted files (toolHelpers/ToolUseBlock/ToolResultBlock/SettingsGeneral) were resolved by accepting the upstream delete — the maintainer will need to integrate the i18n into their post-refactor UI separately or via follow-up commits. Conflict on en.json/zh.json was resolved by taking upstream's refactored structure (memory section removed, credentials section added, sandbox safety keys added) and re-adding our 16-key toolLabels namespace. Net result: en.json/zh.json stay compatible with both upstream's new structure AND our 15 translations (which still contain memory.* for backwards compat). Additive (no conflict): - 15 new locale files: ar, bs, da, de, es, fr, ja, ko, nb, pl, pt-BR, ru, th, tr, zh-TW - 6 new scripts in scripts/i18n/: _util, scaffold_locale, extract_backlog, apply_translations, check_locale, check_placeholders (zero external dependency) Modified (conflicts resolved): - .gitignore: add .i18n-work/ (translation working directory) - src/renderer/i18n/config.ts: add 15 new locale imports + ALLOWLIST - src/renderer/i18n/locales/en.json: add toolLabels namespace, take upstream's refactored structure (memory removed, credentials added) - src/renderer/i18n/locales/zh.json: same as en.json Verification: tsc clean (1 pre-existing TS6133 warning on src/main/config/config-store.ts is unrelated to i18n), vite build OK in 17.55s.
|
???? Fix UI picker ??? commit 557c91e (??tait 05e676e, amend pour corriger un picker upstream incomplet). D??couverte pendant test live (CDP DevTools Protocol) : le picker upstream Correction appliqu??e (8 lignes ajout??es ?? src/renderer/components/SettingsPanel.tsx) :
V??rification live (CDP) :
Verdict : PR #308 livre l''i18n end-to-end fonctionnel, pas seulement l''infrastructure. RTL arabe support?? via attribut Diff size : +21 insertions, -4 deletions sur |
Summary
Add 15 new locale files (ar, bs, da, de, es, fr, ja, ko, nb, pl, pt-BR, ru, th, tr, zh-TW) and refactor tool helpers to use i18n.
Cross-fork PR from Rwanbt/open-cowork:dev (where the feature was integrated via PR #1 merged) to OpenCoworkAI/open-cowork:dev.
Changes
Verification
Translation coverage
14,637 translations bundled (861 keys × 17 locales).
Workflow dev-first (workflow respecté)
This PR was initially opened against \main\ (mistake — the contributor didn't realize upstream has a \dev\ branch at the time). After auditing, the target was retargeted to \dev\ to follow the documented dev-first contribution workflow.
Fork side: \eat/i18n-15-locales\ was merged into \Rwanbt:dev\ via PR #1 (already merged) → cross-fork PR #308 now targets \OpenCoworkAI:dev\ for upstream integration. Once approved here, the maintainer's normal \dev\ → \main\ workflow takes over.