Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 1.8k
Add persistent output locale override setting#6310
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
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
c1f78655e8451fdd556b262d7b58bf625f871d7a29e7a72b24933fcb6f7c3b4d1dd02a38c8df42abe827a48f0e07d2a82625c246a8453ff9c495788File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -10,6 +10,7 @@ | ||
| #include "COMContext.h" | ||
| #include <AppInstallerFileLogger.h> | ||
| #include <winget/OutputDebugStringLogger.h> | ||
| #include <winget/Resources.h> | ||
| #include "Public/ShutdownMonitoring.h" | ||
| #ifndef AICLI_DISABLE_TEST_HOOKS | ||
| @@ -78,6 +79,24 @@ namespace AppInstaller::CLI | ||
| main.Wait = WaitOnMainWaitEvent; | ||
| ShutdownMonitoring::ServerShutdownSynchronization::AddComponent(main); | ||
| } | ||
| std::string ApplyOutputLocaleOverride() | ||
| { | ||
| std::string localeTag{ Settings::User().Get<Settings::Setting::OutputLocale>() }; | ||
| if (localeTag.empty()) | ||
| { | ||
| return {}; | ||
| } | ||
| if (!AppInstaller::Resource::SetLanguageOverride(localeTag)) | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can throw; do we think it important enough to respect the target locale that we should terminate the winget.exe process if we cannot? ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right now the throw should be caught internally to the function and result in the I'm not opposed to making it a hard failure, but without knowing how often it would or could throw, just makes me a bit uneasy. Is this a decision that needs to be made now? Could we do something in the future like adding a telemetry event sampling language overrides as a | ||
| { | ||
| AICLI_LOG(CLI, Warning, << "Failed to apply output locale override from settings: " << localeTag); | ||
| return {}; | ||
| } | ||
| return localeTag; | ||
| } | ||
| } | ||
| int CoreMain(int argc, wchar_t const** argv) try | ||
| @@ -89,7 +108,6 @@ namespace AppInstaller::CLI | ||
| std::signal(SIGABRT, abort_signal_handler); | ||
| init_apartment(); | ||
| #ifndef AICLI_DISABLE_TEST_HOOKS | ||
| // We have to do this here so the auto minidump config initialization gets caught | ||
| Logging::OutputDebugStringLogger::Add(); | ||
| @@ -120,6 +138,12 @@ namespace AppInstaller::CLI | ||
| Logging::OutputDebugStringLogger::Remove(); | ||
| Logging::EnableWilFailureTelemetry(); | ||
| std::string outputLocaleOverride = ApplyOutputLocaleOverride(); | ||
| if (!outputLocaleOverride.empty()) | ||
| { | ||
| AICLI_LOG(CLI, Info, << "Applied output locale override from settings: " << outputLocaleOverride); | ||
| } | ||
| // Set output to UTF8 | ||
| ConsoleOutputCPRestore utf8CP(CP_UTF8); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| // ----------------------------------------------------------------------------- | ||
| // <copyright file="Settings.cs" company="Microsoft Corporation"> | ||
| // Copyright (c) Microsoft Corporation. Licensed under the MIT License. | ||
| // </copyright> | ||
| // ----------------------------------------------------------------------------- | ||
| namespace AppInstallerCLIE2ETests | ||
| { | ||
| using AppInstallerCLIE2ETests.Helpers; | ||
| using NUnit.Framework; | ||
| /// <summary> | ||
| /// Tests user settings behavior. | ||
| /// </summary> | ||
| public class Settings | ||
| { | ||
| /// <summary> | ||
| /// Reset settings before these tests run. | ||
| /// </summary> | ||
| [OneTimeSetUp] | ||
| public void OneTimeSetup() | ||
| { | ||
| WinGetSettingsHelper.InitializeWingetSettings(); | ||
| } | ||
| /// <summary> | ||
| /// Reset settings after these tests complete. | ||
| /// </summary> | ||
| [OneTimeTearDown] | ||
| public void OneTimeTearDown() | ||
| { | ||
| WinGetSettingsHelper.InitializeWingetSettings(); | ||
| } | ||
| /// <summary> | ||
| /// Verifies that changing output.locale changes help output. | ||
| /// </summary> | ||
| [Test] | ||
| public void OutputLocaleChangesHelpOutput() | ||
| { | ||
| WinGetSettingsHelper.ConfigureOutputLocale("en-US"); | ||
| var english = RunHelpCommand(); | ||
| WinGetSettingsHelper.ConfigureOutputLocale("ru-RU"); | ||
| var russian = RunHelpCommand(); | ||
| Assert.That(russian.StdOut, Is.Not.EqualTo(english.StdOut)); | ||
| WinGetSettingsHelper.ConfigureOutputLocale("en-US"); | ||
| var englishAgain = RunHelpCommand(); | ||
| Assert.That(englishAgain.StdOut, Is.EqualTo(english.StdOut)); | ||
| } | ||
| // Shared command helper for settings tests that validate global CLI output. | ||
| private static TestCommon.RunCommandResult RunHelpCommand() | ||
| { | ||
| var result = TestCommon.RunAICLICommand(string.Empty, "-?"); | ||
| Assert.That(result.ExitCode, Is.EqualTo(Constants.ErrorCode.S_OK)); | ||
| return result; | ||
| } | ||
| } | ||
| } |
Trenly marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -925,6 +925,64 @@ TEST_CASE("SettingOutputSortDirection", "[settings]") | ||
| } | ||
| } | ||
| TEST_CASE("SettingOutputLocale", "[settings]") | ||
Trenly marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| { | ||
| auto again = DeleteUserSettingsFiles(); | ||
| SECTION("Default value") | ||
| { | ||
| UserSettingsTest userSettingTest; | ||
| REQUIRE(userSettingTest.Get<Setting::OutputLocale>().empty()); | ||
| REQUIRE(userSettingTest.GetWarnings().size() == 0); | ||
| } | ||
| SECTION("Valid locale") | ||
| { | ||
| std::string_view json = R"({ "output": { "locale": "en-US" } })"; | ||
| SetSetting(Stream::PrimaryUserSettings, json); | ||
| UserSettingsTest userSettingTest; | ||
| REQUIRE(userSettingTest.Get<Setting::OutputLocale>() == "en-US"sv); | ||
| REQUIRE(userSettingTest.GetWarnings().size() == 0); | ||
| } | ||
| SECTION("Case insensitive locale") | ||
| { | ||
| std::string_view json = R"({ "output": { "locale": "EN-us" } })"; | ||
| SetSetting(Stream::PrimaryUserSettings, json); | ||
| UserSettingsTest userSettingTest; | ||
| REQUIRE(userSettingTest.Get<Setting::OutputLocale>() == "en-US"sv); | ||
| REQUIRE(userSettingTest.GetWarnings().size() == 0); | ||
| } | ||
| SECTION("Unsupported locale") | ||
| { | ||
| std::string_view json = R"({ "output": { "locale": "el-GR" } })"; | ||
| SetSetting(Stream::PrimaryUserSettings, json); | ||
| UserSettingsTest userSettingTest; | ||
| REQUIRE(userSettingTest.Get<Setting::OutputLocale>().empty()); | ||
| REQUIRE(userSettingTest.GetWarnings().size() == 1); | ||
| } | ||
| SECTION("Invalid locale") | ||
| { | ||
| std::string_view json = R"({ "output": { "locale": "en_US.UTF-8" } })"; | ||
| SetSetting(Stream::PrimaryUserSettings, json); | ||
| UserSettingsTest userSettingTest; | ||
| REQUIRE(userSettingTest.Get<Setting::OutputLocale>().empty()); | ||
| REQUIRE(userSettingTest.GetWarnings().size() == 1); | ||
| } | ||
| SECTION("Wrong type") | ||
| { | ||
| std::string_view json = R"({ "output": { "locale": ["en-US"] } })"; | ||
| SetSetting(Stream::PrimaryUserSettings, json); | ||
| UserSettingsTest userSettingTest; | ||
| REQUIRE(userSettingTest.Get<Setting::OutputLocale>().empty()); | ||
| REQUIRE(userSettingTest.GetWarnings().size() == 1); | ||
| } | ||
| } | ||
| TEST_CASE("ConvertToSortField", "[settings]") | ||
| { | ||
| SECTION("Valid values - lowercase") | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.