From be3a5a0ce782f70459f0e9d13bc79c204b00c280 Mon Sep 17 00:00:00 2001
From: Carlos Zamora <cazamor@microsoft.com>
Date: Thu, 10 Sep 2020 11:49:42 -0700
Subject: [PATCH 01/11] Add SetValueForKey to JsonUtils

---
 src/cascadia/TerminalApp/JsonUtils.h          | 117 ++++++++++++++++++
 .../TerminalSettingsSerializationHelpers.h    |  15 +++
 src/cascadia/ut_app/JsonUtilsTests.cpp        |  80 +++++++++++-
 3 files changed, 209 insertions(+), 3 deletions(-)

diff --git a/src/cascadia/TerminalApp/JsonUtils.h b/src/cascadia/TerminalApp/JsonUtils.h
index 5e639ecc73a..b4806c16f23 100644
--- a/src/cascadia/TerminalApp/JsonUtils.h
+++ b/src/cascadia/TerminalApp/JsonUtils.h
@@ -103,6 +103,8 @@ namespace TerminalApp::JsonUtils
         T FromJson(const Json::Value&);
         bool CanConvert(const Json::Value& json);
 
+        Json::Value ToJson(const T& val);
+
         std::string TypeDescription() const { return "<unknown>"; }
     };
 
@@ -119,6 +121,11 @@ namespace TerminalApp::JsonUtils
             return json.isString();
         }
 
+        Json::Value ToJson(const std::string& val)
+        {
+            return val;
+        }
+
         std::string TypeDescription() const
         {
             return "string";
@@ -138,6 +145,11 @@ namespace TerminalApp::JsonUtils
             return json.isString();
         }
 
+        Json::Value ToJson(const std::wstring& val)
+        {
+            return til::u16u8(val);
+        }
+
         std::string TypeDescription() const
         {
             return "string";
@@ -153,6 +165,11 @@ namespace TerminalApp::JsonUtils
         {
             return winrt::hstring{ til::u8u16(Detail::GetStringView(json)) };
         }
+
+        Json::Value ToJson(const winrt::hstring& val)
+        {
+            return til::u16u8(val);
+        }
     };
 #endif
 
@@ -169,6 +186,11 @@ namespace TerminalApp::JsonUtils
             return json.isBool();
         }
 
+        Json::Value ToJson(const bool val)
+        {
+            return val;
+        }
+
         std::string TypeDescription() const
         {
             return "true | false";
@@ -188,6 +210,11 @@ namespace TerminalApp::JsonUtils
             return json.isInt();
         }
 
+        Json::Value ToJson(const int& val)
+        {
+            return val;
+        }
+
         std::string TypeDescription() const
         {
             return "number";
@@ -207,6 +234,11 @@ namespace TerminalApp::JsonUtils
             return json.isUInt();
         }
 
+        Json::Value ToJson(const unsigned int& val)
+        {
+            return val;
+        }
+
         std::string TypeDescription() const
         {
             return "number (>= 0)";
@@ -226,6 +258,11 @@ namespace TerminalApp::JsonUtils
             return json.isNumeric();
         }
 
+        Json::Value ToJson(const float& val)
+        {
+            return val;
+        }
+
         std::string TypeDescription() const
         {
             return "number";
@@ -245,6 +282,11 @@ namespace TerminalApp::JsonUtils
             return json.isNumeric();
         }
 
+        Json::Value ToJson(const double& val)
+        {
+            return val;
+        }
+
         std::string TypeDescription() const
         {
             return "number";
@@ -270,6 +312,11 @@ namespace TerminalApp::JsonUtils
             return string.length() == 38 && string.front() == '{' && string.back() == '}';
         }
 
+        Json::Value ToJson(const GUID& val)
+        {
+            return til::u16u8(::Microsoft::Console::Utils::GuidToString(val));
+        }
+
         std::string TypeDescription() const
         {
             return "guid";
@@ -291,6 +338,11 @@ namespace TerminalApp::JsonUtils
             return ConversionTrait<GUID>{}.CanConvert(json);
         }
 
+        Json::Value ToJson(const winrt::guid& val)
+        {
+            return ConversionTrait<GUID>{}.ToJson(val);
+        }
+
         std::string TypeDescription() const
         {
             return ConversionTrait<GUID>{}.TypeDescription();
@@ -316,6 +368,11 @@ namespace TerminalApp::JsonUtils
             return (string.length() == 7 || string.length() == 4) && string.front() == '#';
         }
 
+        Json::Value ToJson(const til::color& val)
+        {
+            return til::u16u8(val.ToHexString(true));
+        }
+
         std::string TypeDescription() const
         {
             return "color (#rrggbb, #rgb)";
@@ -336,6 +393,11 @@ namespace TerminalApp::JsonUtils
             return ConversionTrait<til::color>{}.CanConvert(json);
         }
 
+        Json::Value ToJson(const winrt::Windows::UI::Color& val)
+        {
+            return ConversionTrait<til::color>{}.ToJson(val);
+        }
+
         std::string TypeDescription() const
         {
             return ConversionTrait<til::color>{}.TypeDescription();
@@ -370,6 +432,18 @@ namespace TerminalApp::JsonUtils
             return json.isString();
         }
 
+        Json::Value ToJson(const T& val)
+        {
+            for (const auto& pair : TBase::mappings)
+            {
+                if (pair.second == val)
+                {
+                    return { pair.first.data() };
+                }
+            }
+            FAIL_FAST();
+        }
+
         std::string TypeDescription() const
         {
             std::vector<std::string_view> names;
@@ -427,6 +501,35 @@ namespace TerminalApp::JsonUtils
             return AllClear;
         }
 
+        Json::Value ToJson(const T& val)
+        {
+            if (val == AllClear)
+            {
+                return BaseEnumMapper::ToJson(AllClear);
+            }
+            else if (val == AllSet)
+            {
+                return BaseEnumMapper::ToJson(AllSet);
+            }
+            else if (WI_IsSingleFlagSet(val))
+            {
+                return BaseEnumMapper::ToJson(val);
+            }
+            else
+            {
+                Json::Value json{ Json::ValueType::arrayValue };
+                for (const auto& pair : TBase::mappings)
+                {
+                    if (pair.second != AllClear &&
+                        (val & pair.second) == pair.second)
+                    {
+                        json.append({ pair.first.data() });
+                    }
+                }
+                return json;
+            }
+        }
+
         bool CanConvert(const Json::Value& json)
         {
             return BaseEnumMapper::CanConvert(json) || json.isArray();
@@ -574,6 +677,20 @@ namespace TerminalApp::JsonUtils
         GetValueForKey(json, key1, val1);
         GetValuesForKeys(json, std::forward<Args>(args)...);
     }
+
+    // SetValueForKey, type-deduced, manual converter
+    template<typename T, typename Converter>
+    void SetValueForKey(Json::Value& json, std::string_view key, const T& target, Converter&& conv)
+    {
+        json[key.data()] = conv.ToJson(target);
+    }
+
+    // SetValueForKey, type-deduced, with automatic converter
+    template<typename T>
+    void SetValueForKey(Json::Value& json, std::string_view key, const T& target)
+    {
+        SetValueForKey(json, key, target, ConversionTrait<typename Detail::DeduceOptional<T>::Type>{});
+    }
 };
 
 #define JSON_ENUM_MAPPER(...)                                       \
diff --git a/src/cascadia/TerminalApp/TerminalSettingsSerializationHelpers.h b/src/cascadia/TerminalApp/TerminalSettingsSerializationHelpers.h
index 47e2c681025..3a3eb4f5ffb 100644
--- a/src/cascadia/TerminalApp/TerminalSettingsSerializationHelpers.h
+++ b/src/cascadia/TerminalApp/TerminalSettingsSerializationHelpers.h
@@ -268,6 +268,21 @@ struct ::TerminalApp::JsonUtils::ConversionTrait<::winrt::TerminalApp::LaunchPos
         return json.isString();
     }
 
