Windows: Implement GetPreferredLanguages for UWP and support l18n#27311
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
chinmaygarde
left a comment
There was a problem hiding this comment.
I am not familar with this subsystem but I took a stab at reviewing this since there weren't any other takers. A couple of clarifications and nits but other LGTM.
|
|
||
| #include <sstream> | ||
|
|
||
| #include <third_party/cppwinrt/generated/winrt/Windows.Foundation.Collections.h> |
There was a problem hiding this comment.
Nit: This is not a system header. Please use use quotes instead of angle brackets to include these.
|
|
||
| auto platform_langueages = winrt::Windows::System::UserProfile:: | ||
| GlobalizationPreferences::Languages(); | ||
| for (auto platform_language : platform_langueages) { |
There was a problem hiding this comment.
Does GlobalizationPreferences::Languages need some form of error handling?
There was a problem hiding this comment.
Nit: const auto& platform_language
There was a problem hiding this comment.
- The document GlobalizationPreferences::Languages doesn't say that this function throw any errors, so I think I don't have to handle errors here.
- done
|
Thank you @chinmaygarde for your reviewing. I fix them. |
|
Sorry, one last thing, can this be rebased on ToT please? The presubmits are failing because the commit you built on it too old. |
b6aef71 to
5f4cdf7
Compare
|
done |
|
Thanks so much! |
| languages.push_back(L"en-US"); | ||
| languages.push_back(L"en"); | ||
|
|
||
| auto platform_langueages = winrt::Windows::System::UserProfile:: |
There was a problem hiding this comment.
Sorry. I'm little busy now so I'll fix it with new PR by tomorrow.
Fixes flutter/flutter#74156
It implements
GetPreferredLanguagesfor winuwp platform and replace the stub, and support l18n on UWP.Test on both windows and winuwp for
GetPreferredLanguagesalready exists in master branch.Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.