Add a language switcher using PrimaryLanguageOverride #10309

lhecker · 2021-06-02T00:52:26Z

Summary of the Pull Request

This PR adds a global "language" setting, which may be set to any supported BCP 47 tag.
Additionally a ComboBox is added to the settings UI under "Appearance", listing all languages with their localized names.

This PR introduces one new issue: If you change the language while the app is running, the UI will be in a torn state, as not all UI elements refresh automatically if the PrimaryLanguageOverride is changed.

PR Checklist

Closes Choose language in settings.json #5497
I work here
Tests added/passed
Documentation updated. If checked, please file a pull request on our docs repo and link it here: #xxx
Schema updated

Validation Steps Performed

UI language changes when changing the "language" in settings.json before starting WT / while WT is running. ✔️
"language" field is removed from settings.json if "Use system default" is selected. ✔️
"language" field is added or updated in settings.json if any other language is selected. ✔️
Removes qps- languages if debugFeatures is false. ✔️
Correctly refreshes all UI elements with the new language. ❌

lhecker · 2021-06-02T01:05:42Z

src/cascadia/TerminalApp/pch.h

@@ -25,40 +25,39 @@

 #include <wil/cppwinrt.h>

-#include <unknwn.h>
-
-#include <hstring.h>


Since I had to add <winrt/Windows.Globalization.h> to some pch.h files I decided to cleaned them up by removing unused includes and ordering and reordered the includes alphabetically.

be careful! you must include unknwn and hstring BEFORE any c++/winrt headers; wil is also sensitive to include order (!)

I removed those entirely, because it compiles without them.

src/cascadia/TerminalSettingsEditor/GlobalAppearance.cpp

src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw

lhecker · 2021-06-02T01:08:27Z

src/cascadia/TerminalSettingsEditor/GlobalAppearance.xaml

-            <!--  Theme  -->
-            <local:SettingContainer x:Uid="Globals_Theme"
+            <!--  Language  -->
+            <local:SettingContainer x:Uid="Globals_Language"


I wasn't sure where I should put the ComboBox. Would it look better elsewhere? Maybe not as the first item in the Appearances section?
The reason I put it as the first item was simply because I thought it looked nicer that way.

I think this is fine for now. Ideally, this would go into some kind of "General" page, but we're just not there yet imo.

@cinnamon-msft thoughts?

carlos-zamora

Mostly nits.

Concerned about the torn state where some stuff stays localized and other stuff doesn't update.

doc/cascadia/profiles.schema.json

src/cascadia/TerminalSettingsEditor/Resources/en-US/Resources.resw

src/cascadia/TerminalSettingsModel/GlobalAppSettings.h

src/cascadia/TerminalSettingsEditor/GlobalAppearance.cpp

carlos-zamora · 2021-06-02T17:49:42Z

src/cascadia/TerminalSettingsEditor/GlobalAppearance.cpp

+    // Returns an array of {"und", "en-US", "de-DE", "es-ES", ...}.
+    // "und" is short for "undefined" and is synonym for "Use system language" in this code.


nit: follow the format we use for method descriptions

Oh I had hoped we only use that format for existing code/files...
It's rather verbose and sometimes redundant. (I'll change this comment.)

I've extended the comment, better detailing what it does...
If you don't mind I'll wait for others to concur with you until I change the style to what I perceive as the more verbose form.

src/cascadia/TerminalSettingsEditor/GlobalAppearance.cpp

DHowett · 2021-06-02T18:30:58Z

regarding the resw file: please re-save it using visual studio to "repair" the spacing 😄

lhecker · 2021-06-02T20:33:10Z

@DHowett @carlos-zamora If I change something in the .resw file and re-save it the spaces continue to be gone.
Would you like me to revert the changes to the trailing whitespace regardless?

DHowett · 2021-06-02T20:34:35Z

I am surprised about that. I guess we can just commit it; the next person to change it using VS will reintroduce them. 😄

lhecker · 2021-06-02T21:42:18Z

Oooooh... Nevermind... Apparently VS Code suppresses whitespace differences by default in the diff viewer!

marypq · 2021-06-03T20:38:48Z

X

DHowett

I cannot find fault with this. Very well done!

src/cascadia/TerminalApp/AppLogic.cpp

DHowett · 2021-06-03T22:24:56Z

src/cascadia/TerminalSettingsEditor/GlobalAppearance.cpp

@@ -5,6 +5,10 @@
 #include "GlobalAppearance.h"
 #include "GlobalAppearance.g.cpp"
 #include "GlobalAppearancePageNavigationState.g.cpp"
+
+#include <LibraryResources.h>


This should be part of the precompiled header precomp.h or pch.h and not be necessary here

<numeric> should be in LibraryIncludes

Are you sure putting it into <LibraryIncludes> or the pch.h file is a good idea?
<numeric> is basically only used in 2 places throughout the project.

src/cascadia/TerminalSettingsEditor/GlobalAppearance.cpp

src/inc/til/string.h

DHowett · 2021-06-03T22:34:29Z

Now, what happens if:

the user sets language to qps-ploc in their json while debugFeatures is false?
the user sets language to foo-bar in their json?

lhecker · 2021-06-03T23:52:27Z

@DHowett This PR """abuses""" the fact that the SUI saves the settings.json and triggers a settings reload in AppLogic. Then in AppLogic I compare the "language" settings value with the one returned by PrimaryLanguageOverride(). If they're different I call PrimaryLanguageOverride(newLanguage) (see AppLogic.cpp).
This means you can basically write whatever you want into the settings.json, without being constrained to what the SUI allows you to do. 🙂

DHowett · 2021-06-03T23:55:44Z

