From 1b630ab6f69e2cb1cc21da450aa5268af8ddf743 Mon Sep 17 00:00:00 2001 From: Matthew Daly Date: Fri, 10 Jun 2022 14:37:39 -0400 Subject: [PATCH] Accept color name "magenta" instead of "purple" (#13261) When loading color schemes from Json, check if GetValueForKey failed. If it did, and we were searching for the colors "purple"/"brightPurple", swap out the color name with "magenta"/"brightMagenta" and try again. This was tested manually by creating a new color scheme using the colors "magenta"/"brightMagenta" and loading it, then printing colored text in the terminal to assure that the colors were correctly assigned as purple. This changes the color loader to use an index pair table to add support for alternate color names. ## Validation Steps Performed - Manually edit settings.json, creating a new color scheme with colors "magenta" and "brightMagenta" - View the color scheme in settings - the colors will appear as "purple" and "brightPurple" in the UI - Run a program that prints colored text, purple and brightPurple text will appear in the colors defined as magenta and brightMagenta - Modify the color scheme in the settings UI and save it, the colors will be saved as "purple" and "brightPurple" *(I think this is the right behavior, since calling them "magenta" is technically a mistake)* - Repeat the steps above with a normal color scheme - colors named "purple" and "brightPurple" still load properly Closes #11456 --- .../TerminalSettingsModel/ColorScheme.cpp | 58 ++++++++++++------- 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/src/cascadia/TerminalSettingsModel/ColorScheme.cpp b/src/cascadia/TerminalSettingsModel/ColorScheme.cpp index 0d55c405b79..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,11 +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) { - isValid &= JsonUtils::GetValueForKey(json, til::at(TableColors, i), til::at(_table, i)); + colorCount += JsonUtils::GetValueForKey(json, key, til::at(_table, index)); + if (colorCount == ColorSchemeExpectedSize) + { + break; + } } + isValid &= (colorCount == 16); // Valid schemes should have exactly 16 colors + return isValid; } @@ -122,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;