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

Update ColorScheme with Json Serializer and color table API #7609

Merged
merged 12 commits into from
Sep 17, 2020
Merged

Update ColorScheme with Json Serializer and color table API #7609

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
be3a5a0
Add SetValueForKey to JsonUtils
carlos-zamora Sep 10, 2020
a5fccc6
Add ToJson to ColorScheme
carlos-zamora Sep 10, 2020
c69572b
polish ColorScheme
carlos-zamora Sep 11, 2020
0de07b8
update spell bot
carlos-zamora Sep 11, 2020
0241022
PR Comments - Dustin
carlos-zamora Sep 11, 2020
57620b7
Add comment for demand
carlos-zamora Sep 11, 2020
ce4d5c6
Merge branch 'master' into dev/cazamor/tojson/basic
carlos-zamora Sep 14, 2020
9d4a4ca
add SetColorTable()
carlos-zamora Sep 14, 2020
01c165a
rename to GetColorTableEntry()
carlos-zamora Sep 15, 2020
f217c7a
PR comments - Dustin
carlos-zamora Sep 17, 2020
2dde34b
I knew I missed a code format. whoops!
carlos-zamora Sep 17, 2020
aa6b1e6
rename to ToJson; use til::coalesce
carlos-zamora Sep 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/actions/spell-check/dictionary/apis.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ ACCESSDENIED
alignof
bitfield
bitfields
COLORPROPERTY
COLORTABLEPROPERTY
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
CLASSNOTAVAILABLE
environstrings
EXPCMDFLAGS
Expand Down
58 changes: 29 additions & 29 deletions src/cascadia/LocalTests_TerminalApp/ColorSchemeTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,11 @@ 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(L"scheme0", scheme0->_Name);
VERIFY_ARE_EQUAL(ARGB(0, 0, 0, 0), scheme0->_Foreground);
VERIFY_ARE_EQUAL(ARGB(0, 1, 1, 1), scheme0->_Background);
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]);
Expand All @@ -144,10 +144,10 @@ namespace TerminalAppLocalTests
L"Layering scheme1 on top of scheme0"));
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, 2, 2), scheme0->_Foreground);
VERIFY_ARE_EQUAL(ARGB(0, 3, 3, 3), scheme0->_Background);
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]);
Expand All @@ -156,10 +156,10 @@ namespace TerminalAppLocalTests
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, 4, 4, 4), scheme0->_Foreground);
VERIFY_ARE_EQUAL(ARGB(0, 5, 5, 5), scheme0->_Background);
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]);
Expand Down Expand Up @@ -218,8 +218,8 @@ namespace TerminalAppLocalTests
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->_Foreground);
VERIFY_ARE_EQUAL(ARGB(0, 1, 1, 1), scheme0->_Background);
}

settings->_LayerOrCreateColorScheme(scheme1Json);
Expand All @@ -238,10 +238,10 @@ namespace TerminalAppLocalTests
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->_Foreground);
VERIFY_ARE_EQUAL(ARGB(0, 1, 1, 1), scheme0->_Background);
VERIFY_ARE_EQUAL(ARGB(0, 2, 2, 2), scheme1->_Foreground);
VERIFY_ARE_EQUAL(ARGB(0, 3, 3, 3), scheme1->_Background);
}
settings->_LayerOrCreateColorScheme(scheme2Json);

Expand All @@ -259,10 +259,10 @@ namespace TerminalAppLocalTests
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->_Foreground);
VERIFY_ARE_EQUAL(ARGB(0, 5, 5, 5), scheme0->_Background);
VERIFY_ARE_EQUAL(ARGB(0, 2, 2, 2), scheme1->_Foreground);
VERIFY_ARE_EQUAL(ARGB(0, 3, 3, 3), scheme1->_Background);
}
settings->_LayerOrCreateColorScheme(scheme3Json);

Expand All @@ -283,12 +283,12 @@ namespace TerminalAppLocalTests
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->_Foreground);
VERIFY_ARE_EQUAL(ARGB(0, 5, 5, 5), scheme0->_Background);
VERIFY_ARE_EQUAL(ARGB(0, 2, 2, 2), scheme1->_Foreground);
VERIFY_ARE_EQUAL(ARGB(0, 3, 3, 3), scheme1->_Background);
VERIFY_ARE_EQUAL(ARGB(0, 6, 6, 6), scheme2->_Foreground);
VERIFY_ARE_EQUAL(ARGB(0, 7, 7, 7), scheme2->_Background);
}
}
}
149 changes: 81 additions & 68 deletions src/cascadia/TerminalApp/ColorScheme.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,46 +20,48 @@ static constexpr std::string_view ForegroundKey{ "foreground" };
static constexpr std::string_view BackgroundKey{ "background" };
static constexpr std::string_view SelectionBackgroundKey{ "selectionBackground" };
static constexpr std::string_view CursorColorKey{ "cursorColor" };

static constexpr std::string_view BlackKey{ "black" };
static constexpr std::string_view RedKey{ "red" };
static constexpr std::string_view GreenKey{ "green" };
static constexpr std::string_view YellowKey{ "yellow" };
static constexpr std::string_view BlueKey{ "blue" };
static constexpr std::string_view PurpleKey{ "purple" };
static constexpr std::string_view CyanKey{ "cyan" };
static constexpr std::string_view WhiteKey{ "white" };
static constexpr std::string_view BrightBlackKey{ "brightBlack" };
static constexpr std::string_view BrightRedKey{ "brightRed" };
static constexpr std::string_view BrightGreenKey{ "brightGreen" };
static constexpr std::string_view BrightYellowKey{ "brightYellow" };
static constexpr std::string_view BrightBlueKey{ "brightBlue" };
static constexpr std::string_view BrightPurpleKey{ "brightPurple" };
static constexpr std::string_view BrightCyanKey{ "brightCyan" };
static constexpr std::string_view BrightWhiteKey{ "brightWhite" };

static constexpr std::array<std::string_view, 16> TableColors = {
"black",
"red",
"green",
"yellow",
"blue",
"purple",
"cyan",
"white",
"brightBlack",
"brightRed",
"brightGreen",
"brightYellow",
"brightBlue",
"brightPurple",
"brightCyan",
"brightWhite"
BlackKey,
RedKey,
GreenKey,
YellowKey,
BlueKey,
PurpleKey,
CyanKey,
WhiteKey,
BrightBlackKey,
BrightRedKey,
BrightGreenKey,
BrightYellowKey,
BrightBlueKey,
BrightPurpleKey,
BrightCyanKey,
BrightWhiteKey,
};
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved

ColorScheme::ColorScheme() :
_schemeName{ L"" },
_table{},
_defaultForeground{ DEFAULT_FOREGROUND_WITH_ALPHA },
_defaultBackground{ DEFAULT_BACKGROUND_WITH_ALPHA },
_selectionBackground{ DEFAULT_FOREGROUND },
_cursorColor{ DEFAULT_CURSOR_COLOR }
{
}

ColorScheme::ColorScheme(winrt::hstring name, Color defaultFg, Color defaultBg, Color cursorColor) :
_schemeName{ name },
_table{},
_defaultForeground{ defaultFg },
_defaultBackground{ defaultBg },
_selectionBackground{ DEFAULT_FOREGROUND },
_cursorColor{ cursorColor }
{
}

ColorScheme::~ColorScheme()
_Name{ name },
_Foreground{ defaultFg },
_Background{ defaultBg },
_CursorColor{ cursorColor }
{
}

