Add cursorColor to color scheme (#4651)

Add the option to set the cursor color as part of the color scheme. This is very useful for light themes, where the cursor disappears unless its color is set in the profile. Related to issue #764, but doesn't fully resolve it. ## Validation I tested this manually by creating a light color scheme, setting the cursor color to black and setting the profile color scheme to the newly created color scheme. I validated the cursor is black, then set the cursor color in the profile (to red) and saw it trumps the cursor color from the color scheme.
microsoft · Mar 17, 2020 · c0d704e · c0d704e
1 parent 7621994
commit c0d704e
Show file tree

Hide file tree

Showing 10 changed files with 127 additions and 21 deletions.
diff --git a/doc/cascadia/SettingsSchema.md b/doc/cascadia/SettingsSchema.md
@@ -39,7 +39,7 @@ Properties listed below are specific to each unique profile.
 | `colorScheme` | Optional | String | `Campbell` | Name of the terminal color scheme to use. Color schemes are defined under `schemes`. |
 | `colorTable` | Optional | Array[String] | | Array of colors used in the profile if `colorscheme` is not set. Array follows the format defined in `schemes`. |
 | `commandline` | Optional | String | | Executable used in the profile. |
-| `cursorColor` | Optional | String | `#FFFFFF` | Sets the cursor color for the profile. Uses hex color format: `"#rrggbb"`. |
+| `cursorColor` | Optional | String | | Sets the cursor color of the profile. Overrides `cursorColor` set in color scheme if `colorscheme` is set. Uses hex color format: `"#rrggbb"`. |
 | `cursorHeight` | Optional | Integer | | Sets the percentage height of the cursor starting from the bottom. Only works when `cursorShape` is set to `"vintage"`. Accepts values from 25-100. |
 | `cursorShape` | Optional | String | `bar` | Sets the cursor shape for the profile. Possible values: `"vintage"` ( &#x2583; ), `"bar"` ( &#x2503; ), `"underscore"` ( &#x2581; ), `"filledBox"` ( &#x2588; ), `"emptyBox"` ( &#x25AF; ) |
 | `fontFace` | Optional | String | `Consolas` | Name of the font face used in the profile. We will try to fallback to Consolas if this can't be found or is invalid. |
@@ -68,6 +68,7 @@ Properties listed below are specific to each color scheme. [ColorTool](https://g
 | `foreground` | _Required_ | String | Sets the foreground color of the color scheme. |
 | `background` | _Required_ | String | Sets the background color of the color scheme. |
 | `selectionBackground` | Optional | String | Sets the selection background color of the color scheme. |
+| `cursorColor` | Optional | String | Sets the cursor color of the color scheme. |
 | `black` | _Required_ | String | Sets the color used as ANSI black. |
 | `blue` | _Required_ | String | Sets the color used as ANSI blue. |
 | `brightBlack` | _Required_ | String | Sets the color used as ANSI bright black. |

diff --git a/doc/cascadia/profiles.schema.json b/doc/cascadia/profiles.schema.json
@@ -554,8 +554,7 @@
         },
         "cursorColor": {
           "$ref": "#/definitions/Color",
-          "default": "#FFFFFF",
-          "description": "Sets the cursor color for the profile. Uses hex color format: \"#rrggbb\"."
+          "description": "Sets the cursor color of the profile. Overrides cursor color set in color scheme if colorscheme is set. Uses hex color format: \"#rrggbb\"."
         },
         "cursorHeight": {
           "description": "Sets the percentage height of the cursor starting from the bottom. Only works when cursorShape is set to \"vintage\". Accepts values from 25-100.",
@@ -746,6 +745,11 @@
             "$ref": "#/definitions/Color",
             "description": "Sets the color used as ANSI bright yellow."
           },
+          "cursorColor": {
+            "$ref": "#/definitions/Color",
+            "default": "#FFFFFF",
+            "description": "Sets the cursor color of the color scheme."
+          },
           "cyan": {
             "$ref": "#/definitions/Color",
             "description": "Sets the color used as ANSI cyan."

diff --git a/src/cascadia/LocalTests_TerminalApp/ColorSchemeTests.cpp b/src/cascadia/LocalTests_TerminalApp/ColorSchemeTests.cpp
@@ -100,6 +100,7 @@ namespace TerminalAppLocalTests
             "foreground": "#000000",
             "background": "#010101",
             "selectionBackground": "#010100",
+            "cursorColor": "#010001",
             "red": "#010000",
             "green": "#000100",
             "blue": "#000001"
@@ -109,6 +110,7 @@ namespace TerminalAppLocalTests
             "foreground": "#020202",
             "background": "#030303",
             "selectionBackground": "#020200",
+            "cursorColor": "#040004",
             "red": "#020000",
 
             "blue": "#000002"
@@ -118,6 +120,7 @@ namespace TerminalAppLocalTests
             "foreground": "#040404",
             "background": "#050505",
             "selectionBackground": "#030300",
+            "cursorColor": "#060006",
             "red": "#030000",
             "green": "#000300"
         })" };
@@ -130,8 +133,8 @@ namespace TerminalAppLocalTests
         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, 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]);
