PRE-MERGE #15843 When the profile icon is set to null, fall back to t…

…he icon of the commandline

src/cascadia/LocalTests_SettingsModel/TerminalSettingsTests.cpp

@@ -173,13 +173,13 @@ namespace SettingsModelLocalTests
         {
             const auto commandLine = file2.native() + LR"( -foo "bar1 bar2" -baz)"s;
             const auto expected = file2.native() + L"\0-foo\0bar1 bar2\0-baz"s;
-            const auto actual = implementation::CascadiaSettings::NormalizeCommandLine(commandLine.c_str());
+            const auto actual = implementation::Profile::NormalizeCommandLine(commandLine.c_str());
             VERIFY_ARE_EQUAL(expected, actual);
         }
         {
             const auto commandLine = L"C:\\";
             const auto expected = L"C:\\";
-            const auto actual = implementation::CascadiaSettings::NormalizeCommandLine(commandLine);
+            const auto actual = implementation::Profile::NormalizeCommandLine(commandLine);
             VERIFY_ARE_EQUAL(expected, actual);
         }
     }

src/cascadia/TerminalApp/TabManagement.cpp

@@ -174,9 +174,10 @@ namespace winrt::TerminalApp::implementation
         // Set this tab's icon to the icon from the user's profile
         if (const auto profile{ newTabImpl->GetFocusedProfile() })
         {
-            if (!profile.Icon().empty())
+            const auto& icon = profile.EvaluatedIcon();
+            if (!icon.empty())
             {
-                newTabImpl->UpdateIcon(profile.Icon(), _settings.GlobalSettings().CurrentTheme().Tab().IconStyle());
+                newTabImpl->UpdateIcon(icon, _settings.GlobalSettings().CurrentTheme().Tab().IconStyle());
             }
         }
 
@@ -241,7 +242,7 @@ namespace winrt::TerminalApp::implementation
     {
         if (const auto profile = tab.GetFocusedProfile())
         {
-            tab.UpdateIcon(profile.Icon(), _settings.GlobalSettings().CurrentTheme().Tab().IconStyle());
+            tab.UpdateIcon(profile.EvaluatedIcon(), _settings.GlobalSettings().CurrentTheme().Tab().IconStyle());
         }
     }
 

src/cascadia/TerminalApp/TerminalPage.cpp

@@ -1033,9 +1033,10 @@ namespace winrt::TerminalApp::implementation
 
         // If there's an icon set for this profile, set it as the icon for
         // this flyout item
-        if (!profile.Icon().empty())
+        const auto& iconPath = profile.EvaluatedIcon();
+        if (!iconPath.empty())
         {
-            const auto icon = _CreateNewTabFlyoutIcon(profile.Icon());
+            const auto icon = _CreateNewTabFlyoutIcon(iconPath);
             profileMenuItem.Icon(icon);
         }
 

src/cascadia/TerminalSettingsEditor/MainPage.cpp

@@ -595,7 +595,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         MUX::Controls::NavigationViewItem profileNavItem;
         profileNavItem.Content(box_value(profile.Name()));
         profileNavItem.Tag(box_value<Editor::ProfileViewModel>(profile));
-        profileNavItem.Icon(IconPathConverter::IconWUX(profile.Icon()));
+        profileNavItem.Icon(IconPathConverter::IconWUX(profile.EvaluatedIcon()));
 
         // Update the menu item when the icon/name changes
         auto weakMenuItem{ make_weak(profileNavItem) };

src/cascadia/TerminalSettingsEditor/ProfileViewModel.cpp

@@ -26,6 +26,8 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
     Windows::Foundation::Collections::IObservableVector<Editor::Font> ProfileViewModel::_MonospaceFontList{ nullptr };
     Windows::Foundation::Collections::IObservableVector<Editor::Font> ProfileViewModel::_FontList{ nullptr };
 
