Introduce KeyMapping and Move TerminalSettings construction

src/cascadia/LocalTests_TerminalApp/ProfileTests.cpp

@@ -119,13 +119,13 @@ namespace TerminalAppLocalTests
 
         auto profile0 = implementation::Profile::FromJson(profile0Json);
         VERIFY_IS_NOT_NULL(profile0->Foreground());
-        VERIFY_ARE_EQUAL(til::color(0, 0, 0, 0), til::color{ profile0->Foreground().Value() });
+        VERIFY_ARE_EQUAL(til::color(0, 0, 0), til::color{ profile0->Foreground().Value() });
 
         VERIFY_IS_NOT_NULL(profile0->Background());
-        VERIFY_ARE_EQUAL(til::color(1, 1, 1, 0), til::color{ profile0->Background().Value() });
+        VERIFY_ARE_EQUAL(til::color(1, 1, 1), til::color{ profile0->Background().Value() });
 
         VERIFY_IS_NOT_NULL(profile0->SelectionBackground());
-        VERIFY_ARE_EQUAL(til::color(1, 1, 1, 0), til::color{ profile0->SelectionBackground().Value() });
+        VERIFY_ARE_EQUAL(til::color(1, 1, 1), til::color{ profile0->SelectionBackground().Value() });
 
         VERIFY_ARE_EQUAL(L"profile0", profile0->Name());
 
@@ -136,13 +136,13 @@ namespace TerminalAppLocalTests
         profile0->LayerJson(profile1Json);
 
         VERIFY_IS_NOT_NULL(profile0->Foreground());
-        VERIFY_ARE_EQUAL(til::color(2, 2, 2, 0), til::color{ profile0->Foreground().Value() });
+        VERIFY_ARE_EQUAL(til::color(2, 2, 2), til::color{ profile0->Foreground().Value() });
 
         VERIFY_IS_NOT_NULL(profile0->Background());
-        VERIFY_ARE_EQUAL(til::color(1, 1, 1, 0), til::color{ profile0->Background().Value() });
+        VERIFY_ARE_EQUAL(til::color(1, 1, 1), til::color{ profile0->Background().Value() });
 
         VERIFY_IS_NOT_NULL(profile0->Background());
-        VERIFY_ARE_EQUAL(til::color(1, 1, 1, 0), til::color{ profile0->Background().Value() });
+        VERIFY_ARE_EQUAL(til::color(1, 1, 1), til::color{ profile0->Background().Value() });
 
         VERIFY_ARE_EQUAL(L"profile1", profile0->Name());
 
@@ -154,13 +154,13 @@ namespace TerminalAppLocalTests
         profile0->LayerJson(profile2Json);
 
         VERIFY_IS_NOT_NULL(profile0->Foreground());
-        VERIFY_ARE_EQUAL(til::color(3, 3, 3, 0), til::color{ profile0->Foreground().Value() });
+        VERIFY_ARE_EQUAL(til::color(3, 3, 3), til::color{ profile0->Foreground().Value() });
 
         VERIFY_IS_NOT_NULL(profile0->Background());
-        VERIFY_ARE_EQUAL(til::color(1, 1, 1, 0), til::color{ profile0->Background().Value() });
+        VERIFY_ARE_EQUAL(til::color(1, 1, 1), til::color{ profile0->Background().Value() });
 
         VERIFY_IS_NOT_NULL(profile0->SelectionBackground());
-        VERIFY_ARE_EQUAL(til::color(2, 2, 2, 0), til::color{ profile0->SelectionBackground().Value() });
+        VERIFY_ARE_EQUAL(til::color(2, 2, 2), til::color{ profile0->SelectionBackground().Value() });
 
         VERIFY_ARE_EQUAL(L"profile2", profile0->Name());
 

src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp

@@ -1429,7 +1429,7 @@ namespace TerminalAppLocalTests
         settings._ParseJsonString(settingsJson, false);
         settings.LayerJson(settings._userSettings);
         VERIFY_IS_FALSE(settings._profiles.empty());
-        VERIFY_ARE_EQUAL(expectedPath, settings._profiles[0].GetExpandedIconPath());
+        VERIFY_ARE_EQUAL(expectedPath, settings._profiles[0].ExpandedIconPath());
     }
     void SettingsTests::TestProfileBackgroundImageWithEnvVar()
     {
@@ -3989,7 +3989,7 @@ namespace TerminalAppLocalTests
                 { "name": "scheme_1" },
                 { "name": "scheme_2" },
             ],