@@ -143,6 +146,7 @@ namespace TerminalAppLocalTests
         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]);
@@ -154,6 +158,7 @@ namespace TerminalAppLocalTests
         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]);

diff --git a/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp b/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp
@@ -70,6 +70,8 @@ namespace TerminalAppLocalTests
 
         TEST_METHOD(TestTerminalArgsForBinding);
 
+        TEST_METHOD(TestLayerProfileOnColorScheme);
+
         TEST_METHOD(ValidateKeybindingsWarnings);
 
         TEST_CLASS_SETUP(ClassSetup)
@@ -2092,6 +2094,75 @@ namespace TerminalAppLocalTests
         }
     }
 
+    void SettingsTests::TestLayerProfileOnColorScheme()
+    {
+        Log::Comment(NoThrowString().Format(
+            L"Ensure that setting (or not) a property in the profile that should override a property of the color scheme works correctly."));
+
+        const std::string settings0String{ R"(
+        {
+            "profiles": [
+                {
+                    "name" : "profile0",
+                    "colorScheme": "schemeWithCursorColor"
+                },
+                {
+                    "name" : "profile1",
+                    "colorScheme": "schemeWithoutCursorColor"
+                },
+                {
+                    "name" : "profile2",
+                    "colorScheme": "schemeWithCursorColor",
+                    "cursorColor": "#234567"
+                },
+                {
+                    "name" : "profile3",
+                    "colorScheme": "schemeWithoutCursorColor",
+                    "cursorColor": "#345678"
+                },
+                {
+                    "name" : "profile4",
+                    "cursorColor": "#456789"
+                },
+                {
+                    "name" : "profile5"
+                }
+            ],
+            "schemes": [
+                {
+                    "name": "schemeWithCursorColor",
+                    "cursorColor": "#123456"
+                },
+                {
+                    "name": "schemeWithoutCursorColor"
+                }
+            ]
+        })" };
+
+        VerifyParseSucceeded(settings0String);
+
+        CascadiaSettings settings;
+        settings._ParseJsonString(settings0String, false);
+        settings.LayerJson(settings._userSettings);
+
+        VERIFY_ARE_EQUAL(6u, settings._profiles.size());
+        VERIFY_ARE_EQUAL(2u, settings._globals._colorSchemes.size());
+
+        auto terminalSettings0 = settings._profiles[0].CreateTerminalSettings(settings._globals._colorSchemes);
+        auto terminalSettings1 = settings._profiles[1].CreateTerminalSettings(settings._globals._colorSchemes);
+        auto terminalSettings2 = settings._profiles[2].CreateTerminalSettings(settings._globals._colorSchemes);
+        auto terminalSettings3 = settings._profiles[3].CreateTerminalSettings(settings._globals._colorSchemes);
+        auto terminalSettings4 = settings._profiles[4].CreateTerminalSettings(settings._globals._colorSchemes);
+        auto terminalSettings5 = settings._profiles[5].CreateTerminalSettings(settings._globals._colorSchemes);
+
+        VERIFY_ARE_EQUAL(ARGB(0, 0x12, 0x34, 0x56), terminalSettings0.CursorColor()); // from color scheme
+        VERIFY_ARE_EQUAL(DEFAULT_CURSOR_COLOR, terminalSettings1.CursorColor()); // default
+        VERIFY_ARE_EQUAL(ARGB(0, 0x23, 0x45, 0x67), terminalSettings2.CursorColor()); // from profile (trumps color scheme)
+        VERIFY_ARE_EQUAL(ARGB(0, 0x34, 0x56, 0x78), terminalSettings3.CursorColor()); // from profile (not set in color scheme)
+        VERIFY_ARE_EQUAL(ARGB(0, 0x45, 0x67, 0x89), terminalSettings4.CursorColor()); // from profile (no color scheme)
+        VERIFY_ARE_EQUAL(DEFAULT_CURSOR_COLOR, terminalSettings5.CursorColor()); // default
+    }
+
     void SettingsTests::ValidateKeybindingsWarnings()
     {
         const std::string badSettings{ R"(
@@ -2134,5 +2205,4 @@ namespace TerminalAppLocalTests
         VERIFY_ARE_EQUAL(::TerminalApp::SettingsLoadWarnings::MissingRequiredParameter, settings->_warnings.at(2));
         VERIFY_ARE_EQUAL(::TerminalApp::SettingsLoadWarnings::MissingRequiredParameter, settings->_warnings.at(3));
     }
-
 }
