From 837e86c18c3d0e032251c73cbda7abe81e04b4ac Mon Sep 17 00:00:00 2001 From: "Dustin L. Howett" Date: Tue, 19 Aug 2025 09:35:27 -0500 Subject: [PATCH 1/2] Bust TerminalSettingsCache down to a plain old C++ class (#19254) --- src/cascadia/TerminalApp/TerminalAppLib.vcxproj | 9 ++------- .../TerminalApp/TerminalAppLib.vcxproj.filters | 1 - src/cascadia/TerminalApp/TerminalPage.cpp | 5 +++-- src/cascadia/TerminalApp/TerminalPage.h | 4 +++- src/cascadia/TerminalApp/TerminalPaneContent.cpp | 5 +++-- src/cascadia/TerminalApp/TerminalPaneContent.h | 6 ++++-- src/cascadia/TerminalApp/TerminalPaneContent.idl | 1 - src/cascadia/TerminalApp/TerminalSettingsCache.cpp | 1 - src/cascadia/TerminalApp/TerminalSettingsCache.h | 12 +++--------- src/cascadia/TerminalApp/TerminalSettingsCache.idl | 13 ------------- 10 files changed, 18 insertions(+), 39 deletions(-) delete mode 100644 src/cascadia/TerminalApp/TerminalSettingsCache.idl diff --git a/src/cascadia/TerminalApp/TerminalAppLib.vcxproj b/src/cascadia/TerminalApp/TerminalAppLib.vcxproj index 3d8a82af2a..57e801df9e 100644 --- a/src/cascadia/TerminalApp/TerminalAppLib.vcxproj +++ b/src/cascadia/TerminalApp/TerminalAppLib.vcxproj @@ -174,9 +174,7 @@ TerminalPaneContent.idl - - TerminalSettingsCache.idl - + SuggestionsControl.xaml @@ -289,9 +287,7 @@ - - TerminalSettingsCache.idl - + SuggestionsControl.xaml @@ -366,7 +362,6 @@ TaskPaneContent.xaml Code - MarkdownPaneContent.xaml Code diff --git a/src/cascadia/TerminalApp/TerminalAppLib.vcxproj.filters b/src/cascadia/TerminalApp/TerminalAppLib.vcxproj.filters index d082b2a054..e15e8c8bfb 100644 --- a/src/cascadia/TerminalApp/TerminalAppLib.vcxproj.filters +++ b/src/cascadia/TerminalApp/TerminalAppLib.vcxproj.filters @@ -90,7 +90,6 @@ - diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 47dee8aae7..971903a0de 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -18,6 +18,7 @@ #include "SettingsPaneContent.h" #include "SnippetsPaneContent.h" #include "TabRowControl.h" +#include "TerminalSettingsCache.h" #include "../../types/inc/ColorFix.hpp" #include "../../types/inc/utils.hpp" @@ -240,7 +241,7 @@ namespace winrt::TerminalApp::implementation if (_settings == nullptr) { // Create this only on the first time we load the settings. - _terminalSettingsCache = TerminalApp::TerminalSettingsCache{ settings, *_bindings }; + _terminalSettingsCache = std::make_shared(settings, *_bindings); } _settings = settings; @@ -3733,7 +3734,7 @@ namespace winrt::TerminalApp::implementation // updating terminal panes, so that we don't have to build a _new_ // TerminalSettings for every profile we update - we can just look them // up the previous ones we built. - _terminalSettingsCache.Reset(_settings, *_bindings); + _terminalSettingsCache->Reset(_settings, *_bindings); for (const auto& tab : _tabs) { diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index d095f008dc..dcba1d2880 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -31,6 +31,8 @@ namespace Microsoft::Terminal::Core namespace winrt::TerminalApp::implementation { + struct TerminalSettingsCache; + inline constexpr uint32_t DefaultRowsToScroll{ 3 }; inline constexpr std::wstring_view TabletInputServiceKey{ L"TabletInputService" }; @@ -275,7 +277,7 @@ namespace winrt::TerminalApp::implementation TerminalApp::ContentManager _manager{ nullptr }; - TerminalApp::TerminalSettingsCache _terminalSettingsCache{ nullptr }; + std::shared_ptr _terminalSettingsCache{}; struct StashedDragData { diff --git a/src/cascadia/TerminalApp/TerminalPaneContent.cpp b/src/cascadia/TerminalApp/TerminalPaneContent.cpp index 002e4f026c..b342f6ebef 100644 --- a/src/cascadia/TerminalApp/TerminalPaneContent.cpp +++ b/src/cascadia/TerminalApp/TerminalPaneContent.cpp @@ -6,6 +6,7 @@ #include +#include "TerminalSettingsCache.h" #include "../../types/inc/utils.hpp" #include "BellEventArgs.g.cpp" @@ -20,7 +21,7 @@ using namespace winrt::Microsoft::Terminal::TerminalConnection; namespace winrt::TerminalApp::implementation { TerminalPaneContent::TerminalPaneContent(const winrt::Microsoft::Terminal::Settings::Model::Profile& profile, - const TerminalApp::TerminalSettingsCache& cache, + const std::shared_ptr& cache, const winrt::Microsoft::Terminal::Control::TermControl& control) : _control{ control }, _cache{ cache }, @@ -346,7 +347,7 @@ namespace winrt::TerminalApp::implementation const auto profile{ settings.FindProfile(_profile.Guid()) }; _profile = profile ? profile : settings.ProfileDefaults(); - if (const auto& settings{ _cache.TryLookup(_profile) }) + if (const auto& settings{ _cache->TryLookup(_profile) }) { _control.UpdateControlSettings(settings.DefaultSettings(), settings.UnfocusedSettings()); } diff --git a/src/cascadia/TerminalApp/TerminalPaneContent.h b/src/cascadia/TerminalApp/TerminalPaneContent.h index aa5f4da498..0e828cdc1b 100644 --- a/src/cascadia/TerminalApp/TerminalPaneContent.h +++ b/src/cascadia/TerminalApp/TerminalPaneContent.h @@ -8,6 +8,8 @@ namespace winrt::TerminalApp::implementation { + struct TerminalSettingsCache; + struct BellEventArgs : public BellEventArgsT { public: @@ -20,7 +22,7 @@ namespace winrt::TerminalApp::implementation struct TerminalPaneContent : TerminalPaneContentT, BasicPaneEvents { TerminalPaneContent(const winrt::Microsoft::Terminal::Settings::Model::Profile& profile, - const TerminalApp::TerminalSettingsCache& cache, + const std::shared_ptr& cache, const winrt::Microsoft::Terminal::Control::TermControl& control); winrt::Windows::UI::Xaml::FrameworkElement GetRoot(); @@ -59,7 +61,7 @@ namespace winrt::TerminalApp::implementation winrt::Microsoft::Terminal::Control::TermControl _control{ nullptr }; winrt::Microsoft::Terminal::TerminalConnection::ConnectionState _connectionState{ winrt::Microsoft::Terminal::TerminalConnection::ConnectionState::NotConnected }; winrt::Microsoft::Terminal::Settings::Model::Profile _profile{ nullptr }; - TerminalApp::TerminalSettingsCache _cache{ nullptr }; + std::shared_ptr _cache{}; bool _isDefTermSession{ false }; winrt::Windows::Media::Playback::MediaPlayer _bellPlayer{ nullptr }; diff --git a/src/cascadia/TerminalApp/TerminalPaneContent.idl b/src/cascadia/TerminalApp/TerminalPaneContent.idl index 255ddef547..53dcf4c337 100644 --- a/src/cascadia/TerminalApp/TerminalPaneContent.idl +++ b/src/cascadia/TerminalApp/TerminalPaneContent.idl @@ -2,7 +2,6 @@ // Licensed under the MIT license. import "IPaneContent.idl"; -import "TerminalSettingsCache.idl"; import "FilteredCommand.idl"; namespace TerminalApp diff --git a/src/cascadia/TerminalApp/TerminalSettingsCache.cpp b/src/cascadia/TerminalApp/TerminalSettingsCache.cpp index c6b66169f3..3a32472eb4 100644 --- a/src/cascadia/TerminalApp/TerminalSettingsCache.cpp +++ b/src/cascadia/TerminalApp/TerminalSettingsCache.cpp @@ -3,7 +3,6 @@ #include "pch.h" #include "TerminalSettingsCache.h" -#include "TerminalSettingsCache.g.cpp" namespace winrt { diff --git a/src/cascadia/TerminalApp/TerminalSettingsCache.h b/src/cascadia/TerminalApp/TerminalSettingsCache.h index ec1cd42c65..71fb4282ae 100644 --- a/src/cascadia/TerminalApp/TerminalSettingsCache.h +++ b/src/cascadia/TerminalApp/TerminalSettingsCache.h @@ -13,14 +13,13 @@ Abstract: --*/ #pragma once -#include "TerminalSettingsCache.g.h" -#include +#include "winrt/Microsoft.Terminal.Settings.Model.h" +#include "winrt/TerminalApp.h" namespace winrt::TerminalApp::implementation { - class TerminalSettingsCache : public TerminalSettingsCacheT + struct TerminalSettingsCache { - public: TerminalSettingsCache(const Microsoft::Terminal::Settings::Model::CascadiaSettings& settings, const TerminalApp::AppKeyBindings& bindings); Microsoft::Terminal::Settings::Model::TerminalSettingsCreateResult TryLookup(const Microsoft::Terminal::Settings::Model::Profile& profile); void Reset(const Microsoft::Terminal::Settings::Model::CascadiaSettings& settings, const TerminalApp::AppKeyBindings& bindings); @@ -31,8 +30,3 @@ namespace winrt::TerminalApp::implementation std::unordered_map> profileGuidSettingsMap; }; } - -namespace winrt::TerminalApp::factory_implementation -{ - BASIC_FACTORY(TerminalSettingsCache); -} diff --git a/src/cascadia/TerminalApp/TerminalSettingsCache.idl b/src/cascadia/TerminalApp/TerminalSettingsCache.idl deleted file mode 100644 index 24d766c187..0000000000 --- a/src/cascadia/TerminalApp/TerminalSettingsCache.idl +++ /dev/null @@ -1,13 +0,0 @@ -import "AppKeyBindings.idl"; - -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. -namespace TerminalApp -{ - [default_interface] runtimeclass TerminalSettingsCache - { - TerminalSettingsCache(Microsoft.Terminal.Settings.Model.CascadiaSettings settings, AppKeyBindings bindings); - Microsoft.Terminal.Settings.Model.TerminalSettingsCreateResult TryLookup(Microsoft.Terminal.Settings.Model.Profile profile); - void Reset(Microsoft.Terminal.Settings.Model.CascadiaSettings settings, AppKeyBindings bindings); - } -} From 7055b99acc4b6a54103ae4a14ad30b8c503d3708 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Tue, 19 Aug 2025 12:13:58 -0700 Subject: [PATCH 2/2] Fix names and types of a few telemetry events (#19257) Fixes a few issues with some telemetry events: - The macro is organized as such: `TraceLoggingX(value, argName, [argDescription])`. A few args had a description set on the spot where the name should be. I added a name for a few of these. - `TraceLoggingBool` --> `TraceLoggingInt32` for `themeChoice` (we shouldn't be casting the evaluated int as a bool; it loses some of the data we care about) - improves the description for `themeChoice` to include information about the legacy values Checked through all our telemetry events and all of the args have a proper name set. We tend to use `TraceLoggingValue` too which automatically figures out the type that's being used, so that's also handled. --- .../CascadiaSettingsSerialization.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp b/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp index f251d3e027..7b2a7f17c9 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp @@ -1324,9 +1324,9 @@ void CascadiaSettings::_researchOnLoad() g_hSettingsModelProvider, "ThemesInUse", TraceLoggingDescription("Data about the themes in use"), - TraceLoggingBool(themeChoice, "Identifier for the theme chosen. 0 is system, 1 is light, 2 is dark, and 3 indicates any custom theme."), - TraceLoggingBool(changedTheme, "True if the user actually changed the theme from the default theme"), - TraceLoggingInt32(numThemes, "Number of themes in the user's settings"), + TraceLoggingInt32(themeChoice, "ThemeClass", "Identifier for the theme chosen. 0 is system (legacySystem = 6), 1 is light (legacyLight = 5), 2 is dark (legacyDark = 4), and 3 indicates any custom theme."), + TraceLoggingBool(changedTheme, "ChangedTheme", "True if the user actually changed the theme from the default theme"), + TraceLoggingInt32(numThemes, "NumberOfThemes", "Number of themes in the user's settings"), TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); @@ -1348,7 +1348,7 @@ void CascadiaSettings::_researchOnLoad() g_hSettingsModelProvider, "SendInputUsage", TraceLoggingDescription("Event emitted upon settings load, containing the number of sendInput actions a user has"), - TraceLoggingInt32(collectSendInput(), "Number of sendInput actions in the user's settings"), + TraceLoggingInt32(collectSendInput(), "NumberOfSendInputActions", "Number of sendInput actions in the user's settings"), TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); @@ -1365,8 +1365,8 @@ void CascadiaSettings::_researchOnLoad() g_hSettingsModelProvider, "MarksProfilesUsage", TraceLoggingDescription("Event emitted upon settings load, containing the number of profiles opted-in to scrollbar marks"), - TraceLoggingInt32(totalAutoMark, "Number of profiles for which AutoMarkPrompts is enabled"), - TraceLoggingInt32(totalShowMarks, "Number of profiles for which ShowMarks is enabled"), + TraceLoggingInt32(totalAutoMark, "NumberOfAutoMarkPromptsProfiles", "Number of profiles for which AutoMarkPrompts is enabled"), + TraceLoggingInt32(totalShowMarks, "NumberOfShowMarksProfiles", "Number of profiles for which ShowMarks is enabled"), TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES), TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage)); }