Skip to content

Commit

Permalink
Accept color name "magenta" instead of "purple" (#13261)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
matthewd673 authored Jun 10, 2022
1 parent cac52a9 commit 1b630ab
Showing 1 changed file with 36 additions and 22 deletions.
58 changes: 36 additions & 22 deletions src/cascadia/TerminalSettingsModel/ColorScheme.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string_view, 16> 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<std::pair<std::string_view, size_t>, 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{} }
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
Expand Down

0 comments on commit 1b630ab

Please sign in to comment.