Skip to content

refactor(i18n): one file per locale with keys typed against English; add Italian - #120

Merged
amirlehmam merged 2 commits into
amirlehmam:masterfrom
simplyBarbe:master
Jul 28, 2026
Merged

refactor(i18n): one file per locale with keys typed against English; add Italian#120
amirlehmam merged 2 commits into
amirlehmam:masterfrom
simplyBarbe:master

Conversation

@simplyBarbe

Copy link
Copy Markdown
Contributor

Adding a language meant editing five places, and the fifth was not in the i18n folder at all:

# Location What breaks if you forget
1 core.ts — the Language union won't compile (good)
2 core.ts — the LANGUAGES array missing from the Settings dropdown
3 core.ts — the dictionary const
4 core.ts — the DICTS map won't compile (good)
5 settings-slice.tscandidate === 'en' || 'fr' || 'zh' silent: the language persists, then resets to the detected default on next launch

That last one is the dangerous kind — no compile error, no failing test, just a bug discovered after shipping.

Two other problems: Record<string, string> meant nothing was checked, so t('setings.title') compiled and rendered the raw key; and at ~90 lines per language a single 351-line file would put every translation PR in conflict with every other.

What changed

Dictionaries move out of core.ts into one file per locale. English is the source of truth and exports the key type:

// locales/en.ts
export const en = { 'settings.title': 'Settings', /* … */ } as const;
export type TranslationKey = keyof typeof en;
export type Translation = Partial<Record<TranslationKey, string>>;

Partial is the deliberate choice: a missing key is legal (it falls back to English, so partial translations still ship), but an unknown key is a compile error — that catches typos and keys stranded by an English rename. useT() and translate() now take TranslationKey, extending the same check to all 79 call sites.

core.ts keeps a single table everything derives from:

const REGISTRY = [
  { code: 'en', label: 'English',  dict: en as Translation },
  { code: 'fr', label: 'Français', dict: fr },
  { code: 'it', label: 'Italiano', dict: it },
  { code: 'zh', label: '中文',      dict: zh },
] as const;

export type Language = (typeof REGISTRY)[number]['code'];

Language, SUPPORTED_LANGUAGES, LANGUAGES and the new isLanguage() guard all derive from it, and settings-slice.ts uses isLanguage() instead of the hardcoded list — closing case #5.

Also fixed: SettingsWindow's TAB_LABEL_KEYS was annotated Record<Tab, string>, which widened the literals away. Typing it as TranslationKey checks those nine keys where they are written. This one was found by the compiler, not by review — exactly the drift the refactor exists to catch.

Adding a language is now two edits: a locales/xx.ts file and one row in REGISTRY.

Italian

Added as the first language under the new structure, as a test of that claim — it took the promised two edits. A third file changed because the test pinning the shipped language set failed on its own:

AssertionError: expected [ 'en', 'fr', 'it', 'zh' ] to deeply equal [ 'en', 'fr', 'zh' ]

That guard is working as designed: the shipped set cannot grow unnoticed. Everything else — the Settings dropdown, isLanguage(), the stale-key test — picked up it on its own.

