CmdPal: Refactoring DI, Logging, and Extension Services by michaeljolley · Pull Request #45437 · microsoft/PowerToys

michaeljolley · 2026-02-05T20:42:06Z

IMPORTANT: This is a draft for a reason. You do not want to unintentionally make this real. Let's be honest, you probably don't want this real intentionally.

This is a freaking huge PR. In it we:

Refactor our usage of Dependency Injection to support not passing the service provider through a property on the App (ala App.Current.Services). Rather, services and models are injected into their requesting classes & controls via their constructor.
In previous versions, logging was handled in 3 different ways. With the new DI in place, all logging uses the .NET Microsoft.Extensions.Logging ILogger interface and a logger is injected into anything that might need to log something.
Modified how extensions are loaded. Rather than loading built-ins in the TopLevelCommandManager and, from there, loading 3P extensions in a combination of the TLCM and the ExtensionsService, we now register IExtensionServices and TLCM uses any that are provided to load extensions & commands. This includes a new BuiltInExtensionService and WinRTExtensionService to load built-ins and 3P extension respectively. In the future, we hope to extend this to support extensions written in other languages, ala JsonRPCExtensionService.
Added a new PersistenceService to be the backer of all services that need to persist data (e.g. SettingsService, AppStateService, PersonalizationService). It provides consolidated and generic methods for saving and loading persisted objects in JSON.
Removes .Core. All those viewmodels, etc. are now moved to either the Common, UI, or UI.ViewModels projects.

TODO

Some bug with settings
Additional testing

- Move project from Core/ subfolder to cmdpal/ root - Update namespace from Microsoft.CmdPal.Core.Common to Microsoft.CmdPal.Common - Update all project references and using statements - Update solution files (PowerToys.slnx, CommandPalette.slnf)

…wModels - Move all source files from Core.ViewModels to UI.ViewModels - Update namespace from Microsoft.CmdPal.Core.ViewModels to Microsoft.CmdPal.UI.ViewModels - Remove Core.ViewModels project from solution files - Update all using statements in dependent projects - Delete the Core folder which is now empty

- Extract persistence logic into PersistenceService - Add AppStateService to manage app state - Add SettingsService to manage settings - Add JsonSerializationContext for AOT-compatible JSON - Add EscapeKeyBehavior and MonitorBehavior enums

UI.ViewModels: - CommandSettingsViewModel: Add ILogger injection - CommandItemViewModel: Add ILogger with LoggerMessage methods - ContextMenuViewModel: Add ILogger injection - ExtensionObjectViewModel: Add Logger property with NullLogger default - ShellViewModel: Replace CoreLogger calls with LoggerMessage methods - UpdateCommandBarMessage: Remove logging from interface default method UI helpers/services: - WallpaperHelper: Add ILogger injection with LoggerMessage methods - LocalKeyboardListener: Add ILogger injection with LoggerMessage methods - ImageProvider: Add ILogger injection with LoggerMessage methods - ThemeService: Add ILogger injection with LoggerMessage methods Note: UI project migration is partial - XAML code-behind files still use static Logger

- Update all ViewModels to receive SettingsService via DI - Implement IDisposable pattern for proper event cleanup - Add App.Services property for DI access where needed - Restore GetProviderSettings/GetGlobalFallbacks to SettingsModel - Fix CommandProviderWrapper and TopLevelCommandManager logging - Update all Settings pages to use SettingsService - Add DefaultAppExtensionHost to replace CommandPaletteHost.Instance - Add parameterless constructors for XAML-instantiated types - Fix various ILogger injection issues throughout UI layer

src/modules/cmdpal/Microsoft.CmdPal.UI/Pages/ShellPage.xaml.cs

+
+    [LoggerMessage(
+        Level = LogLevel.Warning,
+        Message = "Unrecognized target for shell navigation: {paramter}")]


src/modules/cmdpal/Microsoft.CmdPal.UI/Pages/ShellPage.xaml.cs

+    [LoggerMessage(
+        Level = LogLevel.Warning,
+        Message = "Unrecognized target for shell navigation: {paramter}")]
+    partial void Log_UnrecognizedShellNavigationTargetWarning(object paramter);


jiripolasek · 2026-02-05T21:09:47Z

Holy squirrel!

zadjii-msft

okay I'm at 232/236

Most of this is fairly straightforward - I just haven't gotten to the extension service & TLVM stuff yet.

Probably would have been easier as like, 4 diffs:

consolidate the core namespace
do the settings -> persistence service refactor
do the TLVM & extension refactor
do the remaining DI refactoring

but beggars can't be choosers. I'm not sure we ever linked an issue to #38831, but I'm including that as an x-link of a previous attempt here

zadjii-msft · 2026-02-09T12:38:52Z

src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/AppearancePage.xaml.cs