+    static constexpr std::wstring_view HideIconValue{ L"none" };
+
     ProfileViewModel::ProfileViewModel(const Model::Profile& profile, const Model::CascadiaSettings& appSettings) :
         _profile{ profile },
         _defaultAppearanceViewModel{ winrt::make<implementation::AppearanceViewModel>(profile.DefaultAppearance().try_as<AppearanceConfig>()) },
@@ -69,6 +71,10 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
             {
                 _NotifyChanges(L"CurrentScrollState");
             }
+            else if (viewModelProperty == L"Icon")
+            {
+                _NotifyChanges(L"HideIcon");
+            }
         });
 
         // Do the same for the starting directory
@@ -348,6 +354,26 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         }
     }
 
+    bool ProfileViewModel::HideIcon()
+    {
+        return Icon() == HideIconValue;
+    }
+    void ProfileViewModel::HideIcon(const bool hide)
+    {
+        if (hide)
+        {
+            // Stash the current value of Icon. If the user
+            // checks and un-checks the "Hide Icon" checkbox, we want
+            // the path that we display in the text box to remain unchanged.
+            _lastIcon = Icon();
+            Icon(HideIconValue);
+        }
+        else
+        {
+            Icon(_lastIcon);
+        }
+    }
+
     bool ProfileViewModel::IsBellStyleFlagSet(const uint32_t flag)
     {
         return (WI_EnumValue(BellStyle()) & flag) == flag;

src/cascadia/TerminalSettingsEditor/ProfileViewModel.h

@@ -58,11 +58,20 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
             Padding(to_hstring(value));
         }
 
+        winrt::hstring EvaluatedIcon() const
+        {
+            return _profile.EvaluatedIcon();
+        }
+
         // starting directory
         bool UseParentProcessDirectory();
         void UseParentProcessDirectory(const bool useParent);
         bool UseCustomStartingDirectory();
 
+        // icon
+        bool HideIcon();
+        void HideIcon(const bool hide);
+
         // general profile knowledge
         winrt::guid OriginalProfileGuid() const noexcept;
         bool CanDeleteProfile() const;
@@ -120,6 +129,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         winrt::guid _originalProfileGuid;
         winrt::hstring _lastBgImagePath;
         winrt::hstring _lastStartingDirectoryPath;
+        winrt::hstring _lastIcon;
         Editor::AppearanceViewModel _defaultAppearanceViewModel;
 
         static Windows::Foundation::Collections::IObservableVector<Editor::Font> _MonospaceFontList;

src/cascadia/TerminalSettingsEditor/ProfileViewModel.idl

@@ -67,6 +67,7 @@ namespace Microsoft.Terminal.Settings.Editor
         ProfileSubPage CurrentPage;
         Boolean UseParentProcessDirectory;
         Boolean UseCustomStartingDirectory { get; };
+        Boolean HideIcon;
         AppearanceViewModel DefaultAppearance { get; };
         Guid OriginalProfileGuid { get; };
         Boolean HasUnfocusedAppearance { get; };
@@ -75,6 +76,8 @@ namespace Microsoft.Terminal.Settings.Editor
         AppearanceViewModel UnfocusedAppearance { get; };
         Boolean VtPassthroughAvailable { get; };
 
+        String EvaluatedIcon { get; };
+
         void CreateUnfocusedAppearance();
         void DeleteUnfocusedAppearance();
 

src/cascadia/TerminalSettingsEditor/Profiles_Base.xaml

@@ -110,13 +110,20 @@
                     <StackPanel>
                         <TextBox x:Uid="Profile_IconBox"
                                  FontFamily="Segoe UI, Segoe Fluent Icons, Segoe MDL2 Assets"
+                                 IsEnabled="{x:Bind local:Converters.InvertBoolean(Profile.HideIcon), Mode=OneWay}"
                                  IsSpellCheckEnabled="False"
                                  Style="{StaticResource TextBoxSettingStyle}"