+    Json::Value ToJson(const ::winrt::TerminalApp::LaunchPosition& val)
+    {
+        std::stringstream ss;
+        if (val.X)
+        {
+            ss << val.X.Value();
+        }
+        ss << ",";
+        if (val.Y)
+        {
+            ss << val.Y.Value();
+        }
+        return ss.str();
+    }
+
     std::string TypeDescription() const
     {
         return "x, y";
diff --git a/src/cascadia/ut_app/JsonUtilsTests.cpp b/src/cascadia/ut_app/JsonUtilsTests.cpp
index 44d2c02fe44..58bbfffa93f 100644
--- a/src/cascadia/ut_app/JsonUtilsTests.cpp
+++ b/src/cascadia/ut_app/JsonUtilsTests.cpp
@@ -295,9 +295,22 @@ namespace TerminalAppUnitTests
     template<typename TExpected, typename TJson>
     static void TryBasicType(TExpected&& expected, TJson&& json)
     {
+        // test FromJson
         Json::Value jsonObject{ json };
         const auto value{ GetValue<TExpected>(jsonObject) };
         VERIFY_ARE_EQUAL(expected, value, NoThrowString{}.Format(L"(type: %hs)", typeid(TExpected).name()));
+
+        // test ToJson
+        {
+            const std::string key{ "myKey" };
+
+            Json::Value expectedJson{};
+            expectedJson[key] = json;
+
+            jsonObject = {};
+            SetValueForKey<TExpected>(jsonObject, key, expected);
+            VERIFY_ARE_EQUAL(expectedJson, jsonObject);
+        }
     }
 
     void JsonUtilsTests::BasicTypeConversion()
@@ -305,7 +318,7 @@ namespace TerminalAppUnitTests
         // Battery of all basic types ;P
         TryBasicType(std::string{ "hello" }, "hello");
         TryBasicType(int{ -1024 }, -1024);
-        TryBasicType(unsigned int{ 1024 }, 1024);
+        TryBasicType(std::numeric_limits<unsigned int>::max(), std::numeric_limits<unsigned int>::max());
         TryBasicType(false, false);
         TryBasicType(1.0f, 1.0f);
 
@@ -318,9 +331,9 @@ namespace TerminalAppUnitTests
         // double -> float
         TryBasicType(1.0f, 1.0);
 
-        TryBasicType(til::color{ 0xab, 0xcd, 0xef }, "#abcdef");
+        TryBasicType(til::color{ 0xab, 0xcd, 0xef }, "#ABCDEF");
 
-        static const std::string testGuidString{ "{AA8147AA-E289-4508-BE83-FB68361EF2F3}" }; // can't use a string_view; jsoncpp hates it
+        static const std::string testGuidString{ "{aa8147aa-e289-4508-be83-fb68361ef2f3}" }; // can't use a string_view; jsoncpp hates it
         static const GUID testGuid{ 0xaa8147aa, 0xe289, 0x4508, { 0xbe, 0x83, 0xfb, 0x68, 0x36, 0x1e, 0xf2, 0xf3 } };
 
         TryBasicType(testGuid, testGuidString);
@@ -371,6 +384,19 @@ namespace TerminalAppUnitTests
         // Unknown value should produce something?
         Json::Value stringUnknown{ "unknown" };
         VERIFY_THROWS_SPECIFIC(GetValue<JsonTestEnum>(stringUnknown), DeserializationError, _ReturnTrueForException);
+
+        // SetValueForKey
+        {
+            const std::string key{ "myKey" };
+            const auto val{ JsonTestEnum::Third };
+
+            Json::Value expected{};
+            expected[key] = "third";
+
+            Json::Value json{};
+            SetValueForKey<JsonTestEnum>(json, key, val);
+            VERIFY_ARE_EQUAL(expected, json);
+        }
     }
 
     void JsonUtilsTests::FlagMapper()
@@ -385,16 +411,64 @@ namespace TerminalAppUnitTests
         Json::Value stringAll{ "all" };
         VERIFY_ARE_EQUAL(JsonTestFlags::All, GetValue<JsonTestFlags>(stringAll));
 
+        {
+            const std::string key{ "myKey" };
+            const auto val{ JsonTestFlags::Third };
+
+            Json::Value expected{};
+            expected[key] = "third";
+
+            Json::Value json{};
+            SetValueForKey<JsonTestFlags>(json, key, val);
+            VERIFY_ARE_EQUAL(expected, json);
+        }
+
+        {
+            const std::string key{ "myKey" };
+            const auto val{ JsonTestFlags::All };
+
+            Json::Value expected{};
+            expected[key] = "all";
+
+            Json::Value json{};
+            SetValueForKey<JsonTestFlags>(json, key, val);
+            VERIFY_ARE_EQUAL(expected, json);
+        }
+
         // Multiple flags
         Json::Value arrayFirstSecond{ Json::arrayValue };
         arrayFirstSecond.append({ "first" });
         arrayFirstSecond.append({ "second" });
         VERIFY_ARE_EQUAL(JsonTestFlags::First | JsonTestFlags::Second, GetValue<JsonTestFlags>(arrayFirstSecond));
 
+        {
+            const std::string key{ "myKey" };
+            const auto val{ JsonTestFlags::First | JsonTestFlags::Second };
+
+            Json::Value expected{};
+            expected[key] = arrayFirstSecond;
+
+            Json::Value json{};
+            SetValueForKey<JsonTestFlags>(json, key, val);
+            VERIFY_ARE_EQUAL(expected, json);
+        }
+
         // No flags
         Json::Value emptyArray{ Json::arrayValue };
         VERIFY_ARE_EQUAL(JsonTestFlags::None, GetValue<JsonTestFlags>(emptyArray));
 
+        {
+            const std::string key{ "myKey" };
+            const auto val{ JsonTestFlags::None };
+
+            Json::Value expected{};
+            expected[key] = "none";
+
+            Json::Value json{};
+            SetValueForKey<JsonTestFlags>(json, key, val);
+            VERIFY_ARE_EQUAL(expected, json);
+        }
+
         // Stacking Always + Any
         Json::Value arrayAllFirst{ Json::arrayValue };
         arrayAllFirst.append({ "all" });

From a5fccc62225cf34e5c34da569267d644a9fdc3e0 Mon Sep 17 00:00:00 2001
From: Carlos Zamora <cazamor@microsoft.com>
Date: Thu, 10 Sep 2020 16:32:34 -0700
Subject: [PATCH 02/11] Add ToJson to ColorScheme

---
 src/cascadia/TerminalApp/ColorScheme.cpp | 36 ++++++++++++++++++++++++
 src/cascadia/TerminalApp/ColorScheme.h   |  3 ++
 src/cascadia/ut_app/JsonTests.cpp        |  6 +++-
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/src/cascadia/TerminalApp/ColorScheme.cpp b/src/cascadia/TerminalApp/ColorScheme.cpp
index 206b7baad67..8b07d00ff9e 100644
--- a/src/cascadia/TerminalApp/ColorScheme.cpp
+++ b/src/cascadia/TerminalApp/ColorScheme.cpp
@@ -141,6 +141,42 @@ void ColorScheme::LayerJson(const Json::Value& json)
     }
 }
 
+// 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)
+{
+    Json::Value json{ Json::ValueType::objectValue };
+    const auto schemeImpl{ winrt::get_self<implementation::ColorScheme>(scheme) };
+    schemeImpl->UpdateJson(json);
+    return json;
+}
+
+// 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)
+{
+    JsonUtils::SetValueForKey(json, NameKey, _schemeName);
+    JsonUtils::SetValueForKey(json, ForegroundKey, _defaultForeground);
+    JsonUtils::SetValueForKey(json, BackgroundKey, _defaultBackground);
+    JsonUtils::SetValueForKey(json, SelectionBackgroundKey, _selectionBackground);
+    JsonUtils::SetValueForKey(json, CursorColorKey, _cursorColor);
+
+    int i = 0;
+    for (const auto& current : TableColors)
+    {
+        JsonUtils::SetValueForKey(json, current, _table.at(i));
+        i++;
+    }
+}
+
 winrt::hstring ColorScheme::Name() const noexcept
 {
     return _schemeName;
diff --git a/src/cascadia/TerminalApp/ColorScheme.h b/src/cascadia/TerminalApp/ColorScheme.h
index fe7c8186a69..34d86785730 100644
--- a/src/cascadia/TerminalApp/ColorScheme.h
+++ b/src/cascadia/TerminalApp/ColorScheme.h
@@ -42,6 +42,9 @@ namespace winrt::TerminalApp::implementation
         bool ShouldBeLayered(const Json::Value& json) const;
         void LayerJson(const Json::Value& json);
 
+        static Json::Value ToJson(const TerminalApp::ColorScheme& scheme);
+        void UpdateJson(Json::Value& json);
+
         hstring Name() const noexcept;
         com_array<Windows::UI::Color> Table() const noexcept;
         Windows::UI::Color Foreground() const noexcept;
diff --git a/src/cascadia/ut_app/JsonTests.cpp b/src/cascadia/ut_app/JsonTests.cpp
index d36f97123d6..6db8430ba5e 100644
--- a/src/cascadia/ut_app/JsonTests.cpp
+++ b/src/cascadia/ut_app/JsonTests.cpp
@@ -87,7 +87,7 @@ namespace TerminalAppUnitTests
                                           "\"purple\" : \"#881798\","
                                           "\"red\" : \"#C50F1F\","
                                           "\"selectionBackground\" : \"#131313\","
-                                          "\"white\" : \"#CCC\","
+                                          "\"white\" : \"#CCCCCC\","
                                           "\"yellow\" : \"#C19C00\""
                                           "}" };
 
@@ -110,6 +110,10 @@ namespace TerminalAppUnitTests
             const til::color actual{ scheme->Table().at(static_cast<uint32_t>(i)) };
             VERIFY_ARE_EQUAL(expected, actual);
         }
+
+        Log::Comment(L"Roundtrip Test for Color Scheme");
+        const auto outJson = implementation::ColorScheme::ToJson(*scheme);
+        VERIFY_ARE_EQUAL(schemeObject, outJson);
     }
 
     void JsonTests::ProfileGeneratesGuid()

From c69572bdeaa7b0a072cabc3ecef680343d612089 Mon Sep 17 00:00:00 2001
From: Carlos Zamora <cazamor@microsoft.com>
Date: Fri, 11 Sep 2020 11:20:57 -0700
Subject: [PATCH 03/11] polish ColorScheme

---
 src/cascadia/TerminalApp/ColorScheme.cpp | 129 ++++++++++-------------
 src/cascadia/TerminalApp/ColorScheme.h   |  64 ++++++++---
 src/cascadia/TerminalApp/ColorScheme.idl |  25 ++++-
 src/cascadia/inc/cppwinrt_utils.h        |  12 +++
 4 files changed, 137 insertions(+), 93 deletions(-)

diff --git a/src/cascadia/TerminalApp/ColorScheme.cpp b/src/cascadia/TerminalApp/ColorScheme.cpp
index 8b07d00ff9e..896350f621c 100644
--- a/src/cascadia/TerminalApp/ColorScheme.cpp
+++ b/src/cascadia/TerminalApp/ColorScheme.cpp
@@ -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,
 };
 
-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 }
 {
 }
 
@@ -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++)
@@ -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;
 }
@@ -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)
@@ -163,11 +165,11 @@ Json::Value ColorScheme::ToJson(const TerminalApp::ColorScheme& scheme)
 // <none>
 void ColorScheme::UpdateJson(Json::Value& json)
 {
-    JsonUtils::SetValueForKey(json, NameKey, _schemeName);
-    JsonUtils::SetValueForKey(json, ForegroundKey, _defaultForeground);
-    JsonUtils::SetValueForKey(json, BackgroundKey, _defaultBackground);
-    JsonUtils::SetValueForKey(json, SelectionBackgroundKey, _selectionBackground);
-    JsonUtils::SetValueForKey(json, CursorColorKey, _cursorColor);
+    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);
 
     int i = 0;
     for (const auto& current : TableColors)
@@ -177,11 +179,6 @@ void ColorScheme::UpdateJson(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 };
@@ -189,26 +186,6 @@ winrt::com_array<Color> ColorScheme::Table() const noexcept
     return result;
 }
 