Behavior change: detectDefaultLanguage() now resolves it-IT, so on an Italian-display Windows a fresh profile starts in Italian instead of English (the issue #56 case, now covering Italian). Existing installs keep whatever they persisted.

Contract preserved

LANGUAGES still yields exactly { code, label } (asserted — the registry's dict field does not leak), and translate, detectDefaultLanguage, useT, Language and the '../i18n/core' import path are unchanged.

Verification

  • Dictionaries verified byte-identical to the previous core.ts — old and new transpiled and diffed object-for-object, 84 keys × 3 languages, zero differences.
  • Contract proven negatively: a scratch file with three bad cases (unknown key in a locale, typo at a call site, unshipped language code) produced exactly three errors, while the three valid cases compiled clean.
  • npm run typecheck → 0
  • npm test430/430 across 46 files
  • npm run lint → clean on every touched file (32 pre-existing problems repo-wide, unchanged vs. base)
  • Coverage: en 84/84 · fr 84/84 · it 84/84 · zh 84/84

Test changes

The suite gains a stale-key guard per language, a coverage report, isLanguage cases, and a cast in the fallback-chain test — that key is now provably invalid, so it needs as TranslationKey to still exercise the runtime chain. Worth knowing: vitest strips types, so that one would have stayed green while typecheck failed.

simplyBarbe and others added 2 commits July 27, 2026 11:50
Adding a language meant editing five places, and the fifth was not in the
i18n folder: loadPersistedLanguage() in settings-slice.ts hardcoded
`candidate === 'en' || 'fr' || 'zh'`. Miss it and a new language persists,
then silently resets to the detected default on the next launch — no
compile error, no failing test.

Dictionaries move out of the 351-line core.ts into locales/{en,fr,zh}.ts.
English is the source of truth and exports TranslationKey/Translation;
the other locales are typed against it, so a partial translation is still
legal (missing keys fall back to English) but an unknown key — a typo, or
one stranded by an English rename — no longer compiles. useT() and
translate() take TranslationKey, which extends the same check to call
sites.

core.ts keeps a single REGISTRY table that Language, SUPPORTED_LANGUAGES,
LANGUAGES and the new isLanguage() guard all derive from. Adding a
language is now one locales/xx.ts file plus one row.

SettingsWindow's TAB_LABEL_KEYS was annotated Record<Tab, string>, which
widened the literals away; typing it as TranslationKey checks those nine
keys where they are written.

Exported contract unchanged: LANGUAGES still yields exactly {code, label},
and translate/detectDefaultLanguage/Language/'../i18n/core' are as before.
Dictionaries verified byte-identical to the previous core.ts (84 keys x 3).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
All 84 keys translated, so nothing falls back to English. The registry
refactor held: this took one locales/it.ts plus one row in REGISTRY —
Language, SUPPORTED_LANGUAGES, the Settings dropdown, isLanguage() and the
stale-key test all picked up 'it' on their own.

The third file is the guard doing its job: the test pinning the shipped
language set failed on its own ("expected [en, fr, it, zh] to deeply equal
[en, fr, zh]"), which is how a language is kept from appearing unnoticed.
Updated to the new set.

Side effect worth noting: detectDefaultLanguage() now resolves it-IT to
'it', so on an Italian-display Windows a fresh profile starts in Italian
instead of English — the issue amirlehmam#56 case, now covering Italian. Existing
installs keep whatever they persisted.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@amirlehmam
amirlehmam merged commit 1696aa9 into amirlehmam:master Jul 28, 2026
amirlehmam added a commit that referenced this pull request Jul 28, 2026
The last open piece of the markdown viewer: read → edit → save now
happens where the reading happens, instead of needing an editor pane to
change three characters.

The interesting design is not the textarea, it is the write path. This
is the first renderer→disk write in this surface, the renderer holds the
backing path in its store, and markdown content is explicitly untrusted
— so a renderer-supplied write path is treated as attacker-controlled.
markdown-grants.ts keeps a per-window set of paths that a *native
dialog* or an *authenticated pipe client* opened this session, and
refuses to write anywhere else. MARKDOWN_READ_FILE is deliberately NOT a
grant source: it takes a renderer-supplied path, so granting on it would
let the renderer mint its own grants and the set would mean nothing. The
cost is that a dragged-in file saves through Save As the first time —
which is the user confirming the destination, i.e. the point.

writeMarkdownFile re-applies every read guard on the way out (extension
whitelist, 5 MB cap, symlink refusal — a grant must not be launderable
into writing a .ps1), adds optimistic concurrency on mtime, and renames
a temp file into place, because a half-written spec is worse than an
unsaved one.

Conflicts are detected by re-stat on focus rather than a watcher per
pane: it needs no lifecycle and covers the real case, an agent rewriting
the file while the user was in another pane. With a dirty buffer the
save is blocked and the banner offers overwrite / reload / save a copy.

The editor is a plain textarea — CodeMirror would add 1–3 MB for a
surface whose job is "fix a typo, add a bullet" — with Tab indenting
instead of moving focus, and Enter continuing list/quote prefixes
(ending the list on an empty item, as every editor does).

Unsaved buffers get a `•` on the tab and confirm before closing, through
the same request/pending/confirm split ConfirmCloseDialog already uses
for workspaces, so `wmux close-surface` never blocks on a modal.

Also closes the two remaining "worth fixing" items from the issue:
`--title` on `markdown set` (every CLI-pushed surface was labelled
"Markdown") and `wmux markdown get`, which lets an agent verify what it
actually pushed.

PR #120's typed keys earned themselves immediately: every one of the 15
new i18n keys was a compile error until it was declared in en.ts. One
hole found and closed while adding them — the toolbar's menu builder had
typed `t` as `(key: any)`, which exempted its labels from that check.

Closes #116

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MNZmKeLgMpkhvZtLFrm6kG
@amirlehmam

Copy link
Copy Markdown
Owner

Merged in 1696aa9 — thank you, this is a good refactor and the reasoning in the description holds up.

What I verified before merging

The claim the whole PR rests on is that the dictionaries came across untouched, so I checked it independently rather than taking the diff's word for it: extracted the three dicts from the old core.ts and the three new locales/*.ts, evaluated both, and compared object-for-object. 84 keys each in en/fr/zh, zero value differences. Beyond that: 449 tests pass, typecheck is clean on both projects, and lint is clean on every touched file (the 32 repo-wide problems are pre-existing and unchanged against base).

Case #5 in your table is the one that made this worth doing. A candidate === 'en' || 'fr' || 'zh' sitting in settings-slice.ts is exactly the kind of drift that ships: no compile error, no failing test, and the symptom (language resets on next launch) shows up far enough from the cause that nobody connects the two. Deriving isLanguage() from the registry is the right shape — the guard can't fall behind the thing it guards.

Partial<Record<TranslationKey, string>> is also the right call over a full Record. A missing key falling back to English is what lets a partial translation ship at all; an unknown key being an error is what catches the typo. Getting both from one type is neat.

One follow-up applied on merge

The Italian strings used French-style spaced guillemets — « Apri in wmux ». Italian sets them tight: «Apri in wmux». Four strings, fixed in the merge commit.

It earned itself within the hour

I shipped issue #116's F3 (markdown edit & save) straight after merging this, which needed 15 new i18n keys. Every single one was a compile error until it was declared in en.ts — which is precisely the failure mode you built this to catch, and it caught them before they could render as raw keys in the UI.

It also surfaced a hole I'd have shipped otherwise: I'd typed a t parameter as (key: any, fallback?: string) => string when threading the translator into a helper, which silently exempted that whole table of labels from the check. Typing it TranslationKey turned it back on. Worth knowing as a general hazard with this design — the contract is only as strong as the narrowest t in the call chain.

Italian ships in v0.37.0, out now.

amirlehmam pushed a commit that referenced this pull request Jul 29, 2026
Second language added under the per-locale structure from #120, and the
two-edit claim holds: a locales/es.ts file plus one row in REGISTRY.
Language, SUPPORTED_LANGUAGES, the Settings dropdown and the isLanguage()
persistence guard all derive from that table and picked up `es` on their
own; the only other change is the test that deliberately pins the shipped
language set, which fails by design when the set grows.

Coverage is complete: es 107/107 keys, so nothing falls back to English.

detectDefaultLanguage() now resolves es-*, so a fresh profile on a
Spanish-display Windows starts in Spanish instead of English (the issue
#56 case). Existing installs keep whatever they persisted.


Claude-Session: https://claude.ai/code/session_0137J8T5Ckm8GodkfkJc1MQi

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants