From 66d46ed8eddc29f75771afff245d6597a5923f44 Mon Sep 17 00:00:00 2001 From: Michael Ratanapintha <1039862+metathinker@users.noreply.github.com> Date: Thu, 25 Jul 2019 10:44:58 -0700 Subject: [PATCH] Allow empty strings and env vars in profile "icon" settings - fixes #1468, #1867 (#2050) First, I tried reusing the existing ExpandEnvironmentVariableStrings() helper in TerminalApp/CascadiaSettings.cpp, but then I realized that WIL already provides its own wrapper for ExpandEnvironmentStrings(), so instead I deleted ExpandEnvironmentVariableStrings() and replaced its usages with wil::ExpandEnvironmentStringsW(). I then used wil::ExpandEnvironmentStringsW() when resolving the icon path as well. In addition, to allow empty strings, I made changes to treat empty strings for "icon" the same as JSON `null` or not setting the property at all. Co-Authored-By: Michael Niksa --- src/cascadia/TerminalApp/App.cpp | 5 +- src/cascadia/TerminalApp/CascadiaSettings.cpp | 24 +--- src/cascadia/TerminalApp/CascadiaSettings.h | 1 - src/cascadia/TerminalApp/Profile.cpp | 2 +- .../ut_app/TerminalApp.Unit.Tests.manifest | 118 ------------------ src/inc/LibraryIncludes.h | 1 + 6 files changed, 7 insertions(+), 144 deletions(-) diff --git a/src/cascadia/TerminalApp/App.cpp b/src/cascadia/TerminalApp/App.cpp index 92004ccb68..83fc490cbf 100644 --- a/src/cascadia/TerminalApp/App.cpp +++ b/src/cascadia/TerminalApp/App.cpp @@ -1257,8 +1257,9 @@ namespace winrt::TerminalApp::implementation { if (profile.HasIcon()) { - auto path = profile.GetIconPath(); - winrt::hstring iconPath{ path }; + std::wstring path{ profile.GetIconPath() }; + const auto envExpandedPath{ wil::ExpandEnvironmentStringsW(path.data()) }; + winrt::hstring iconPath{ envExpandedPath }; winrt::Windows::Foundation::Uri iconUri{ iconPath }; Controls::BitmapIconSource iconSource; // Make sure to set this to false, so we keep the RGB data of the diff --git a/src/cascadia/TerminalApp/CascadiaSettings.cpp b/src/cascadia/TerminalApp/CascadiaSettings.cpp index 09cb0f7481..5d663f1717 100644 --- a/src/cascadia/TerminalApp/CascadiaSettings.cpp +++ b/src/cascadia/TerminalApp/CascadiaSettings.cpp @@ -498,7 +498,8 @@ bool CascadiaSettings::_isPowerShellCoreInstalled(std::filesystem::path& cmdline // - true iff powershell core (pwsh.exe) is present in the given path bool CascadiaSettings::_isPowerShellCoreInstalledInPath(const std::wstring_view programFileEnv, std::filesystem::path& cmdline) { - std::filesystem::path psCorePath = ExpandEnvironmentVariableString(programFileEnv.data()); + std::wstring programFileEnvNulTerm{ programFileEnv }; + std::filesystem::path psCorePath{ wil::ExpandEnvironmentStringsW(programFileEnvNulTerm.data()) }; psCorePath /= L"PowerShell"; if (std::filesystem::exists(psCorePath)) { @@ -604,27 +605,6 @@ void CascadiaSettings::_AppendWslProfiles(std::vector& pro } } -// Function Description: -// - Get a environment variable string. -// Arguments: -// - A string that contains an environment-variable string in the form: %variableName%. -// Return Value: -// - a string of the expending environment-variable string. -std::wstring CascadiaSettings::ExpandEnvironmentVariableString(std::wstring_view source) -{ - std::wstring result{}; - DWORD requiredSize = 0; - do - { - result.resize(requiredSize); - requiredSize = ::ExpandEnvironmentStringsW(source.data(), result.data(), gsl::narrow(result.size())); - } while (requiredSize != result.size()); - - // Trim the terminating null character - result.resize(requiredSize - 1); - return result; -} - // Method Description: // - Helper function for creating a skeleton default profile with a pre-populated // guid and name. diff --git a/src/cascadia/TerminalApp/CascadiaSettings.h b/src/cascadia/TerminalApp/CascadiaSettings.h index f1ae32c908..c5e2ccdf27 100644 --- a/src/cascadia/TerminalApp/CascadiaSettings.h +++ b/src/cascadia/TerminalApp/CascadiaSettings.h @@ -66,6 +66,5 @@ private: static bool _isPowerShellCoreInstalledInPath(const std::wstring_view programFileEnv, std::filesystem::path& cmdline); static bool _isPowerShellCoreInstalled(std::filesystem::path& cmdline); static void _AppendWslProfiles(std::vector& profileStorage); - static std::wstring ExpandEnvironmentVariableString(std::wstring_view source); static Profile _CreateDefaultProfile(const std::wstring_view name); }; diff --git a/src/cascadia/TerminalApp/Profile.cpp b/src/cascadia/TerminalApp/Profile.cpp index 92e6553aeb..dc3e2c09da 100644 --- a/src/cascadia/TerminalApp/Profile.cpp +++ b/src/cascadia/TerminalApp/Profile.cpp @@ -513,7 +513,7 @@ void Profile::SetDefaultBackground(COLORREF defaultBackground) noexcept bool Profile::HasIcon() const noexcept { - return _icon.has_value(); + return _icon.has_value() && !_icon.value().empty(); } // Method Description diff --git a/src/cascadia/ut_app/TerminalApp.Unit.Tests.manifest b/src/cascadia/ut_app/TerminalApp.Unit.Tests.manifest index 4c2159f374..ef9516047a 100644 --- a/src/cascadia/ut_app/TerminalApp.Unit.Tests.manifest +++ b/src/cascadia/ut_app/TerminalApp.Unit.Tests.manifest @@ -20,122 +20,4 @@ true - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/src/inc/LibraryIncludes.h b/src/inc/LibraryIncludes.h index 7e4d40a4be..eb4ae8db0b 100644 --- a/src/inc/LibraryIncludes.h +++ b/src/inc/LibraryIncludes.h @@ -49,6 +49,7 @@ #include #include #include +#include #include #include