-Color ColorScheme::Foreground() const noexcept
-{
-    return _defaultForeground;
-}
-
-Color ColorScheme::Background() const noexcept
-{
-    return _defaultBackground;
-}
-
-Color ColorScheme::SelectionBackground() const noexcept
-{
-    return _selectionBackground;
-}
-
-Color ColorScheme::CursorColor() 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
index 34d86785730..a650625b83f 100644
--- a/src/cascadia/TerminalApp/ColorScheme.h
+++ b/src/cascadia/TerminalApp/ColorScheme.h
@@ -29,12 +29,20 @@ namespace TerminalAppLocalTests
 
 namespace winrt::TerminalApp::implementation
 {
+// Use this macro to quick implement both the getter and setter for a color property.
+// This should only be used for color types where there's no logic in the
+// getter/setter beyond just accessing/updating the value.
+// This takes advantage of til::color
+#define GETSET_COLORTABLEPROPERTY(name)                                               \
+public:                                                                               \
+    winrt::Windows::UI::Color name() const noexcept { return _table[##name##Index]; } \
+    void name(const winrt::Windows::UI::Color& value) noexcept { _table[##name##Index] = value; }
+
     struct ColorScheme : ColorSchemeT<ColorScheme>
     {
     public:
-        ColorScheme();
+        ColorScheme() = default;
         ColorScheme(hstring name, Windows::UI::Color defaultFg, Windows::UI::Color defaultBg, Windows::UI::Color cursorColor);
-        ~ColorScheme();
 
         void ApplyScheme(const winrt::Microsoft::Terminal::TerminalControl::IControlSettings& terminalSettings) const;
 
@@ -45,22 +53,52 @@ namespace winrt::TerminalApp::implementation
         static Json::Value ToJson(const TerminalApp::ColorScheme& scheme);
         void UpdateJson(Json::Value& json);
 
-        hstring Name() const noexcept;
+        static std::optional<std::wstring> GetNameFromJson(const Json::Value& json);
+
         com_array<Windows::UI::Color> Table() const noexcept;
-        Windows::UI::Color Foreground() const noexcept;
-        Windows::UI::Color Background() const noexcept;
-        Windows::UI::Color SelectionBackground() const noexcept;
-        Windows::UI::Color CursorColor() const noexcept;
 
-        static std::optional<std::wstring> GetNameFromJson(const Json::Value& json);
+        GETSET_PROPERTY(winrt::hstring, Name, L"");
+        GETSET_COLORPROPERTY(Foreground, DEFAULT_FOREGROUND_WITH_ALPHA);
+        GETSET_COLORPROPERTY(Background, DEFAULT_BACKGROUND_WITH_ALPHA);
+        GETSET_COLORPROPERTY(SelectionBackground, DEFAULT_FOREGROUND);
+        GETSET_COLORPROPERTY(CursorColor, DEFAULT_CURSOR_COLOR);
+
+        GETSET_COLORTABLEPROPERTY(Black);
+        GETSET_COLORTABLEPROPERTY(Red);
+        GETSET_COLORTABLEPROPERTY(Green);
+        GETSET_COLORTABLEPROPERTY(Yellow);
+        GETSET_COLORTABLEPROPERTY(Blue);
+        GETSET_COLORTABLEPROPERTY(Purple);
+        GETSET_COLORTABLEPROPERTY(Cyan);
+        GETSET_COLORTABLEPROPERTY(White);
+        GETSET_COLORTABLEPROPERTY(BrightBlack);
+        GETSET_COLORTABLEPROPERTY(BrightRed);
+        GETSET_COLORTABLEPROPERTY(BrightGreen);
+        GETSET_COLORTABLEPROPERTY(BrightYellow);
+        GETSET_COLORTABLEPROPERTY(BrightBlue);
+        GETSET_COLORTABLEPROPERTY(BrightPurple);
+        GETSET_COLORTABLEPROPERTY(BrightCyan);
+        GETSET_COLORTABLEPROPERTY(BrightWhite);
 
     private:
-        hstring _schemeName;
         std::array<til::color, COLOR_TABLE_SIZE> _table;
-        til::color _defaultForeground;
-        til::color _defaultBackground;
-        til::color _selectionBackground;
-        til::color _cursorColor;
+
+        static constexpr unsigned int BlackIndex{ 0 };
+        static constexpr unsigned int RedIndex{ 1 };
+        static constexpr unsigned int GreenIndex{ 2 };
+        static constexpr unsigned int YellowIndex{ 3 };
+        static constexpr unsigned int BlueIndex{ 4 };
+        static constexpr unsigned int PurpleIndex{ 5 };
+        static constexpr unsigned int CyanIndex{ 6 };
+        static constexpr unsigned int WhiteIndex{ 7 };
+        static constexpr unsigned int BrightBlackIndex{ 8 };
+        static constexpr unsigned int BrightRedIndex{ 9 };
+        static constexpr unsigned int BrightGreenIndex{ 10 };
+        static constexpr unsigned int BrightYellowIndex{ 11 };
+        static constexpr unsigned int BrightBlueIndex{ 12 };
+        static constexpr unsigned int BrightPurpleIndex{ 13 };
+        static constexpr unsigned int BrightCyanIndex{ 14 };
+        static constexpr unsigned int BrightWhiteIndex{ 15 };
 
         friend class TerminalAppLocalTests::SettingsTests;
         friend class TerminalAppLocalTests::ColorSchemeTests;
diff --git a/src/cascadia/TerminalApp/ColorScheme.idl b/src/cascadia/TerminalApp/ColorScheme.idl
index ea0d2979aad..f922c1ffcca 100644
--- a/src/cascadia/TerminalApp/ColorScheme.idl
+++ b/src/cascadia/TerminalApp/ColorScheme.idl
@@ -11,10 +11,27 @@ namespace TerminalApp
 
         String Name { get; };
 
-        Windows.UI.Color Foreground { get; };
-        Windows.UI.Color Background { get; };
-        Windows.UI.Color SelectionBackground { get; };
-        Windows.UI.Color CursorColor { get; };
+        Windows.UI.Color Foreground;
+        Windows.UI.Color Background;
+        Windows.UI.Color SelectionBackground;
+        Windows.UI.Color CursorColor;
+
+        Windows.UI.Color Black;
+        Windows.UI.Color Red;
+        Windows.UI.Color Green;
+        Windows.UI.Color Yellow;
+        Windows.UI.Color Blue;
+        Windows.UI.Color Purple;
+        Windows.UI.Color Cyan;
+        Windows.UI.Color White;
+        Windows.UI.Color BrightBlack;
+        Windows.UI.Color BrightRed;
+        Windows.UI.Color BrightGreen;
+        Windows.UI.Color BrightYellow;
+        Windows.UI.Color BrightBlue;
+        Windows.UI.Color BrightPurple;
+        Windows.UI.Color BrightCyan;
+        Windows.UI.Color BrightWhite;
 
         Windows.UI.Color[] Table { get; };
     }
diff --git a/src/cascadia/inc/cppwinrt_utils.h b/src/cascadia/inc/cppwinrt_utils.h
index dcc53ef23ce..72f0526d3ca 100644
--- a/src/cascadia/inc/cppwinrt_utils.h
+++ b/src/cascadia/inc/cppwinrt_utils.h
@@ -107,6 +107,18 @@ public:                                                        \
 private:                                                       \
     type _##name{ __VA_ARGS__ };
 
+// Use this macro to quick implement both the getter and setter for a color property.
+// This should only be used for color types where there's no logic in the
+// getter/setter beyond just accessing/updating the value.
+// This takes advantage of til::color
+#define GETSET_COLORPROPERTY(name, ...)                                             \
+public:                                                                             \
+    winrt::Windows::UI::Color name() const noexcept { return _##name; }             \
+    void name(const winrt::Windows::UI::Color& value) noexcept { _##name = value; } \
+                                                                                    \
+private:                                                                            \
+    til::color _##name{ __VA_ARGS__ };
+
 // Use this macro to quickly implement both the getter and setter for an
 // observable property. This is similar to the GETSET_PROPERTY macro above,
 // except this will also raise a PropertyChanged event with the name of the

From 0de07b81163f75b5b37b68d7315074c278b03b15 Mon Sep 17 00:00:00 2001
From: Carlos Zamora <cazamor@microsoft.com>
Date: Fri, 11 Sep 2020 11:27:50 -0700
Subject: [PATCH 04/11] update spell bot

---
 .github/actions/spell-check/dictionary/apis.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.github/actions/spell-check/dictionary/apis.txt b/.github/actions/spell-check/dictionary/apis.txt
index 6e450ee12bb..708ab9f857a 100644
--- a/.github/actions/spell-check/dictionary/apis.txt
+++ b/.github/actions/spell-check/dictionary/apis.txt
@@ -3,6 +3,8 @@ ACCESSDENIED
 alignof
 bitfield
 bitfields
+COLORPROPERTY
+COLORTABLEPROPERTY
 CLASSNOTAVAILABLE
 environstrings
 EXPCMDFLAGS

From 024102226ce6069c6398d14212673206f3d62a61 Mon Sep 17 00:00:00 2001
From: Carlos Zamora <cazamor@microsoft.com>
Date: Fri, 11 Sep 2020 16:04:40 -0700
Subject: [PATCH 05/11] PR Comments - Dustin

---
 .../ColorSchemeTests.cpp                      | 58 +++++++++----------
 src/cascadia/TerminalApp/JsonUtils.h          |  4 +-
 src/cascadia/ut_app/JsonUtilsTests.cpp        | 10 ++--
 3 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/src/cascadia/LocalTests_TerminalApp/ColorSchemeTests.cpp b/src/cascadia/LocalTests_TerminalApp/ColorSchemeTests.cpp
index 10e070db90a..ec597da2333 100644
--- a/src/cascadia/LocalTests_TerminalApp/ColorSchemeTests.cpp
+++ b/src/cascadia/LocalTests_TerminalApp/ColorSchemeTests.cpp
@@ -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]);
@@ -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]);
@@ -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]);
@@ -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);
@@ -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);
 
@@ -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);
 
@@ -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);
         }
     }
 }
diff --git a/src/cascadia/TerminalApp/JsonUtils.h b/src/cascadia/TerminalApp/JsonUtils.h
index b4806c16f23..a33cd3f962f 100644
--- a/src/cascadia/TerminalApp/JsonUtils.h
+++ b/src/cascadia/TerminalApp/JsonUtils.h
@@ -523,7 +523,7 @@ namespace TerminalApp::JsonUtils
                     if (pair.second != AllClear &&
                         (val & pair.second) == pair.second)
                     {
-                        json.append({ pair.first.data() });
+                        json.append(BaseEnumMapper::ToJson(pair.second));
                     }
                 }
                 return json;