diff --git a/src/cascadia/TerminalApp/ColorScheme.cpp b/src/cascadia/TerminalApp/ColorScheme.cpp
@@ -18,6 +18,7 @@ static constexpr std::string_view TableKey{ "colors" };
 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::array<std::string_view, 16> TableColors = {
     "black",
     "red",
@@ -42,16 +43,18 @@ ColorScheme::ColorScheme() :
     _table{},
     _defaultForeground{ DEFAULT_FOREGROUND_WITH_ALPHA },
     _defaultBackground{ DEFAULT_BACKGROUND_WITH_ALPHA },
-    _selectionBackground{ DEFAULT_FOREGROUND }
+    _selectionBackground{ DEFAULT_FOREGROUND },
+    _cursorColor{ DEFAULT_CURSOR_COLOR }
 {
 }
 
-ColorScheme::ColorScheme(std::wstring name, COLORREF defaultFg, COLORREF defaultBg) :
+ColorScheme::ColorScheme(std::wstring name, COLORREF defaultFg, COLORREF defaultBg, COLORREF cursorColor) :
     _schemeName{ name },
     _table{},
     _defaultForeground{ defaultFg },
     _defaultBackground{ defaultBg },
-    _selectionBackground{ DEFAULT_FOREGROUND }
+    _selectionBackground{ DEFAULT_FOREGROUND },
+    _cursorColor{ cursorColor }
 {
 }
 
@@ -71,6 +74,7 @@ void ColorScheme::ApplyScheme(TerminalSettings terminalSettings) const
     terminalSettings.DefaultForeground(_defaultForeground);
     terminalSettings.DefaultBackground(_defaultBackground);
     terminalSettings.SelectionBackground(_selectionBackground);
+    terminalSettings.CursorColor(_cursorColor);
 
     auto const tableCount = gsl::narrow_cast<int>(_table.size());
     for (int i = 0; i < tableCount; i++)
@@ -92,6 +96,7 @@ Json::Value ColorScheme::ToJson() const
     root[JsonKey(ForegroundKey)] = Utils::ColorToHexString(_defaultForeground);
     root[JsonKey(BackgroundKey)] = Utils::ColorToHexString(_defaultBackground);
     root[JsonKey(SelectionBackgroundKey)] = Utils::ColorToHexString(_selectionBackground);
+    root[JsonKey(CursorColorKey)] = Utils::ColorToHexString(_cursorColor);
 
     int i = 0;
     for (const auto& colorName : TableColors)
@@ -166,6 +171,11 @@ void ColorScheme::LayerJson(const Json::Value& json)
         const auto color = Utils::ColorFromHexString(sbString.asString());
         _selectionBackground = color;
     }
+    if (auto sbString{ json[JsonKey(CursorColorKey)] })
+    {
+        const auto color = Utils::ColorFromHexString(sbString.asString());
+        _cursorColor = color;
+    }
 
     // Legacy Deserialization. Leave in place to allow forward compatibility
     if (auto table{ json[JsonKey(TableKey)] })
@@ -220,6 +230,11 @@ COLORREF ColorScheme::GetSelectionBackground() const noexcept
     return _selectionBackground;
 }
 
+COLORREF ColorScheme::GetCursorColor() const noexcept
+{
+    return _cursorColor;
+}
+
 // Method Description:
 // - Parse the name from the JSON representation of a ColorScheme.
 // Arguments:

