Repository: microsoft/terminal (language: cpp)

Issue:
Writing settings includes all of Theme's optional keys
Including `useMica`, which might confuse users when they find it and set it to true and it doesn't work.


Discussion / hints:

Fix patch:
diff --git a/src/cascadia/TerminalSettingsModel/Theme.cpp b/src/cascadia/TerminalSettingsModel/Theme.cpp
index 236cb5ab987..dfb108634ac 100644
--- a/src/cascadia/TerminalSettingsModel/Theme.cpp
+++ b/src/cascadia/TerminalSettingsModel/Theme.cpp
@@ -31,6 +31,35 @@ static constexpr std::string_view NameKey{ "name" };
 static constexpr wchar_t RegKeyDwm[] = L"Software\\Microsoft\\Windows\\DWM";
 static constexpr wchar_t RegKeyAccentColor[] = L"AccentColor";
 
+#define THEME_SETTINGS_COPY(type, name, jsonKey, ...) \
+    result->_##name = _##name;
+
+#define THEME_SETTINGS_TO_JSON(type, name, jsonKey, ...) \
+    JsonUtils::SetValueForKey(json, jsonKey, _##name);
+
+#define THEME_OBJECT(className, macro)                    \
+    winrt::com_ptr<className> className::Copy()           \
+    {                                                     \
+        auto result{ winrt::make_self<className>() };     \
+        macro(THEME_SETTINGS_COPY);                       \
+        return result;                                    \
+    }                                                     \
+                                                          \
+    Json::Value className::ToJson()                       \
+    {                                                     \
+        Json::Value json{ Json::ValueType::objectValue }; \
+        macro(THEME_SETTINGS_TO_JSON);                    \
+        return json;                                      \
+    }
+
+THEME_OBJECT(WindowTheme, MTSM_THEME_WINDOW_SETTINGS);
+THEME_OBJECT(TabRowTheme, MTSM_THEME_TABROW_SETTINGS);
+THEME_OBJECT(TabTheme, MTSM_THEME_TAB_SETTINGS);
+
+#undef THEME_SETTINGS_COPY
+#undef THEME_SETTINGS_TO_JSON
+#undef THEME_OBJECT
+
 winrt::Microsoft::Terminal::Settings::Model::ThemeColor ThemeColor::FromColor(const winrt::Microsoft::Terminal::Core::Color& coreColor) noexcept
 {
     auto result = winrt::make_self<implementation::ThemeColor>();
@@ -178,9 +207,6 @@ winrt::WUX::Media::Brush ThemeColor::Evaluate(const winrt::WUX::ResourceDictiona
             result->name(*_val);                                              \
     }
 
-#define THEME_SETTINGS_TO_JSON(type, name, jsonKey, ...) \
-    JsonUtils::SetValueForKey(json, jsonKey, val.name());
-
 #define THEME_OBJECT_CONVERTER(nameSpace, name, macro)                                         \
     template<>                                                                                 \
     struct ::Microsoft::Terminal::Settings::Model::JsonUtils::ConversionTrait<nameSpace::name> \
@@ -201,11 +227,7 @@ winrt::WUX::Media::Brush ThemeColor::Evaluate(const winrt::WUX::ResourceDictiona
                                                                                                \
         Json::Value ToJson(const nameSpace::name& val)                                         \
         {                                                                                      \
-            if (val == nullptr)                                                                \
-                return Json::Value::null;                                                      \
-            Json::Value json{ Json::ValueType::objectValue };                                  \
-            macro(THEME_SETTINGS_TO_JSON);                                                     \
-            return json;                                                                       \
+            return val ? winrt::get_self<name>(val)->ToJson() : Json::Value::null;             \
         }                                                                                      \
                                                                                                \
         std::string TypeDescription() const                                                    \
@@ -222,11 +244,6 @@ THEME_OBJECT_CONVERTER(winrt::Microsoft::Terminal::Settings::Model, TabTheme, MT
 #undef THEME_SETTINGS_TO_JSON
 #undef THEME_OBJECT_CONVERTER
 
-Theme::Theme() noexcept :
-    Theme{ winrt::WUX::ElementTheme::Default }
-{
-}
-
 Theme::Theme(const winrt::WUX::ElementTheme& requestedTheme) noexcept
 {
     auto window{ winrt::make_self<implementation::WindowTheme>() };
diff --git a/src/cascadia/TerminalSettingsModel/Theme.h b/src/cascadia/TerminalSettingsModel/Theme.h
index b27e42baa46..96a496b908c 100644
--- a/src/cascadia/TerminalSettingsModel/Theme.h
+++ b/src/cascadia/TerminalSettingsModel/Theme.h
@@ -47,24 +47,13 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
 #define THEME_SETTINGS_INITIALIZE(type, name, jsonKey, ...) \
     WINRT_PROPERTY(type, name, ##__VA_ARGS__)
 
-#define THEME_SETTINGS_COPY(type, name, jsonKey, ...) \
-    result->_##name = _##name;
-
-#define COPY_THEME_OBJECT(T, macro)           \
-    winrt::com_ptr<T> Copy()                  \
-    {                                         \
-        auto result{ winrt::make_self<T>() }; \
-        macro(THEME_SETTINGS_COPY);           \
-        return result;                        \
-    }
-
 #define THEME_OBJECT(className, macro)         \
     struct className : className##T<className> \
     {                                          \
-        macro(THEME_SETTINGS_INITIALIZE);      \
+        winrt::com_ptr<className> Copy();      \
+        Json::Value ToJson();                  \
                                                \
-    public:                                    \
-        COPY_THEME_OBJECT(className, macro);   \
+        macro(THEME_SETTINGS_INITIALIZE);      \
     };
 
     THEME_OBJECT(WindowTheme, MTSM_THEME_WINDOW_SETTINGS);
@@ -74,7 +63,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
     struct Theme : ThemeT<Theme>
     {
     public:
-        Theme() noexcept;
+        Theme() = default;
         Theme(const winrt::Windows::UI::Xaml::ElementTheme& requestedTheme) noexcept;
 
         com_ptr<Theme> Copy() const;
@@ -95,8 +84,6 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
     };
 
 #undef THEME_SETTINGS_INITIALIZE
-#undef THEME_SETTINGS_COPY
-#undef COPY_THEME_OBJECT
 #undef THEME_OBJECT
 }
 


Test patch: