-
Notifications
You must be signed in to change notification settings - Fork 305
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[MRTCore] Add global PrimaryLanguageOverride switch #4181
base: main
Are you sure you want to change the base?
[MRTCore] Add global PrimaryLanguageOverride switch #4181
Conversation
/azp run |
Commenter does not have sufficient privileges for PR 4181 in repo microsoft/WindowsAppSDK |
while doubtful /azp run |
/azp run |
Commenter does not have sufficient privileges for PR 4181 in repo microsoft/WindowsAppSDK |
This is needed for PowerToys to fully enable language override |
static hstring PrimaryLanguageOverride(); | ||
|
||
private: | ||
static hstring m_language; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens with multiple threads?
[contract(MrtCoreContract, 1)] | ||
runtimeclass ApplicationLanguages | ||
{ | ||
static String PrimaryLanguageOverride; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has Xaml/WinUI3 been hooked up to use this new API? Or does it all just work?
[contract(MrtCoreContract, 1)] | ||
runtimeclass ApplicationLanguages | ||
{ | ||
static String PrimaryLanguageOverride; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New API, so where's the test / sample in the peer Windows App SDK Samples repo?
} // namespace Microsoft.Windows.ApplicationModel.Resources | ||
|
||
[contract(MrtCoreContract, 1)] | ||
runtimeclass ApplicationLanguages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New API, so please include a markdown file for API review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Often better to make a PR with just a markdown spec for review, separate from implementation/test/etc. Streamlines the API Review process for everyone involved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added comments. Please address and update.
{ | ||
hstring ApplicationLanguages::m_language; | ||
|
||
void ApplicationLanguages::PrimaryLanguageOverride(hstring language) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this file from the generated skeleton from C++/WinRT or hand crafted? Generated files usually have string parameters as 'hstring const& blah' (hstring
will cause a deep copy on each call)
After compiling your .idl grab the code from d:\source\repos\windowsappsdk\obj\Debug\x64\...dir-for-this-project...\Generated Files\sources\Microsoft.Windows.ApplicationModel.Resources.ApplicationLanguages.cpp
and matching .h, and follow their contained directions.
Changing the application language by setting
Windows.Globalization.ApplicationLanguages.PrimaryLanguageOverride
to a specific language is not supported for WASDK unpackaged app, only for packaged ones. To support the same behavior for unpackaged applications similar switch is introduced into WASDK MRTCore module.Sample app used to verify the change: https://github.com/stefansjfw/testwasdk/blob/master/testlocalwinappsdk/App.xaml.cs#L36
A microsoft employee must use /azp run to validate using the pipelines below.
WARNING:
Comments made by azure-pipelines bot maybe inaccurate.
Please see pipeline link to verify that the build is being ran.
For status checks on the main branch, please use TransportPackage-Foundation-PR
(https://microsoft.visualstudio.com/ProjectReunion/_build?definitionId=81063&_a=summary)
and run the build against your PR branch with the default parameters.