From 0a9cbd09d8665861d73dcdf5b8d8f40b9599acff Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Fri, 23 Aug 2024 12:28:19 -0700 Subject: [PATCH] Track and log changes to settings (#17678) Adds functionality throughout the settings model to keep track of which settings have been set. There are two entry points: - AppLogic.cpp: this is where we perform a settings reload by loading the JSON - MainPage.cpp: this is where the Save button is clicked in the settings UI Both of these entry points call into `CascadiaSettings::LogSettingChanges()` where we aggregate the list of changes (specifically, _which_ settings changed, not _what_ their value is). Just about all of the settings model objects now have a `LogSettingChanges(std::set& changes, std::string_view context)` on them. - `changes` is where we aggregate all of the changes to. In it being a set, we don't need to worry about duplicates and can do things like iterate across all of the profiles. - `context` prepends a string to the setting. This'll allow us to better identify where a setting was changes (i.e. "global.X" are global settings). We also use this to distinguish between settings set in the ~base layer~ profile defaults vs individual profiles. The change log in each object is modified via two ways: - `LayerJson()` changes: this is useful for detecting JSON changes! All we're doing is checking if the setting has a value (due to inheritance, just about everything is an optional here!). If the value is set, we add the json key to the change log - `INHERITABLE_SETTING_WITH_LOGGING` in IInheritable.h: we already use this macro to define getters and setters. This new macro updates the setter to check if the value was set to something different. If so, log it! Other notes: - We're not distinguishing between `defaultAppearance` and `unfocusedAppearance` - We are distinguishing between `profileDefaults` and `profile` (any other profile) - New Tab Menu Customization: - we really just care about the entry types. Handled in `GlobalAppSettings` - Font: - We still have support for legacy values here. We still want to track them, but just use the modern keys. - `Theme`: - We don't do inheritance here, so we have to approach it differently. During the JSON load, we log each setting. However, we don't have `LayerJson`! So instead, do the work in `CascadiaSettings` and store the changes there. Note that we don't track any changes made via setters. This is fine for now since themes aren't even in the settings UI, so we wouldn't get much use out of it anyways. - Actions: - Actions are weird because we can have nested and iterable actions too, but `ActionsAndArgs` as a whole add a ton of functionality. I handled it over in `Command::LogSettingChanges` and we generally just serialize it to JSON to get the keys. It's a lot easier than dealing with the object model. Epic: #10000 Auto-Save (ish): #12424 --- .github/actions/spelling/allow/allow.txt | 2 + src/cascadia/TerminalApp/AppLogic.cpp | 4 + .../TerminalSettingsEditor/MainPage.cpp | 1 + .../TerminalSettingsModel/ActionMap.cpp | 1 + .../TerminalSettingsModel/ActionMap.h | 3 + .../ActionMapSerialization.cpp | 34 +++++- .../AppearanceConfig.cpp | 38 ++++++- .../TerminalSettingsModel/AppearanceConfig.h | 5 + .../TerminalSettingsModel/CascadiaSettings.h | 5 + .../CascadiaSettings.idl | 1 + .../CascadiaSettingsSerialization.cpp | 78 +++++++++++++ .../TerminalSettingsModel/Command.cpp | 53 +++++++++ src/cascadia/TerminalSettingsModel/Command.h | 1 + .../TerminalSettingsModel/FontConfig.cpp | 50 ++++++++- .../TerminalSettingsModel/FontConfig.h | 5 + .../GlobalAppSettings.cpp | 106 +++++++++++++++++- .../TerminalSettingsModel/GlobalAppSettings.h | 8 +- .../TerminalSettingsModel/IInheritable.h | 21 ++++ .../TerminalSettingsModel/Profile.cpp | 69 +++++++++++- src/cascadia/TerminalSettingsModel/Profile.h | 7 +- src/cascadia/TerminalSettingsModel/Theme.cpp | 47 ++++++++ src/cascadia/TerminalSettingsModel/Theme.h | 2 +- 22 files changed, 531 insertions(+), 10 deletions(-) diff --git a/.github/actions/spelling/allow/allow.txt b/.github/actions/spelling/allow/allow.txt index ef4f666f47..3b54d30497 100644 --- a/.github/actions/spelling/allow/allow.txt +++ b/.github/actions/spelling/allow/allow.txt @@ -27,6 +27,7 @@ gje godbolt hyperlinking hyperlinks +Kbds kje libfuzzer liga @@ -43,6 +44,7 @@ mkmk mnt mru nje +NTMTo notwrapped ogonek overlined diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp index 18b9a1ffea..812a1cf1b7 100644 --- a/src/cascadia/TerminalApp/AppLogic.cpp +++ b/src/cascadia/TerminalApp/AppLogic.cpp @@ -432,6 +432,10 @@ namespace winrt::TerminalApp::implementation return; } } + else + { + _settings.LogSettingChanges(true); + } if (initialLoad) { diff --git a/src/cascadia/TerminalSettingsEditor/MainPage.cpp b/src/cascadia/TerminalSettingsEditor/MainPage.cpp index ea98ee83f1..76bbeacb24 100644 --- a/src/cascadia/TerminalSettingsEditor/MainPage.cpp +++ b/src/cascadia/TerminalSettingsEditor/MainPage.cpp @@ -481,6 +481,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation void MainPage::SaveButton_Click(const IInspectable& /*sender*/, const RoutedEventArgs& /*args*/) { + _settingsClone.LogSettingChanges(false); _settingsClone.WriteSettingsToDisk(); } diff --git a/src/cascadia/TerminalSettingsModel/ActionMap.cpp b/src/cascadia/TerminalSettingsModel/ActionMap.cpp index 3d80220f79..bebe9ac76b 100644 --- a/src/cascadia/TerminalSettingsModel/ActionMap.cpp +++ b/src/cascadia/TerminalSettingsModel/ActionMap.cpp @@ -570,6 +570,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation const auto action = cmd.ActionAndArgs().Action(); const auto id = action == ShortcutAction::Invalid ? hstring{} : cmd.ID(); _KeyMap.insert_or_assign(keys, id); + _changeLog.emplace(KeysKey); } // Method Description: diff --git a/src/cascadia/TerminalSettingsModel/ActionMap.h b/src/cascadia/TerminalSettingsModel/ActionMap.h index e3ceb3f518..8da139a30d 100644 --- a/src/cascadia/TerminalSettingsModel/ActionMap.h +++ b/src/cascadia/TerminalSettingsModel/ActionMap.h @@ -73,6 +73,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation Json::Value ToJson() const; Json::Value KeyBindingsToJson() const; bool FixupsAppliedDuringLoad() const; + void LogSettingChanges(std::set& changes, const std::string_view& context) const; // modification bool RebindKeys(const Control::KeyChord& oldKeys, const Control::KeyChord& newKeys); @@ -138,6 +139,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation til::shared_mutex>> _cwdLocalSnippetsCache{}; + std::set _changeLog; + friend class SettingsModelUnitTests::KeyBindingsTests; friend class SettingsModelUnitTests::DeserializationTests; friend class SettingsModelUnitTests::TerminalSettingsTests; diff --git a/src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp b/src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp index b6216971ce..11b6fe649b 100644 --- a/src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp +++ b/src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp @@ -78,7 +78,9 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // Now check if this is a command block if (jsonBlock.isMember(JsonKey(CommandsKey)) || jsonBlock.isMember(JsonKey(ActionKey))) { - AddAction(*Command::FromJson(jsonBlock, warnings, origin), keys); + auto command = Command::FromJson(jsonBlock, warnings, origin); + command->LogSettingChanges(_changeLog); + AddAction(*command, keys); if (jsonBlock.isMember(JsonKey(KeysKey))) { @@ -105,6 +107,28 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // any existing keybinding with the same keychord in this layer will get overwritten _KeyMap.insert_or_assign(keys, idJson); + + if (!_changeLog.contains(KeysKey.data())) + { + // Log "keys" field, but only if it's one that isn't in userDefaults.json + static constexpr std::array, 3> userDefaultKbds{ { { L"Terminal.CopyToClipboard", "ctrl+c" }, + { L"Terminal.PasteFromClipboard", "ctrl+v" }, + { L"Terminal.DuplicatePaneAuto", "alt+shift+d" } } }; + bool isUserDefaultKbd = false; + for (const auto& [id, kbd] : userDefaultKbds) + { + const auto keyJson{ jsonBlock.find(&*KeysKey.cbegin(), (&*KeysKey.cbegin()) + KeysKey.size()) }; + if (idJson == id && keyJson->asString() == kbd) + { + isUserDefaultKbd = true; + break; + } + } + if (!isUserDefaultKbd) + { + _changeLog.emplace(KeysKey); + } + } } } @@ -156,4 +180,12 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation return keybindingsList; } + + void ActionMap::LogSettingChanges(std::set& changes, const std::string_view& context) const + { + for (const auto& setting : _changeLog) + { + changes.emplace(fmt::format(FMT_COMPILE("{}.{}"), context, setting)); + } + } } diff --git a/src/cascadia/TerminalSettingsModel/AppearanceConfig.cpp b/src/cascadia/TerminalSettingsModel/AppearanceConfig.cpp index f513f66fa2..4abfc97f5d 100644 --- a/src/cascadia/TerminalSettingsModel/AppearanceConfig.cpp +++ b/src/cascadia/TerminalSettingsModel/AppearanceConfig.cpp @@ -90,27 +90,42 @@ Json::Value AppearanceConfig::ToJson() const void AppearanceConfig::LayerJson(const Json::Value& json) { JsonUtils::GetValueForKey(json, ForegroundKey, _Foreground); + _logSettingIfSet(ForegroundKey, _Foreground.has_value()); + JsonUtils::GetValueForKey(json, BackgroundKey, _Background); + _logSettingIfSet(BackgroundKey, _Background.has_value()); + JsonUtils::GetValueForKey(json, SelectionBackgroundKey, _SelectionBackground); + _logSettingIfSet(SelectionBackgroundKey, _SelectionBackground.has_value()); + JsonUtils::GetValueForKey(json, CursorColorKey, _CursorColor); + _logSettingIfSet(CursorColorKey, _CursorColor.has_value()); JsonUtils::GetValueForKey(json, LegacyAcrylicTransparencyKey, _Opacity); JsonUtils::GetValueForKey(json, OpacityKey, _Opacity, JsonUtils::OptionalConverter{}); + _logSettingIfSet(OpacityKey, _Opacity.has_value()); + if (json["colorScheme"].isString()) { // to make the UI happy, set ColorSchemeName. JsonUtils::GetValueForKey(json, ColorSchemeKey, _DarkColorSchemeName); _LightColorSchemeName = _DarkColorSchemeName; + _logSettingSet(ColorSchemeKey); } else if (json["colorScheme"].isObject()) { // to make the UI happy, set ColorSchemeName to whatever the dark value is. JsonUtils::GetValueForKey(json["colorScheme"], "dark", _DarkColorSchemeName); JsonUtils::GetValueForKey(json["colorScheme"], "light", _LightColorSchemeName); + + _logSettingSet("colorScheme.dark"); + _logSettingSet("colorScheme.light"); } #define APPEARANCE_SETTINGS_LAYER_JSON(type, name, jsonKey, ...) \ - JsonUtils::GetValueForKey(json, jsonKey, _##name); + JsonUtils::GetValueForKey(json, jsonKey, _##name); \ + _logSettingIfSet(jsonKey, _##name.has_value()); + MTSM_APPEARANCE_SETTINGS(APPEARANCE_SETTINGS_LAYER_JSON) #undef APPEARANCE_SETTINGS_LAYER_JSON } @@ -156,3 +171,24 @@ winrt::hstring AppearanceConfig::ExpandedBackgroundImagePath() return winrt::hstring{ wil::ExpandEnvironmentStringsW(path.c_str()) }; } } + +void AppearanceConfig::_logSettingSet(const std::string_view& setting) +{ + _changeLog.emplace(setting); +} + +void AppearanceConfig::_logSettingIfSet(const std::string_view& setting, const bool isSet) +{ + if (isSet) + { + _logSettingSet(setting); + } +} + +void AppearanceConfig::LogSettingChanges(std::set& changes, const std::string_view& context) const +{ + for (const auto& setting : _changeLog) + { + changes.emplace(fmt::format(FMT_COMPILE("{}.{}"), context, setting)); + } +} diff --git a/src/cascadia/TerminalSettingsModel/AppearanceConfig.h b/src/cascadia/TerminalSettingsModel/AppearanceConfig.h index cffbdede6d..87beb7ea1b 100644 --- a/src/cascadia/TerminalSettingsModel/AppearanceConfig.h +++ b/src/cascadia/TerminalSettingsModel/AppearanceConfig.h @@ -31,6 +31,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation static winrt::com_ptr CopyAppearance(const AppearanceConfig* source, winrt::weak_ref sourceProfile); Json::Value ToJson() const; void LayerJson(const Json::Value& json); + void LogSettingChanges(std::set& changes, const std::string_view& context) const; Model::Profile SourceProfile(); @@ -52,5 +53,9 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation private: winrt::weak_ref _sourceProfile; + std::set _changeLog; + + void _logSettingSet(const std::string_view& setting); + void _logSettingIfSet(const std::string_view& setting, const bool isSet); }; } diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettings.h b/src/cascadia/TerminalSettingsModel/CascadiaSettings.h index becc6749c9..75eb9ee96e 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettings.h +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettings.h @@ -48,6 +48,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation std::unordered_map> colorSchemes; std::unordered_map colorSchemeRemappings; bool fixupsAppliedDuringLoad{ false }; + std::set themesChangeLog; void clear(); }; @@ -96,6 +97,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation void _executeGenerator(const IDynamicProfileGenerator& generator); std::unordered_set _ignoredNamespaces; + std::set themesChangeLog; // See _getNonUserOriginProfiles(). size_t _userProfileCount = 0; }; @@ -150,6 +152,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation void ExpandCommands(); + void LogSettingChanges(bool isJsonLoad) const; + private: static const std::filesystem::path& _settingsPath(); static const std::filesystem::path& _releaseSettingsPath(); @@ -180,6 +184,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation winrt::com_ptr _baseLayerProfile = winrt::make_self(); winrt::Windows::Foundation::Collections::IObservableVector _allProfiles = winrt::single_threaded_observable_vector(); winrt::Windows::Foundation::Collections::IObservableVector _activeProfiles = winrt::single_threaded_observable_vector(); + std::set _themesChangeLog{}; // load errors winrt::Windows::Foundation::Collections::IVector _warnings = winrt::single_threaded_vector(); diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettings.idl b/src/cascadia/TerminalSettingsModel/CascadiaSettings.idl index 0d8c0c5b15..2fa41941d6 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettings.idl +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettings.idl @@ -24,6 +24,7 @@ namespace Microsoft.Terminal.Settings.Model CascadiaSettings Copy(); void WriteSettingsToDisk(); + void LogSettingChanges(Boolean isJsonLoad); String Hash { get; }; diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp b/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp index f5cc10fced..5ec7275b05 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp @@ -109,6 +109,7 @@ void ParsedSettings::clear() profilesByGuid.clear(); colorSchemes.clear(); fixupsAppliedDuringLoad = false; + themesChangeLog.clear(); } // This is a convenience method used by the CascadiaSettings constructor. @@ -658,6 +659,12 @@ void SettingsLoader::_parse(const OriginTag origin, const winrt::hstring& source // versions of these themes overriding the built-in ones. continue; } + + if (origin != OriginTag::InBox) + { + static std::string themesContext{ "themes" }; + theme->LogSettingChanges(settings.themesChangeLog, themesContext); + } settings.globals->AddTheme(*theme); } } @@ -1222,6 +1229,7 @@ CascadiaSettings::CascadiaSettings(SettingsLoader&& loader) : _allProfiles = winrt::single_threaded_observable_vector(std::move(allProfiles)); _activeProfiles = winrt::single_threaded_observable_vector(std::move(activeProfiles)); _warnings = winrt::single_threaded_vector(std::move(warnings)); + _themesChangeLog = std::move(loader.userSettings.themesChangeLog); _resolveDefaultProfile(); _resolveNewTabMenuProfiles(); @@ -1596,3 +1604,73 @@ void CascadiaSettings::_resolveNewTabMenuProfilesSet(const IVector changes; + static constexpr std::string_view globalContext{ "global" }; + _globals->LogSettingChanges(changes, globalContext); + + // Actions are not expected to change when loaded from the settings UI + static constexpr std::string_view actionContext{ "action" }; + winrt::get_self(_globals->ActionMap())->LogSettingChanges(changes, actionContext); + + static constexpr std::string_view profileContext{ "profile" }; + for (const auto& profile : _allProfiles) + { + winrt::get_self(profile)->LogSettingChanges(changes, profileContext); + } + + static constexpr std::string_view profileDefaultsContext{ "profileDefaults" }; + _baseLayerProfile->LogSettingChanges(changes, profileDefaultsContext); + + // Themes are not expected to change when loaded from the settings UI + // DO NOT CALL Theme::LogSettingChanges!! + // We already collected the changes when we loaded the JSON + for (const auto& change : _themesChangeLog) + { + changes.insert(change); + } + + // report changes + for (const auto& change : changes) + { +#ifndef _DEBUG + // A `isJsonLoad ? "JsonSettingsChanged" : "UISettingsChanged"` + // would be nice, but that apparently isn't allowed in the macro below. + // Also, there's guidance to not send too much data all in one event, + // so we'll be sending a ton of events here. + if (isJsonLoad) + { + TraceLoggingWrite(g_hSettingsModelProvider, + "JsonSettingsChanged", + TraceLoggingDescription("Event emitted when settings.json change"), + TraceLoggingValue(change.data()), + TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), + TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); + } + else + { + TraceLoggingWrite(g_hSettingsModelProvider, + "UISettingsChanged", + TraceLoggingDescription("Event emitted when settings change via the UI"), + TraceLoggingValue(change.data()), + TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), + TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); + } +#else + OutputDebugStringA(isJsonLoad ? "JsonSettingsChanged - " : "UISettingsChanged - "); + OutputDebugStringA(change.data()); + OutputDebugStringA("\n"); +#endif // !_DEBUG + } +} diff --git a/src/cascadia/TerminalSettingsModel/Command.cpp b/src/cascadia/TerminalSettingsModel/Command.cpp index 730d883463..2f4cc17f70 100644 --- a/src/cascadia/TerminalSettingsModel/Command.cpp +++ b/src/cascadia/TerminalSettingsModel/Command.cpp @@ -741,4 +741,57 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation return winrt::single_threaded_vector(std::move(result)); } + + void Command::LogSettingChanges(std::set& changes) + { + if (_IterateOn != ExpandCommandType::None) + { + switch (_IterateOn) + { + case ExpandCommandType::Profiles: + changes.emplace(fmt::format(FMT_COMPILE("{}.{}"), IterateOnKey, "profiles")); + break; + case ExpandCommandType::ColorSchemes: + changes.emplace(fmt::format(FMT_COMPILE("{}.{}"), IterateOnKey, "schemes")); + break; + } + } + + if (!_Description.empty()) + { + changes.emplace(DescriptionKey); + } + + if (IsNestedCommand()) + { + changes.emplace(CommandsKey); + } + else + { + const auto json{ ActionAndArgs::ToJson(ActionAndArgs()) }; + if (json.isString()) + { + // covers actions w/out args + // - "command": "unbound" --> "unbound" + // - "command": "copy" --> "copy" + changes.emplace(fmt::format(FMT_COMPILE("{}"), json.asString())); + } + else + { + // covers actions w/ args + // - "command": { "action": "copy", "singleLine": true } --> "copy.singleLine" + // - "command": { "action": "copy", "singleLine": true, "dismissSelection": true } --> "copy.singleLine", "copy.dismissSelection" + + const std::string shortcutActionName{ json[JsonKey("action")].asString() }; + + auto members = json.getMemberNames(); + members.erase(std::remove_if(members.begin(), members.end(), [](const auto& member) { return member == "action"; }), members.end()); + + for (const auto& actionArg : members) + { + changes.emplace(fmt::format(FMT_COMPILE("{}.{}"), shortcutActionName, actionArg)); + } + } + } + } } diff --git a/src/cascadia/TerminalSettingsModel/Command.h b/src/cascadia/TerminalSettingsModel/Command.h index 1bc92220ca..4d372bea9c 100644 --- a/src/cascadia/TerminalSettingsModel/Command.h +++ b/src/cascadia/TerminalSettingsModel/Command.h @@ -61,6 +61,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation const Json::Value& json, const OriginTag origin); Json::Value ToJson() const; + void LogSettingChanges(std::set& changes); bool HasNestedCommands() const; bool IsNestedCommand() const noexcept; diff --git a/src/cascadia/TerminalSettingsModel/FontConfig.cpp b/src/cascadia/TerminalSettingsModel/FontConfig.cpp index 746ceee649..0fe0da5439 100644 --- a/src/cascadia/TerminalSettingsModel/FontConfig.cpp +++ b/src/cascadia/TerminalSettingsModel/FontConfig.cpp @@ -83,17 +83,25 @@ void FontConfig::LayerJson(const Json::Value& json) { // A font object is defined, use that const auto fontInfoJson = json[JsonKey(FontInfoKey)]; -#define FONT_SETTINGS_LAYER_JSON(type, name, jsonKey, ...) \ - JsonUtils::GetValueForKey(fontInfoJson, jsonKey, _##name); +#define FONT_SETTINGS_LAYER_JSON(type, name, jsonKey, ...) \ + JsonUtils::GetValueForKey(fontInfoJson, jsonKey, _##name); \ + _logSettingIfSet(jsonKey, _##name.has_value()); + MTSM_FONT_SETTINGS(FONT_SETTINGS_LAYER_JSON) #undef FONT_SETTINGS_LAYER_JSON } else { // No font object is defined + // Log settings as if they were a part of the font object JsonUtils::GetValueForKey(json, LegacyFontFaceKey, _FontFace); + _logSettingIfSet("face", _FontFace.has_value()); + JsonUtils::GetValueForKey(json, LegacyFontSizeKey, _FontSize); + _logSettingIfSet("size", _FontSize.has_value()); + JsonUtils::GetValueForKey(json, LegacyFontWeightKey, _FontWeight); + _logSettingIfSet("weight", _FontWeight.has_value()); } } @@ -101,3 +109,41 @@ winrt::Microsoft::Terminal::Settings::Model::Profile FontConfig::SourceProfile() { return _sourceProfile.get(); } + +void FontConfig::_logSettingSet(const std::string_view& setting) +{ + if (setting == "axes" && _FontAxes.has_value()) + { + for (const auto& [mapKey, _] : _FontAxes.value()) + { + _changeLog.emplace(fmt::format(FMT_COMPILE("{}.{}"), setting, til::u16u8(mapKey))); + } + } + else if (setting == "features" && _FontFeatures.has_value()) + { + for (const auto& [mapKey, _] : _FontFeatures.value()) + { + _changeLog.emplace(fmt::format(FMT_COMPILE("{}.{}"), setting, til::u16u8(mapKey))); + } + } + else + { + _changeLog.emplace(setting); + } +} + +void FontConfig::_logSettingIfSet(const std::string_view& setting, const bool isSet) +{ + if (isSet) + { + _logSettingSet(setting); + } +} + +void FontConfig::LogSettingChanges(std::set& changes, const std::string_view& context) const +{ + for (const auto& setting : _changeLog) + { + changes.emplace(fmt::format(FMT_COMPILE("{}.{}"), context, setting)); + } +} diff --git a/src/cascadia/TerminalSettingsModel/FontConfig.h b/src/cascadia/TerminalSettingsModel/FontConfig.h index c249f5a1b4..99d503d56d 100644 --- a/src/cascadia/TerminalSettingsModel/FontConfig.h +++ b/src/cascadia/TerminalSettingsModel/FontConfig.h @@ -35,6 +35,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation static winrt::com_ptr CopyFontInfo(const FontConfig* source, winrt::weak_ref sourceProfile); Json::Value ToJson() const; void LayerJson(const Json::Value& json); + void LogSettingChanges(std::set& changes, const std::string_view& context) const; Model::Profile SourceProfile(); @@ -45,5 +46,9 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation private: winrt::weak_ref _sourceProfile; + std::set _changeLog; + + void _logSettingSet(const std::string_view& setting); + void _logSettingIfSet(const std::string_view& setting, const bool isSet); }; } diff --git a/src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp b/src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp index f51ece15e1..e7f930c175 100644 --- a/src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp +++ b/src/cascadia/TerminalSettingsModel/GlobalAppSettings.cpp @@ -128,13 +128,16 @@ winrt::com_ptr GlobalAppSettings::FromJson(const Json::Value& void GlobalAppSettings::LayerJson(const Json::Value& json, const OriginTag origin) { JsonUtils::GetValueForKey(json, DefaultProfileKey, _UnparsedDefaultProfile); + // GH#8076 - when adding enum values to this key, we also changed it from // "useTabSwitcher" to "tabSwitcherMode". Continue supporting // "useTabSwitcher", but prefer "tabSwitcherMode" JsonUtils::GetValueForKey(json, LegacyUseTabSwitcherModeKey, _TabSwitcherMode); #define GLOBAL_SETTINGS_LAYER_JSON(type, name, jsonKey, ...) \ - JsonUtils::GetValueForKey(json, jsonKey, _##name); + JsonUtils::GetValueForKey(json, jsonKey, _##name); \ + _logSettingIfSet(jsonKey, _##name.has_value()); + MTSM_GLOBAL_SETTINGS(GLOBAL_SETTINGS_LAYER_JSON) #undef GLOBAL_SETTINGS_LAYER_JSON @@ -152,6 +155,25 @@ void GlobalAppSettings::LayerJson(const Json::Value& json, const OriginTag origi LayerActionsFrom(json, origin, true); JsonUtils::GetValueForKey(json, LegacyReloadEnvironmentVariablesKey, _legacyReloadEnvironmentVariables); + if (json[LegacyReloadEnvironmentVariablesKey.data()]) + { + _logSettingSet(LegacyReloadEnvironmentVariablesKey); + } + + // Remove settings included in userDefaults + static constexpr std::array, 2> userDefaultSettings{ { { "copyOnSelect", "false" }, + { "copyFormatting", "false" } } }; + for (const auto& [setting, val] : userDefaultSettings) + { + if (const auto settingJson{ json.find(&*setting.cbegin(), (&*setting.cbegin()) + setting.size()) }) + { + if (settingJson->asString() == val) + { + // false positive! + _changeLog.erase(std::string{ setting }); + } + } + } } void GlobalAppSettings::LayerActionsFrom(const Json::Value& json, const OriginTag origin, const bool withKeybindings) @@ -317,3 +339,85 @@ bool GlobalAppSettings::ShouldUsePersistedLayout() const { return FirstWindowPreference() == FirstWindowPreference::PersistedWindowLayout && !IsolatedMode(); } + +void GlobalAppSettings::_logSettingSet(const std::string_view& setting) +{ + if (setting == "theme") + { + if (_Theme.has_value()) + { + // ThemePair always has a Dark/Light value, + // so we need to check if they were explicitly set + if (_Theme->DarkName() == _Theme->LightName()) + { + _changeLog.emplace(setting); + } + else + { + _changeLog.emplace(fmt::format(FMT_COMPILE("{}.{}"), setting, "dark")); + _changeLog.emplace(fmt::format(FMT_COMPILE("{}.{}"), setting, "light")); + } + } + } + else if (setting == "newTabMenu") + { + if (_NewTabMenu.has_value()) + { + for (const auto& entry : *_NewTabMenu) + { + std::string entryType; + switch (entry.Type()) + { + case NewTabMenuEntryType::Profile: + entryType = "profile"; + break; + case NewTabMenuEntryType::Separator: + entryType = "separator"; + break; + case NewTabMenuEntryType::Folder: + entryType = "folder"; + break; + case NewTabMenuEntryType::RemainingProfiles: + entryType = "remainingProfiles"; + break; + case NewTabMenuEntryType::MatchProfiles: + entryType = "matchProfiles"; + break; + case NewTabMenuEntryType::Action: + entryType = "action"; + break; + case NewTabMenuEntryType::Invalid: + // ignore invalid + continue; + } + _changeLog.emplace(fmt::format(FMT_COMPILE("{}.{}"), setting, entryType)); + } + } + } + else + { + _changeLog.emplace(setting); + } +} + +void GlobalAppSettings::_logSettingIfSet(const std::string_view& setting, const bool isSet) +{ + if (isSet) + { + // Exclude some false positives from userDefaults.json + const bool settingCopyFormattingToDefault = til::equals_insensitive_ascii(setting, "copyFormatting") && _CopyFormatting.has_value() && _CopyFormatting.value() == static_cast(0); + const bool settingNTMToDefault = til::equals_insensitive_ascii(setting, "newTabMenu") && _NewTabMenu.has_value() && _NewTabMenu->Size() == 1 && _NewTabMenu->GetAt(0).Type() == NewTabMenuEntryType::RemainingProfiles; + if (!settingCopyFormattingToDefault && !settingNTMToDefault) + { + _logSettingSet(setting); + } + } +} + +void GlobalAppSettings::LogSettingChanges(std::set& changes, const std::string_view& context) const +{ + for (const auto& setting : _changeLog) + { + changes.emplace(fmt::format(FMT_COMPILE("{}.{}"), context, setting)); + } +} diff --git a/src/cascadia/TerminalSettingsModel/GlobalAppSettings.h b/src/cascadia/TerminalSettingsModel/GlobalAppSettings.h index 7b55b7007b..fe60347cfc 100644 --- a/src/cascadia/TerminalSettingsModel/GlobalAppSettings.h +++ b/src/cascadia/TerminalSettingsModel/GlobalAppSettings.h @@ -72,10 +72,12 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation bool LegacyReloadEnvironmentVariables() const noexcept { return _legacyReloadEnvironmentVariables; } + void LogSettingChanges(std::set& changes, const std::string_view& context) const; + INHERITABLE_SETTING(Model::GlobalAppSettings, hstring, UnparsedDefaultProfile, L""); #define GLOBAL_SETTINGS_INITIALIZE(type, name, jsonKey, ...) \ - INHERITABLE_SETTING(Model::GlobalAppSettings, type, name, ##__VA_ARGS__) + INHERITABLE_SETTING_WITH_LOGGING(Model::GlobalAppSettings, type, name, jsonKey, ##__VA_ARGS__) MTSM_GLOBAL_SETTINGS(GLOBAL_SETTINGS_INITIALIZE) #undef GLOBAL_SETTINGS_INITIALIZE @@ -89,9 +91,13 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation winrt::guid _defaultProfile{}; bool _legacyReloadEnvironmentVariables{ true }; winrt::com_ptr _actionMap{ winrt::make_self() }; + std::set _changeLog; std::vector _keybindingsWarnings; Windows::Foundation::Collections::IMap _colorSchemes{ winrt::single_threaded_map() }; Windows::Foundation::Collections::IMap _themes{ winrt::single_threaded_map() }; + + void _logSettingSet(const std::string_view& setting); + void _logSettingIfSet(const std::string_view& setting, const bool isSet); }; } diff --git a/src/cascadia/TerminalSettingsModel/IInheritable.h b/src/cascadia/TerminalSettingsModel/IInheritable.h index 091cd653a1..1099c128bc 100644 --- a/src/cascadia/TerminalSettingsModel/IInheritable.h +++ b/src/cascadia/TerminalSettingsModel/IInheritable.h @@ -185,6 +185,27 @@ public: \ _##name = value; \ } +#define INHERITABLE_SETTING_WITH_LOGGING(projectedType, type, name, jsonKey, ...) \ + _BASE_INHERITABLE_SETTING(projectedType, std::optional, name, ...) \ +public: \ + /* Returns the resolved value for this setting */ \ + /* fallback: user set value --> inherited value --> system set value */ \ + type name() const \ + { \ + const auto val{ _get##name##Impl() }; \ + return val ? *val : type{ __VA_ARGS__ }; \ + } \ + \ + /* Overwrite the user set value */ \ + void name(const type& value) \ + { \ + if (!_##name.has_value() || _##name.value() != value) \ + { \ + _logSettingSet(jsonKey); \ + } \ + _##name = value; \ + } + // This macro is similar to the one above, but is reserved for optional settings // like Profile.Foreground (where null is interpreted // as an acceptable value, rather than "inherit") diff --git a/src/cascadia/TerminalSettingsModel/Profile.cpp b/src/cascadia/TerminalSettingsModel/Profile.cpp index 66382c857b..3d1d278247 100644 --- a/src/cascadia/TerminalSettingsModel/Profile.cpp +++ b/src/cascadia/TerminalSettingsModel/Profile.cpp @@ -175,15 +175,22 @@ void Profile::LayerJson(const Json::Value& json) JsonUtils::GetValueForKey(json, NameKey, _Name); JsonUtils::GetValueForKey(json, UpdatesKey, _Updates); JsonUtils::GetValueForKey(json, GuidKey, _Guid); - JsonUtils::GetValueForKey(json, HiddenKey, _Hidden); + + // Make sure Source is before Hidden! We use that to exclude false positives from the settings logger! JsonUtils::GetValueForKey(json, SourceKey, _Source); + JsonUtils::GetValueForKey(json, HiddenKey, _Hidden); + _logSettingIfSet(HiddenKey, _Hidden.has_value()); + JsonUtils::GetValueForKey(json, IconKey, _Icon); + _logSettingIfSet(IconKey, _Icon.has_value()); // Padding was never specified as an integer, but it was a common working mistake. // Allow it to be permissive. JsonUtils::GetValueForKey(json, PaddingKey, _Padding, JsonUtils::OptionalConverter>{}); + _logSettingIfSet(PaddingKey, _Padding.has_value()); JsonUtils::GetValueForKey(json, TabColorKey, _TabColor); + _logSettingIfSet(TabColorKey, _TabColor.has_value()); // Try to load some legacy keys, to migrate them. // Done _before_ the MTSM_PROFILE_SETTINGS, which have the updated keys. @@ -191,7 +198,8 @@ void Profile::LayerJson(const Json::Value& json) JsonUtils::GetValueForKey(json, LegacyAutoMarkPromptsKey, _AutoMarkPrompts); #define PROFILE_SETTINGS_LAYER_JSON(type, name, jsonKey, ...) \ - JsonUtils::GetValueForKey(json, jsonKey, _##name); + JsonUtils::GetValueForKey(json, jsonKey, _##name); \ + _logSettingIfSet(jsonKey, _##name.has_value()); MTSM_PROFILE_SETTINGS(PROFILE_SETTINGS_LAYER_JSON) #undef PROFILE_SETTINGS_LAYER_JSON @@ -208,6 +216,8 @@ void Profile::LayerJson(const Json::Value& json) unfocusedAppearance->LayerJson(json[JsonKey(UnfocusedAppearanceKey)]); _UnfocusedAppearance = *unfocusedAppearance; + + _logSettingSet(UnfocusedAppearanceKey); } } @@ -518,3 +528,58 @@ std::wstring Profile::NormalizeCommandLine(LPCWSTR commandLine) return normalized; } + +void Profile::_logSettingSet(const std::string_view& setting) +{ + _changeLog.emplace(setting); +} + +void Profile::_logSettingIfSet(const std::string_view& setting, const bool isSet) +{ + if (isSet) + { + // make sure this matches defaults.json. + static constexpr winrt::guid DEFAULT_WINDOWS_POWERSHELL_GUID{ 0x61c54bbd, 0xc2c6, 0x5271, { 0x96, 0xe7, 0x00, 0x9a, 0x87, 0xff, 0x44, 0xbf } }; + static constexpr winrt::guid DEFAULT_COMMAND_PROMPT_GUID{ 0x0caa0dad, 0x35be, 0x5f56, { 0xa8, 0xff, 0xaf, 0xce, 0xee, 0xaa, 0x61, 0x01 } }; + + // Exclude some false positives from userDefaults.json + // NOTE: we can't use the OriginTag here because it hasn't been set yet! + const bool isWinPow = _Guid.has_value() && *_Guid == DEFAULT_WINDOWS_POWERSHELL_GUID; //_Name.has_value() && til::equals_insensitive_ascii(*_Name, L"Windows PowerShell"); + const bool isCmd = _Guid.has_value() && *_Guid == DEFAULT_COMMAND_PROMPT_GUID; //_Name.has_value() && til::equals_insensitive_ascii(*_Name, L"Command Prompt"); + const bool isACS = _Name.has_value() && til::equals_insensitive_ascii(*_Name, L"Azure Cloud Shell"); + const bool isWTDynamicProfile = _Source.has_value() && til::starts_with(*_Source, L"Windows.Terminal"); + const bool settingHiddenToFalse = til::equals_insensitive_ascii(setting, HiddenKey) && _Hidden.has_value() && _Hidden == false; + const bool settingCommandlineToWinPow = til::equals_insensitive_ascii(setting, "commandline") && _Commandline.has_value() && til::equals_insensitive_ascii(*_Commandline, L"%SystemRoot%\\System32\\WindowsPowerShell\\v1.0\\powershell.exe"); + const bool settingCommandlineToCmd = til::equals_insensitive_ascii(setting, "commandline") && _Commandline.has_value() && til::equals_insensitive_ascii(*_Commandline, L"%SystemRoot%\\System32\\cmd.exe"); + // clang-format off + if (!(isWinPow && (settingHiddenToFalse || settingCommandlineToWinPow)) + && !(isCmd && (settingHiddenToFalse || settingCommandlineToCmd)) + && !(isACS && settingHiddenToFalse) + && !(isWTDynamicProfile && settingHiddenToFalse)) + { + // clang-format on + _logSettingSet(setting); + } + } +} + +void Profile::LogSettingChanges(std::set& changes, const std::string_view& context) const +{ + for (const auto& setting : _changeLog) + { + changes.emplace(fmt::format(FMT_COMPILE("{}.{}"), context, setting)); + } + + std::string fontContext{ fmt::format(FMT_COMPILE("{}.{}"), context, FontInfoKey) }; + winrt::get_self(_FontInfo)->LogSettingChanges(changes, fontContext); + + // We don't want to distinguish between "profile.defaultAppearance.*" and "profile.unfocusedAppearance.*" settings, + // but we still want to aggregate all of the appearance settings from both appearances. + // Log them as "profile.appearance.*" + std::string appContext{ fmt::format(FMT_COMPILE("{}.{}"), context, "appearance") }; + winrt::get_self(_DefaultAppearance)->LogSettingChanges(changes, appContext); + if (_UnfocusedAppearance) + { + winrt::get_self(*_UnfocusedAppearance)->LogSettingChanges(changes, appContext); + } +} diff --git a/src/cascadia/TerminalSettingsModel/Profile.h b/src/cascadia/TerminalSettingsModel/Profile.h index 6fdd5c3ed5..24fbb7d1f2 100644 --- a/src/cascadia/TerminalSettingsModel/Profile.h +++ b/src/cascadia/TerminalSettingsModel/Profile.h @@ -108,6 +108,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation void _FinalizeInheritance() override; + void LogSettingChanges(std::set& changes, const std::string_view& context) const; + // Special fields hstring Icon() const; void Icon(const hstring& value); @@ -131,7 +133,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation public: #define PROFILE_SETTINGS_INITIALIZE(type, name, jsonKey, ...) \ - INHERITABLE_SETTING(Model::Profile, type, name, ##__VA_ARGS__) + INHERITABLE_SETTING_WITH_LOGGING(Model::Profile, type, name, jsonKey, ##__VA_ARGS__) MTSM_PROFILE_SETTINGS(PROFILE_SETTINGS_INITIALIZE) #undef PROFILE_SETTINGS_INITIALIZE @@ -140,12 +142,15 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation Model::FontConfig _FontInfo{ winrt::make(weak_ref(*this)) }; std::optional _evaluatedIcon{ std::nullopt }; + std::set _changeLog; static std::wstring EvaluateStartingDirectory(const std::wstring& directory); static guid _GenerateGuidForProfile(const std::wstring_view& name, const std::wstring_view& source) noexcept; winrt::hstring _evaluateIcon() const; + void _logSettingSet(const std::string_view& setting); + void _logSettingIfSet(const std::string_view& setting, const bool isSet); friend class SettingsModelUnitTests::DeserializationTests; friend class SettingsModelUnitTests::ProfileTests; diff --git a/src/cascadia/TerminalSettingsModel/Theme.cpp b/src/cascadia/TerminalSettingsModel/Theme.cpp index 52c283a743..90fd1d1a76 100644 --- a/src/cascadia/TerminalSettingsModel/Theme.cpp +++ b/src/cascadia/TerminalSettingsModel/Theme.cpp @@ -292,6 +292,53 @@ winrt::com_ptr Theme::FromJson(const Json::Value& json) return result; } +void Theme::LogSettingChanges(std::set& changes, const std::string_view& context) +{ +#pragma warning(push) +#pragma warning(disable : 5103) // pasting '{' and 'winrt' does not result in a valid preprocessing token + +#define GENERATE_SET_CHECK_AND_JSON_KEYS(type, name, jsonKey, ...) \ + const bool is##name##Set = _##name != nullptr; \ + std::string_view outer##name##JsonKey = jsonKey; + + MTSM_THEME_SETTINGS(GENERATE_SET_CHECK_AND_JSON_KEYS) + +#define LOG_IF_SET(type, name, jsonKey, ...) \ + if (obj.name() != type{##__VA_ARGS__ }) \ + changes.emplace(fmt::format(FMT_COMPILE("{}.{}.{}"), context, outerJsonKey, jsonKey)); + + if (isWindowSet) + { + const auto obj = _Window; + const auto outerJsonKey = outerWindowJsonKey; + MTSM_THEME_WINDOW_SETTINGS(LOG_IF_SET) + } + + if (isSettingsSet) + { + const auto obj = _Settings; + const auto outerJsonKey = outerSettingsJsonKey; + MTSM_THEME_SETTINGS_SETTINGS(LOG_IF_SET) + } + + if (isTabRowSet) + { + const auto obj = _TabRow; + const auto outerJsonKey = outerTabRowJsonKey; + MTSM_THEME_TABROW_SETTINGS(LOG_IF_SET) + } + + if (isTabSet) + { + const auto obj = _Tab; + const auto outerJsonKey = outerTabJsonKey; + MTSM_THEME_TAB_SETTINGS(LOG_IF_SET) + } +#undef LOG_IF_SET +#undef GENERATE_SET_CHECK_AND_JSON_KEYS +#pragma warning(pop) +} + // Method Description: // - Create a new serialized JsonObject from an instance of this class // Arguments: diff --git a/src/cascadia/TerminalSettingsModel/Theme.h b/src/cascadia/TerminalSettingsModel/Theme.h index 2c0ac139aa..e0892733ae 100644 --- a/src/cascadia/TerminalSettingsModel/Theme.h +++ b/src/cascadia/TerminalSettingsModel/Theme.h @@ -97,8 +97,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation hstring ToString(); static com_ptr FromJson(const Json::Value& json); - void LayerJson(const Json::Value& json); Json::Value ToJson() const; + void LogSettingChanges(std::set& changes, const std::string_view& context); winrt::Windows::UI::Xaml::ElementTheme RequestedTheme() const noexcept;