+
+    private void BindScreenPreview(ScreenPreview screenPreview)
+    {
+        // Create CommandPalettePreview and set up bindings


can we just move the ViewModel = settingsViewModel; line above the InitializeComponent() call, so that we can still bind this in XAML (rather than codebehind?)

zadjii-msft · 2026-02-09T12:43:23Z

src/modules/cmdpal/Microsoft.CmdPal.UI/App.xaml.cs

+        // Core services
+        services.AddSingleton<IExtensionService, BuiltInExtensionService>();
+
+        // services.AddSingleton<IExtensionService, WinRTExtensionService>();


zadjii-msft · 2026-02-09T12:50:40Z

src/modules/cmdpal/Microsoft.CmdPal.UI/MainWindow.xaml.cs

can this stay static?

zadjii-msft · 2026-02-09T12:51:33Z

src/modules/cmdpal/Microsoft.CmdPal.UI/PowerToysRootPageService.cs

+        // new Lazy<MainListPage>(() =>
+        // {
+        // return new MainListPage(_tlcManager, settingsService, aliasManager, appStateService, _logger);
+        // });


zadjii-msft · 2026-02-09T12:52:00Z

src/modules/cmdpal/Microsoft.CmdPal.UI/PowerToysRootPageService.cs

    private readonly TopLevelCommandManager _tlcManager;

+    private readonly MainListPage _mainListPage; // Lazy<MainListPage> _mainListPage;


dead comments

zadjii-msft · 2026-02-09T12:57:08Z

src/modules/cmdpal/Microsoft.CmdPal.UI/ExtViews/ContentPage.xaml.cs

+        var markdownConfig = new MarkdownConfig()
+        {
+            ImageProvider = imageProvider,
+            Themes = _markdownThemes,
+        };
+
+        // Add the MarkdownConfig as a resource so DataTemplates can reference it
+        Resources["DefaultMarkdownConfig"] = markdownConfig;


why did we move all of this out of XAML into codebehind?

zadjii-msft · 2026-02-09T12:57:22Z

src/modules/cmdpal/Microsoft.CmdPal.UI/ExtViews/ContentPage.xaml.cs

+
+        // Add the MarkdownConfig as a resource so DataTemplates can reference it
+        Resources["DefaultMarkdownConfig"] = markdownConfig;
+


is this just duplicated work now?

zadjii-msft · 2026-02-11T12:29:14Z

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/AliasManager.cs

-                {
-                    topLevelCommand.AliasText = string.Empty;
-                }
+                // var topLevelCommand = _topLevelCommandManager.LookupCommand(kv.Value.CommandId);


zadjii-msft · 2026-02-11T12:29:24Z

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/AliasManager.cs

-                    WeakReferenceMessenger.Default.Send<PerformCommandMessage>(topLevelCommand.GetPerformCommandMessage());
-                    return true;
-                }
+                // var topLevelCommand = _topLevelCommandManager.LookupCommand(alias.CommandId);


📝 wait where did this actual implementation go?)

zadjii-msft · 2026-02-11T12:29:49Z

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/AliasManager.cs

 public partial class AliasManager : ObservableObject
 {
-    private readonly TopLevelCommandManager _topLevelCommandManager;
+    // private readonly TopLevelCommandManager _topLevelCommandManager;


(dead code here and on on L17 & L20 too)

jiripolasek · 2026-02-12T22:36:55Z

src/modules/cmdpal/Microsoft.CmdPal.Common/CmdPalLogger.cs

+
+                if (exception is not null)
+                {
+                    Logger.LogError(exception.Message, exception);


I don't like this, as a developer I know what severity I want to use. The logger has no clue about the context.

jiripolasek · 2026-02-12T22:49:13Z

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/AliasManager.cs

    {
-        _topLevelCommandManager = tlcManager;
-        _aliases = settings.Aliases;
+        // _topLevelCommandManager = tlcManager;


jiripolasek · 2026-02-12T22:54:16Z

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/AppExtensionHost.cs

@@ -60,7 +67,7 @@ public IAsyncAction LogMessage(ILogMessage? message)
            return Task.CompletedTask.AsAsyncAction();
        }

-        CoreLogger.LogDebug(message.Message);
+        Log_Message(message.Message);


This is hilarious :D

jiripolasek · 2026-02-12T23:20:17Z

src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/SearchBar.xaml.cs

@@ -51,7 +52,7 @@ public sealed partial class SearchBar : UserControl,
    // 0.6+ suggestions
    private string? _textToSuggest;

-    private SettingsModel Settings => App.Current.Services.GetRequiredService<SettingsModel>();
+    private SettingsModel settings;


michaeljolley · 2026-02-13T20:37:51Z

Thanks for the reviews. For clarity, this was not ready for review. 😄

Much clean-up and testing to do before the draft status is removed.

michaeljolley and others added 14 commits January 26, 2026 15:52

Rename CoreLogger to CmdPalLogger

b5ae2ef

Use ILogger in AppExtensionHost instead of CoreLogger

81aeb74

Working on DI

303be86

Working on DI

fa4bc0a

Working on DI

94a699d

Working on DI

54c5d7b

Working on DI

10f953f

DI

d3d39f9

I don't know what I've done. Send help... or Jiri

2851838

github-advanced-security bot found potential problems Feb 5, 2026

View reviewed changes

Working on DI

99a6c8c

jiripolasek added the Product-Command Palette Refers to the Command Palette utility label Feb 5, 2026

michaeljolley added 3 commits February 9, 2026 13:21

WinRTExtensionService

8267166

WinRTExtensionService

54d1ab3

Fixing spelling

12bfa13

zadjii-msft reviewed Feb 11, 2026

View reviewed changes

zadjii-msft marked this pull request as ready for review February 12, 2026 19:37

jiripolasek reviewed Feb 12, 2026

View reviewed changes

michaeljolley marked this pull request as draft February 13, 2026 20:28

Merging main (broken)

c7f8ac0

		private readonly TopLevelCommandManager _tlcManager;

		private readonly MainListPage _mainListPage; // Lazy<MainListPage> _mainListPage;


		// Add the MarkdownConfig as a resource so DataTemplates can reference it
		Resources["DefaultMarkdownConfig"] = markdownConfig;

Conversation

michaeljolley commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODO

Uh oh!

Check failure

Check failure

jiripolasek commented Feb 5, 2026

Uh oh!

zadjii-msft left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jiripolasek Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michaeljolley commented Feb 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

michaeljolley commented Feb 5, 2026 •

edited

Loading

jiripolasek Feb 12, 2026 •

edited

Loading