Expand All @@ -72,10 +74,10 @@ ColorScheme::~ColorScheme()
// - <none>
void ColorScheme::ApplyScheme(const winrt::Microsoft::Terminal::TerminalControl::IControlSettings& terminalSettings) const
{
terminalSettings.DefaultForeground(static_cast<COLORREF>(_defaultForeground));
terminalSettings.DefaultBackground(static_cast<COLORREF>(_defaultBackground));
terminalSettings.SelectionBackground(static_cast<COLORREF>(_selectionBackground));
terminalSettings.CursorColor(static_cast<COLORREF>(_cursorColor));
terminalSettings.DefaultForeground(static_cast<COLORREF>(_Foreground));
terminalSettings.DefaultBackground(static_cast<COLORREF>(_Background));
terminalSettings.SelectionBackground(static_cast<COLORREF>(_SelectionBackground));
terminalSettings.CursorColor(static_cast<COLORREF>(_CursorColor));

auto const tableCount = gsl::narrow_cast<int>(_table.size());
for (int i = 0; i < tableCount; i++)
Expand Down Expand Up @@ -110,7 +112,7 @@ bool ColorScheme::ShouldBeLayered(const Json::Value& json) const
std::wstring nameFromJson{};
if (JsonUtils::GetValueForKey(json, NameKey, nameFromJson))
{
return nameFromJson == _schemeName;
return nameFromJson == _Name;
}
return false;
}
Expand All @@ -127,11 +129,11 @@ bool ColorScheme::ShouldBeLayered(const Json::Value& json) const
// <none>
void ColorScheme::LayerJson(const Json::Value& json)
{
JsonUtils::GetValueForKey(json, NameKey, _schemeName);
JsonUtils::GetValueForKey(json, ForegroundKey, _defaultForeground);
JsonUtils::GetValueForKey(json, BackgroundKey, _defaultBackground);
JsonUtils::GetValueForKey(json, SelectionBackgroundKey, _selectionBackground);
JsonUtils::GetValueForKey(json, CursorColorKey, _cursorColor);
JsonUtils::GetValueForKey(json, NameKey, _Name);
JsonUtils::GetValueForKey(json, ForegroundKey, _Foreground);
JsonUtils::GetValueForKey(json, BackgroundKey, _Background);
JsonUtils::GetValueForKey(json, SelectionBackgroundKey, _SelectionBackground);
JsonUtils::GetValueForKey(json, CursorColorKey, _CursorColor);

int i = 0;
for (const auto& current : TableColors)
Expand All @@ -141,36 +143,47 @@ void ColorScheme::LayerJson(const Json::Value& json)
}
}

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

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

Color ColorScheme::Foreground() const noexcept
// Method Description:
// - Create a new serialized JsonObject from an instance of this class
// Arguments:
// - scheme: a ColorScheme object that will be converted into a serialized JsonObject
// Return Value:
// - a serialized JsonObject from the values in scheme
Json::Value ColorScheme::ToJson(const TerminalApp::ColorScheme& scheme)
{
return _defaultForeground;
Json::Value json{ Json::ValueType::objectValue };
const auto schemeImpl{ winrt::get_self<implementation::ColorScheme>(scheme) };
schemeImpl->UpdateJson(json);
return json;
}

Color ColorScheme::Background() const noexcept
// Method Description:
// - Update the given json object with values from this object.
// Arguments:
// - json: an object which will be a serialization of a ColorScheme object.
// Return Value:
// <none>
void ColorScheme::UpdateJson(Json::Value& json)
Copy link
Member

Choose a reason for hiding this comment

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

i 100% expected you to go a different direction with this and just .. make it a function called ToJson that returned the json object.

Copy link
Member

Choose a reason for hiding this comment

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

Is there ever a reason for us to call UpdateJson multiple times with different color schemes? (or profiles, etc.)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ideally, I could name it LayerJson with an out param because we're really just layering our values onto the Json. But I guess that isn't something we're doing yet, so yeah, renaming to ToJson as you expected haha

{
return _defaultBackground;
}
JsonUtils::SetValueForKey(json, NameKey, _Name);
JsonUtils::SetValueForKey(json, ForegroundKey, _Foreground);
JsonUtils::SetValueForKey(json, BackgroundKey, _Background);
JsonUtils::SetValueForKey(json, SelectionBackgroundKey, _SelectionBackground);
JsonUtils::SetValueForKey(json, CursorColorKey, _CursorColor);

Color ColorScheme::SelectionBackground() const noexcept
{
return _selectionBackground;
int i = 0;
for (const auto& current : TableColors)
{
JsonUtils::SetValueForKey(json, current, _table.at(i));
i++;
}
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
}

Color ColorScheme::CursorColor() const noexcept
winrt::com_array<Color> ColorScheme::Table() const noexcept
{
return _cursorColor;
winrt::com_array<Color> result{ COLOR_TABLE_SIZE };
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
std::transform(_table.begin(), _table.end(), result.begin(), [](til::color c) -> Color { return c; });
return result;
}

// Method Description:
Expand Down