From f78ec0c8e5ea538da32df64e3528018b06796bc6 Mon Sep 17 00:00:00 2001 From: Michael Jolley Date: Tue, 27 Jan 2026 14:05:21 -0600 Subject: [PATCH] Migrate CoreLogger to ILogger with LoggerMessage pattern (partial) 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 --- .../CommandItemViewModel.cs | 24 +++++++++--- .../CommandSettingsViewModel.cs | 27 +++++++++++--- .../Commands/MainListPage.cs | 14 ++++--- .../ContentFormViewModel.cs | 3 +- .../ContextMenuViewModel.cs | 12 ++++-- .../ExtensionObjectViewModel.cs | 19 +++++++++- .../Messages/UpdateCommandBarMessage.cs | 9 ++--- .../ShellViewModel.cs | 37 +++++++++++++++---- .../Helpers/LocalKeyboardListener.cs | 20 ++++++++-- .../MarkdownImageProviders/ImageProvider.cs | 13 ++++++- .../Helpers/WallpaperHelper.cs | 32 ++++++++++++---- .../PowerToysRootPageService.cs | 4 +- .../Microsoft.CmdPal.UI/RunHistoryService.cs | 22 ++++++----- .../Services/ThemeService.cs | 11 ++++-- 14 files changed, 181 insertions(+), 66 deletions(-) diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandItemViewModel.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandItemViewModel.cs index 65e707fc48..da95abda86 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandItemViewModel.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandItemViewModel.cs @@ -3,11 +3,12 @@ // See the LICENSE file in the project root for more information. using System.Diagnostics.CodeAnalysis; -using Microsoft.CmdPal.Common; using Microsoft.CmdPal.UI.ViewModels.Messages; using Microsoft.CmdPal.UI.ViewModels.Models; using Microsoft.CommandPalette.Extensions; using Microsoft.CommandPalette.Extensions.Toolkit; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using Windows.ApplicationModel.DataTransfer; namespace Microsoft.CmdPal.UI.ViewModels; @@ -15,6 +16,9 @@ namespace Microsoft.CmdPal.UI.ViewModels; [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] public partial class CommandItemViewModel : ExtensionObjectViewModel, ICommandBarContext { + // Local logger field required for [LoggerMessage] source generator + private readonly ILogger _logger; + public ExtensionObject Model => _commandItemModel; private ExtensionObject? ExtendedAttributesProvider { get; set; } @@ -91,9 +95,10 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel, ICommandBa _errorIcon.InitializeProperties(); } - public CommandItemViewModel(ExtensionObject item, WeakReference errorContext) + public CommandItemViewModel(ExtensionObject item, WeakReference errorContext, ILogger? logger = null) : base(errorContext) { + _logger = logger ?? NullLogger.Instance; _commandItemModel = item; Command = new(null, errorContext); } @@ -243,7 +248,7 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel, ICommandBa } catch (Exception ex) { - CoreLogger.LogError("error fast initializing CommandItemViewModel", ex); + Log_FastInitError(ex); Command = new(null, PageContext); _itemTitle = "Error"; Subtitle = "Item failed to load"; @@ -265,7 +270,7 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel, ICommandBa catch (Exception ex) { Initialized |= InitializedState.Error; - CoreLogger.LogError("error slow initializing CommandItemViewModel", ex); + Log_SlowInitError(ex); } return false; @@ -280,7 +285,7 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel, ICommandBa } catch (Exception ex) { - CoreLogger.LogError("error initializing CommandItemViewModel", ex); + Log_InitError(ex); Command = new(null, PageContext); _itemTitle = "Error"; Subtitle = "Item failed to load"; @@ -489,6 +494,15 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel, ICommandBa base.SafeCleanup(); Initialized |= InitializedState.CleanedUp; } + + [LoggerMessage(Level = LogLevel.Error, Message = "Error fast initializing CommandItemViewModel")] + partial void Log_FastInitError(Exception ex); + + [LoggerMessage(Level = LogLevel.Error, Message = "Error slow initializing CommandItemViewModel")] + partial void Log_SlowInitError(Exception ex); + + [LoggerMessage(Level = LogLevel.Error, Message = "Error initializing CommandItemViewModel")] + partial void Log_InitError(Exception ex); } [Flags] diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandSettingsViewModel.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandSettingsViewModel.cs index c59c7f9b6a..9d3f1bd5f1 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandSettingsViewModel.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandSettingsViewModel.cs @@ -2,16 +2,28 @@ // The Microsoft Corporation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using Microsoft.CmdPal.Common; using Microsoft.CmdPal.UI.ViewModels; using Microsoft.CmdPal.UI.ViewModels.Models; using Microsoft.CommandPalette.Extensions; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; namespace Microsoft.CmdPal.UI.ViewModels; -public partial class CommandSettingsViewModel(ICommandSettings? _unsafeSettings, CommandProviderWrapper provider, TaskScheduler mainThread) +public partial class CommandSettingsViewModel { - private readonly ExtensionObject _model = new(_unsafeSettings); + private readonly ILogger _logger; + private readonly ExtensionObject _model; + private readonly CommandProviderWrapper _provider; + private readonly TaskScheduler _mainThread; + + public CommandSettingsViewModel(ICommandSettings? unsafeSettings, CommandProviderWrapper provider, TaskScheduler mainThread, ILogger? logger = null) + { + _model = new(unsafeSettings); + _provider = provider; + _mainThread = mainThread; + _logger = logger ?? NullLogger.Instance; + } public ContentPageViewModel? SettingsPage { get; private set; } @@ -31,7 +43,7 @@ public partial class CommandSettingsViewModel(ICommandSettings? _unsafeSettings, if (model.SettingsPage is not null) { - SettingsPage = new CommandPaletteContentPageViewModel(model.SettingsPage, mainThread, provider.ExtensionHost); + SettingsPage = new CommandPaletteContentPageViewModel(model.SettingsPage, _mainThread, _provider.ExtensionHost); SettingsPage.InitializeProperties(); } } @@ -44,7 +56,7 @@ public partial class CommandSettingsViewModel(ICommandSettings? _unsafeSettings, } catch (Exception ex) { - CoreLogger.LogError($"Failed to load settings page", ex: ex); + Log_FailedToLoadSettingsPage(ex); } Initialized = true; @@ -56,6 +68,9 @@ public partial class CommandSettingsViewModel(ICommandSettings? _unsafeSettings, action, CancellationToken.None, TaskCreationOptions.None, - mainThread); + _mainThread); } + + [LoggerMessage(Level = LogLevel.Error, Message = "Failed to load settings page")] + partial void Log_FailedToLoadSettingsPage(Exception ex); } diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/MainListPage.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/MainListPage.cs index e8ed2f1626..354ad5dd33 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/MainListPage.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/MainListPage.cs @@ -30,7 +30,7 @@ public partial class MainListPage : DynamicListPage, private readonly TopLevelCommandManager _tlcManager; private readonly AliasManager _aliasManager; private readonly SettingsModel _settings; - private readonly AppStateModel _appStateModel; + private readonly AppStateService _appStateService; private List>? _filteredItems; private List>? _filteredApps; @@ -47,7 +47,9 @@ public partial class MainListPage : DynamicListPage, private CancellationTokenSource? _cancellationTokenSource; - public MainListPage(TopLevelCommandManager topLevelCommandManager, SettingsModel settings, AliasManager aliasManager, AppStateModel appStateModel) + private AppStateModel AppState => _appStateService.CurrentSettings; + + public MainListPage(TopLevelCommandManager topLevelCommandManager, SettingsModel settings, AliasManager aliasManager, AppStateService appStateService) { Title = Resources.builtin_home_name; Icon = IconHelpers.FromRelativePath("Assets\\StoreLogo.scale-200.png"); @@ -55,7 +57,7 @@ public partial class MainListPage : DynamicListPage, _settings = settings; _aliasManager = aliasManager; - _appStateModel = appStateModel; + _appStateService = appStateService; _tlcManager = topLevelCommandManager; _tlcManager.PropertyChanged += TlcManager_PropertyChanged; _tlcManager.TopLevelCommands.CollectionChanged += Commands_CollectionChanged; @@ -375,7 +377,7 @@ public partial class MainListPage : DynamicListPage, } } - var history = _appStateModel.RecentCommands!; + var history = AppState.RecentCommands!; Func scoreItem = (a, b) => { return ScoreTopLevelItem(a, b, history); }; // Produce a list of everything that matches the current filter. @@ -593,9 +595,9 @@ public partial class MainListPage : DynamicListPage, public void UpdateHistory(IListItem topLevelOrAppItem) { var id = IdForTopLevelOrAppItem(topLevelOrAppItem); - var history = _appStateModel.RecentCommands; + var history = AppState.RecentCommands; history.AddHistoryItem(id); - AppStateModel.SaveState(_appStateModel); + _appStateService.SaveSettings(AppState); } private static string IdForTopLevelOrAppItem(IListItem topLevelOrAppItem) diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ContentFormViewModel.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ContentFormViewModel.cs index 9872f707bc..b9497b7588 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ContentFormViewModel.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ContentFormViewModel.cs @@ -7,7 +7,6 @@ using System.Text.Json; using AdaptiveCards.ObjectModel.WinUI3; using AdaptiveCards.Templating; using CommunityToolkit.Mvvm.Messaging; -using ManagedCommon; using Microsoft.CmdPal.UI.ViewModels; using Microsoft.CmdPal.UI.ViewModels.Messages; using Microsoft.CmdPal.UI.ViewModels.Models; @@ -52,7 +51,7 @@ public partial class ContentFormViewModel(IFormContent _form, WeakReference { + private readonly ILogger _logger; + public ICommandBarContext? SelectedItem { get => field; @@ -39,8 +41,9 @@ public partial class ContextMenuViewModel : ObservableObject, private string _lastSearchText = string.Empty; - public ContextMenuViewModel() + public ContextMenuViewModel(ILogger logger) { + _logger = logger; WeakReferenceMessenger.Default.Register(this); } @@ -141,7 +144,7 @@ public partial class ContextMenuViewModel : ObservableObject, var added = result.TryAdd(key, cmd); if (!added) { - CoreLogger.LogWarning($"Ignoring duplicate keyboard shortcut {KeyChordHelpers.FormatForDebug(key)} on command '{cmd.Title ?? cmd.Name ?? "(unknown)"}'"); + Log_DuplicateKeyboardShortcut(KeyChordHelpers.FormatForDebug(key), cmd.Title ?? cmd.Name ?? "(unknown)"); } } } @@ -223,4 +226,7 @@ public partial class ContextMenuViewModel : ObservableObject, return ContextKeybindingResult.Hide; } } + + [LoggerMessage(Level = LogLevel.Warning, Message = "Ignoring duplicate keyboard shortcut {KeyChord} on command '{CommandName}'")] + partial void Log_DuplicateKeyboardShortcut(string keyChord, string commandName); } diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ExtensionObjectViewModel.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ExtensionObjectViewModel.cs index 71fbf25505..5f4a7d7cbc 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ExtensionObjectViewModel.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ExtensionObjectViewModel.cs @@ -3,12 +3,24 @@ // See the LICENSE file in the project root for more information. using CommunityToolkit.Mvvm.ComponentModel; -using Microsoft.CmdPal.Common; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; namespace Microsoft.CmdPal.UI.ViewModels; public abstract partial class ExtensionObjectViewModel : ObservableObject { + private ILogger _logger = NullLogger.Instance; + + /// + /// Gets or sets the logger for this ViewModel. Subclasses can set this in their constructor. + /// + protected ILogger Logger + { + get => _logger; + set => _logger = value; + } + public WeakReference PageContext { get; set; } internal ExtensionObjectViewModel(IPageContext? context) @@ -114,7 +126,10 @@ public abstract partial class ExtensionObjectViewModel : ObservableObject } catch (Exception ex) { - CoreLogger.LogDebug(ex.ToString()); + Log_CleanupException(ex); } } + + [LoggerMessage(Level = LogLevel.Debug, Message = "Exception during cleanup")] + partial void Log_CleanupException(Exception ex); } diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Messages/UpdateCommandBarMessage.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Messages/UpdateCommandBarMessage.cs index ab97a178fa..2868e3d744 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Messages/UpdateCommandBarMessage.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Messages/UpdateCommandBarMessage.cs @@ -3,7 +3,6 @@ // See the LICENSE file in the project root for more information. using System.ComponentModel; -using Microsoft.CmdPal.Common; using Microsoft.CommandPalette.Extensions; using Microsoft.CommandPalette.Extensions.Toolkit; @@ -47,11 +46,9 @@ public interface IContextMenuContext : INotifyPropertyChanged if (item is CommandContextItemViewModel cmd && cmd.HasRequestedShortcut) { var key = cmd.RequestedShortcut ?? new KeyChord(0, 0, 0); - var added = result.TryAdd(key, cmd); - if (!added) - { - CoreLogger.LogWarning($"Ignoring duplicate keyboard shortcut {KeyChordHelpers.FormatForDebug(key)} on command '{cmd.Title ?? cmd.Name ?? "(unknown)"}'"); - } + + // Silently ignore duplicate shortcuts - ContextMenuViewModel logs these + result.TryAdd(key, cmd); } } diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ShellViewModel.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ShellViewModel.cs index 946bd38215..0c367a0596 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ShellViewModel.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ShellViewModel.cs @@ -166,7 +166,7 @@ public partial class ShellViewModel : ObservableObject, { if (viewModel.InitializeCommand.ExecutionTask.Exception is AggregateException ex) { - CoreLogger.LogError(ex.ToString()); + Log_PageViewModelInitError(ex); } } else @@ -184,7 +184,7 @@ public partial class ShellViewModel : ObservableObject, } catch (Exception ex) { - CoreLogger.LogError(ex.ToString()); + Log_DisposeError(ex); } } @@ -214,7 +214,7 @@ public partial class ShellViewModel : ObservableObject, } catch (Exception ex) { - CoreLogger.LogError(ex.ToString()); + Log_DisposeError(ex); } } @@ -244,7 +244,7 @@ public partial class ShellViewModel : ObservableObject, } catch (Exception ex) { - CoreLogger.LogError(ex.ToString()); + Log_CancelNavigationError(ex); } finally { @@ -268,7 +268,7 @@ public partial class ShellViewModel : ObservableObject, { if (command is IPage page) { - CoreLogger.LogDebug($"Navigating to page"); + Log_NavigatingToPage(); var isMainPage = command == _rootPage; _isNested = !isMainPage; @@ -287,7 +287,7 @@ public partial class ShellViewModel : ObservableObject, var pageViewModel = _pageViewModelFactory.TryCreatePageViewModel(page, _isNested, host!); if (pageViewModel is null) { - CoreLogger.LogError($"Failed to create ViewModel for page {page.GetType().Name}"); + Log_FailedToCreatePageViewModel(page.GetType().Name); throw new NotSupportedException(); } @@ -317,7 +317,7 @@ public partial class ShellViewModel : ObservableObject, } else if (command is IInvokableCommand invokable) { - CoreLogger.LogDebug($"Invoking command"); + Log_InvokingCommand(); WeakReferenceMessenger.Default.Send(); StartInvoke(message, invokable, host); @@ -403,7 +403,7 @@ public partial class ShellViewModel : ObservableObject, } var kind = result.Kind; - CoreLogger.LogDebug($"handling {kind.ToString()}"); + Log_HandlingCommandResult(kind.ToString()); WeakReferenceMessenger.Default.Send(new(kind)); switch (kind) @@ -507,4 +507,25 @@ public partial class ShellViewModel : ObservableObject, Level = LogLevel.Error, Message = "Error disposing old page")] partial void Log_ErrorDisposingOldPage(Exception ex); + + [LoggerMessage(Level = LogLevel.Error, Message = "Error during page ViewModel initialization")] + partial void Log_PageViewModelInitError(Exception ex); + + [LoggerMessage(Level = LogLevel.Error, Message = "Error disposing ViewModel")] + partial void Log_DisposeError(Exception ex); + + [LoggerMessage(Level = LogLevel.Error, Message = "Error cancelling navigation")] + partial void Log_CancelNavigationError(Exception ex); + + [LoggerMessage(Level = LogLevel.Debug, Message = "Navigating to page")] + partial void Log_NavigatingToPage(); + + [LoggerMessage(Level = LogLevel.Error, Message = "Failed to create ViewModel for page {PageTypeName}")] + partial void Log_FailedToCreatePageViewModel(string pageTypeName); + + [LoggerMessage(Level = LogLevel.Debug, Message = "Invoking command")] + partial void Log_InvokingCommand(); + + [LoggerMessage(Level = LogLevel.Debug, Message = "Handling command result: {ResultKind}")] + partial void Log_HandlingCommandResult(string resultKind); } diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/LocalKeyboardListener.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/LocalKeyboardListener.cs index 788127a5d1..05aa31c83e 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/LocalKeyboardListener.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/LocalKeyboardListener.cs @@ -2,8 +2,7 @@ // The Microsoft Corporation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using ManagedCommon; - +using Microsoft.Extensions.Logging; using Windows.System; using Windows.Win32; using Windows.Win32.Foundation; @@ -16,6 +15,8 @@ namespace Microsoft.CmdPal.UI.Helpers; /// internal sealed partial class LocalKeyboardListener : IDisposable { + private readonly ILogger _logger; + /// /// Event that is raised when a key is pressed down. /// @@ -25,6 +26,11 @@ internal sealed partial class LocalKeyboardListener : IDisposable private UnhookWindowsHookExSafeHandle? _handle; private HOOKPROC? _hookProc; // Keep reference to prevent GC collection + public LocalKeyboardListener(ILogger logger) + { + _logger = logger; + } + /// /// Registers a global keyboard hook to listen for key down events. /// @@ -68,7 +74,7 @@ internal sealed partial class LocalKeyboardListener : IDisposable } catch (Exception ex) { - Logger.LogError("Failed to register hook", ex); + Log_FailedToRegisterHook(ex); return false; } } @@ -121,7 +127,7 @@ internal sealed partial class LocalKeyboardListener : IDisposable } catch (Exception ex) { - Logger.LogError("Failed when invoking key down keyboard hook event", ex); + Log_KeyDownHookError(ex); } // Call next hook in chain - pass null as first parameter for current hook @@ -154,4 +160,10 @@ internal sealed partial class LocalKeyboardListener : IDisposable _disposed = true; } } + + [LoggerMessage(Level = LogLevel.Error, Message = "Failed to register hook")] + partial void Log_FailedToRegisterHook(Exception ex); + + [LoggerMessage(Level = LogLevel.Error, Message = "Failed when invoking key down keyboard hook event")] + partial void Log_KeyDownHookError(Exception ex); } diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/MarkdownImageProviders/ImageProvider.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/MarkdownImageProviders/ImageProvider.cs index 6f3d3b446c..ac724cf37a 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/MarkdownImageProviders/ImageProvider.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/MarkdownImageProviders/ImageProvider.cs @@ -3,15 +3,21 @@ // See the LICENSE file in the project root for more information. using CommunityToolkit.WinUI.Controls; -using ManagedCommon; +using Microsoft.Extensions.Logging; using Microsoft.UI.Xaml.Controls; namespace Microsoft.CmdPal.UI.Helpers.MarkdownImageProviders; internal sealed partial class ImageProvider : IImageProvider { + private readonly ILogger _logger; private readonly CompositeImageSourceProvider _compositeProvider = new(); + public ImageProvider(ILogger logger) + { + _logger = logger; + } + public async Task GetImage(string url) { try @@ -31,7 +37,7 @@ internal sealed partial class ImageProvider : IImageProvider } catch (Exception ex) { - Logger.LogError($"Failed to provide an image from URI '{url}'", ex); + Log_FailedToProvideImage(url, ex); return null!; } } @@ -40,4 +46,7 @@ internal sealed partial class ImageProvider : IImageProvider { return _compositeProvider.ShouldUseThisProvider(url); } + + [LoggerMessage(Level = LogLevel.Error, Message = "Failed to provide an image from URI '{Url}'")] + partial void Log_FailedToProvideImage(string url, Exception ex); } diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/WallpaperHelper.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/WallpaperHelper.cs index 9772d33b1d..92b205320e 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/WallpaperHelper.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/WallpaperHelper.cs @@ -5,8 +5,8 @@ using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Runtime.InteropServices.Marshalling; -using ManagedCommon; using ManagedCsWin32; +using Microsoft.Extensions.Logging; using Microsoft.UI; using Microsoft.UI.Xaml.Media.Imaging; using Windows.UI; @@ -18,10 +18,13 @@ namespace Microsoft.CmdPal.UI.Helpers; /// internal sealed partial class WallpaperHelper { + private readonly ILogger _logger; private readonly IDesktopWallpaper? _desktopWallpaper; - public WallpaperHelper() + public WallpaperHelper(ILogger logger) { + _logger = logger; + try { var desktopWallpaper = ComHelper.CreateComInstance( @@ -33,7 +36,7 @@ internal sealed partial class WallpaperHelper catch (Exception ex) { // If COM initialization fails, keep helper usable with safe fallbacks - Logger.LogError("Failed to initialize DesktopWallpaper COM interface", ex); + Log_FailedToInitializeDesktopWallpaper(ex); _desktopWallpaper = null; } } @@ -67,7 +70,7 @@ internal sealed partial class WallpaperHelper } catch (Exception ex) { - Logger.LogError("Failed to query wallpaper path", ex); + Log_FailedToQueryWallpaperPath(ex); } return null; @@ -94,7 +97,7 @@ internal sealed partial class WallpaperHelper } catch (Exception ex) { - Logger.LogError("Failed to load wallpaper color", ex); + Log_FailedToLoadWallpaperColor(ex); return Colors.Black; } } @@ -118,7 +121,7 @@ internal sealed partial class WallpaperHelper var randomAccessStream = stream.AsRandomAccessStream(); if (randomAccessStream == null) { - Logger.LogError("Failed to convert file stream to RandomAccessStream for wallpaper image."); + Log_FailedToConvertStream(); return null; } @@ -127,11 +130,26 @@ internal sealed partial class WallpaperHelper } catch (Exception ex) { - Logger.LogError("Failed to load wallpaper image", ex); + Log_FailedToLoadWallpaperImage(ex); return null; } } + [LoggerMessage(Level = LogLevel.Error, Message = "Failed to initialize DesktopWallpaper COM interface")] + partial void Log_FailedToInitializeDesktopWallpaper(Exception ex); + + [LoggerMessage(Level = LogLevel.Error, Message = "Failed to query wallpaper path")] + partial void Log_FailedToQueryWallpaperPath(Exception ex); + + [LoggerMessage(Level = LogLevel.Error, Message = "Failed to load wallpaper color")] + partial void Log_FailedToLoadWallpaperColor(Exception ex); + + [LoggerMessage(Level = LogLevel.Error, Message = "Failed to convert file stream to RandomAccessStream for wallpaper image")] + partial void Log_FailedToConvertStream(); + + [LoggerMessage(Level = LogLevel.Error, Message = "Failed to load wallpaper image")] + partial void Log_FailedToLoadWallpaperImage(Exception ex); + // blittable type for COM interop [StructLayout(LayoutKind.Sequential)] internal readonly partial struct COLORREF diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/PowerToysRootPageService.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/PowerToysRootPageService.cs index fbb9b56199..fdccc320e5 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/PowerToysRootPageService.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/PowerToysRootPageService.cs @@ -22,13 +22,13 @@ internal sealed class PowerToysRootPageService : IRootPageService private IExtensionWrapper? _activeExtension; private Lazy _mainListPage; - public PowerToysRootPageService(TopLevelCommandManager topLevelCommandManager, SettingsModel settings, AliasManager aliasManager, AppStateModel appStateModel) + public PowerToysRootPageService(TopLevelCommandManager topLevelCommandManager, SettingsModel settings, AliasManager aliasManager, AppStateService appStateService) { _tlcManager = topLevelCommandManager; _mainListPage = new Lazy(() => { - return new MainListPage(_tlcManager, settings, aliasManager, appStateModel); + return new MainListPage(_tlcManager, settings, aliasManager, appStateService); }); } diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/RunHistoryService.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/RunHistoryService.cs index e1ef0cb57f..4f1e4d5274 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/RunHistoryService.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/RunHistoryService.cs @@ -9,27 +9,29 @@ namespace Microsoft.CmdPal.UI; internal sealed class RunHistoryService : IRunHistoryService { - private readonly AppStateModel _appStateModel; + private readonly AppStateService _appStateService; - public RunHistoryService(AppStateModel appStateModel) + private AppStateModel AppState => _appStateService.CurrentSettings; + + public RunHistoryService(AppStateService appStateService) { - _appStateModel = appStateModel; + _appStateService = appStateService; } public IReadOnlyList GetRunHistory() { - if (_appStateModel.RunHistory.Count == 0) + if (AppState.RunHistory.Count == 0) { var history = Microsoft.Terminal.UI.RunHistory.CreateRunHistory(); - _appStateModel.RunHistory.AddRange(history); + AppState.RunHistory.AddRange(history); } - return _appStateModel.RunHistory; + return AppState.RunHistory; } public void ClearRunHistory() { - _appStateModel.RunHistory.Clear(); + AppState.RunHistory.Clear(); } public void AddRunHistoryItem(string item) @@ -40,11 +42,11 @@ internal sealed class RunHistoryService : IRunHistoryService return; // Do not add empty or whitespace items } - _appStateModel.RunHistory.Remove(item); + AppState.RunHistory.Remove(item); // Add the item to the front of the history - _appStateModel.RunHistory.Insert(0, item); + AppState.RunHistory.Insert(0, item); - AppStateModel.SaveState(_appStateModel); + _appStateService.SaveSettings(AppState); } } diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Services/ThemeService.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Services/ThemeService.cs index 65fbfb24d7..220f45bcf8 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Services/ThemeService.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Services/ThemeService.cs @@ -3,10 +3,10 @@ // See the LICENSE file in the project root for more information. using CommunityToolkit.WinUI; -using ManagedCommon; using Microsoft.CmdPal.UI.Helpers; using Microsoft.CmdPal.UI.ViewModels; using Microsoft.CmdPal.UI.ViewModels.Services; +using Microsoft.Extensions.Logging; using Microsoft.UI; using Microsoft.UI.Dispatching; using Microsoft.UI.Xaml; @@ -25,6 +25,7 @@ internal sealed partial class ThemeService : IThemeService, IDisposable { private static readonly TimeSpan ReloadDebounceInterval = TimeSpan.FromMilliseconds(500); + private readonly ILogger _logger; private readonly UISettings _uiSettings; private readonly SettingsModel _settings; private readonly ResourceSwapper _resourceSwapper; @@ -162,16 +163,17 @@ internal sealed partial class ThemeService : IThemeService, IDisposable } catch (Exception ex) { - Logger.LogWarning($"Failed to load background image '{path}'. {ex.Message}"); + Log_FailedToLoadBackgroundImage(path, ex); return null; } } - public ThemeService(SettingsModel settings, ResourceSwapper resourceSwapper) + public ThemeService(SettingsModel settings, ResourceSwapper resourceSwapper, ILogger logger) { ArgumentNullException.ThrowIfNull(settings); ArgumentNullException.ThrowIfNull(resourceSwapper); + _logger = logger; _settings = settings; _settings.SettingsChanged += SettingsOnSettingsChanged; @@ -258,4 +260,7 @@ internal sealed partial class ThemeService : IThemeService, IDisposable public required IThemeProvider Provider { get; init; } } + + [LoggerMessage(Level = LogLevel.Warning, Message = "Failed to load background image '{Path}'")] + partial void Log_FailedToLoadBackgroundImage(string path, Exception ex); }