@@ -682,7 +682,7 @@ namespace TerminalApp::JsonUtils
     template<typename T, typename Converter>
     void SetValueForKey(Json::Value& json, std::string_view key, const T& target, Converter&& conv)
     {
-        json[key.data()] = conv.ToJson(target);
+        *json.demand(&*key.cbegin(), (&*key.cbegin()) + key.size()) = conv.ToJson(target);
     }
 
     // SetValueForKey, type-deduced, with automatic converter
diff --git a/src/cascadia/ut_app/JsonUtilsTests.cpp b/src/cascadia/ut_app/JsonUtilsTests.cpp
index 58bbfffa93f..153d767df13 100644
--- a/src/cascadia/ut_app/JsonUtilsTests.cpp
+++ b/src/cascadia/ut_app/JsonUtilsTests.cpp
@@ -394,7 +394,7 @@ namespace TerminalAppUnitTests
             expected[key] = "third";
 
             Json::Value json{};
-            SetValueForKey<JsonTestEnum>(json, key, val);
+            SetValueForKey(json, key, val);
             VERIFY_ARE_EQUAL(expected, json);
         }
     }
@@ -419,7 +419,7 @@ namespace TerminalAppUnitTests
             expected[key] = "third";
 
             Json::Value json{};
-            SetValueForKey<JsonTestFlags>(json, key, val);
+            SetValueForKey(json, key, val);
             VERIFY_ARE_EQUAL(expected, json);
         }
 
@@ -431,7 +431,7 @@ namespace TerminalAppUnitTests
             expected[key] = "all";
 
             Json::Value json{};
-            SetValueForKey<JsonTestFlags>(json, key, val);
+            SetValueForKey(json, key, val);
             VERIFY_ARE_EQUAL(expected, json);
         }
 
@@ -449,7 +449,7 @@ namespace TerminalAppUnitTests
             expected[key] = arrayFirstSecond;
 
             Json::Value json{};
-            SetValueForKey<JsonTestFlags>(json, key, val);
+            SetValueForKey(json, key, val);
             VERIFY_ARE_EQUAL(expected, json);
         }
 
@@ -465,7 +465,7 @@ namespace TerminalAppUnitTests
             expected[key] = "none";
 
             Json::Value json{};
-            SetValueForKey<JsonTestFlags>(json, key, val);
+            SetValueForKey(json, key, val);
             VERIFY_ARE_EQUAL(expected, json);
         }
 

From 57620b7921f79529e71780af2400c13edbdad684 Mon Sep 17 00:00:00 2001
From: Carlos Zamora <carlos.zamora@microsoft.com>
Date: Fri, 11 Sep 2020 16:34:54 -0700
Subject: [PATCH 06/11] Add comment for demand

Co-authored-by: Dustin L. Howett <duhowett@microsoft.com>
---
 src/cascadia/TerminalApp/JsonUtils.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/cascadia/TerminalApp/JsonUtils.h b/src/cascadia/TerminalApp/JsonUtils.h
index a33cd3f962f..11c1a40d1d3 100644
--- a/src/cascadia/TerminalApp/JsonUtils.h
+++ b/src/cascadia/TerminalApp/JsonUtils.h
@@ -682,6 +682,7 @@ namespace TerminalApp::JsonUtils
     template<typename T, typename Converter>
     void SetValueForKey(Json::Value& json, std::string_view key, const T& target, Converter&& conv)
     {
+        // demand guarantees that it will return a value or throw an exception
         *json.demand(&*key.cbegin(), (&*key.cbegin()) + key.size()) = conv.ToJson(target);
     }
 

From 9d4a4cada70d78d2653bca02579f86c60da6ebb7 Mon Sep 17 00:00:00 2001
From: Carlos Zamora <cazamor@microsoft.com>
Date: Mon, 14 Sep 2020 15:21:16 -0700
Subject: [PATCH 07/11] add SetColorTable()

---
 src/cascadia/TerminalApp/ColorScheme.cpp | 22 ++++++++++
 src/cascadia/TerminalApp/ColorScheme.h   | 54 +++---------------------
 src/cascadia/TerminalApp/ColorScheme.idl | 21 +--------
 3 files changed, 30 insertions(+), 67 deletions(-)

diff --git a/src/cascadia/TerminalApp/ColorScheme.cpp b/src/cascadia/TerminalApp/ColorScheme.cpp
index ec40341bfb5..9026ad24420 100644
--- a/src/cascadia/TerminalApp/ColorScheme.cpp
+++ b/src/cascadia/TerminalApp/ColorScheme.cpp
@@ -57,10 +57,19 @@ static constexpr std::array<std::string_view, 16> TableColors = {
     BrightWhiteKey,
 };
 
+ColorScheme::ColorScheme() :
+    _Foreground{ DEFAULT_FOREGROUND_WITH_ALPHA },
+    _Background{ DEFAULT_BACKGROUND_WITH_ALPHA },
+    _SelectionBackground{ DEFAULT_FOREGROUND },
+    _CursorColor{ DEFAULT_CURSOR_COLOR }
+{
+}
+
 ColorScheme::ColorScheme(winrt::hstring name, Color defaultFg, Color defaultBg, Color cursorColor) :
     _Name{ name },
     _Foreground{ defaultFg },
     _Background{ defaultBg },
+    _SelectionBackground{ DEFAULT_FOREGROUND },
     _CursorColor{ cursorColor }
 {
 }
@@ -165,6 +174,19 @@ winrt::com_array<Color> ColorScheme::Table() const noexcept
     return result;
 }
 
+// Method Description:
+// - Set a color in the color table
+// Arguments:
+// - index: the index of the desired color within the table
+// - value: the color value we are setting the color table color to
+// Return Value:
+// - none
+void ColorScheme::SetColorTable(uint8_t index, const winrt::Windows::UI::Color& value) noexcept
+{
+    THROW_HR_IF(E_INVALIDARG, index > COLOR_TABLE_SIZE - 1);
+    _table[index] = value;
+}
+
 // 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