-            "bindings": [
+            "actions": [
                 {
                     "name": "iterable command ${scheme.name}",
                     "iterateOn": "schemes",

src/cascadia/TerminalApp/CascadiaSettings.cpp

@@ -443,7 +443,7 @@ void CascadiaSettings::_ValidateMediaResources()
             // This covers file paths on the machine, app data, URLs, and other resource paths.
             try
             {
-                winrt::Windows::Foundation::Uri imagePath{ profile.GetExpandedBackgroundImagePath() };
+                winrt::Windows::Foundation::Uri imagePath{ profile.ExpandedBackgroundImagePath() };
             }
             catch (...)
             {
@@ -457,7 +457,7 @@ void CascadiaSettings::_ValidateMediaResources()
         {
             try
             {
-                winrt::Windows::Foundation::Uri imagePath{ profile.GetExpandedIconPath() };
+                winrt::Windows::Foundation::Uri imagePath{ profile.ExpandedIconPath() };
             }
             catch (...)
             {

src/cascadia/TerminalApp/ColorScheme.h

@@ -18,7 +18,6 @@ Author(s):
 #include "../../inc/conattrs.hpp"
 #include "inc/cppwinrt_utils.h"
 
-#include <winrt/Microsoft.Terminal.TerminalControl.h>
 #include "ColorScheme.g.h"
 
 // fwdecl unittest classes

src/cascadia/TerminalApp/Command.cpp

@@ -467,7 +467,7 @@ namespace winrt::TerminalApp::implementation
 
                 // - Escape the profile name for JSON appropriately
                 auto escapedProfileName = _escapeForJson(til::u16u8(p.Name()));
-                auto escapedProfileIcon = _escapeForJson(til::u16u8(p.GetExpandedIconPath()));
+                auto escapedProfileIcon = _escapeForJson(til::u16u8(p.ExpandedIconPath()));
                 auto newJsonString = til::replace_needle_in_haystack(oldJsonString,
                                                                      ProfileNameToken,
                                                                      escapedProfileName);

src/cascadia/TerminalApp/GlobalAppSettings.cpp

@@ -173,6 +173,7 @@ void GlobalAppSettings::LayerJson(const Json::Value& json)
             warnings = winrt::TerminalApp::implementation::Command::LayerJson(_commands, bindings);
             // It's possible that the user provided commands have some warnings
             // in them, similar to the keybindings.
+            _keybindingsWarnings.insert(_keybindingsWarnings.end(), warnings.begin(), warnings.end());
         }
     };
     parseBindings(LegacyKeybindingsKey);

src/cascadia/TerminalApp/KeyMapping.h

@@ -75,8 +75,3 @@ namespace winrt::TerminalApp::implementation
         friend class TerminalAppLocalTests::TestUtils;
     };
 }
-
-namespace winrt::TerminalApp::factory_implementation
-{
-    BASIC_FACTORY(KeyMapping);
-}

src/cascadia/TerminalApp/KeyMapping.idl

@@ -57,8 +57,6 @@ namespace TerminalApp
 
     [default_interface] runtimeclass KeyMapping
     {
-        KeyMapping();
-
         ActionAndArgs TryLookup(Microsoft.Terminal.TerminalControl.KeyChord chord);
 
         void SetKeyBinding(ActionAndArgs actionAndArgs, Microsoft.Terminal.TerminalControl.KeyChord chord);

src/cascadia/TerminalApp/Profile.cpp

@@ -250,7 +250,7 @@ void Profile::LayerJson(const Json::Value& json)
 //   path, if there are any.
 // Return Value:
 // - this profile's icon path, if one is set. Otherwise returns the empty string.
-winrt::hstring Profile::GetExpandedIconPath() const
+winrt::hstring Profile::ExpandedIconPath() const
 {
     if (_IconPath.empty())
     {
@@ -265,7 +265,7 @@ winrt::hstring Profile::GetExpandedIconPath() const
 //   any environment variables in the path, if there are any.
 // Return Value:
 // - This profile's expanded background image path / the empty string.
-winrt::hstring Profile::GetExpandedBackgroundImagePath() const
+winrt::hstring Profile::ExpandedBackgroundImagePath() const
 {
     if (_BackgroundImagePath.empty())
     {
@@ -274,7 +274,7 @@ winrt::hstring Profile::GetExpandedBackgroundImagePath() const
     return winrt::hstring{ wil::ExpandEnvironmentStringsW<std::wstring>(_BackgroundImagePath.c_str()) };
 }
 
-winrt::hstring Profile::GetEvaluatedStartingDirectory() const
+winrt::hstring Profile::EvaluatedStartingDirectory() const
 {
     return winrt::hstring{ Profile::EvaluateStartingDirectory(_StartingDirectory.c_str()) };
 }

src/cascadia/TerminalApp/Profile.h

@@ -51,9 +51,9 @@ namespace winrt::TerminalApp::implementation
         void LayerJson(const Json::Value& json);
         static bool IsDynamicProfileObject(const Json::Value& json);
 
-        hstring GetEvaluatedStartingDirectory() const;
-        hstring GetExpandedIconPath() const;
-        hstring GetExpandedBackgroundImagePath() const;
+        hstring EvaluatedStartingDirectory() const;
+        hstring ExpandedIconPath() const;
+        hstring ExpandedBackgroundImagePath() const;
         void GenerateGuidIfNecessary() noexcept;
         static guid GetGuidOrGenerateForJson(const Json::Value& json) noexcept;
 

src/cascadia/TerminalApp/Profile.idl

@@ -23,7 +23,7 @@ namespace TerminalApp
         Boolean Hidden;
 
         String IconPath;
-        String GetExpandedIconPath();
+        String ExpandedIconPath { get; };
 
         CloseOnExitMode CloseOnExit;
         String TabTitle;
@@ -41,10 +41,10 @@ namespace TerminalApp
 
         String Commandline;
         String StartingDirectory;
-        String GetEvaluatedStartingDirectory();
+        String EvaluatedStartingDirectory { get; };
 
         String BackgroundImagePath;
-        String GetExpandedBackgroundImagePath();
+        String ExpandedBackgroundImagePath { get; };
         Double BackgroundImageOpacity;
         Windows.UI.Xaml.Media.Stretch BackgroundImageStretchMode;
         Windows.UI.Xaml.HorizontalAlignment BackgroundImageHorizontalAlignment;

src/cascadia/TerminalApp/TerminalPage.cpp

@@ -484,7 +484,7 @@ namespace winrt::TerminalApp::implementation
             // this flyout item.
             if (!profile.IconPath().empty())
             {
-                auto iconSource = GetColoredIcon<WUX::Controls::IconSource>(profile.GetExpandedIconPath());
+                auto iconSource = GetColoredIcon<WUX::Controls::IconSource>(profile.ExpandedIconPath());
 
                 WUX::Controls::IconSourceElement iconElement;
                 iconElement.IconSource(iconSource);
@@ -728,7 +728,7 @@ namespace winrt::TerminalApp::implementation
         const auto profile = _settings->FindProfile(profileGuid);
         if (profile != nullptr && !profile.IconPath().empty())
         {
-            newTabImpl->UpdateIcon(profile.GetExpandedIconPath());
+            newTabImpl->UpdateIcon(profile.ExpandedIconPath());
         }
 
         tabViewItem.PointerPressed({ this, &TerminalPage::_OnTabClick });
@@ -882,7 +882,7 @@ namespace winrt::TerminalApp::implementation
     // as the object to handle dispatching ShortcutAction events.
     // Arguments:
     // - bindings: A AppKeyBindings object to wire up with our event handlers
-    void TerminalPage::_HookupKeyBindings(TerminalApp::KeyMapping keymap) noexcept
+    void TerminalPage::_HookupKeyBindings(const TerminalApp::KeyMapping& keymap) noexcept
     {
         _bindings->SetDispatch(*_actionDispatch);
         _bindings->SetKeyMapping(keymap);
@@ -970,7 +970,7 @@ namespace winrt::TerminalApp::implementation
             const auto matchingProfile = _settings->FindProfile(lastFocusedProfile);
             if (matchingProfile)
             {
-                tab.UpdateIcon(matchingProfile.GetExpandedIconPath());
+                tab.UpdateIcon(matchingProfile.ExpandedIconPath());
             }
             else
             {

src/cascadia/TerminalApp/TerminalPage.h

@@ -131,7 +131,7 @@ namespace winrt::TerminalApp::implementation
         void _CloseWarningPrimaryButtonOnClick(Windows::UI::Xaml::Controls::ContentDialog sender, Windows::UI::Xaml::Controls::ContentDialogButtonClickEventArgs eventArgs);
         void _ThirdPartyNoticesOnClick(const IInspectable& sender, const Windows::UI::Xaml::RoutedEventArgs& eventArgs);
 
-        void _HookupKeyBindings(TerminalApp::KeyMapping keymap) noexcept;
+        void _HookupKeyBindings(const TerminalApp::KeyMapping& keymap) noexcept;
         void _RegisterActionCallbacks();
 
         void _UpdateTitle(const Tab& tab);

src/cascadia/TerminalApp/TerminalSettings.cpp

@@ -95,7 +95,7 @@ namespace winrt::TerminalApp::implementation
 
         if (!profile.StartingDirectory().empty())
         {
-            _StartingDirectory = profile.GetEvaluatedStartingDirectory();
+            _StartingDirectory = profile.EvaluatedStartingDirectory();
         }
 
         // GH#2373: Use the tabTitle as the starting title if it exists, otherwise
@@ -135,7 +135,7 @@ namespace winrt::TerminalApp::implementation
 
         if (!profile.BackgroundImagePath().empty())
         {
-            _BackgroundImage = profile.GetExpandedBackgroundImagePath();
+            _BackgroundImage = profile.ExpandedBackgroundImagePath();
         }
 
         _BackgroundImageOpacity = profile.BackgroundImageOpacity();
@@ -188,22 +188,13 @@ namespace winrt::TerminalApp::implementation
         _CursorColor = til::color{ scheme.CursorColor() };
 
         const auto table = scheme.Table();
-        auto const tableCount = gsl::narrow_cast<int>(table.size());
-        for (int i = 0; i < tableCount; i++)
-        {
-            SetColorTableEntry(i, til::color{ table[i] });
-        }
+        std::transform(table.cbegin(), table.cend(), _colorTable.begin(), [](auto&& color) {
+            return static_cast<uint32_t>(til::color{ color });
+        });
     }
 
     uint32_t TerminalSettings::GetColorTableEntry(int32_t index) const noexcept
     {
         return _colorTable.at(index);
     }
-
-    void TerminalSettings::SetColorTableEntry(int32_t index, uint32_t value)
-    {
-        auto const colorTableCount = gsl::narrow_cast<decltype(index)>(_colorTable.size());
-        THROW_HR_IF(E_INVALIDARG, index >= colorTableCount);
-        _colorTable.at(index) = value;
-    }
 }

src/cascadia/TerminalApp/TerminalSettings.h

@@ -51,10 +51,9 @@ namespace winrt::TerminalApp::implementation
         // --------------------------- Core Settings ---------------------------
         //  All of these settings are defined in ICoreSettings.
 
-        // Get/Set ColorTableEntry needs to be implemented manually, to get a
+        // GetColorTableEntry needs to be implemented manually, to get a
         // particular value from the array.
         uint32_t GetColorTableEntry(int32_t index) const noexcept;
-        void SetColorTableEntry(int32_t index, uint32_t value);
 
         GETSET_PROPERTY(uint32_t, DefaultForeground, DEFAULT_FOREGROUND_WITH_ALPHA);
         GETSET_PROPERTY(uint32_t, DefaultBackground, DEFAULT_BACKGROUND_WITH_ALPHA);

src/cascadia/TerminalCore/ICoreSettings.idl

@@ -17,7 +17,6 @@ namespace Microsoft.Terminal.TerminalControl
         UInt32 DefaultForeground;
         UInt32 DefaultBackground;
         UInt32 GetColorTableEntry(Int32 index);
-        void SetColorTableEntry(Int32 index, UInt32 value);
         // TODO:MSFT:20642297 - define a sentinel for Infinite Scrollback
         Int32 HistorySize;
         Int32 InitialRows;

src/cascadia/UnitTests_TerminalCore/MockTermSettings.h

@@ -61,9 +61,6 @@ namespace TerminalCoreUnitTests
         void SelectionBackground(uint32_t) {}
         void ForceVTInput(bool) {}
 
-        // other unimplemented methods
-        void SetColorTableEntry(int32_t /* index */, uint32_t /* value */) {}
-
         GETSET_PROPERTY(winrt::Windows::Foundation::IReference<uint32_t>, TabColor, nullptr);
 
     private: