Skip to content

Commit

Permalink
Allow empty strings and env vars in profile "icon" settings - fixes m…
Browse files Browse the repository at this point in the history 
…icrosoft#1468, microsoft#1867 (microsoft#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 <miniksa@microsoft.com>
  • Loading branch information
@metathinker @miniksa @DHowett
2 people authored and DHowett committed Jul 25, 2019
1 parent 8ae4f2f commit 66d46ed
Show file tree
Hide file tree
Showing 6 changed files with 7 additions and 144 deletions.
5 changes: 3 additions & 2 deletions src/cascadia/TerminalApp/App.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::wstring>(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
Expand Down
24 changes: 2 additions & 22 deletions src/cascadia/TerminalApp/CascadiaSettings.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::wstring>(programFileEnvNulTerm.data()) };
psCorePath /= L"PowerShell";
if (std::filesystem::exists(psCorePath))
{
Expand Down Expand Up @@ -604,27 +605,6 @@ void CascadiaSettings::_AppendWslProfiles(std::vector<TerminalApp::Profile>& 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<DWORD>(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.
Expand Down
1 change: 0 additions & 1 deletion src/cascadia/TerminalApp/CascadiaSettings.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,5 @@ class TerminalApp::CascadiaSettings final
static bool _isPowerShellCoreInstalledInPath(const std::wstring_view programFileEnv, std::filesystem::path& cmdline);
static bool _isPowerShellCoreInstalled(std::filesystem::path& cmdline);
static void _AppendWslProfiles(std::vector<TerminalApp::Profile>& profileStorage);
static std::wstring ExpandEnvironmentVariableString(std::wstring_view source);
static Profile _CreateDefaultProfile(const std::wstring_view name);
};
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/Profile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
118 changes: 0 additions & 118 deletions src/cascadia/ut_app/TerminalApp.Unit.Tests.manifest
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,122 +20,4 @@
<longPathAware xmlns="http://schemas.microsoft.com/SMI/2016/WindowsSettings">true</longPathAware>
</windowsSettings>
</application>
<!--
This manifest makes launching the exe directly via unpackaged winrt possible.
The exe can quickly be tested with `openterm` from a razzle window.
The more official way of testing however is to use the CascadiaPackage appx.
Whenever you add new cppwinrt classes, make sure to add them here.
If you need help with the syntax, the generated AppxManifest.xml should have
the classes and files all listed in it.
TODO MSFT#21300206: Switch to automatically generating these.
-->
<file name="TerminalSettings.dll" hashalg="SHA1">
<activatableClass name="Microsoft.Terminal.Settings.KeyChord" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.Terminal.Settings.TerminalSettings" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
</file>
<file name="TerminalApp.dll" hashalg="SHA1">
<activatableClass name="TerminalApp.App" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="TerminalApp.AppKeyBindings" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="TerminalApp.XamlmetaDataProvider" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
</file>
<file name="TerminalConnection.dll" hashalg="SHA1">
<activatableClass name="Microsoft.Terminal.TerminalConnection.ConhostConnection" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.Terminal.TerminalConnection.EchoConnection" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
</file>
<file name="TerminalControl.dll" hashalg="SHA1">
<activatableClass name="Microsoft.Terminal.TerminalControl.TermControl" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
</file>
<file name="Microsoft.UI.Xaml.dll" hashalg="SHA1">
<activatableClass name="Microsoft.UI.Xaml.Controls.IconSource" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.BitmapIconSource" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.ColorPicker" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.CommandBarFlyout" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.DropDownButton" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.ElementAnimator" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.ElementFactory" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.ElementFactoryGetArgs" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.ElementFactoryRecycleArgs" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.Layout" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.VirtualizingLayout" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.FlowLayout" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.FlowLayoutState" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.FontIconSource" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.IndexPath" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.ItemsRepeater" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.ItemsRepeaterScrollHost" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.ItemsSourceView" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.LayoutContext" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.LayoutPanel" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.MenuBar" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.MenuBarItem" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.MenuBarItemFlyout" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.NavigationView" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.NavigationViewItemBase" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.NavigationViewItem" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.NavigationViewItemHeader" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.NavigationViewItemInvokedEventArgs" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.NavigationViewItemSeparator" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.NavigationViewList" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.NavigationViewTemplateSettings" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.NonVirtualizingLayout" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.NonVirtualizingLayoutContext" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.ParallaxView" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.PathIconSource" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.PersonPicture" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.Primitives.ColorPickerSlider" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.Primitives.ColorSpectrum" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.Primitives.CommandBarFlyoutCommandBar" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.Primitives.NavigationViewItemPresenter" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.Primitives.RadioButtonsListView" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.Primitives.RadioButtonsListViewItem" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.Primitives.SnapPointBase" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.Primitives.ScrollSnapPointBase" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.Primitives.RepeatedScrollSnapPoint" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.Primitives.ZoomSnapPointBase" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.Primitives.RepeatedZoomSnapPoint" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.Primitives.ScrollControllerInteractionRequestedEventArgs" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.Primitives.ScrollControllerScrollByRequestedEventArgs" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.Primitives.ScrollControllerScrollFromRequestedEventArgs" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.Primitives.ScrollControllerScrollToRequestedEventArgs" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.Primitives.ScrollSnapPoint" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.Primitives.Scroller" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.Primitives.ZoomSnapPoint" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.RadioButtons" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.RadioMenuFlyoutItem" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.RatingControl" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.RatingItemInfo" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.RatingItemFontInfo" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.RatingItemImageInfo" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.RecyclePool" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.RecyclingElementFactory" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.RefreshContainer" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.RefreshVisualizer" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.RevealListViewItemPresenter" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.ScrollOptions" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.ScrollViewer" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.SelectionModel" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.SplitButton" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.StackLayout" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.StackLayoutState" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.SwipeControl" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.SwipeItem" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.SwipeItems" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.SymbolIconSource" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.TabView" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.TabViewItem" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.UniformGridLayout" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.UniformGridLayoutState" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.VirtualizingLayoutContext" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.XamlControlsResources" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Controls.ZoomOptions" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Media.AcrylicBrush" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Media.RevealBrush" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Media.RevealBackgroundBrush" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.Media.RevealBorderBrush" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
<activatableClass name="Microsoft.UI.Xaml.XamlTypeInfo.XamlControlsXamlMetaDataProvider" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
</file>
<file name="Microsoft.Toolkit.Win32.UI.XamlHost.dll" hashalg="SHA1">
<activatableClass name="Microsoft.Toolkit.Win32.UI.XamlHost.XamlApplication" threadingModel="both" xmlns="urn:schemas-microsoft-com:winrt.v1"></activatableClass>
</file>
</assembly>
1 change: 1 addition & 0 deletions src/inc/LibraryIncludes.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
#include <wil/Result.h>
#include <wil/resource.h>
#include <wil/wistd_memory.h>
#include <wil/stl.h>
#include <wil/com.h>
#include <wil/filesystem.h>

Expand Down

0 comments on commit 66d46ed

Please sign in to comment.