@DHowett This PR """abuses""" the fact that the SUI saves the settings.json and triggers a settings reload in AppLogic. Then in AppLogic I compare the "language" settings value with the one returned by PrimaryLanguageOverride(). If they're different I call PrimaryLanguageOverride(newLanguage) (see AppLogic.cpp).

This means you can basically write whatever you want into the settings.json, without being constrained to what the SUI allows you to do. 🙂

Indeed. I'm just hoping that it doesn't crash or overwrite the setting or do something weird if the JSON value doesn't exist in the actual language Combo list.

lhecker · 2021-06-06T15:05:03Z

src/cascadia/TerminalSettingsEditor/GlobalAppearance.cpp

+            if (!_State.Globals().DebugFeaturesEnabled())
+            {
+                // [4] part 1:
+                it = std::remove_if(it, tagsEnd, [](const winrt::hstring& tag) mutable -> bool {


@DHowett FYI I just realized this is a programming error and should be std::remove_if(tagsBegin, it, ...) instead. std::unique returns an iterator for the new end of the range, which I have to use instead of tagsEnd here.

carlos-zamora

Looks good! Just create a follow-up task for...

Settings UI not being reloaded entirely
- offline discussion: looks like you need to reload all of the NavigationView's menu items and also reload the page footer (with the save button)
Command Palette sometimes not reloading (nested?) commands

ghost · 2021-06-10T23:24:09Z

Hello @lhecker!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

ghost · 2021-07-14T17:08:41Z

🎉Windows Terminal Preview v1.10.1933.0 has been released which incorporates this pull request.:tada:

Handy links:

lhecker requested a review from carlos-zamora June 2, 2021 00:52

ghost added Area-Settings Issues related to settings and customizability, for console or terminal Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Terminal The new Windows Terminal. labels Jun 2, 2021

lhecker force-pushed the dev/lhecker/language-setting branch 2 times, most recently from 48a2cd9 to 8e7b994 Compare June 2, 2021 01:01

microsoft deleted a comment from github-actions bot Jun 2, 2021

lhecker force-pushed the dev/lhecker/language-setting branch from 8e7b994 to d92f4ca Compare June 2, 2021 01:04

Add a language switcher using PrimaryLanguageOverride

249f152

lhecker force-pushed the dev/lhecker/language-setting branch from d92f4ca to 249f152 Compare June 2, 2021 01:12

lhecker commented Jun 2, 2021

View reviewed changes

carlos-zamora requested changes Jun 2, 2021

View reviewed changes

ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jun 2, 2021

ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jun 2, 2021

Address reviewer comments

fbec8a7

This comment has been minimized.

Sign in to view

lhecker added 2 commits June 3, 2021 00:05

Make the spell checker happy

22c48e8

Make the audit mode happy

d7ef775

Address reviewer comments

248be79

DHowett approved these changes Jun 3, 2021

View reviewed changes

Address reviewer comments

5991876

lhecker commented Jun 6, 2021

View reviewed changes

Fix qps tag filtering

5d02173

carlos-zamora approved these changes Jun 10, 2021

View reviewed changes

lhecker added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jun 10, 2021

ghost merged commit e34897c into main Jun 10, 2021

ghost deleted the dev/lhecker/language-setting branch June 10, 2021 23:24

ghost mentioned this pull request Jul 14, 2021

Choose language in settings.json #5497

Closed

skyline75489 mentioned this pull request Jul 15, 2021

Windows Terminal may need to be translated into more languages #10667

Open

zadjii-msft mentioned this pull request May 23, 2023

Add setting to change language microsoft/devhome#708

Open

cjmaxik mentioned this pull request Jun 16, 2023

UI Language switcher microsoft/PowerToys#26884

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a language switcher using PrimaryLanguageOverride #10309

Add a language switcher using PrimaryLanguageOverride #10309

lhecker commented Jun 2, 2021 •

edited

Loading

lhecker Jun 2, 2021

DHowett Jun 2, 2021

lhecker Jun 2, 2021

lhecker Jun 2, 2021

carlos-zamora Jun 2, 2021

carlos-zamora left a comment

carlos-zamora Jun 2, 2021

lhecker Jun 2, 2021

lhecker Jun 2, 2021

DHowett commented Jun 2, 2021

lhecker commented Jun 2, 2021

DHowett commented Jun 2, 2021

lhecker commented Jun 2, 2021

This comment has been minimized.

marypq commented Jun 3, 2021

DHowett left a comment

DHowett Jun 3, 2021

lhecker Jun 4, 2021

DHowett commented Jun 3, 2021

lhecker commented Jun 3, 2021

DHowett commented Jun 3, 2021

lhecker Jun 6, 2021

carlos-zamora left a comment

ghost commented Jun 10, 2021

ghost commented Jul 14, 2021

		// Returns an array of {"und", "en-US", "de-DE", "es-ES", ...}.
		// "und" is short for "undefined" and is synonym for "Use system language" in this code.

Add a language switcher using PrimaryLanguageOverride #10309

Add a language switcher using PrimaryLanguageOverride #10309

Conversation

lhecker commented Jun 2, 2021 • edited Loading

Summary of the Pull Request

PR Checklist

Validation Steps Performed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carlos-zamora left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DHowett commented Jun 2, 2021

lhecker commented Jun 2, 2021

DHowett commented Jun 2, 2021

lhecker commented Jun 2, 2021

This comment has been minimized.

marypq commented Jun 3, 2021

DHowett left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DHowett commented Jun 3, 2021

lhecker commented Jun 3, 2021

DHowett commented Jun 3, 2021

Choose a reason for hiding this comment

carlos-zamora left a comment

Choose a reason for hiding this comment

ghost commented Jun 10, 2021

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

ghost commented Jul 14, 2021

lhecker commented Jun 2, 2021 •

edited

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.