diff --git a/src/cascadia/TerminalApp/ColorScheme.h b/src/cascadia/TerminalApp/ColorScheme.h
@@ -35,7 +35,7 @@ class TerminalApp::ColorScheme
 {
 public:
     ColorScheme();
-    ColorScheme(std::wstring name, COLORREF defaultFg, COLORREF defaultBg);
+    ColorScheme(std::wstring name, COLORREF defaultFg, COLORREF defaultBg, COLORREF cursorColor);
     ~ColorScheme();
 
     void ApplyScheme(winrt::Microsoft::Terminal::Settings::TerminalSettings terminalSettings) const;
@@ -50,6 +50,7 @@ class TerminalApp::ColorScheme
     COLORREF GetForeground() const noexcept;
     COLORREF GetBackground() const noexcept;
     COLORREF GetSelectionBackground() const noexcept;
+    COLORREF GetCursorColor() const noexcept;
 
     static std::optional<std::wstring> GetNameFromJson(const Json::Value& json);
 
@@ -59,6 +60,7 @@ class TerminalApp::ColorScheme
     COLORREF _defaultForeground;
     COLORREF _defaultBackground;
     COLORREF _selectionBackground;
+    COLORREF _cursorColor;
 
     friend class TerminalAppLocalTests::SettingsTests;
     friend class TerminalAppLocalTests::ColorSchemeTests;

diff --git a/src/cascadia/TerminalApp/Profile.cpp b/src/cascadia/TerminalApp/Profile.cpp
@@ -104,12 +104,12 @@ Profile::Profile(const std::optional<GUID>& guid) :
     _defaultForeground{},
     _defaultBackground{},
     _selectionBackground{},
+    _cursorColor{},
     _colorTable{},
     _tabTitle{},
     _suppressApplicationTitle{},
     _historySize{ DEFAULT_HISTORY_SIZE },
     _snapOnInput{ true },
-    _cursorColor{ DEFAULT_CURSOR_COLOR },
     _cursorShape{ CursorStyle::Bar },
     _cursorHeight{ DEFAULT_CURSOR_HEIGHT },
 
@@ -178,7 +178,6 @@ TerminalSettings Profile::CreateTerminalSettings(const std::unordered_map<std::w
     }
     terminalSettings.HistorySize(_historySize);
     terminalSettings.SnapOnInput(_snapOnInput);
-    terminalSettings.CursorColor(_cursorColor);
     terminalSettings.CursorHeight(_cursorHeight);
     terminalSettings.CursorShape(_cursorShape);
 
@@ -228,6 +227,10 @@ TerminalSettings Profile::CreateTerminalSettings(const std::unordered_map<std::w
     {
         terminalSettings.SelectionBackground(_selectionBackground.value());
     }
+    if (_cursorColor)
+    {
+        terminalSettings.CursorColor(_cursorColor.value());
+    }
 
     if (_scrollbarState)
     {
@@ -296,6 +299,10 @@ Json::Value Profile::ToJson() const
     {
         root[JsonKey(SelectionBackgroundKey)] = Utils::ColorToHexString(_selectionBackground.value());
     }
+    if (_cursorColor)
+    {
+        root[JsonKey(CursorColorKey)] = Utils::ColorToHexString(_cursorColor.value());
+    }
     if (_schemeName)
     {
         const auto scheme = winrt::to_string(_schemeName.value());
@@ -312,7 +319,6 @@ Json::Value Profile::ToJson() const
     }
     root[JsonKey(HistorySizeKey)] = _historySize;
     root[JsonKey(SnapOnInputKey)] = _snapOnInput;
-    root[JsonKey(CursorColorKey)] = Utils::ColorToHexString(_cursorColor);
     // Only add the cursor height property if we're a legacy-style cursor.
     if (_cursorShape == CursorStyle::Vintage)
     {
@@ -643,6 +649,8 @@ void Profile::LayerJson(const Json::Value& json)
 
     JsonUtils::GetOptionalColor(json, SelectionBackgroundKey, _selectionBackground);
 
+    JsonUtils::GetOptionalColor(json, CursorColorKey, _cursorColor);
+
     JsonUtils::GetOptionalString(json, ColorSchemeKey, _schemeName);
     // TODO:GH#1069 deprecate old settings key
     JsonUtils::GetOptionalString(json, ColorSchemeKeyOld, _schemeName);
@@ -675,12 +683,6 @@ void Profile::LayerJson(const Json::Value& json)
         auto snapOnInput{ json[JsonKey(SnapOnInputKey)] };
         _snapOnInput = snapOnInput.asBool();
     }
-    if (json.isMember(JsonKey(CursorColorKey)))
-    {
-        auto cursorColor{ json[JsonKey(CursorColorKey)] };
-        const auto color = Utils::ColorFromHexString(cursorColor.asString());
-        _cursorColor = color;
-    }
     if (json.isMember(JsonKey(CursorHeightKey)))
     {
         auto cursorHeight{ json[JsonKey(CursorHeightKey)] };

diff --git a/src/cascadia/TerminalApp/Profile.h b/src/cascadia/TerminalApp/Profile.h
@@ -142,12 +142,12 @@ class TerminalApp::Profile final
     std::optional<uint32_t> _defaultForeground;
     std::optional<uint32_t> _defaultBackground;
     std::optional<uint32_t> _selectionBackground;
+    std::optional<uint32_t> _cursorColor;
     std::array<uint32_t, COLOR_TABLE_SIZE> _colorTable;
     std::optional<std::wstring> _tabTitle;
     bool _suppressApplicationTitle;
     int32_t _historySize;
     bool _snapOnInput;
-    uint32_t _cursorColor;
     uint32_t _cursorHeight;
     winrt::Microsoft::Terminal::Settings::CursorStyle _cursorShape;