index 20864a01013..39daacd5b99 100644
--- a/src/cascadia/TerminalApp/ColorScheme.h
+++ b/src/cascadia/TerminalApp/ColorScheme.h
@@ -29,19 +29,10 @@ namespace TerminalAppLocalTests
 
 namespace winrt::TerminalApp::implementation
 {
-// Use this macro to quick implement both the getter and setter for a color property.
-// This should only be used for color types where there's no logic in the
-// getter/setter beyond just accessing/updating the value.
-// This takes advantage of til::color
-#define GETSET_COLORTABLEPROPERTY(name)                                               \
-public:                                                                               \
-    winrt::Windows::UI::Color name() const noexcept { return _table[##name##Index]; } \
-    void name(const winrt::Windows::UI::Color& value) noexcept { _table[##name##Index] = value; }
-
     struct ColorScheme : ColorSchemeT<ColorScheme>
     {
     public:
-        ColorScheme() = default;
+        ColorScheme();
         ColorScheme(hstring name, Windows::UI::Color defaultFg, Windows::UI::Color defaultBg, Windows::UI::Color cursorColor);
 
         static com_ptr<ColorScheme> FromJson(const Json::Value& json);
@@ -54,50 +45,17 @@ public:
         static std::optional<std::wstring> GetNameFromJson(const Json::Value& json);
 
         com_array<Windows::UI::Color> Table() const noexcept;
+        void SetColorTable(uint8_t index, const winrt::Windows::UI::Color& value) noexcept;
 
         GETSET_PROPERTY(winrt::hstring, Name, L"");
-        GETSET_COLORPROPERTY(Foreground, DEFAULT_FOREGROUND_WITH_ALPHA);
-        GETSET_COLORPROPERTY(Background, DEFAULT_BACKGROUND_WITH_ALPHA);
-        GETSET_COLORPROPERTY(SelectionBackground, DEFAULT_FOREGROUND);
-        GETSET_COLORPROPERTY(CursorColor, DEFAULT_CURSOR_COLOR);
-
-        GETSET_COLORTABLEPROPERTY(Black);
-        GETSET_COLORTABLEPROPERTY(Red);
-        GETSET_COLORTABLEPROPERTY(Green);
-        GETSET_COLORTABLEPROPERTY(Yellow);
-        GETSET_COLORTABLEPROPERTY(Blue);
-        GETSET_COLORTABLEPROPERTY(Purple);
-        GETSET_COLORTABLEPROPERTY(Cyan);
-        GETSET_COLORTABLEPROPERTY(White);
-        GETSET_COLORTABLEPROPERTY(BrightBlack);
-        GETSET_COLORTABLEPROPERTY(BrightRed);
-        GETSET_COLORTABLEPROPERTY(BrightGreen);
-        GETSET_COLORTABLEPROPERTY(BrightYellow);
-        GETSET_COLORTABLEPROPERTY(BrightBlue);
-        GETSET_COLORTABLEPROPERTY(BrightPurple);
-        GETSET_COLORTABLEPROPERTY(BrightCyan);
-        GETSET_COLORTABLEPROPERTY(BrightWhite);
+        GETSET_COLORPROPERTY(Foreground); // defined in constructor
+        GETSET_COLORPROPERTY(Background); // defined in constructor
+        GETSET_COLORPROPERTY(SelectionBackground); // defined in constructor
+        GETSET_COLORPROPERTY(CursorColor); // defined in constructor
 
     private:
         std::array<til::color, COLOR_TABLE_SIZE> _table;
 
-        static constexpr unsigned int BlackIndex{ 0 };
-        static constexpr unsigned int RedIndex{ 1 };
-        static constexpr unsigned int GreenIndex{ 2 };
-        static constexpr unsigned int YellowIndex{ 3 };
-        static constexpr unsigned int BlueIndex{ 4 };
-        static constexpr unsigned int PurpleIndex{ 5 };
-        static constexpr unsigned int CyanIndex{ 6 };
-        static constexpr unsigned int WhiteIndex{ 7 };
-        static constexpr unsigned int BrightBlackIndex{ 8 };
-        static constexpr unsigned int BrightRedIndex{ 9 };
-        static constexpr unsigned int BrightGreenIndex{ 10 };
-        static constexpr unsigned int BrightYellowIndex{ 11 };
-        static constexpr unsigned int BrightBlueIndex{ 12 };
-        static constexpr unsigned int BrightPurpleIndex{ 13 };
-        static constexpr unsigned int BrightCyanIndex{ 14 };
-        static constexpr unsigned int BrightWhiteIndex{ 15 };
-
         friend class TerminalAppLocalTests::SettingsTests;
         friend class TerminalAppLocalTests::ColorSchemeTests;
     };
diff --git a/src/cascadia/TerminalApp/ColorScheme.idl b/src/cascadia/TerminalApp/ColorScheme.idl
index a0da352a3c8..c3ef208a105 100644
--- a/src/cascadia/TerminalApp/ColorScheme.idl
+++ b/src/cascadia/TerminalApp/ColorScheme.idl
@@ -5,32 +5,15 @@ namespace TerminalApp
 {
     [default_interface] runtimeclass ColorScheme {
         ColorScheme();
-        ColorScheme(String name, Windows.UI.Color defaultFg, Windows.UI.Color defaultBg, Windows.UI.Color cursorColor);
 
-        String Name { get; };
+        String Name;
 
         Windows.UI.Color Foreground;
         Windows.UI.Color Background;
         Windows.UI.Color SelectionBackground;
         Windows.UI.Color CursorColor;
 
-        Windows.UI.Color Black;
-        Windows.UI.Color Red;
-        Windows.UI.Color Green;
-        Windows.UI.Color Yellow;
-        Windows.UI.Color Blue;
-        Windows.UI.Color Purple;
-        Windows.UI.Color Cyan;
-        Windows.UI.Color White;
-        Windows.UI.Color BrightBlack;
-        Windows.UI.Color BrightRed;
-        Windows.UI.Color BrightGreen;
-        Windows.UI.Color BrightYellow;
-        Windows.UI.Color BrightBlue;
-        Windows.UI.Color BrightPurple;
-        Windows.UI.Color BrightCyan;
-        Windows.UI.Color BrightWhite;
-
         Windows.UI.Color[] Table { get; };
+        void SetColorTable(UInt8 index, Windows.UI.Color value);
     }
 }

From 01c165af87c21171e1fb37a566f8ed4c42090ca8 Mon Sep 17 00:00:00 2001
From: Carlos Zamora <cazamor@microsoft.com>
Date: Tue, 15 Sep 2020 09:15:39 -0700
Subject: [PATCH 08/11] rename to GetColorTableEntry()

---
 src/cascadia/TerminalApp/ColorScheme.cpp | 2 +-
 src/cascadia/TerminalApp/ColorScheme.h   | 2 +-
 src/cascadia/TerminalApp/ColorScheme.idl | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/cascadia/TerminalApp/ColorScheme.cpp b/src/cascadia/TerminalApp/ColorScheme.cpp
index 9026ad24420..43fac39ba1e 100644
--- a/src/cascadia/TerminalApp/ColorScheme.cpp
+++ b/src/cascadia/TerminalApp/ColorScheme.cpp
@@ -181,7 +181,7 @@ winrt::com_array<Color> ColorScheme::Table() const noexcept
 // - value: the color value we are setting the color table color to
 // Return Value:
 // - none
-void ColorScheme::SetColorTable(uint8_t index, const winrt::Windows::UI::Color& value) noexcept
+void ColorScheme::SetColorTableEntry(uint8_t index, const winrt::Windows::UI::Color& value) noexcept
 {
     THROW_HR_IF(E_INVALIDARG, index > COLOR_TABLE_SIZE - 1);
     _table[index] = value;
diff --git a/src/cascadia/TerminalApp/ColorScheme.h b/src/cascadia/TerminalApp/ColorScheme.h
index 39daacd5b99..88ffe5cc324 100644
--- a/src/cascadia/TerminalApp/ColorScheme.h
+++ b/src/cascadia/TerminalApp/ColorScheme.h
@@ -45,7 +45,7 @@ namespace winrt::TerminalApp::implementation
         static std::optional<std::wstring> GetNameFromJson(const Json::Value& json);
 
         com_array<Windows::UI::Color> Table() const noexcept;
-        void SetColorTable(uint8_t index, const winrt::Windows::UI::Color& value) noexcept;
+        void SetColorTableEntry(uint8_t index, const winrt::Windows::UI::Color& value) noexcept;
 
         GETSET_PROPERTY(winrt::hstring, Name, L"");
         GETSET_COLORPROPERTY(Foreground); // defined in constructor
diff --git a/src/cascadia/TerminalApp/ColorScheme.idl b/src/cascadia/TerminalApp/ColorScheme.idl
index c3ef208a105..f1603f8a309 100644
--- a/src/cascadia/TerminalApp/ColorScheme.idl
+++ b/src/cascadia/TerminalApp/ColorScheme.idl
@@ -14,6 +14,6 @@ namespace TerminalApp
         Windows.UI.Color CursorColor;
 
         Windows.UI.Color[] Table { get; };
-        void SetColorTable(UInt8 index, Windows.UI.Color value);
+        void SetColorTableEntry(UInt8 index, Windows.UI.Color value);
     }
 }

From f217c7a17feaafda0b4ba27a522eff51d2e28a39 Mon Sep 17 00:00:00 2001
From: Carlos Zamora <cazamor@microsoft.com>
Date: Wed, 16 Sep 2020 18:34:34 -0700
Subject: [PATCH 09/11] PR comments - Dustin

---
 .../actions/spell-check/dictionary/apis.txt   |  1 -
 src/cascadia/TerminalApp/ColorScheme.cpp      | 79 ++++++-------------
 src/cascadia/TerminalApp/ColorScheme.h        |  3 +-
 src/cascadia/ut_app/JsonTests.cpp             |  3 +-
 src/cascadia/ut_app/JsonUtilsTests.cpp        | 20 +++--
 5 files changed, 39 insertions(+), 67 deletions(-)

diff --git a/.github/actions/spell-check/dictionary/apis.txt b/.github/actions/spell-check/dictionary/apis.txt
index 708ab9f857a..2ce6c20c31f 100644
--- a/.github/actions/spell-check/dictionary/apis.txt
+++ b/.github/actions/spell-check/dictionary/apis.txt
@@ -4,7 +4,6 @@ alignof
 bitfield
 bitfields
 COLORPROPERTY
-COLORTABLEPROPERTY
 CLASSNOTAVAILABLE
 environstrings
 EXPCMDFLAGS
diff --git a/src/cascadia/TerminalApp/ColorScheme.cpp b/src/cascadia/TerminalApp/ColorScheme.cpp
index 43fac39ba1e..406b0a3da4e 100644
--- a/src/cascadia/TerminalApp/ColorScheme.cpp
+++ b/src/cascadia/TerminalApp/ColorScheme.cpp
@@ -21,40 +21,23 @@ 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 = {
-    BlackKey,
-    RedKey,
-    GreenKey,
-    YellowKey,
-    BlueKey,
-    PurpleKey,
-    CyanKey,
-    WhiteKey,
-    BrightBlackKey,
-    BrightRedKey,
-    BrightGreenKey,
-    BrightYellowKey,
-    BrightBlueKey,
-    BrightPurpleKey,
-    BrightCyanKey,
-    BrightWhiteKey,
+    "black",
+    "red",
+    "green",
+    "yellow",
+    "blue",
+    "purple",
+    "cyan",
+    "white",
+    "brightBlack",
+    "brightRed",
+    "brightGreen",
+    "brightYellow",
+    "brightBlue",
+    "brightPurple",
+    "brightCyan",
+    "brightWhite"
 };
 
 ColorScheme::ColorScheme() :
@@ -123,28 +106,12 @@ void ColorScheme::LayerJson(const Json::Value& json)
     JsonUtils::GetValueForKey(json, SelectionBackgroundKey, _SelectionBackground);
     JsonUtils::GetValueForKey(json, CursorColorKey, _CursorColor);
 
-    int i = 0;
-    for (const auto& current : TableColors)
+    for (unsigned int i=0; i<TableColors.size(); ++i)
     {
-        JsonUtils::GetValueForKey(json, current, _table.at(i));
-        i++;
+        JsonUtils::GetValueForKey(json, til::at(TableColors, i), _table.at(i));
     }
 }
 
-// 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)
-{
-    Json::Value json{ Json::ValueType::objectValue };
-    const auto schemeImpl{ winrt::get_self<implementation::ColorScheme>(scheme) };
-    schemeImpl->UpdateJson(json);
-    return json;
-}
-
 // Method Description:
 // - Update the given json object with values from this object.
 // Arguments:
@@ -159,17 +126,15 @@ void ColorScheme::UpdateJson(Json::Value& json)
     JsonUtils::SetValueForKey(json, SelectionBackgroundKey, _SelectionBackground);
     JsonUtils::SetValueForKey(json, CursorColorKey, _CursorColor);
 
-    int i = 0;
-    for (const auto& current : TableColors)
+    for (unsigned int i = 0; i < TableColors.size(); ++i)
     {
-        JsonUtils::SetValueForKey(json, current, _table.at(i));
-        i++;
+        JsonUtils::SetValueForKey(json, til::at(TableColors, i), _table.at(i));
     }
 }
 
 winrt::com_array<Color> ColorScheme::Table() const noexcept
 {
-    winrt::com_array<Color> result{ COLOR_TABLE_SIZE };
+    winrt::com_array<Color> result{ base::checked_cast<uint32_t>(_table.size()) };
     std::transform(_table.begin(), _table.end(), result.begin(), [](til::color c) -> Color { return c; });
     return result;
 }
@@ -183,7 +148,7 @@ winrt::com_array<Color> ColorScheme::Table() const noexcept
 // - none
 void ColorScheme::SetColorTableEntry(uint8_t index, const winrt::Windows::UI::Color& value) noexcept
 {
-    THROW_HR_IF(E_INVALIDARG, index > COLOR_TABLE_SIZE - 1);
+    THROW_HR_IF(E_INVALIDARG, index > _table.size() - 1);
     _table[index] = value;
 }
 
diff --git a/src/cascadia/TerminalApp/ColorScheme.h b/src/cascadia/TerminalApp/ColorScheme.h
index 88ffe5cc324..10f1624a377 100644
--- a/src/cascadia/TerminalApp/ColorScheme.h
+++ b/src/cascadia/TerminalApp/ColorScheme.h
@@ -39,7 +39,6 @@ namespace winrt::TerminalApp::implementation
         bool ShouldBeLayered(const Json::Value& json) const;
         void LayerJson(const Json::Value& json);
 
-        static Json::Value ToJson(const TerminalApp::ColorScheme& scheme);
         void UpdateJson(Json::Value& json);
 
         static std::optional<std::wstring> GetNameFromJson(const Json::Value& json);
@@ -47,7 +46,7 @@ namespace winrt::TerminalApp::implementation
         com_array<Windows::UI::Color> Table() const noexcept;
         void SetColorTableEntry(uint8_t index, const winrt::Windows::UI::Color& value) noexcept;
 
-        GETSET_PROPERTY(winrt::hstring, Name, L"");
+        GETSET_PROPERTY(winrt::hstring, Name);
         GETSET_COLORPROPERTY(Foreground); // defined in constructor
         GETSET_COLORPROPERTY(Background); // defined in constructor
         GETSET_COLORPROPERTY(SelectionBackground); // defined in constructor
diff --git a/src/cascadia/ut_app/JsonTests.cpp b/src/cascadia/ut_app/JsonTests.cpp
index 6db8430ba5e..662a22f30ce 100644
--- a/src/cascadia/ut_app/JsonTests.cpp
+++ b/src/cascadia/ut_app/JsonTests.cpp
@@ -112,7 +112,8 @@ namespace TerminalAppUnitTests
         }
 
         Log::Comment(L"Roundtrip Test for Color Scheme");
-        const auto outJson = implementation::ColorScheme::ToJson(*scheme);
+        Json::Value outJson{ Json::ValueType::objectValue };
+        scheme->UpdateJson(outJson);
         VERIFY_ARE_EQUAL(schemeObject, outJson);
     }
 
