CmdPal: Wire up ILogger<T> in ShellViewModel and PowerToysRootPageService

Agent-Logs-Url: https://github.com/microsoft/PowerToys/sessions/07d975df-d306-4734-8ba5-4b1f0784d79a

Co-authored-by: crutkas <1462282+crutkas@users.noreply.github.com>

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Microsoft.CmdPal.UI.ViewModels.csproj

@@ -23,6 +23,7 @@
     <PackageReference Include="CommunityToolkit.Common" />
     <PackageReference Include="CommunityToolkit.Mvvm" />
     <PackageReference Include="AdaptiveCards.Templating" />
+    <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" />
     <PackageReference Include="CommunityToolkit.WinUI.Extensions" />
     <PackageReference Include="Microsoft.Bot.AdaptiveExpressions.Core" />
     <PackageReference Include="AdaptiveCards.ObjectModel.WinUI3" GeneratePathProperty="true">

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

@@ -6,11 +6,11 @@ using System.ComponentModel;
 using CommunityToolkit.Mvvm.ComponentModel;
 using CommunityToolkit.Mvvm.Input;
 using CommunityToolkit.Mvvm.Messaging;
-using Microsoft.CmdPal.Common;
 using Microsoft.CmdPal.UI.ViewModels.Messages;
 using Microsoft.CmdPal.UI.ViewModels.Models;
 using Microsoft.CmdPal.ViewModels.Messages;
 using Microsoft.CommandPalette.Extensions;
+using Microsoft.Extensions.Logging;
 
 namespace Microsoft.CmdPal.UI.ViewModels;
 
@@ -24,6 +24,7 @@ public partial class ShellViewModel : ObservableObject,
     private readonly IAppHostService _appHostService;
     private readonly TaskScheduler _scheduler;
     private readonly IPageViewModelFactoryService _pageViewModelFactory;
+    private readonly ILogger<ShellViewModel> _logger;
     private readonly Lock _invokeLock = new();
     private Task? _handleInvokeTask;
 
@@ -63,7 +64,7 @@ public partial class ShellViewModel : ObservableObject,
                     }
                     catch (Exception ex)
                     {
-                        CoreLogger.LogError(ex.ToString());
+                        _logger.LogError(ex, "Failed to dispose previous page ViewModel");
                     }
                 }
             }
@@ -91,12 +92,14 @@ public partial class ShellViewModel : ObservableObject,
         TaskScheduler scheduler,
         IRootPageService rootPageService,
         IPageViewModelFactoryService pageViewModelFactory,
-        IAppHostService appHostService)
+        IAppHostService appHostService,
+        ILogger<ShellViewModel> logger)
     {
         _pageViewModelFactory = pageViewModelFactory;
         _scheduler = scheduler;
         _rootPageService = rootPageService;
         _appHostService = appHostService;
+        _logger = logger;
 
         NullPage = new NullPageViewModel(_scheduler, appHostService.GetDefaultHost());
         _currentPage = new LoadingPageViewModel(null, _scheduler, appHostService.GetDefaultHost());
@@ -167,7 +170,7 @@ public partial class ShellViewModel : ObservableObject,
                     {
                         if (viewModel.InitializeCommand.ExecutionTask.Exception is AggregateException ex)
                         {
-                            CoreLogger.LogError(ex.ToString());
+                            _logger.LogError(ex, "Failed to initialize page ViewModel");
                         }
                     }
                     else
@@ -185,7 +188,7 @@ public partial class ShellViewModel : ObservableObject,
                                         }
                                         catch (Exception ex)
                                         {
-                                            CoreLogger.LogError(ex.ToString());
+                                            _logger.LogError(ex, "Failed to dispose page ViewModel after navigation cancellation");
                                         }
                                     }
 
@@ -215,7 +218,7 @@ public partial class ShellViewModel : ObservableObject,
                     }
                     catch (Exception ex)
                     {
-                        CoreLogger.LogError(ex.ToString());
+                        _logger.LogError(ex, "Failed to dispose page ViewModel after navigation cancellation");
                     }
                 }
 
