Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make ColorScheme a WinRT object #7238

Merged
merged 9 commits into from
Aug 15, 2020
Merged

Make ColorScheme a WinRT object #7238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a44ef99
Make ColorScheme a WinRT object
carlos-zamora Aug 10, 2020
8417c9d
fix build
carlos-zamora Aug 11, 2020
7c42199
fix tests
carlos-zamora Aug 12, 2020
27c6207
fix filter
carlos-zamora Aug 12, 2020
d9cdc19
remove TerminalSettings dependency
carlos-zamora Aug 12, 2020
3f712e7
Revert "remove TerminalSettings dependency"
carlos-zamora Aug 12, 2020
1fb018d
fix test + PR comment
carlos-zamora Aug 12, 2020
38760fe
PR Comments
carlos-zamora Aug 14, 2020
c2518b5
one less wstring<-->hstring
carlos-zamora Aug 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 71 additions & 62 deletions src/cascadia/LocalTests_TerminalApp/ColorSchemeTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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<ColorScheme>(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);
Expand All @@ -226,60 +228,67 @@ 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<ColorScheme>(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<ColorScheme>(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);

{
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<ColorScheme>(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<ColorScheme>(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);

{
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<ColorScheme>(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<ColorScheme>(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<ColorScheme>(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);
}
}
}
4 changes: 2 additions & 2 deletions src/cascadia/TerminalApp/CascadiaSettings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
{
Expand Down
6 changes: 4 additions & 2 deletions src/cascadia/TerminalApp/CascadiaSettings.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ Author(s):
#include "Profile.h"
#include "IDynamicProfileGenerator.h"

#include "ColorScheme.h"

// fwdecl unittest classes
namespace TerminalAppLocalTests
{
Expand Down Expand Up @@ -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<TerminalApp::SettingsLoadWarnings>& GetWarnings();

Expand All @@ -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<winrt::TerminalApp::implementation::ColorScheme> _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);
Expand Down
13 changes: 5 additions & 8 deletions src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand All @@ -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<implementation::ColorScheme> 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<implementation::ColorScheme>(iterator->second)->get_strong();
}
}
return nullptr;
Expand Down
31 changes: 18 additions & 13 deletions src/cascadia/TerminalApp/ColorScheme.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" };
Expand Down Expand Up @@ -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 },
Expand Down Expand Up @@ -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> ColorScheme::FromJson(const Json::Value& json)
{
ColorScheme result;
result.LayerJson(json);
auto result = winrt::make_self<ColorScheme>();
result->LayerJson(json);
return result;
}

Expand Down Expand Up @@ -138,32 +141,34 @@ void ColorScheme::LayerJson(const Json::Value& json)
}
}

std::wstring_view ColorScheme::GetName() const noexcept
winrt::hstring ColorScheme::Name() const noexcept
{
return { _schemeName };
return _schemeName;
}

std::array<til::color, COLOR_TABLE_SIZE>& ColorScheme::GetTable() noexcept
winrt::com_array<Color> ColorScheme::Table() const noexcept
{
return _table;
winrt::com_array<Color> result{ COLOR_TABLE_SIZE };
std::transform(_table.begin(), _table.end(), result.begin(), [](til::color c) -> Color { return c; });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is fine because a til::color is small enough to fit in a single register (or two registers), but it could be an expensive copy for any other struct. prefer const x& unless otherwise okay

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;
}
Expand Down
Loading