diff --git a/src/cascadia/ut_app/JsonUtilsTests.cpp b/src/cascadia/ut_app/JsonUtilsTests.cpp
index 153d767df13..ad08dcabf12 100644
--- a/src/cascadia/ut_app/JsonUtilsTests.cpp
+++ b/src/cascadia/ut_app/JsonUtilsTests.cpp
@@ -293,7 +293,7 @@ namespace TerminalAppUnitTests
     // versions.
 
     template<typename TExpected, typename TJson>
-    static void TryBasicType(TExpected&& expected, TJson&& json)
+    static void TryBasicType(TExpected&& expected, TJson&& json, std::optional<std::string> overrideToJsonOutput = std::nullopt)
     {
         // test FromJson
         Json::Value jsonObject{ json };
@@ -305,11 +305,18 @@ namespace TerminalAppUnitTests
             const std::string key{ "myKey" };
 
             Json::Value expectedJson{};
-            expectedJson[key] = json;
-
-            jsonObject = {};
-            SetValueForKey<TExpected>(jsonObject, key, expected);
-            VERIFY_ARE_EQUAL(expectedJson, jsonObject);
+            if (overrideToJsonOutput.has_value())
+            {
+                expectedJson[key] = *overrideToJsonOutput;
+            }
+            else
+            {
+                expectedJson[key] = jsonObject;
+            }
+
+            Json::Value toJsonResult{};
+            SetValueForKey(toJsonResult, key, expected);
+            VERIFY_ARE_EQUAL(expectedJson, toJsonResult);
         }
     }
 
