From 8f799b67c4b00ee5bfa1875d2d9d8d3e8347e1ab Mon Sep 17 00:00:00 2001 From: Matthew Daly Date: Thu, 9 Jun 2022 21:53:07 -0400 Subject: [PATCH 1/2] Check alt color names in Json (#11456) --- .../TerminalSettingsModel/ColorScheme.cpp | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/cascadia/TerminalSettingsModel/ColorScheme.cpp b/src/cascadia/TerminalSettingsModel/ColorScheme.cpp index 0d55c405b79..ae818c2c2f0 100644 --- a/src/cascadia/TerminalSettingsModel/ColorScheme.cpp +++ b/src/cascadia/TerminalSettingsModel/ColorScheme.cpp @@ -100,7 +100,25 @@ bool ColorScheme::_layerJson(const Json::Value& json) // Required fields for (unsigned int i = 0; i < TableColors.size(); ++i) { - isValid &= JsonUtils::GetValueForKey(json, til::at(TableColors, i), til::at(_table, i)); + std::string_view tableColorName = til::at(TableColors, i); + bool valueForKeyIsValid = JsonUtils::GetValueForKey(json, tableColorName, til::at(_table, i)); + + // If GetValueForKey failed, try again with alternate color names + if (!valueForKeyIsValid) + { + if (tableColorName == "purple") + { + tableColorName = "magenta"; + } + else if (tableColorName == "brightPurple") + { + tableColorName = "brightMagenta"; + } + + valueForKeyIsValid = JsonUtils::GetValueForKey(json, tableColorName, til::at(_table, i)); + } + + isValid &= valueForKeyIsValid; } return isValid; From 3fc5cd0a3fbacc8269e4ab1cdae6fc3c0944ce11 Mon Sep 17 00:00:00 2001 From: Matthew Daly Date: Fri, 10 Jun 2022 14:02:42 -0400 Subject: [PATCH 2/2] Update check for alternate color names --- .../TerminalSettingsModel/ColorScheme.cpp | 72 +++++++++---------- 1 file changed, 34 insertions(+), 38 deletions(-) diff --git a/src/cascadia/TerminalSettingsModel/ColorScheme.cpp b/src/cascadia/TerminalSettingsModel/ColorScheme.cpp index ae818c2c2f0..62f405e48fb 100644 --- a/src/cascadia/TerminalSettingsModel/ColorScheme.cpp +++ b/src/cascadia/TerminalSettingsModel/ColorScheme.cpp @@ -21,24 +21,30 @@ static constexpr std::string_view BackgroundKey{ "background" }; static constexpr std::string_view SelectionBackgroundKey{ "selectionBackground" }; static constexpr std::string_view CursorColorKey{ "cursorColor" }; -static constexpr std::array TableColors = { - "black", - "red", - "green", - "yellow", - "blue", - "purple", - "cyan", - "white", - "brightBlack", - "brightRed", - "brightGreen", - "brightYellow", - "brightBlue", - "brightPurple", - "brightCyan", - "brightWhite" -}; +static constexpr size_t ColorSchemeExpectedSize = 16; +static constexpr std::array, 18> TableColorsMapping{ { + // Primary color mappings + { "black", 0 }, + { "red", 1 }, + { "green", 2 }, + { "yellow", 3 }, + { "blue", 4 }, + { "purple", 5 }, + { "cyan", 6 }, + { "white", 7 }, + { "brightBlack", 8 }, + { "brightRed", 9 }, + { "brightGreen", 10 }, + { "brightYellow", 11 }, + { "brightBlue", 12 }, + { "brightPurple", 13 }, + { "brightCyan", 14 }, + { "brightWhite", 15 }, + + // Alternate color mappings (GH#11456) + { "magenta", 5 }, + { "brightMagenta", 13 }, +} }; ColorScheme::ColorScheme() noexcept : ColorScheme{ winrt::hstring{} } @@ -98,29 +104,18 @@ bool ColorScheme::_layerJson(const Json::Value& json) JsonUtils::GetValueForKey(json, CursorColorKey, _CursorColor); // Required fields - for (unsigned int i = 0; i < TableColors.size(); ++i) + size_t colorCount = 0; + for (const auto& [key, index] : TableColorsMapping) { - std::string_view tableColorName = til::at(TableColors, i); - bool valueForKeyIsValid = JsonUtils::GetValueForKey(json, tableColorName, til::at(_table, i)); - - // If GetValueForKey failed, try again with alternate color names - if (!valueForKeyIsValid) + colorCount += JsonUtils::GetValueForKey(json, key, til::at(_table, index)); + if (colorCount == ColorSchemeExpectedSize) { - if (tableColorName == "purple") - { - tableColorName = "magenta"; - } - else if (tableColorName == "brightPurple") - { - tableColorName = "brightMagenta"; - } - - valueForKeyIsValid = JsonUtils::GetValueForKey(json, tableColorName, til::at(_table, i)); + break; } - - isValid &= valueForKeyIsValid; } + isValid &= (colorCount == 16); // Valid schemes should have exactly 16 colors + return isValid; } @@ -140,9 +135,10 @@ Json::Value ColorScheme::ToJson() const JsonUtils::SetValueForKey(json, SelectionBackgroundKey, _SelectionBackground); JsonUtils::SetValueForKey(json, CursorColorKey, _CursorColor); - for (unsigned int i = 0; i < TableColors.size(); ++i) + for (size_t i = 0; i < ColorSchemeExpectedSize; ++i) { - JsonUtils::SetValueForKey(json, til::at(TableColors, i), til::at(_table, i)); + const auto& key = til::at(TableColorsMapping, i).first; + JsonUtils::SetValueForKey(json, key, til::at(_table, i)); } return json;