-                                 Text="{x:Bind Profile.Icon, Mode=TwoWay}" />
+                                 Text="{x:Bind Profile.Icon, Mode=TwoWay}"
+                                 Visibility="{x:Bind local:Converters.InvertedBooleanToVisibility(Profile.HideIcon), Mode=OneWay}" />
                         <Button x:Uid="Profile_IconBrowse"
                                 Margin="0,10,0,0"
                                 Click="Icon_Click"
-                                Style="{StaticResource BrowseButtonStyle}" />
+                                Style="{StaticResource BrowseButtonStyle}"
+                                Visibility="{x:Bind local:Converters.InvertedBooleanToVisibility(Profile.HideIcon), Mode=OneWay}" />
+                        <CheckBox x:Name="HideIconCheckbox"
+                                  x:Uid="Profile_HideIconCheckbox"
+                                  Margin="0,5,0,0"
+                                  IsChecked="{x:Bind Profile.HideIcon, Mode=TwoWay}" />
                     </StackPanel>
                 </local:SettingContainer>
 

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

@@ -1034,6 +1034,14 @@
     <value>If enabled, this profile will spawn in the directory from which Terminal was launched.</value>
     <comment>A description for what the supplementary "use parent process directory" setting does. Presented near "Profile_StartingDirectoryUseParentCheckbox".</comment>
   </data>
+  <data name="Profile_HideIconCheckbox.Content" xml:space="preserve">
+    <value>Hide icon</value>
+    <comment>A supplementary setting to the "icon" setting.</comment>
+  </data>
+  <data name="Profile_HideIconCheckbox.[using:Windows.UI.Xaml.Controls]ToolTipService.ToolTip" xml:space="preserve">
+    <value>If enabled, this will have no icon.</value>
+    <comment>A description for what the supplementary "Hide icon" setting does. Presented near "Profile_HideIconCheckbox".</comment>
+  </data>
   <data name="Profile_SuppressApplicationTitle.Header" xml:space="preserve">
     <value>Suppress title changes</value>
     <comment>Header for a control to toggle changes in the app title.</comment>

src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp

@@ -515,8 +515,12 @@ void CascadiaSettings::_validateMediaResources()
             }
         }
 