@@ -332,6 +339,7 @@ namespace TerminalAppUnitTests
         TryBasicType(1.0f, 1.0);
 
         TryBasicType(til::color{ 0xab, 0xcd, 0xef }, "#ABCDEF");
+        TryBasicType(til::color{ 0xcc, 0xcc, 0xcc }, "#CCC", "#CCCCCC");
 
         static const std::string testGuidString{ "{aa8147aa-e289-4508-be83-fb68361ef2f3}" }; // can't use a string_view; jsoncpp hates it
         static const GUID testGuid{ 0xaa8147aa, 0xe289, 0x4508, { 0xbe, 0x83, 0xfb, 0x68, 0x36, 0x1e, 0xf2, 0xf3 } };

From 2dde34b51de35c39fce398234643271a2af62612 Mon Sep 17 00:00:00 2001
From: Carlos Zamora <cazamor@microsoft.com>
Date: Wed, 16 Sep 2020 18:36:57 -0700
Subject: [PATCH 10/11] I knew I missed a code format. whoops!

---
 src/cascadia/TerminalApp/ColorScheme.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/cascadia/TerminalApp/ColorScheme.cpp b/src/cascadia/TerminalApp/ColorScheme.cpp
index 406b0a3da4e..3dfbc7dda79 100644
--- a/src/cascadia/TerminalApp/ColorScheme.cpp
+++ b/src/cascadia/TerminalApp/ColorScheme.cpp
@@ -106,7 +106,7 @@ void ColorScheme::LayerJson(const Json::Value& json)
     JsonUtils::GetValueForKey(json, SelectionBackgroundKey, _SelectionBackground);
     JsonUtils::GetValueForKey(json, CursorColorKey, _CursorColor);
 
-    for (unsigned int i=0; i<TableColors.size(); ++i)
+    for (unsigned int i = 0; i < TableColors.size(); ++i)
     {
         JsonUtils::GetValueForKey(json, til::at(TableColors, i), _table.at(i));
     }

From aa6b1e62a256f664ffef6db99c7e1e13b8ad837c Mon Sep 17 00:00:00 2001
From: Carlos Zamora <cazamor@microsoft.com>
Date: Thu, 17 Sep 2020 09:28:32 -0700
Subject: [PATCH 11/11] rename to ToJson; use til::coalesce

---
 src/cascadia/TerminalApp/ColorScheme.cpp | 10 +++++++---
 src/cascadia/TerminalApp/ColorScheme.h   |  2 +-
 src/cascadia/ut_app/JsonTests.cpp        |  3 +--
 src/cascadia/ut_app/JsonUtilsTests.cpp   | 11 ++---------
 4 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/src/cascadia/TerminalApp/ColorScheme.cpp b/src/cascadia/TerminalApp/ColorScheme.cpp
index 3dfbc7dda79..4c4ee9cca42 100644
--- a/src/cascadia/TerminalApp/ColorScheme.cpp
+++ b/src/cascadia/TerminalApp/ColorScheme.cpp
@@ -113,13 +113,15 @@ void ColorScheme::LayerJson(const Json::Value& json)
 }
 
 // Method Description:
-// - Update the given json object with values from this object.
+// - Create a new serialized JsonObject from an instance of this class
 // Arguments:
-// - json: an object which will be a serialization of a ColorScheme object.
+// - <none>
 // Return Value:
 // <none>
-void ColorScheme::UpdateJson(Json::Value& json)
+Json::Value ColorScheme::ToJson()
 {
+    Json::Value json{ Json::ValueType::objectValue };
+
     JsonUtils::SetValueForKey(json, NameKey, _Name);
     JsonUtils::SetValueForKey(json, ForegroundKey, _Foreground);
     JsonUtils::SetValueForKey(json, BackgroundKey, _Background);
@@ -130,6 +132,8 @@ void ColorScheme::UpdateJson(Json::Value& json)
     {
         JsonUtils::SetValueForKey(json, til::at(TableColors, i), _table.at(i));
     }
+
+    return json;
 }
 
 winrt::com_array<Color> ColorScheme::Table() const noexcept
diff --git a/src/cascadia/TerminalApp/ColorScheme.h b/src/cascadia/TerminalApp/ColorScheme.h
index 10f1624a377..68456dc15b3 100644
--- a/src/cascadia/TerminalApp/ColorScheme.h
+++ b/src/cascadia/TerminalApp/ColorScheme.h
@@ -39,7 +39,7 @@ namespace winrt::TerminalApp::implementation
         bool ShouldBeLayered(const Json::Value& json) const;
         void LayerJson(const Json::Value& json);
 
-        void UpdateJson(Json::Value& json);
+        Json::Value ToJson();
 
         static std::optional<std::wstring> GetNameFromJson(const Json::Value& json);
 
diff --git a/src/cascadia/ut_app/JsonTests.cpp b/src/cascadia/ut_app/JsonTests.cpp
index 662a22f30ce..178ccd8cffb 100644
--- a/src/cascadia/ut_app/JsonTests.cpp
+++ b/src/cascadia/ut_app/JsonTests.cpp
@@ -112,8 +112,7 @@ namespace TerminalAppUnitTests
         }
 
         Log::Comment(L"Roundtrip Test for Color Scheme");
-        Json::Value outJson{ Json::ValueType::objectValue };
-        scheme->UpdateJson(outJson);
+        Json::Value outJson{ scheme->ToJson() };
         VERIFY_ARE_EQUAL(schemeObject, outJson);
     }
 
diff --git a/src/cascadia/ut_app/JsonUtilsTests.cpp b/src/cascadia/ut_app/JsonUtilsTests.cpp
index ad08dcabf12..b9bc020eca4 100644
--- a/src/cascadia/ut_app/JsonUtilsTests.cpp
+++ b/src/cascadia/ut_app/JsonUtilsTests.cpp
@@ -293,7 +293,7 @@ namespace TerminalAppUnitTests
     // versions.
 
     template<typename TExpected, typename TJson>
-    static void TryBasicType(TExpected&& expected, TJson&& json, std::optional<std::string> overrideToJsonOutput = std::nullopt)
+    static void TryBasicType(TExpected&& expected, TJson&& json, std::optional<Json::Value> overrideToJsonOutput = std::nullopt)
     {
         // test FromJson
         Json::Value jsonObject{ json };
@@ -305,14 +305,7 @@ namespace TerminalAppUnitTests
             const std::string key{ "myKey" };
 
             Json::Value expectedJson{};
-            if (overrideToJsonOutput.has_value())
-            {
-                expectedJson[key] = *overrideToJsonOutput;
-            }
-            else
-            {
-                expectedJson[key] = jsonObject;
-            }
+            expectedJson[key] = til::coalesce_value(overrideToJsonOutput, jsonObject);
 
             Json::Value toJsonResult{};
             SetValueForKey(toJsonResult, key, expected);