Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Copilot wants to merge 1 commit into
base: main
Choose a base branch
from
+22 −17
Draft

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Microsoft.CmdPal.UI.ViewModels.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down
25 changes: 14 additions & 11 deletions src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ShellViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
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;

Expand All @@ -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;

Expand Down Expand Up @@ -63,7 +64,7 @@ public PageViewModel CurrentPage
}
catch (Exception ex)
{
CoreLogger.LogError(ex.ToString());
_logger.LogError(ex, "Failed to dispose previous page ViewModel");
}
}
}
Expand Down Expand Up @@ -91,12 +92,14 @@ public ShellViewModel(
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());
Expand Down Expand Up @@ -167,7 +170,7 @@ private async Task LoadPageViewModelAsync(PageViewModel viewModel, CancellationT
{
if (viewModel.InitializeCommand.ExecutionTask.Exception is AggregateException ex)
{
CoreLogger.LogError(ex.ToString());
_logger.LogError(ex, "Failed to initialize page ViewModel");
}
}
else
Expand All @@ -185,7 +188,7 @@ private async Task LoadPageViewModelAsync(PageViewModel viewModel, CancellationT
}
catch (Exception ex)
{
CoreLogger.LogError(ex.ToString());
_logger.LogError(ex, "Failed to dispose page ViewModel after navigation cancellation");
}
}

Expand Down Expand Up @@ -215,7 +218,7 @@ private async Task LoadPageViewModelAsync(PageViewModel viewModel, CancellationT
}
catch (Exception ex)
{
CoreLogger.LogError(ex.ToString());
_logger.LogError(ex, "Failed to dispose page ViewModel after navigation cancellation");
}
}

Expand Down Expand Up @@ -245,7 +248,7 @@ private void PerformCommand(PerformCommandMessage message)
}
catch (Exception ex)
{
CoreLogger.LogError(ex.ToString());
_logger.LogError(ex, "Failed to cancel previous navigation");
}
finally
{
Expand Down Expand Up @@ -282,7 +285,7 @@ private void PerformCommand(PerformCommandMessage message)
{
if (command is IPage page)
{
CoreLogger.LogDebug($"Navigating to page");
_logger.LogDebug("Navigating to page");

_isNested = !isMainPage;
_currentlyTransient = message.TransientPage;
Expand All @@ -301,7 +304,7 @@ private void PerformCommand(PerformCommandMessage message)
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();
}

Expand Down Expand Up @@ -335,7 +338,7 @@ private void PerformCommand(PerformCommandMessage message)
}
else if (command is IInvokableCommand invokable)
{
CoreLogger.LogDebug($"Invoking command");
_logger.LogDebug("Invoking command");

WeakReferenceMessenger.Default.Send<TelemetryBeginInvokeMessage>();
StartInvoke(message, invokable, host);
Expand Down Expand Up @@ -421,7 +424,7 @@ private void UnsafeHandleCommandResult(ICommandResult? result)
}

var kind = result.Kind;
CoreLogger.LogDebug($"handling {kind.ToString()}");
_logger.LogDebug("Handling command result {Kind}", kind);

WeakReferenceMessenger.Default.Send<TelemetryInvokeResultMessage>(new(kind));
switch (kind)
Expand Down
13 changes: 7 additions & 6 deletions src/modules/cmdpal/Microsoft.CmdPal.UI/PowerToysRootPageService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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>(() =>
{
Expand Down Expand Up @@ -67,8 +69,7 @@ private void OnPerformTopLevelCommand(object? context)
}
catch (Exception ex)
{
Logger.LogError("Failed to update history in PowerToysRootPageService");
Logger.LogError(ex.ToString());
_logger.LogError(ex, "Failed to update history in PowerToysRootPageService");
}
}

Expand Down Expand Up @@ -109,13 +110,13 @@ public void SetActiveExtension(IExtensionWrapper? extension)
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");
}
}
}
Expand Down
Loading