-        // Anything longer than 2 wchar_t's _isn't_ an emoji or symbol,
-        // so treat it as an invalid path.
+        // Anything longer than 2 wchar_t's _isn't_ an emoji or symbol, so treat
+        // it as an invalid path.
+        //
+        // Explicitly just use the Icon here, not the EvaluatedIcon. We don't
+        // want to blow up if we fell back to the commandline and the
+        // commandline _isn't an icon_.
         if (const auto icon = profile.Icon(); icon.size() > 2)
         {
             const auto iconPath{ wil::ExpandEnvironmentStringsW<std::wstring>(icon.c_str()) };
@@ -662,7 +666,7 @@ Model::Profile CascadiaSettings::_getProfileForCommandLine(const winrt::hstring&
 
             try
             {
-                _commandLinesCache.emplace_back(NormalizeCommandLine(cmd.c_str()), profile);
+                _commandLinesCache.emplace_back(Profile::NormalizeCommandLine(cmd.c_str()), profile);
             }
             CATCH_LOG()
         }
@@ -681,7 +685,7 @@ Model::Profile CascadiaSettings::_getProfileForCommandLine(const winrt::hstring&
 
     try
     {
-        const auto needle = NormalizeCommandLine(commandLine.c_str());
+        const auto needle = Profile::NormalizeCommandLine(commandLine.c_str());
 
         // til::starts_with(string, prefix) will always return false if prefix.size() > string.size().
         // --> Using binary search we can safely skip all items in _commandLinesCache where .first.size() > needle.size().
@@ -710,129 +714,6 @@ Model::Profile CascadiaSettings::_getProfileForCommandLine(const winrt::hstring&
     return nullptr;
 }
 
-// Given a commandLine like the following:
-// * "C:\WINDOWS\System32\cmd.exe"
-// * "pwsh -WorkingDirectory ~"
-// * "C:\Program Files\PowerShell\7\pwsh.exe"
-// * "C:\Program Files\PowerShell\7\pwsh.exe -WorkingDirectory ~"
-//
-// This function returns:
-// * "C:\Windows\System32\cmd.exe"
-// * "C:\Program Files\PowerShell\7\pwsh.exe\0-WorkingDirectory\0~"
-// * "C:\Program Files\PowerShell\7\pwsh.exe"
-// * "C:\Program Files\PowerShell\7\pwsh.exe\0-WorkingDirectory\0~"
-//
-// The resulting strings are then used for comparisons in _getProfileForCommandLine().
-// For instance a resulting string of
-//   "C:\Program Files\PowerShell\7\pwsh.exe"
-// is considered a compatible profile with
-//   "C:\Program Files\PowerShell\7\pwsh.exe -WorkingDirectory ~"
-// as it shares the same (normalized) prefix.
-std::wstring CascadiaSettings::NormalizeCommandLine(LPCWSTR commandLine)
-{
-    // Turn "%SystemRoot%\System32\cmd.exe" into "C:\WINDOWS\System32\cmd.exe".
-    // We do this early, as environment variables might occur anywhere in the commandLine.
-    std::wstring normalized;
-    THROW_IF_FAILED(wil::ExpandEnvironmentStringsW(commandLine, normalized));
-
-    // One of the most important things this function does is to strip quotes.
-    // That way the commandLine "foo.exe -bar" and "\"foo.exe\" \"-bar\"" appear identical.
-    // We'll abuse CommandLineToArgvW for that as it's close to what CreateProcessW uses.
-    auto argc = 0;
-    wil::unique_hlocal_ptr<PWSTR[]> argv{ CommandLineToArgvW(normalized.c_str(), &argc) };
-    THROW_LAST_ERROR_IF(!argc);
-
-    // The index of the first argument in argv for our executable in argv[0].
-    // Given {"C:\Program Files\PowerShell\7\pwsh.exe", "-WorkingDirectory", "~"} this will be 1.
-    auto startOfArguments = 1;
-
-    // The given commandLine should start with an executable name or path.
-    // For instance given the following argv arrays:
-    // * {"C:\WINDOWS\System32\cmd.exe"}
-    // * {"pwsh", "-WorkingDirectory", "~"}
-    // * {"C:\Program", "Files\PowerShell\7\pwsh.exe"}
-    //               ^^^^
-    //   Notice how there used to be a space in the path, which was split by ExpandEnvironmentStringsW().
-    //   CreateProcessW() supports such atrocities, so we got to do the same.
-    // * {"C:\Program Files\PowerShell\7\pwsh.exe", "-WorkingDirectory", "~"}
-    //
-    // This loop tries to resolve relative paths, as well as executable names in %PATH%
-    // into absolute paths and normalizes them. The results for the above would be:
-    // * "C:\Windows\System32\cmd.exe"
-    // * "C:\Program Files\PowerShell\7\pwsh.exe"
-    // * "C:\Program Files\PowerShell\7\pwsh.exe"
-    // * "C:\Program Files\PowerShell\7\pwsh.exe"
-    for (;;)
-    {
-        // CreateProcessW uses RtlGetExePath to get the lpPath for SearchPathW.
-        // The difference between the behavior of SearchPathW if lpPath is nullptr and what RtlGetExePath returns
-        // seems to be mostly whether SafeProcessSearchMode is respected and the support for relative paths.
-        // Windows Terminal makes the use of relative paths rather impractical which is why we simply dropped the call to RtlGetExePath.
-        const auto status = wil::SearchPathW(nullptr, argv[0], L".exe", normalized);
-
-        if (status == S_OK)
-        {
-            const auto attributes = GetFileAttributesW(normalized.c_str());
-
-            if (attributes != INVALID_FILE_ATTRIBUTES && WI_IsFlagClear(attributes, FILE_ATTRIBUTE_DIRECTORY))
-            {
-                std::filesystem::path path{ std::move(normalized) };
-
-                // canonical() will resolve symlinks, etc. for us.
-                {
-                    std::error_code ec;
-                    auto canonicalPath = std::filesystem::canonical(path, ec);
-                    if (!ec)
-                    {
-                        path = std::move(canonicalPath);
-                    }
-                }
-
-                // std::filesystem::path has no way to extract the internal path.
-                // So about that.... I own you, computer. Give me that path.
-                normalized = std::move(const_cast<std::wstring&>(path.native()));
-                break;
-            }
-        }
-        // All other error types aren't handled at the moment.
-        else if (status != HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND))
-        {
-            break;
-        }
-        // If the file path couldn't be found by SearchPathW this could be the result of us being given a commandLine
-        // like "C:\foo bar\baz.exe -arg" which is resolved to the argv array {"C:\foo", "bar\baz.exe", "-arg"},
-        // or we were erroneously given a directory to execute (e.g. someone ran `wt .`).
-        // Just like CreateProcessW() we thus try to concatenate arguments until we successfully resolve a valid path.
-        // Of course we can only do that if we have at least 2 remaining arguments in argv.
-        if ((argc - startOfArguments) < 2)
-        {
-            break;
-        }
-
-        // As described in the comment right above, we concatenate arguments in an attempt to resolve a valid path.
-        // The code below turns argv from {"C:\foo", "bar\baz.exe", "-arg"} into {"C:\foo bar\baz.exe", "-arg"}.
-        // The code abuses the fact that CommandLineToArgvW allocates all arguments back-to-back on the heap separated by '\0'.
-        argv[startOfArguments][-1] = L' ';
-        ++startOfArguments;
-    }
-
-    // We've (hopefully) finished resolving the path to the executable.
-    // We're now going to append all remaining arguments to the resulting string.
-    // If argv is {"C:\Program Files\PowerShell\7\pwsh.exe", "-WorkingDirectory", "~"},
-    // then we'll get "C:\Program Files\PowerShell\7\pwsh.exe\0-WorkingDirectory\0~"
-    if (startOfArguments < argc)
-    {
-        // normalized contains a canonical form of argv[0] at this point.
-        // -1 allows us to include the \0 between argv[0] and argv[1] in the call to append().
-        const auto beg = argv[startOfArguments] - 1;
-        const auto lastArg = argv[argc - 1];
-        const auto end = lastArg + wcslen(lastArg);
-        normalized.append(beg, end);
-    }
-
-    return normalized;
-}
-
 // Method Description:
 // - Helper to get a profile given a name that could be a guid or an actual name.
 // Arguments:

src/cascadia/TerminalSettingsModel/CascadiaSettings.h

@@ -136,7 +136,6 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         winrt::hstring GetSerializationErrorMessage() const;
 
         // defterm
-        static std::wstring NormalizeCommandLine(LPCWSTR commandLine);
         static bool IsDefaultTerminalAvailable() noexcept;
         static bool IsDefaultTerminalSet() noexcept;
         winrt::Windows::Foundation::Collections::IObservableVector<Model::DefaultTerminal> DefaultTerminals() noexcept;

src/cascadia/TerminalSettingsModel/Command.cpp

@@ -602,7 +602,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
 
                 // - Escape the profile name for JSON appropriately
                 auto escapedProfileName = _escapeForJson(til::u16u8(p.Name()));
-                auto escapedProfileIcon = _escapeForJson(til::u16u8(p.Icon()));
+                auto escapedProfileIcon = _escapeForJson(til::u16u8(p.EvaluatedIcon()));
                 auto newJsonString = til::replace_needle_in_haystack(oldJsonString,
                                                                      ProfileNameToken,
                                                                      escapedProfileName);