@@ -245,7 +248,7 @@ public partial class ShellViewModel : ObservableObject,
             }
             catch (Exception ex)
             {
-                CoreLogger.LogError(ex.ToString());
+                _logger.LogError(ex, "Failed to cancel previous navigation");
             }
             finally
             {
@@ -282,7 +285,7 @@ public partial class ShellViewModel : ObservableObject,
         {
             if (command is IPage page)
             {
-                CoreLogger.LogDebug($"Navigating to page");
+                _logger.LogDebug("Navigating to page");
 
                 _isNested = !isMainPage;
                 _currentlyTransient = message.TransientPage;
@@ -301,7 +304,7 @@ public partial class ShellViewModel : ObservableObject,
                 var pageViewModel = _pageViewModelFactory.TryCreatePageViewModel(page, _isNested, host!, providerContext);
                 if (pageViewModel is null)
                 {
-                    CoreLogger.LogError($"Failed to create ViewModel for page {page.GetType().Name}");
+                    _logger.LogError("Failed to create ViewModel for page {PageType}", page.GetType().Name);
                     throw new NotSupportedException();
                 }
 
@@ -335,7 +338,7 @@ public partial class ShellViewModel : ObservableObject,
             }
             else if (command is IInvokableCommand invokable)
             {
-                CoreLogger.LogDebug($"Invoking command");
+                _logger.LogDebug("Invoking command");
 
                 WeakReferenceMessenger.Default.Send<TelemetryBeginInvokeMessage>();
                 StartInvoke(message, invokable, host);
@@ -421,7 +424,7 @@ public partial class ShellViewModel : ObservableObject,
         }
 
         var kind = result.Kind;
-        CoreLogger.LogDebug($"handling {kind.ToString()}");
+        _logger.LogDebug("Handling command result {Kind}", kind);
 
         WeakReferenceMessenger.Default.Send<TelemetryInvokeResultMessage>(new(kind));
         switch (kind)

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

@@ -4,13 +4,13 @@
 
 using System.Runtime.InteropServices;
 using System.Runtime.Versioning;
-using ManagedCommon;
 using Microsoft.CmdPal.Common.Services;
 using Microsoft.CmdPal.Common.Text;
 using Microsoft.CmdPal.UI.ViewModels;
 using Microsoft.CmdPal.UI.ViewModels.MainPage;
 using Microsoft.CmdPal.UI.ViewModels.Services;
 using Microsoft.CommandPalette.Extensions;
+using Microsoft.Extensions.Logging;
 using WinRT;
 
 // To learn more about WinUI, the WinUI project structure,
@@ -20,13 +20,15 @@ namespace Microsoft.CmdPal.UI;
 internal sealed class PowerToysRootPageService : IRootPageService
 {
     private readonly TopLevelCommandManager _tlcManager;
+    private readonly ILogger<PowerToysRootPageService> _logger;
 
     private IExtensionWrapper? _activeExtension;
     private Lazy<MainListPage> _mainListPage;
 
-    public PowerToysRootPageService(TopLevelCommandManager topLevelCommandManager, AliasManager aliasManager, IFuzzyMatcherProvider fuzzyMatcherProvider, ISettingsService settingsService, IAppStateService appStateService)
+    public PowerToysRootPageService(TopLevelCommandManager topLevelCommandManager, AliasManager aliasManager, IFuzzyMatcherProvider fuzzyMatcherProvider, ISettingsService settingsService, IAppStateService appStateService, ILogger<PowerToysRootPageService> logger)
     {
         _tlcManager = topLevelCommandManager;
+        _logger = logger;
 
         _mainListPage = new Lazy<MainListPage>(() =>
         {
@@ -67,8 +69,7 @@ internal sealed class PowerToysRootPageService : IRootPageService
         }
         catch (Exception ex)
         {
-            Logger.LogError("Failed to update history in PowerToysRootPageService");
-            Logger.LogError(ex.ToString());
+            _logger.LogError(ex, "Failed to update history in PowerToysRootPageService");
         }
     }
 
@@ -109,13 +110,13 @@ internal sealed class PowerToysRootPageService : IRootPageService
                         var hr = Native.CoAllowSetForegroundWindow(intPtr);
                         if (hr != 0)
                         {
-                            Logger.LogWarning($"Error giving foreground rights: 0x{hr.Value:X8}");
+                            _logger.LogWarning("Error giving foreground rights: 0x{HRESULT:X8}", hr.Value);
                         }
                     }
                 }
                 catch (Exception ex)
                 {
-                    ManagedCommon.Logger.LogError(ex.ToString());
+                    _logger.LogError(ex, "Failed to grant foreground window rights to extension");
                 }
             }
         }