Repository navigation
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where Locale.system would incorrectly fall back to 'und' on various platforms because the raw platform locale strings were not compatible with BCP47 parsing. By implementing a normalization layer, the library now correctly interprets diverse locale formats from Linux, Windows, and Apple platforms, significantly improving the reliability of system locale detection. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request improves Locale.system resolution by converting Platform.localeName to a BCP47 tag, resolving issues where it returned und on Linux, Windows, and Apple platforms. Feedback on the changes highlights a bug in the parsing logic where hybrid locale names (such as en_US-POSIX or sr_Latn-RS) would be incorrectly truncated. The reviewer suggested refining the truncation logic to ensure the underscore appears after the hyphen before truncating, and adding corresponding test cases to verify this behavior.
| final tag = platformLocaleNameToBcp47(localeName); | ||
| expect(tag, expected); | ||
| // Only check that the tag is accepted, as the canonical form differs | ||
| // between backends: ICU4X turns `en-US-POSIX` into `en-US-posix`, |
There was a problem hiding this comment.
this might be an ICU4X bug
Platform.localeName returns POSIX locale names on Linux (en_US.UTF-8), Windows sort orders (de-DE_phoneb), and Apple CFLocale identifiers (en_US@calendar=japanese), none of which ICU4X can parse, so Locale.system silently fell back to und. Fixes dart-lang#1026
Map Apple's ICU-style keywords (en_US@calendar=japanese;hours=h23) to -u- extensions, Windows sort orders (de-DE_phoneb) to -u-co-, and glibc's @euro modifier to -u-cu-eur.
b1957ba to
cf30036
Compare
Note
This PR was generated by an AI coding agent (Jetski) on behalf of @mosuem.
Locale.systempassedPlatform.localeNamestraight toLocale.parse. On most non-web platforms that name is not a BCP47 tag, so ICU4X rejects it andLocale.systemsilently falls back tound.What
Platform.localeNamereturns, per the Dart SDK (runtime/bin/platform_*.cc):getenv("LANG"), defaulten_USen_US.UTF-8,sr_RS.UTF-8@latin,C.UTF-8unden-US,sr-Latn-RS,undGetUserDefaultLocaleNameen-US,de-DE_phoneb(sort order)en-US,unden-US,de-DE-u-co-phonebkCFLocaleidentifierzh-Hans-CN,en_US@calendar=japanese;hours=h23zh-Hans-CN,undzh-Hans-CN,en-US-u-ca-japanese-hc-h23Changes:
platformLocaleNameToBcp47inlib/src/locale/platform_locale_name.dart. It strips the POSIX codeset, maps glibc's@latin/@cyrillicmodifiers to script subtags, mapsC/POSIXtound, and replaces_with-. The regexes are top-levelfinals, so they are compiled once, and the main regex is documented piece by piece.-u-keywords, mapping legacy names and values (calendar=gregorian→ca-gregory,collation=phonebook→co-phonebk,hours→hc,numbers→nu,measure=imperial→ms-uksystem, …). Keys are sorted, and unknown keys or invalid values (such astimezone=America/New_York) are dropped._phoneb→co-phonebk,_tradnl→co-trad,_stroke→co-stroke,_pronun→co-zhuyin,_radstr→co-unihan. Hungarian_tchncland Georgian_modernhave no CLDR equivalent and are dropped.@eurobecomes-u-cu-eur.findSystemLocale()on non-web platforms. Web is unchanged, asnavigator.languageis already BCP47.Locale.systemthatPlatform.localeNamedoes not reflect the device locale on Android, asLANGis usually unset there, and how Flutter apps should pass the locale instead.Locale.parseaccepts it.Verified locally:
dart analyze --fatal-infos,dart format, anddart test(VM) pass. WithLANG=en_US.UTF-8,Locale.systemnow returnsen-USinstead ofund. Checked end to end on native that the extensions take effect:en_US@calendar=japaneseformats asJan 2, 8 Reiwa,en_US@hours=h23as22:23:00,th_TH@calendar=buddhist;numbers=thaias๒ ม.ค. ๒๕๖๙, andde-DE_phonebsortsMüllernext toMueller. Chrome and wasm tests run in CI.Fixes #1026