Repository navigation
Conversation
`formatTrackingSince()` builds its date with `toLocaleDateString(undefined, …)`, which follows the OS locale rather than the language chosen in Settings. On a machine set to en-US the "Tracking since …" header stays English even when the rest of the pane is translated. Added `getIntlLocale()` and used it for that call. The helper handles the case that makes this non-trivial: plugin language packs register under the synthetic `plugin<hex>` resource language from `pluginLanguageResourceId()`, which `Intl` rejects outright, so it resolves to the BCP-47 tag the pack declares and falls back to the default locale when `Intl` has no data for the tag. This was reviewed and confirmed in stablyai#11826 but did not survive the merge, so it is re-sent on its own.
Greptile SummaryThis PR fixes a localization bug where
Confidence Score: 5/5Safe to merge — the change is a targeted, renderer-only fix to a single date-formatting call site, with robust fallback logic and no new external surfaces. The helper is small, self-contained, and its three execution paths (built-in locale, plugin-pack locale, fallback) are each covered by a test. The try/catch around supportedLocalesOf ensures invalid tags never propagate. The only observable difference from main is that the tracking-since date now follows the UI language rather than the OS locale — which is the intended fix. Files Needing Attention: No files require special attention.
|
| Filename | Overview |
|---|---|
| src/renderer/src/i18n/i18n.ts | Adds getIntlLocale() — correctly resolves plugin resource languages, validates with supportedLocalesOf, and falls back to DEFAULT_LOCALE; logic is sound and well-guarded. |
| src/renderer/src/components/stats/StatsPane.tsx | Single-line fix replacing undefined with getIntlLocale() in toLocaleDateString; change is minimal and correct. |
| src/renderer/src/i18n/intl-locale.test.ts | New test file with five well-structured cases exercising getIntlLocale() through the public API; stubs supportedLocalesOf to decouple from ICU data availability. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["formatTrackingSince(timestamp)"] --> B["getIntlLocale()"]
B --> C["active = i18n.language"]
C --> D{"Is it a plugin\nresource language?"}
D -- "Yes: find in pluginLanguagePacks" --> E["candidate = pack.locale\n(e.g. 'ru-RU')"]
D -- "No" --> F["candidate = active\n(e.g. 'es')"]
E --> G["Intl.DateTimeFormat\n.supportedLocalesOf(candidate)"]
F --> G
G -- "returns non-empty array" --> H["return result[0]"]
G -- "returns empty array" --> I["return DEFAULT_LOCALE ('en')"]
G -- "throws RangeError" --> I
H --> J["date.toLocaleDateString(locale, opts)"]
I --> J
Reviews (3): Last reviewed commit: "test(i18n): stub ICU lookup so the local..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe i18n module adds 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/renderer/src/i18n/intl-locale.test.ts (1)
10-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest
getIntlLocale()directly.These tests only verify
Intl.DateTimeFormat.supportedLocalesOf(). They do not exercisegetIntlLocale(), plugin-pack lookup, or the fallback branches. Add cases that configure a synthetic plugin resource language with a declared locale, an unsupported declared locale, and an invalid active locale. Assert that the helper returns the declared locale orDEFAULT_LOCALEas appropriate.Based on the supplied test body and
getIntlLocale()implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: bf024afd-32b0-407a-9345-4107fb812ae5
📒 Files selected for processing (3)
src/renderer/src/components/stats/StatsPane.tsxsrc/renderer/src/i18n/i18n.tssrc/renderer/src/i18n/intl-locale.test.ts
…nditions The suite pinned the Intl behaviour that motivates the helper but never called it, so a regression in the pack lookup or the supportedLocalesOf guard would have gone unnoticed. Drives the function through the public API — registering a pack and switching languages — rather than mocking module internals, so the plugin path is covered end to end.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 504ea730-7d6d-4a18-8880-ddcd2019e777
📒 Files selected for processing (1)
src/renderer/src/i18n/intl-locale.test.ts
supportedLocalesOf() answers from whatever ICU data the runtime carries, so asserting on real tags made the branch coverage depend on the build: a small-icu Node would fail the passthrough case for reasons unrelated to the helper. The branch assertions now stub the lookup and describe the logic instead. The RangeError case keeps its unstubbed assertion, since rejecting the synthetic tag is a property of the tag itself rather than of ICU data.
|
@brennanb2025 — flagging this one for you since CODEOWNERS did not pick it up. It touches
No rush on any of these — mostly wanted to make sure #12105 is not invisible for want of a CODEOWNERS entry. |
formatTrackingSince() used the OS locale rather than the language chosen in Settings. getIntlLocale() resolves the active i18n language to a BCP-47 tag, mapping the synthetic plugin<hex> resource language that Intl rejects back to the pack's real locale. Co-authored-by: Evgenii <kumiro@me.com>
Eight renderer call sites built Intl.RelativeTimeFormat(undefined, ...), which resolves to the OS locale, so relative timestamps rendered in Korean on a Korean-locale machine even with the UI language set to English. Unlike DateTimeFormat, RelativeTimeFormat emits language words, so it must follow the UI language. Rebased onto main: the formatter now resolves through getIntlLocale() (#12105) rather than i18n.resolvedLanguage, so the synthetic plugin<hex> resource language cannot reach the constructor and throw. GitHubItemDialog and PullRequestPage no longer own their formatter — main moved it into work-item-state-presentation, which is converted instead, along with the newly added site there. Co-authored-by: moseoh <azqazq195@gmail.com> Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Co-authored-by: Orca <help@stably.ai>
|
Merged into Closing this in favour of that. Thank you for the fix, and sorry it took as long as it did to get through. |
4 participants
Summary
formatTrackingSince()inStatsPaneformats its date withtoLocaleDateString(undefined, …), which follows the OS locale rather than the language selected in Settings. On a machine set toen-US, the "Tracking since 11 Jun 2026" header stays English while the rest of the pane is translated.This was reviewed and confirmed in #11826 (thread) but did not survive the merge, so it is re-sent on its own.
Why it needs a helper
Passing
i18n.languagestraight toIntlis not safe. Plugin language packs register under the syntheticplugin<hex>resource language produced bypluginLanguageResourceId(), andIntlthrows aRangeErroron it:getIntlLocale()resolves the active language to a tagIntlaccepts:localethe pack declares (ru-RU);Intlhas data for it;DEFAULT_LOCALE— never to the runtime locale, which is the bug being fixed.The helper is exported so other
toLocaleString()call sites can adopt it later; this PR deliberately changes only the one call site the review flagged.Screenshots
No visual changefor an English UI — the date renders identically. The difference appears only when the selected language and the OS locale differ.Testing
pnpm lintpnpm typecheckpnpm testpnpm buildsrc/renderer/src/i18n/intl-locale.test.ts— five cases drivinggetIntlLocale()through the public API (registering a pack, switching languages) rather than mocking internals: built-in locale passthrough, plugin resource language resolving to the declaredru-RU, unregistered plugin tag and unsupported-but-valid tag both falling back toDEFAULT_LOCALE, and the synthetic tag still throwing when handed toIntldirectly.pnpm test: 43 491 passed, 79 skipped. Two files fail on this machine —src/main/ssh/ssh-posix-command-wrapper.test.ts(3) andsrc/relay/agent-exec-handler.test.ts(2). Both fail identically on a cleanmainwith this branch stashed, so they are pre-existing environment failures.AI Review Report
Reviewed with Claude Opus 5.
Intldata availability differs between Electron builds and Node versions, which is exactly why the result is validated throughsupportedLocalesOf()instead of being trusted; the fallback keeps behaviour deterministic on every platform.supportedLocalesOf()call per render of a single header; negligible, and the previous code already constructed a formatter on the same path.Security Audit
Reviewed with Claude Opus 5. No new input handling, command execution, path handling, auth, secrets, dependency, or IPC surface. The only external input is the
localestring a plugin declares in its manifest, which is already validated byparsePluginLanguagePackArtifactand is passed toIntlinside atry/catchthat cannot escape. No follow-up needed.Notes
Found while maintaining a full Russian language pack for Orca — smwbev/orca-russian, 11 676 strings, 98.2 % of
en.json. The plugin-pack branch of the helper is not hypothetical: that is the path every plugin-supplied locale takes.X: @silentmegawatt · GitHub: @smwbev