From a44ef99460914ee62c9c115f533e74692c2eb5ee Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Mon, 10 Aug 2020 14:26:29 -0700 Subject: [PATCH 1/9] Make ColorScheme a WinRT object --- .../ColorSchemeTests.cpp | 133 ++++++++++-------- src/cascadia/TerminalApp/CascadiaSettings.h | 6 +- .../CascadiaSettingsSerialization.cpp | 13 +- src/cascadia/TerminalApp/ColorScheme.cpp | 34 +++-- src/cascadia/TerminalApp/ColorScheme.h | 65 +++++---- src/cascadia/TerminalApp/ColorScheme.idl | 23 +++ .../TerminalApp/GlobalAppSettings.cpp | 2 +- src/cascadia/TerminalApp/GlobalAppSettings.h | 11 +- src/cascadia/TerminalApp/Profile.h | 4 +- src/cascadia/TerminalApp/TerminalPage.cpp | 2 +- .../TerminalApp/lib/TerminalAppLib.vcxproj | 9 +- .../lib/TerminalAppLib.vcxproj.filters | 3 + src/cascadia/ut_app/JsonTests.cpp | 14 +- src/cascadia/ut_app/precomp.h | 3 +- 14 files changed, 188 insertions(+), 134 deletions(-) create mode 100644 src/cascadia/TerminalApp/ColorScheme.idl diff --git a/src/cascadia/LocalTests_TerminalApp/ColorSchemeTests.cpp b/src/cascadia/LocalTests_TerminalApp/ColorSchemeTests.cpp index 55c2026bada..ab56263cad0 100644 --- a/src/cascadia/LocalTests_TerminalApp/ColorSchemeTests.cpp +++ b/src/cascadia/LocalTests_TerminalApp/ColorSchemeTests.cpp @@ -9,6 +9,7 @@ using namespace Microsoft::Console; using namespace TerminalApp; +using namespace winrt::TerminalApp::implementation; using namespace WEX::Logging; using namespace WEX::TestExecution; using namespace WEX::Common; @@ -73,24 +74,24 @@ namespace TerminalAppLocalTests const auto scheme0 = ColorScheme::FromJson(scheme0Json); - VERIFY_IS_TRUE(scheme0.ShouldBeLayered(scheme0Json)); - VERIFY_IS_FALSE(scheme0.ShouldBeLayered(scheme1Json)); - VERIFY_IS_TRUE(scheme0.ShouldBeLayered(scheme2Json)); - VERIFY_IS_FALSE(scheme0.ShouldBeLayered(scheme3Json)); + VERIFY_IS_TRUE(scheme0->ShouldBeLayered(scheme0Json)); + VERIFY_IS_FALSE(scheme0->ShouldBeLayered(scheme1Json)); + VERIFY_IS_TRUE(scheme0->ShouldBeLayered(scheme2Json)); + VERIFY_IS_FALSE(scheme0->ShouldBeLayered(scheme3Json)); const auto scheme1 = ColorScheme::FromJson(scheme1Json); - VERIFY_IS_FALSE(scheme1.ShouldBeLayered(scheme0Json)); - VERIFY_IS_TRUE(scheme1.ShouldBeLayered(scheme1Json)); - VERIFY_IS_FALSE(scheme1.ShouldBeLayered(scheme2Json)); - VERIFY_IS_FALSE(scheme1.ShouldBeLayered(scheme3Json)); + VERIFY_IS_FALSE(scheme1->ShouldBeLayered(scheme0Json)); + VERIFY_IS_TRUE(scheme1->ShouldBeLayered(scheme1Json)); + VERIFY_IS_FALSE(scheme1->ShouldBeLayered(scheme2Json)); + VERIFY_IS_FALSE(scheme1->ShouldBeLayered(scheme3Json)); const auto scheme3 = ColorScheme::FromJson(scheme3Json); - VERIFY_IS_FALSE(scheme3.ShouldBeLayered(scheme0Json)); - VERIFY_IS_FALSE(scheme3.ShouldBeLayered(scheme1Json)); - VERIFY_IS_FALSE(scheme3.ShouldBeLayered(scheme2Json)); - VERIFY_IS_FALSE(scheme3.ShouldBeLayered(scheme3Json)); + VERIFY_IS_FALSE(scheme3->ShouldBeLayered(scheme0Json)); + VERIFY_IS_FALSE(scheme3->ShouldBeLayered(scheme1Json)); + VERIFY_IS_FALSE(scheme3->ShouldBeLayered(scheme2Json)); + VERIFY_IS_FALSE(scheme3->ShouldBeLayered(scheme3Json)); } void ColorSchemeTests::LayerColorSchemeProperties() @@ -130,38 +131,38 @@ namespace TerminalAppLocalTests const auto scheme2Json = VerifyParseSucceeded(scheme2String); auto scheme0 = ColorScheme::FromJson(scheme0Json); - VERIFY_ARE_EQUAL(L"scheme0", scheme0._schemeName); - VERIFY_ARE_EQUAL(ARGB(0, 0, 0, 0), scheme0._defaultForeground); - VERIFY_ARE_EQUAL(ARGB(0, 1, 1, 1), scheme0._defaultBackground); - VERIFY_ARE_EQUAL(ARGB(0, 1, 1, 0), scheme0._selectionBackground); - VERIFY_ARE_EQUAL(ARGB(0, 1, 0, 1), scheme0._cursorColor); - VERIFY_ARE_EQUAL(ARGB(0, 1, 0, 0), scheme0._table[XTERM_RED_ATTR]); - VERIFY_ARE_EQUAL(ARGB(0, 0, 1, 0), scheme0._table[XTERM_GREEN_ATTR]); - VERIFY_ARE_EQUAL(ARGB(0, 0, 0, 1), scheme0._table[XTERM_BLUE_ATTR]); + VERIFY_ARE_EQUAL(L"scheme0", scheme0->_schemeName); + VERIFY_ARE_EQUAL(ARGB(0, 0, 0, 0), scheme0->_defaultForeground); + VERIFY_ARE_EQUAL(ARGB(0, 1, 1, 1), scheme0->_defaultBackground); + VERIFY_ARE_EQUAL(ARGB(0, 1, 1, 0), scheme0->_selectionBackground); + VERIFY_ARE_EQUAL(ARGB(0, 1, 0, 1), scheme0->_cursorColor); + VERIFY_ARE_EQUAL(ARGB(0, 1, 0, 0), scheme0->_table[XTERM_RED_ATTR]); + VERIFY_ARE_EQUAL(ARGB(0, 0, 1, 0), scheme0->_table[XTERM_GREEN_ATTR]); + VERIFY_ARE_EQUAL(ARGB(0, 0, 0, 1), scheme0->_table[XTERM_BLUE_ATTR]); Log::Comment(NoThrowString().Format( L"Layering scheme1 on top of scheme0")); - scheme0.LayerJson(scheme1Json); + scheme0->LayerJson(scheme1Json); - VERIFY_ARE_EQUAL(ARGB(0, 2, 2, 2), scheme0._defaultForeground); - VERIFY_ARE_EQUAL(ARGB(0, 3, 3, 3), scheme0._defaultBackground); - VERIFY_ARE_EQUAL(ARGB(0, 2, 2, 0), scheme0._selectionBackground); - VERIFY_ARE_EQUAL(ARGB(0, 4, 0, 4), scheme0._cursorColor); - VERIFY_ARE_EQUAL(ARGB(0, 2, 0, 0), scheme0._table[XTERM_RED_ATTR]); - VERIFY_ARE_EQUAL(ARGB(0, 0, 1, 0), scheme0._table[XTERM_GREEN_ATTR]); - VERIFY_ARE_EQUAL(ARGB(0, 0, 0, 2), scheme0._table[XTERM_BLUE_ATTR]); + VERIFY_ARE_EQUAL(ARGB(0, 2, 2, 2), scheme0->_defaultForeground); + VERIFY_ARE_EQUAL(ARGB(0, 3, 3, 3), scheme0->_defaultBackground); + VERIFY_ARE_EQUAL(ARGB(0, 2, 2, 0), scheme0->_selectionBackground); + VERIFY_ARE_EQUAL(ARGB(0, 4, 0, 4), scheme0->_cursorColor); + VERIFY_ARE_EQUAL(ARGB(0, 2, 0, 0), scheme0->_table[XTERM_RED_ATTR]); + VERIFY_ARE_EQUAL(ARGB(0, 0, 1, 0), scheme0->_table[XTERM_GREEN_ATTR]); + VERIFY_ARE_EQUAL(ARGB(0, 0, 0, 2), scheme0->_table[XTERM_BLUE_ATTR]); Log::Comment(NoThrowString().Format( L"Layering scheme2Json on top of (scheme0+scheme1)")); - scheme0.LayerJson(scheme2Json); - - VERIFY_ARE_EQUAL(ARGB(0, 4, 4, 4), scheme0._defaultForeground); - VERIFY_ARE_EQUAL(ARGB(0, 5, 5, 5), scheme0._defaultBackground); - VERIFY_ARE_EQUAL(ARGB(0, 3, 3, 0), scheme0._selectionBackground); - VERIFY_ARE_EQUAL(ARGB(0, 6, 0, 6), scheme0._cursorColor); - VERIFY_ARE_EQUAL(ARGB(0, 3, 0, 0), scheme0._table[XTERM_RED_ATTR]); - VERIFY_ARE_EQUAL(ARGB(0, 0, 3, 0), scheme0._table[XTERM_GREEN_ATTR]); - VERIFY_ARE_EQUAL(ARGB(0, 0, 0, 2), scheme0._table[XTERM_BLUE_ATTR]); + scheme0->LayerJson(scheme2Json); + + VERIFY_ARE_EQUAL(ARGB(0, 4, 4, 4), scheme0->_defaultForeground); + VERIFY_ARE_EQUAL(ARGB(0, 5, 5, 5), scheme0->_defaultBackground); + VERIFY_ARE_EQUAL(ARGB(0, 3, 3, 0), scheme0->_selectionBackground); + VERIFY_ARE_EQUAL(ARGB(0, 6, 0, 6), scheme0->_cursorColor); + VERIFY_ARE_EQUAL(ARGB(0, 3, 0, 0), scheme0->_table[XTERM_RED_ATTR]); + VERIFY_ARE_EQUAL(ARGB(0, 0, 3, 0), scheme0->_table[XTERM_GREEN_ATTR]); + VERIFY_ARE_EQUAL(ARGB(0, 0, 0, 2), scheme0->_table[XTERM_BLUE_ATTR]); } void ColorSchemeTests::LayerColorSchemesOnArray() @@ -205,19 +206,20 @@ namespace TerminalAppLocalTests for (auto& kv : settings._globals._colorSchemes) { Log::Comment(NoThrowString().Format( - L"kv:%s->%s", kv.first.data(), kv.second.GetName().data())); + L"kv:%s->%s", kv.first.data(), kv.second.Name().data())); } VERIFY_ARE_EQUAL(1u, settings._globals.GetColorSchemes().size()); VERIFY_IS_TRUE(settings._globals._colorSchemes.find(L"scheme0") != settings._globals._colorSchemes.end()); - auto scheme0 = settings._globals._colorSchemes.find(L"scheme0")->second; + auto scheme0Proj = settings._globals._colorSchemes.find(L"scheme0")->second; + auto scheme0 = winrt::get_self(scheme0Proj); VERIFY_IS_NOT_NULL(settings._FindMatchingColorScheme(scheme0Json)); VERIFY_IS_NULL(settings._FindMatchingColorScheme(scheme1Json)); VERIFY_IS_NOT_NULL(settings._FindMatchingColorScheme(scheme2Json)); VERIFY_IS_NULL(settings._FindMatchingColorScheme(scheme3Json)); - VERIFY_ARE_EQUAL(ARGB(0, 0, 0, 0), scheme0._defaultForeground); - VERIFY_ARE_EQUAL(ARGB(0, 1, 1, 1), scheme0._defaultBackground); + VERIFY_ARE_EQUAL(ARGB(0, 0, 0, 0), scheme0->_defaultForeground); + VERIFY_ARE_EQUAL(ARGB(0, 1, 1, 1), scheme0->_defaultBackground); } settings._LayerOrCreateColorScheme(scheme1Json); @@ -226,18 +228,20 @@ namespace TerminalAppLocalTests VERIFY_ARE_EQUAL(2u, settings._globals.GetColorSchemes().size()); VERIFY_IS_TRUE(settings._globals._colorSchemes.find(L"scheme0") != settings._globals._colorSchemes.end()); - auto scheme0 = settings._globals._colorSchemes.find(L"scheme0")->second; + auto scheme0Proj = settings._globals._colorSchemes.find(L"scheme0")->second; + auto scheme0 = winrt::get_self(scheme0Proj); VERIFY_IS_TRUE(settings._globals._colorSchemes.find(L"scheme1") != settings._globals._colorSchemes.end()); - auto scheme1 = settings._globals._colorSchemes.find(L"scheme1")->second; + auto scheme1Proj = settings._globals._colorSchemes.find(L"scheme1")->second; + auto scheme1 = winrt::get_self(scheme1Proj); VERIFY_IS_NOT_NULL(settings._FindMatchingColorScheme(scheme0Json)); VERIFY_IS_NOT_NULL(settings._FindMatchingColorScheme(scheme1Json)); VERIFY_IS_NOT_NULL(settings._FindMatchingColorScheme(scheme2Json)); VERIFY_IS_NULL(settings._FindMatchingColorScheme(scheme3Json)); - VERIFY_ARE_EQUAL(ARGB(0, 0, 0, 0), scheme0._defaultForeground); - VERIFY_ARE_EQUAL(ARGB(0, 1, 1, 1), scheme0._defaultBackground); - VERIFY_ARE_EQUAL(ARGB(0, 2, 2, 2), scheme1._defaultForeground); - VERIFY_ARE_EQUAL(ARGB(0, 3, 3, 3), scheme1._defaultBackground); + VERIFY_ARE_EQUAL(ARGB(0, 0, 0, 0), scheme0->_defaultForeground); + VERIFY_ARE_EQUAL(ARGB(0, 1, 1, 1), scheme0->_defaultBackground); + VERIFY_ARE_EQUAL(ARGB(0, 2, 2, 2), scheme1->_defaultForeground); + VERIFY_ARE_EQUAL(ARGB(0, 3, 3, 3), scheme1->_defaultBackground); } settings._LayerOrCreateColorScheme(scheme2Json); @@ -245,18 +249,20 @@ namespace TerminalAppLocalTests VERIFY_ARE_EQUAL(2u, settings._globals.GetColorSchemes().size()); VERIFY_IS_TRUE(settings._globals._colorSchemes.find(L"scheme0") != settings._globals._colorSchemes.end()); - auto scheme0 = settings._globals._colorSchemes.find(L"scheme0")->second; + auto scheme0Proj = settings._globals._colorSchemes.find(L"scheme0")->second; + auto scheme0 = winrt::get_self(scheme0Proj); VERIFY_IS_TRUE(settings._globals._colorSchemes.find(L"scheme1") != settings._globals._colorSchemes.end()); - auto scheme1 = settings._globals._colorSchemes.find(L"scheme1")->second; + auto scheme1Proj = settings._globals._colorSchemes.find(L"scheme1")->second; + auto scheme1 = winrt::get_self(scheme1Proj); VERIFY_IS_NOT_NULL(settings._FindMatchingColorScheme(scheme0Json)); VERIFY_IS_NOT_NULL(settings._FindMatchingColorScheme(scheme1Json)); VERIFY_IS_NOT_NULL(settings._FindMatchingColorScheme(scheme2Json)); VERIFY_IS_NULL(settings._FindMatchingColorScheme(scheme3Json)); - VERIFY_ARE_EQUAL(ARGB(0, 4, 4, 4), scheme0._defaultForeground); - VERIFY_ARE_EQUAL(ARGB(0, 5, 5, 5), scheme0._defaultBackground); - VERIFY_ARE_EQUAL(ARGB(0, 2, 2, 2), scheme1._defaultForeground); - VERIFY_ARE_EQUAL(ARGB(0, 3, 3, 3), scheme1._defaultBackground); + VERIFY_ARE_EQUAL(ARGB(0, 4, 4, 4), scheme0->_defaultForeground); + VERIFY_ARE_EQUAL(ARGB(0, 5, 5, 5), scheme0->_defaultBackground); + VERIFY_ARE_EQUAL(ARGB(0, 2, 2, 2), scheme1->_defaultForeground); + VERIFY_ARE_EQUAL(ARGB(0, 3, 3, 3), scheme1->_defaultBackground); } settings._LayerOrCreateColorScheme(scheme3Json); @@ -264,22 +270,25 @@ namespace TerminalAppLocalTests VERIFY_ARE_EQUAL(3u, settings._globals.GetColorSchemes().size()); VERIFY_IS_TRUE(settings._globals._colorSchemes.find(L"scheme0") != settings._globals._colorSchemes.end()); - auto scheme0 = settings._globals._colorSchemes.find(L"scheme0")->second; + auto scheme0Proj = settings._globals._colorSchemes.find(L"scheme0")->second; + auto scheme0 = winrt::get_self(scheme0Proj); VERIFY_IS_TRUE(settings._globals._colorSchemes.find(L"scheme1") != settings._globals._colorSchemes.end()); - auto scheme1 = settings._globals._colorSchemes.find(L"scheme1")->second; + auto scheme1Proj = settings._globals._colorSchemes.find(L"scheme1")->second; + auto scheme1 = winrt::get_self(scheme1Proj); VERIFY_IS_TRUE(settings._globals._colorSchemes.find(L"") != settings._globals._colorSchemes.end()); - auto scheme2 = settings._globals._colorSchemes.find(L"")->second; + auto scheme2Proj = settings._globals._colorSchemes.find(L"")->second; + auto scheme2 = winrt::get_self(scheme2Proj); VERIFY_IS_NOT_NULL(settings._FindMatchingColorScheme(scheme0Json)); VERIFY_IS_NOT_NULL(settings._FindMatchingColorScheme(scheme1Json)); VERIFY_IS_NOT_NULL(settings._FindMatchingColorScheme(scheme2Json)); VERIFY_IS_NULL(settings._FindMatchingColorScheme(scheme3Json)); - VERIFY_ARE_EQUAL(ARGB(0, 4, 4, 4), scheme0._defaultForeground); - VERIFY_ARE_EQUAL(ARGB(0, 5, 5, 5), scheme0._defaultBackground); - VERIFY_ARE_EQUAL(ARGB(0, 2, 2, 2), scheme1._defaultForeground); - VERIFY_ARE_EQUAL(ARGB(0, 3, 3, 3), scheme1._defaultBackground); - VERIFY_ARE_EQUAL(ARGB(0, 6, 6, 6), scheme2._defaultForeground); - VERIFY_ARE_EQUAL(ARGB(0, 7, 7, 7), scheme2._defaultBackground); + VERIFY_ARE_EQUAL(ARGB(0, 4, 4, 4), scheme0->_defaultForeground); + VERIFY_ARE_EQUAL(ARGB(0, 5, 5, 5), scheme0->_defaultBackground); + VERIFY_ARE_EQUAL(ARGB(0, 2, 2, 2), scheme1->_defaultForeground); + VERIFY_ARE_EQUAL(ARGB(0, 3, 3, 3), scheme1->_defaultBackground); + VERIFY_ARE_EQUAL(ARGB(0, 6, 6, 6), scheme2->_defaultForeground); + VERIFY_ARE_EQUAL(ARGB(0, 7, 7, 7), scheme2->_defaultBackground); } } } diff --git a/src/cascadia/TerminalApp/CascadiaSettings.h b/src/cascadia/TerminalApp/CascadiaSettings.h index a4b61893112..6d94e7f8b4b 100644 --- a/src/cascadia/TerminalApp/CascadiaSettings.h +++ b/src/cascadia/TerminalApp/CascadiaSettings.h @@ -22,6 +22,8 @@ Author(s): #include "Profile.h" #include "IDynamicProfileGenerator.h" +#include "ColorScheme.h" + // fwdecl unittest classes namespace TerminalAppLocalTests { @@ -78,7 +80,7 @@ class TerminalApp::CascadiaSettings final static std::filesystem::path GetDefaultSettingsPath(); const Profile* FindProfile(GUID profileGuid) const noexcept; - const ColorScheme* GetColorSchemeForProfile(const GUID profileGuid) const; + const winrt::TerminalApp::ColorScheme* GetColorSchemeForProfile(const GUID profileGuid) const; std::vector& GetWarnings(); @@ -99,7 +101,7 @@ class TerminalApp::CascadiaSettings final void _LayerOrCreateProfile(const Json::Value& profileJson); Profile* _FindMatchingProfile(const Json::Value& profileJson); void _LayerOrCreateColorScheme(const Json::Value& schemeJson); - ColorScheme* _FindMatchingColorScheme(const Json::Value& schemeJson); + winrt::com_ptr _FindMatchingColorScheme(const Json::Value& schemeJson); void _ParseJsonString(std::string_view fileData, const bool isDefaultSettings); static const Json::Value& _GetProfilesJsonObject(const Json::Value& json); static const Json::Value& _GetDisabledProfileSourcesJsonObject(const Json::Value& json); diff --git a/src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp b/src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp index 8a4c1c50086..33433a3b9eb 100644 --- a/src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp +++ b/src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp @@ -708,7 +708,8 @@ void CascadiaSettings::_LayerOrCreateColorScheme(const Json::Value& schemeJson) } else { - _globals.AddColorScheme(ColorScheme::FromJson(schemeJson)); + const auto scheme = implementation::ColorScheme::FromJson(schemeJson); + _globals.AddColorScheme(*scheme); } } @@ -723,19 +724,15 @@ void CascadiaSettings::_LayerOrCreateColorScheme(const Json::Value& schemeJson) // Return Value: // - a ColorScheme that can be layered with the given json object, iff such a // color scheme exists. -ColorScheme* CascadiaSettings::_FindMatchingColorScheme(const Json::Value& schemeJson) +winrt::com_ptr CascadiaSettings::_FindMatchingColorScheme(const Json::Value& schemeJson) { - if (auto schemeName = ColorScheme::GetNameFromJson(schemeJson)) + if (auto schemeName = implementation::ColorScheme::GetNameFromJson(schemeJson)) { auto& schemes = _globals.GetColorSchemes(); auto iterator = schemes.find(*schemeName); if (iterator != schemes.end()) { - // HERE BE DRAGONS: Returning a pointer to a type in the vector is - // maybe not the _safest_ thing, but we have a mind to make Profile - // and ColorScheme winrt types in the future, so this will be safer - // then. - return &iterator->second; + return winrt::get_self(iterator->second)->get_strong(); } } return nullptr; diff --git a/src/cascadia/TerminalApp/ColorScheme.cpp b/src/cascadia/TerminalApp/ColorScheme.cpp index af70c3595e4..dd385746ca7 100644 --- a/src/cascadia/TerminalApp/ColorScheme.cpp +++ b/src/cascadia/TerminalApp/ColorScheme.cpp @@ -8,9 +8,12 @@ #include "Utils.h" #include "JsonUtils.h" +#include "ColorScheme.g.cpp" + using namespace ::Microsoft::Console; using namespace TerminalApp; -using namespace winrt::TerminalApp; +using namespace winrt::TerminalApp::implementation; +using namespace winrt::Windows::UI; static constexpr std::string_view NameKey{ "name" }; static constexpr std::string_view ForegroundKey{ "foreground" }; @@ -46,7 +49,7 @@ ColorScheme::ColorScheme() : { } -ColorScheme::ColorScheme(std::wstring name, til::color defaultFg, til::color defaultBg, til::color cursorColor) : +ColorScheme::ColorScheme(winrt::hstring name, Color defaultFg, Color defaultBg, Color cursorColor) : _schemeName{ name }, _table{}, _defaultForeground{ defaultFg }, @@ -87,10 +90,10 @@ void ColorScheme::ApplyScheme(const winrt::Microsoft::Terminal::TerminalControl: // - json: an object which should be a serialization of a ColorScheme object. // Return Value: // - a new ColorScheme instance created from the values in `json` -ColorScheme ColorScheme::FromJson(const Json::Value& json) +winrt::com_ptr ColorScheme::FromJson(const Json::Value& json) { - ColorScheme result; - result.LayerJson(json); + auto result = winrt::make_self(); + result->LayerJson(json); return result; } @@ -138,32 +141,37 @@ void ColorScheme::LayerJson(const Json::Value& json) } } -std::wstring_view ColorScheme::GetName() const noexcept +winrt::hstring ColorScheme::Name() const noexcept { - return { _schemeName }; + return { _schemeName.c_str() }; } -std::array& ColorScheme::GetTable() noexcept +winrt::com_array ColorScheme::Table() noexcept { - return _table; + winrt::com_array result = winrt::com_array(COLOR_TABLE_SIZE); + for (int i = 0; i < _table.size(); i++) + { + result[i] = _table[i]; + } + return result; } -til::color ColorScheme::GetForeground() const noexcept +Color ColorScheme::Foreground() const noexcept { return _defaultForeground; } -til::color ColorScheme::GetBackground() const noexcept +Color ColorScheme::Background() const noexcept { return _defaultBackground; } -til::color ColorScheme::GetSelectionBackground() const noexcept +Color ColorScheme::SelectionBackground() const noexcept { return _selectionBackground; } -til::color ColorScheme::GetCursorColor() const noexcept +Color ColorScheme::CursorColor() const noexcept { return _cursorColor; } diff --git a/src/cascadia/TerminalApp/ColorScheme.h b/src/cascadia/TerminalApp/ColorScheme.h index 2267eb8f006..3c78183681b 100644 --- a/src/cascadia/TerminalApp/ColorScheme.h +++ b/src/cascadia/TerminalApp/ColorScheme.h @@ -18,6 +18,8 @@ Author(s): #include "TerminalSettings.h" #include "../../inc/conattrs.hpp" +#include "ColorScheme.g.h" + // fwdecl unittest classes namespace TerminalAppLocalTests { @@ -25,41 +27,44 @@ namespace TerminalAppLocalTests class ColorSchemeTests; }; -namespace TerminalApp +namespace winrt::TerminalApp::implementation { - class ColorScheme; -}; + struct ColorScheme : ColorSchemeT + { + public: + ColorScheme(); + ColorScheme(hstring name, Windows::UI::Color defaultFg, Windows::UI::Color defaultBg, Windows::UI::Color cursorColor); + ~ColorScheme(); -class TerminalApp::ColorScheme -{ -public: - ColorScheme(); - ColorScheme(std::wstring name, til::color defaultFg, til::color defaultBg, til::color cursorColor); - ~ColorScheme(); + void ApplyScheme(const winrt::Microsoft::Terminal::TerminalControl::IControlSettings& terminalSettings) const; - void ApplyScheme(const winrt::Microsoft::Terminal::TerminalControl::IControlSettings& terminalSettings) const; + static com_ptr FromJson(const Json::Value& json); + bool ShouldBeLayered(const Json::Value& json) const; + void LayerJson(const Json::Value& json); - static ColorScheme FromJson(const Json::Value& json); - bool ShouldBeLayered(const Json::Value& json) const; - void LayerJson(const Json::Value& json); + hstring Name() const noexcept; + com_array Table() noexcept; + Windows::UI::Color Foreground() const noexcept; + Windows::UI::Color Background() const noexcept; + Windows::UI::Color SelectionBackground() const noexcept; + Windows::UI::Color CursorColor() const noexcept; - std::wstring_view GetName() const noexcept; - std::array& GetTable() noexcept; - til::color GetForeground() const noexcept; - til::color GetBackground() const noexcept; - til::color GetSelectionBackground() const noexcept; - til::color GetCursorColor() const noexcept; + static std::optional GetNameFromJson(const Json::Value& json); - static std::optional GetNameFromJson(const Json::Value& json); + private: + std::wstring _schemeName; + std::array _table; + til::color _defaultForeground; + til::color _defaultBackground; + til::color _selectionBackground; + til::color _cursorColor; -private: - std::wstring _schemeName; - std::array _table; - til::color _defaultForeground; - til::color _defaultBackground; - til::color _selectionBackground; - til::color _cursorColor; + friend class TerminalAppLocalTests::SettingsTests; + friend class TerminalAppLocalTests::ColorSchemeTests; + }; +} - friend class TerminalAppLocalTests::SettingsTests; - friend class TerminalAppLocalTests::ColorSchemeTests; -}; +namespace winrt::TerminalApp::factory_implementation +{ + BASIC_FACTORY(ColorScheme); +} diff --git a/src/cascadia/TerminalApp/ColorScheme.idl b/src/cascadia/TerminalApp/ColorScheme.idl new file mode 100644 index 00000000000..4f27f1d56e9 --- /dev/null +++ b/src/cascadia/TerminalApp/ColorScheme.idl @@ -0,0 +1,23 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import "../TerminalSettings.idl"; + +namespace TerminalApp +{ + [default_interface] runtimeclass ColorScheme { + ColorScheme(); + ColorScheme(String name, Windows.UI.Color defaultFg, Windows.UI.Color defaultBg, Windows.UI.Color cursorColor); + + void ApplyScheme(Microsoft.Terminal.TerminalControl.IControlSettings terminalSettings); + + String Name { get; }; + + Windows.UI.Color Foreground { get; }; + Windows.UI.Color Background { get; }; + Windows.UI.Color SelectionBackground { get; }; + Windows.UI.Color CursorColor { get; }; + + Windows.UI.Color[] Table { get; }; + } +} diff --git a/src/cascadia/TerminalApp/GlobalAppSettings.cpp b/src/cascadia/TerminalApp/GlobalAppSettings.cpp index b9d6175df01..834038bfad8 100644 --- a/src/cascadia/TerminalApp/GlobalAppSettings.cpp +++ b/src/cascadia/TerminalApp/GlobalAppSettings.cpp @@ -213,7 +213,7 @@ void GlobalAppSettings::LayerJson(const Json::Value& json) // - void GlobalAppSettings::AddColorScheme(ColorScheme scheme) { - std::wstring name{ scheme.GetName() }; + std::wstring name{ scheme.Name() }; _colorSchemes[name] = std::move(scheme); } diff --git a/src/cascadia/TerminalApp/GlobalAppSettings.h b/src/cascadia/TerminalApp/GlobalAppSettings.h index 6a72d0ad857..b7386429046 100644 --- a/src/cascadia/TerminalApp/GlobalAppSettings.h +++ b/src/cascadia/TerminalApp/GlobalAppSettings.h @@ -15,10 +15,11 @@ Author(s): --*/ #pragma once #include "AppKeyBindings.h" -#include "ColorScheme.h" #include "Command.h" #include "SettingsTypes.h" +#include "ColorScheme.g.h" + // fwdecl unittest classes namespace TerminalAppLocalTests { @@ -37,9 +38,9 @@ class TerminalApp::GlobalAppSettings final GlobalAppSettings(); ~GlobalAppSettings(); - std::unordered_map& GetColorSchemes() noexcept; - const std::unordered_map& GetColorSchemes() const noexcept; - void AddColorScheme(ColorScheme scheme); + std::unordered_map& GetColorSchemes() noexcept; + const std::unordered_map& GetColorSchemes() const noexcept; + void AddColorScheme(winrt::TerminalApp::ColorScheme scheme); winrt::TerminalApp::AppKeyBindings GetKeybindings() const noexcept; @@ -89,7 +90,7 @@ class TerminalApp::GlobalAppSettings final winrt::com_ptr _keybindings; std::vector<::TerminalApp::SettingsLoadWarnings> _keybindingsWarnings; - std::unordered_map _colorSchemes; + std::unordered_map _colorSchemes; winrt::Windows::Foundation::Collections::IMap _commands; friend class TerminalAppLocalTests::SettingsTests; diff --git a/src/cascadia/TerminalApp/Profile.h b/src/cascadia/TerminalApp/Profile.h index a6794bbcc07..2ab88f0c884 100644 --- a/src/cascadia/TerminalApp/Profile.h +++ b/src/cascadia/TerminalApp/Profile.h @@ -14,7 +14,7 @@ Author(s): --*/ #pragma once -#include "ColorScheme.h" +#include "ColorScheme.g.h" #include "SettingsTypes.h" // fwdecl unittest classes @@ -46,7 +46,7 @@ class TerminalApp::Profile final ~Profile(); - winrt::TerminalApp::TerminalSettings CreateTerminalSettings(const std::unordered_map& schemes) const; + winrt::TerminalApp::TerminalSettings CreateTerminalSettings(const std::unordered_map& schemes) const; Json::Value GenerateStub() const; static Profile FromJson(const Json::Value& json); diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index abd7768afc9..d9ba7af17b0 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -637,7 +637,7 @@ namespace winrt::TerminalApp::implementation const auto* scheme = _settings->GetColorSchemeForProfile(profileGuid); // If they explicitly specified `null` as the scheme (indicating _no_ scheme), log // that as the empty string. - const auto schemeName = scheme ? scheme->GetName() : L"\0"; + const auto schemeName = scheme ? scheme->Name() : L"\0"; TraceLoggingWrite( g_hTerminalAppProvider, // handle to TerminalApp tracelogging provider diff --git a/src/cascadia/TerminalApp/lib/TerminalAppLib.vcxproj b/src/cascadia/TerminalApp/lib/TerminalAppLib.vcxproj index aecf0ef0847..95f01337336 100644 --- a/src/cascadia/TerminalApp/lib/TerminalAppLib.vcxproj +++ b/src/cascadia/TerminalApp/lib/TerminalAppLib.vcxproj @@ -101,7 +101,9 @@ ../Tab.idl - + + ../ColorScheme.idl + @@ -175,7 +177,9 @@ ../Tab.idl - + + ../ColorScheme.idl + @@ -264,6 +268,7 @@ + diff --git a/src/cascadia/TerminalApp/lib/TerminalAppLib.vcxproj.filters b/src/cascadia/TerminalApp/lib/TerminalAppLib.vcxproj.filters index 9a80a146c43..731b54eada4 100644 --- a/src/cascadia/TerminalApp/lib/TerminalAppLib.vcxproj.filters +++ b/src/cascadia/TerminalApp/lib/TerminalAppLib.vcxproj.filters @@ -148,6 +148,9 @@ settings + + settings + diff --git a/src/cascadia/ut_app/JsonTests.cpp b/src/cascadia/ut_app/JsonTests.cpp index f18579b1d0d..cd600f53c09 100644 --- a/src/cascadia/ut_app/JsonTests.cpp +++ b/src/cascadia/ut_app/JsonTests.cpp @@ -92,12 +92,12 @@ namespace TerminalAppUnitTests "}" }; const auto schemeObject = VerifyParseSucceeded(campbellScheme); - auto scheme = ColorScheme::FromJson(schemeObject); - VERIFY_ARE_EQUAL(L"Campbell", scheme.GetName()); - VERIFY_ARE_EQUAL(ARGB(0, 0xf2, 0xf2, 0xf2), scheme.GetForeground()); - VERIFY_ARE_EQUAL(ARGB(0, 0x0c, 0x0c, 0x0c), scheme.GetBackground()); - VERIFY_ARE_EQUAL(ARGB(0, 0x13, 0x13, 0x13), scheme.GetSelectionBackground()); - VERIFY_ARE_EQUAL(ARGB(0, 0xFF, 0xFF, 0xFF), scheme.GetCursorColor()); + auto scheme = implementation::ColorScheme::FromJson(schemeObject); + VERIFY_ARE_EQUAL(L"Campbell", scheme->Name()); + VERIFY_ARE_EQUAL(til::color{ARGB(0, 0xf2, 0xf2, 0xf2)}, scheme->Foreground()); + VERIFY_ARE_EQUAL(til::color{ARGB(0, 0x0c, 0x0c, 0x0c)}, scheme->Background()); + VERIFY_ARE_EQUAL(til::color{ARGB(0, 0x13, 0x13, 0x13)}, scheme->SelectionBackground()); + VERIFY_ARE_EQUAL(til::color{ARGB(0, 0xFF, 0xFF, 0xFF)}, scheme->CursorColor()); std::array expectedCampbellTable; auto campbellSpan = gsl::span(&expectedCampbellTable[0], COLOR_TABLE_SIZE); @@ -107,7 +107,7 @@ namespace TerminalAppUnitTests for (size_t i = 0; i < expectedCampbellTable.size(); i++) { const auto& expected = expectedCampbellTable.at(i); - const auto& actual = scheme.GetTable().at(i); + const auto& actual = til::color{ scheme->Table().at(static_cast(i)) }; VERIFY_ARE_EQUAL(expected, actual); } } diff --git a/src/cascadia/ut_app/precomp.h b/src/cascadia/ut_app/precomp.h index 9097a630411..3ab75f96a8b 100644 --- a/src/cascadia/ut_app/precomp.h +++ b/src/cascadia/ut_app/precomp.h @@ -46,4 +46,5 @@ Author(s): #include #include #include -#include +#include +#include From 8417c9dab344f60137e92ed8bfd0f1b8ce4acbed Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Tue, 11 Aug 2020 16:52:27 -0700 Subject: [PATCH 2/9] fix build --- src/cascadia/TerminalApp/ColorScheme.cpp | 2 +- src/cascadia/ut_app/JsonTests.cpp | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/cascadia/TerminalApp/ColorScheme.cpp b/src/cascadia/TerminalApp/ColorScheme.cpp index dd385746ca7..4d3589e6a23 100644 --- a/src/cascadia/TerminalApp/ColorScheme.cpp +++ b/src/cascadia/TerminalApp/ColorScheme.cpp @@ -149,7 +149,7 @@ winrt::hstring ColorScheme::Name() const noexcept winrt::com_array ColorScheme::Table() noexcept { winrt::com_array result = winrt::com_array(COLOR_TABLE_SIZE); - for (int i = 0; i < _table.size(); i++) + for (uint32_t i = 0; i < _table.size(); i++) { result[i] = _table[i]; } diff --git a/src/cascadia/ut_app/JsonTests.cpp b/src/cascadia/ut_app/JsonTests.cpp index cd600f53c09..91340a21850 100644 --- a/src/cascadia/ut_app/JsonTests.cpp +++ b/src/cascadia/ut_app/JsonTests.cpp @@ -94,10 +94,10 @@ namespace TerminalAppUnitTests const auto schemeObject = VerifyParseSucceeded(campbellScheme); auto scheme = implementation::ColorScheme::FromJson(schemeObject); VERIFY_ARE_EQUAL(L"Campbell", scheme->Name()); - VERIFY_ARE_EQUAL(til::color{ARGB(0, 0xf2, 0xf2, 0xf2)}, scheme->Foreground()); - VERIFY_ARE_EQUAL(til::color{ARGB(0, 0x0c, 0x0c, 0x0c)}, scheme->Background()); - VERIFY_ARE_EQUAL(til::color{ARGB(0, 0x13, 0x13, 0x13)}, scheme->SelectionBackground()); - VERIFY_ARE_EQUAL(til::color{ARGB(0, 0xFF, 0xFF, 0xFF)}, scheme->CursorColor()); + VERIFY_ARE_EQUAL(til::color{ ARGB(0, 0xf2, 0xf2, 0xf2) }, scheme->Foreground()); + VERIFY_ARE_EQUAL(til::color{ ARGB(0, 0x0c, 0x0c, 0x0c) }, scheme->Background()); + VERIFY_ARE_EQUAL(til::color{ ARGB(0, 0x13, 0x13, 0x13) }, scheme->SelectionBackground()); + VERIFY_ARE_EQUAL(til::color{ ARGB(0, 0xFF, 0xFF, 0xFF) }, scheme->CursorColor()); std::array expectedCampbellTable; auto campbellSpan = gsl::span(&expectedCampbellTable[0], COLOR_TABLE_SIZE); From 7c4219961233181a7fb1d87d6a4a628a0e56b5a5 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Wed, 12 Aug 2020 10:11:25 -0700 Subject: [PATCH 3/9] fix tests --- src/cascadia/ut_app/JsonTests.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/cascadia/ut_app/JsonTests.cpp b/src/cascadia/ut_app/JsonTests.cpp index 91340a21850..4211fa7c233 100644 --- a/src/cascadia/ut_app/JsonTests.cpp +++ b/src/cascadia/ut_app/JsonTests.cpp @@ -94,10 +94,10 @@ namespace TerminalAppUnitTests const auto schemeObject = VerifyParseSucceeded(campbellScheme); auto scheme = implementation::ColorScheme::FromJson(schemeObject); VERIFY_ARE_EQUAL(L"Campbell", scheme->Name()); - VERIFY_ARE_EQUAL(til::color{ ARGB(0, 0xf2, 0xf2, 0xf2) }, scheme->Foreground()); - VERIFY_ARE_EQUAL(til::color{ ARGB(0, 0x0c, 0x0c, 0x0c) }, scheme->Background()); - VERIFY_ARE_EQUAL(til::color{ ARGB(0, 0x13, 0x13, 0x13) }, scheme->SelectionBackground()); - VERIFY_ARE_EQUAL(til::color{ ARGB(0, 0xFF, 0xFF, 0xFF) }, scheme->CursorColor()); + VERIFY_ARE_EQUAL(til::color(0xf2, 0xf2, 0xf2, 0), scheme->Foreground()); + VERIFY_ARE_EQUAL(til::color(0x0c, 0x0c, 0x0c, 0), scheme->Background()); + VERIFY_ARE_EQUAL(til::color(0x13, 0x13, 0x13, 0), scheme->SelectionBackground()); + VERIFY_ARE_EQUAL(til::color(0xFF, 0xFF, 0xFF, 0), scheme->CursorColor()); std::array expectedCampbellTable; auto campbellSpan = gsl::span(&expectedCampbellTable[0], COLOR_TABLE_SIZE); @@ -107,7 +107,7 @@ namespace TerminalAppUnitTests for (size_t i = 0; i < expectedCampbellTable.size(); i++) { const auto& expected = expectedCampbellTable.at(i); - const auto& actual = til::color{ scheme->Table().at(static_cast(i)) }; + const til::color actual{ scheme->Table().at(static_cast(i)) }; VERIFY_ARE_EQUAL(expected, actual); } } From 27c6207e9ef26015e9f9c1ccf59fc9880b6134fd Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Wed, 12 Aug 2020 10:18:02 -0700 Subject: [PATCH 4/9] fix filter --- src/cascadia/TerminalApp/lib/TerminalAppLib.vcxproj.filters | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cascadia/TerminalApp/lib/TerminalAppLib.vcxproj.filters b/src/cascadia/TerminalApp/lib/TerminalAppLib.vcxproj.filters index 731b54eada4..3056b71130d 100644 --- a/src/cascadia/TerminalApp/lib/TerminalAppLib.vcxproj.filters +++ b/src/cascadia/TerminalApp/lib/TerminalAppLib.vcxproj.filters @@ -148,7 +148,7 @@ settings - + settings From d9cdc19a38b01ffdef0d223d8111a79b4c0c521b Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Wed, 12 Aug 2020 10:37:49 -0700 Subject: [PATCH 5/9] remove TerminalSettings dependency --- src/cascadia/TerminalApp/ColorScheme.cpp | 14 +++++++------- src/cascadia/TerminalApp/ColorScheme.h | 2 +- src/cascadia/TerminalApp/ColorScheme.idl | 4 +--- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/cascadia/TerminalApp/ColorScheme.cpp b/src/cascadia/TerminalApp/ColorScheme.cpp index 4d3589e6a23..22cd580000a 100644 --- a/src/cascadia/TerminalApp/ColorScheme.cpp +++ b/src/cascadia/TerminalApp/ColorScheme.cpp @@ -67,20 +67,20 @@ ColorScheme::~ColorScheme() // - Apply our values to the given TerminalSettings object. Sets the foreground, // background, and color table of the settings object. // Arguments: -// - terminalSettings: the object to apply our settings to. +// - settings: the object to apply our settings to. // Return Value: // - -void ColorScheme::ApplyScheme(const winrt::Microsoft::Terminal::TerminalControl::IControlSettings& terminalSettings) const +void ColorScheme::ApplyScheme(const winrt::Microsoft::Terminal::TerminalControl::IControlSettings& settings) const { - terminalSettings.DefaultForeground(static_cast(_defaultForeground)); - terminalSettings.DefaultBackground(static_cast(_defaultBackground)); - terminalSettings.SelectionBackground(static_cast(_selectionBackground)); - terminalSettings.CursorColor(static_cast(_cursorColor)); + settings.DefaultForeground(static_cast(_defaultForeground)); + settings.DefaultBackground(static_cast(_defaultBackground)); + settings.SelectionBackground(static_cast(_selectionBackground)); + settings.CursorColor(static_cast(_cursorColor)); auto const tableCount = gsl::narrow_cast(_table.size()); for (int i = 0; i < tableCount; i++) { - terminalSettings.SetColorTableEntry(i, static_cast(_table[i])); + settings.SetColorTableEntry(i, static_cast(_table[i])); } } diff --git a/src/cascadia/TerminalApp/ColorScheme.h b/src/cascadia/TerminalApp/ColorScheme.h index 3c78183681b..aa99fb5e9e0 100644 --- a/src/cascadia/TerminalApp/ColorScheme.h +++ b/src/cascadia/TerminalApp/ColorScheme.h @@ -36,7 +36,7 @@ namespace winrt::TerminalApp::implementation ColorScheme(hstring name, Windows::UI::Color defaultFg, Windows::UI::Color defaultBg, Windows::UI::Color cursorColor); ~ColorScheme(); - void ApplyScheme(const winrt::Microsoft::Terminal::TerminalControl::IControlSettings& terminalSettings) const; + void ApplyScheme(const winrt::Microsoft::Terminal::TerminalControl::IControlSettings& settings) const; static com_ptr FromJson(const Json::Value& json); bool ShouldBeLayered(const Json::Value& json) const; diff --git a/src/cascadia/TerminalApp/ColorScheme.idl b/src/cascadia/TerminalApp/ColorScheme.idl index 4f27f1d56e9..e34cf7de08a 100644 --- a/src/cascadia/TerminalApp/ColorScheme.idl +++ b/src/cascadia/TerminalApp/ColorScheme.idl @@ -1,15 +1,13 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -import "../TerminalSettings.idl"; - namespace TerminalApp { [default_interface] runtimeclass ColorScheme { ColorScheme(); ColorScheme(String name, Windows.UI.Color defaultFg, Windows.UI.Color defaultBg, Windows.UI.Color cursorColor); - void ApplyScheme(Microsoft.Terminal.TerminalControl.IControlSettings terminalSettings); + void ApplyScheme(Microsoft.Terminal.TerminalControl.IControlSettings settings); String Name { get; }; From 3f712e7338dc74d1c537e5d8c95c807c60fb7a20 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Wed, 12 Aug 2020 12:50:15 -0700 Subject: [PATCH 6/9] Revert "remove TerminalSettings dependency" This reverts commit 7a3ca00a3df630a50802cd177a89c739803e3d8b. --- src/cascadia/TerminalApp/ColorScheme.cpp | 14 +++++++------- src/cascadia/TerminalApp/ColorScheme.h | 2 +- src/cascadia/TerminalApp/ColorScheme.idl | 4 +++- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/cascadia/TerminalApp/ColorScheme.cpp b/src/cascadia/TerminalApp/ColorScheme.cpp index 22cd580000a..4d3589e6a23 100644 --- a/src/cascadia/TerminalApp/ColorScheme.cpp +++ b/src/cascadia/TerminalApp/ColorScheme.cpp @@ -67,20 +67,20 @@ ColorScheme::~ColorScheme() // - Apply our values to the given TerminalSettings object. Sets the foreground, // background, and color table of the settings object. // Arguments: -// - settings: the object to apply our settings to. +// - terminalSettings: the object to apply our settings to. // Return Value: // - -void ColorScheme::ApplyScheme(const winrt::Microsoft::Terminal::TerminalControl::IControlSettings& settings) const +void ColorScheme::ApplyScheme(const winrt::Microsoft::Terminal::TerminalControl::IControlSettings& terminalSettings) const { - settings.DefaultForeground(static_cast(_defaultForeground)); - settings.DefaultBackground(static_cast(_defaultBackground)); - settings.SelectionBackground(static_cast(_selectionBackground)); - settings.CursorColor(static_cast(_cursorColor)); + terminalSettings.DefaultForeground(static_cast(_defaultForeground)); + terminalSettings.DefaultBackground(static_cast(_defaultBackground)); + terminalSettings.SelectionBackground(static_cast(_selectionBackground)); + terminalSettings.CursorColor(static_cast(_cursorColor)); auto const tableCount = gsl::narrow_cast(_table.size()); for (int i = 0; i < tableCount; i++) { - settings.SetColorTableEntry(i, static_cast(_table[i])); + terminalSettings.SetColorTableEntry(i, static_cast(_table[i])); } } diff --git a/src/cascadia/TerminalApp/ColorScheme.h b/src/cascadia/TerminalApp/ColorScheme.h index aa99fb5e9e0..3c78183681b 100644 --- a/src/cascadia/TerminalApp/ColorScheme.h +++ b/src/cascadia/TerminalApp/ColorScheme.h @@ -36,7 +36,7 @@ namespace winrt::TerminalApp::implementation ColorScheme(hstring name, Windows::UI::Color defaultFg, Windows::UI::Color defaultBg, Windows::UI::Color cursorColor); ~ColorScheme(); - void ApplyScheme(const winrt::Microsoft::Terminal::TerminalControl::IControlSettings& settings) const; + void ApplyScheme(const winrt::Microsoft::Terminal::TerminalControl::IControlSettings& terminalSettings) const; static com_ptr FromJson(const Json::Value& json); bool ShouldBeLayered(const Json::Value& json) const; diff --git a/src/cascadia/TerminalApp/ColorScheme.idl b/src/cascadia/TerminalApp/ColorScheme.idl index e34cf7de08a..4f27f1d56e9 100644 --- a/src/cascadia/TerminalApp/ColorScheme.idl +++ b/src/cascadia/TerminalApp/ColorScheme.idl @@ -1,13 +1,15 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. +import "../TerminalSettings.idl"; + namespace TerminalApp { [default_interface] runtimeclass ColorScheme { ColorScheme(); ColorScheme(String name, Windows.UI.Color defaultFg, Windows.UI.Color defaultBg, Windows.UI.Color cursorColor); - void ApplyScheme(Microsoft.Terminal.TerminalControl.IControlSettings settings); + void ApplyScheme(Microsoft.Terminal.TerminalControl.IControlSettings terminalSettings); String Name { get; }; From 1fb018de9fb06ca71fc82fb70c4bae8455b20119 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Wed, 12 Aug 2020 13:19:33 -0700 Subject: [PATCH 7/9] fix test + PR comment --- src/cascadia/TerminalApp/CascadiaSettings.cpp | 4 ++-- src/cascadia/TerminalApp/CascadiaSettings.h | 2 +- src/cascadia/TerminalApp/TerminalPage.cpp | 4 ++-- src/cascadia/ut_app/JsonTests.cpp | 8 ++++---- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/cascadia/TerminalApp/CascadiaSettings.cpp b/src/cascadia/TerminalApp/CascadiaSettings.cpp index e8b8a06e428..2ecfa174543 100644 --- a/src/cascadia/TerminalApp/CascadiaSettings.cpp +++ b/src/cascadia/TerminalApp/CascadiaSettings.cpp @@ -734,7 +734,7 @@ std::string CascadiaSettings::_ApplyFirstRunChangesToSettingsTemplate(std::strin // - profileGuid: the GUID of the profile to find the scheme for. // Return Value: // - a non-owning pointer to the scheme. -const ColorScheme* CascadiaSettings::GetColorSchemeForProfile(const GUID profileGuid) const +const ColorScheme CascadiaSettings::GetColorSchemeForProfile(const GUID profileGuid) const { auto* profile = FindProfile(profileGuid); if (!profile) @@ -745,7 +745,7 @@ const ColorScheme* CascadiaSettings::GetColorSchemeForProfile(const GUID profile auto scheme = _globals.GetColorSchemes().find(schemeName); if (scheme != _globals.GetColorSchemes().end()) { - return &scheme->second; + return scheme->second; } else { diff --git a/src/cascadia/TerminalApp/CascadiaSettings.h b/src/cascadia/TerminalApp/CascadiaSettings.h index 6d94e7f8b4b..def568424f5 100644 --- a/src/cascadia/TerminalApp/CascadiaSettings.h +++ b/src/cascadia/TerminalApp/CascadiaSettings.h @@ -80,7 +80,7 @@ class TerminalApp::CascadiaSettings final static std::filesystem::path GetDefaultSettingsPath(); const Profile* FindProfile(GUID profileGuid) const noexcept; - const winrt::TerminalApp::ColorScheme* GetColorSchemeForProfile(const GUID profileGuid) const; + const winrt::TerminalApp::ColorScheme GetColorSchemeForProfile(const GUID profileGuid) const; std::vector& GetWarnings(); diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index d9ba7af17b0..1c9bc9fd212 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -634,10 +634,10 @@ namespace winrt::TerminalApp::implementation newTerminalArgs.Profile().empty()); // Lookup the name of the color scheme used by this profile. - const auto* scheme = _settings->GetColorSchemeForProfile(profileGuid); + const auto scheme = _settings->GetColorSchemeForProfile(profileGuid); // If they explicitly specified `null` as the scheme (indicating _no_ scheme), log // that as the empty string. - const auto schemeName = scheme ? scheme->Name() : L"\0"; + const auto schemeName = scheme ? scheme.Name() : L"\0"; TraceLoggingWrite( g_hTerminalAppProvider, // handle to TerminalApp tracelogging provider diff --git a/src/cascadia/ut_app/JsonTests.cpp b/src/cascadia/ut_app/JsonTests.cpp index 4211fa7c233..c145c48c1dc 100644 --- a/src/cascadia/ut_app/JsonTests.cpp +++ b/src/cascadia/ut_app/JsonTests.cpp @@ -94,10 +94,10 @@ namespace TerminalAppUnitTests const auto schemeObject = VerifyParseSucceeded(campbellScheme); auto scheme = implementation::ColorScheme::FromJson(schemeObject); VERIFY_ARE_EQUAL(L"Campbell", scheme->Name()); - VERIFY_ARE_EQUAL(til::color(0xf2, 0xf2, 0xf2, 0), scheme->Foreground()); - VERIFY_ARE_EQUAL(til::color(0x0c, 0x0c, 0x0c, 0), scheme->Background()); - VERIFY_ARE_EQUAL(til::color(0x13, 0x13, 0x13, 0), scheme->SelectionBackground()); - VERIFY_ARE_EQUAL(til::color(0xFF, 0xFF, 0xFF, 0), scheme->CursorColor()); + VERIFY_ARE_EQUAL(til::color(0xf2, 0xf2, 0xf2, 255), scheme->Foreground()); + VERIFY_ARE_EQUAL(til::color(0x0c, 0x0c, 0x0c, 255), scheme->Background()); + VERIFY_ARE_EQUAL(til::color(0x13, 0x13, 0x13, 255), scheme->SelectionBackground()); + VERIFY_ARE_EQUAL(til::color(0xFF, 0xFF, 0xFF, 255), scheme->CursorColor()); std::array expectedCampbellTable; auto campbellSpan = gsl::span(&expectedCampbellTable[0], COLOR_TABLE_SIZE); From 38760fe135e851d6578b09aea530b00e71c8edd8 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Fri, 14 Aug 2020 12:09:14 -0700 Subject: [PATCH 8/9] PR Comments --- src/cascadia/TerminalApp/ColorScheme.cpp | 11 ++++------- src/cascadia/TerminalApp/ColorScheme.h | 2 +- src/cascadia/TerminalApp/ColorScheme.idl | 2 -- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/cascadia/TerminalApp/ColorScheme.cpp b/src/cascadia/TerminalApp/ColorScheme.cpp index 4d3589e6a23..551ad3ac95c 100644 --- a/src/cascadia/TerminalApp/ColorScheme.cpp +++ b/src/cascadia/TerminalApp/ColorScheme.cpp @@ -143,16 +143,13 @@ void ColorScheme::LayerJson(const Json::Value& json) winrt::hstring ColorScheme::Name() const noexcept { - return { _schemeName.c_str() }; + return winrt::hstring(_schemeName); } -winrt::com_array ColorScheme::Table() noexcept +winrt::com_array ColorScheme::Table() const noexcept { - winrt::com_array result = winrt::com_array(COLOR_TABLE_SIZE); - for (uint32_t i = 0; i < _table.size(); i++) - { - result[i] = _table[i]; - } + winrt::com_array result{ COLOR_TABLE_SIZE }; + std::transform(_table.begin(), _table.end(), result.begin(), [](til::color c) -> Color { return c; }); return result; } diff --git a/src/cascadia/TerminalApp/ColorScheme.h b/src/cascadia/TerminalApp/ColorScheme.h index 3c78183681b..75ddc865413 100644 --- a/src/cascadia/TerminalApp/ColorScheme.h +++ b/src/cascadia/TerminalApp/ColorScheme.h @@ -43,7 +43,7 @@ namespace winrt::TerminalApp::implementation void LayerJson(const Json::Value& json); hstring Name() const noexcept; - com_array Table() noexcept; + com_array Table() const noexcept; Windows::UI::Color Foreground() const noexcept; Windows::UI::Color Background() const noexcept; Windows::UI::Color SelectionBackground() const noexcept; diff --git a/src/cascadia/TerminalApp/ColorScheme.idl b/src/cascadia/TerminalApp/ColorScheme.idl index 4f27f1d56e9..ea0d2979aad 100644 --- a/src/cascadia/TerminalApp/ColorScheme.idl +++ b/src/cascadia/TerminalApp/ColorScheme.idl @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -import "../TerminalSettings.idl"; - namespace TerminalApp { [default_interface] runtimeclass ColorScheme { From c2518b555d3f3923f6a56e20256d509ef1ffd8e8 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Fri, 14 Aug 2020 13:06:45 -0700 Subject: [PATCH 9/9] one less wstring<-->hstring --- src/cascadia/TerminalApp/ColorScheme.cpp | 2 +- src/cascadia/TerminalApp/ColorScheme.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cascadia/TerminalApp/ColorScheme.cpp b/src/cascadia/TerminalApp/ColorScheme.cpp index 551ad3ac95c..206b7baad67 100644 --- a/src/cascadia/TerminalApp/ColorScheme.cpp +++ b/src/cascadia/TerminalApp/ColorScheme.cpp @@ -143,7 +143,7 @@ void ColorScheme::LayerJson(const Json::Value& json) winrt::hstring ColorScheme::Name() const noexcept { - return winrt::hstring(_schemeName); + return _schemeName; } winrt::com_array ColorScheme::Table() const noexcept diff --git a/src/cascadia/TerminalApp/ColorScheme.h b/src/cascadia/TerminalApp/ColorScheme.h index 75ddc865413..fe7c8186a69 100644 --- a/src/cascadia/TerminalApp/ColorScheme.h +++ b/src/cascadia/TerminalApp/ColorScheme.h @@ -52,7 +52,7 @@ namespace winrt::TerminalApp::implementation static std::optional GetNameFromJson(const Json::Value& json); private: - std::wstring _schemeName; + hstring _schemeName; std::array _table; til::color _defaultForeground; til::color _defaultBackground;