[cmdpal][AOT] fix some cmdpal aot issue #40301
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Changed `Logger.LogDebug` to `Logger.LogError` in `ExtensionWrapper.cs` for better error handling. - Modified `HotkeySettingsControlHook` to be a partial class for improved organization. - Updated `NativeKeyboardHelper` class and structures for clarity, including using `Marshal.SizeOf<INPUT>()`. - Converted `NativeMethods` to a partial class and updated PInvoke declarations to use `LibraryImport`. - Improved type safety in `TrayIconService.cs` by updating size calculations and adjusted executable path for compatibility. - Cleaned up `NativeMethods.txt` by removing unused function references.
There was a problem hiding this comment.
Pull Request Overview
This PR addresses AOT issues in the cmdpal module by adjusting interop declarations and updating certain API usages. Key changes include:
- Updates to interop method declarations (using LibraryImport and generic Marshal.SizeOf).
- Modification of the tray icon’s executable path resolution in TrayIconService.
- Change of log level in ExtensionWrapper from Debug to Error.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/modules/cmdpal/Microsoft.CmdPal.UI/NativeMethods.txt | Removal of unused native method references |
| src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/TrayIconService.cs | Updated exe path resolution and generic size calculation |
| src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ShortcutControl/NativeMethods.cs | Converted to partial class with LibraryImport attributes |
| src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ShortcutControl/NativeKeyboardHelper.cs | Updated file-scoped namespace and generic Marshal.SizeOf |
| src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/ShortcutControl/HotkeySettingsControlHook.cs | Made the hook class partial |
| src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/ExtensionWrapper.cs | Changed logging from Debug to Error |
Comments suppressed due to low confidence (2)
src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/TrayIconService.cs:136
- Using a hard-coded executable name may lead to issues if the application name changes or if the expected file is not present in the base directory. Consider adding validation or a fallback mechanism to ensure this path resolves correctly.
var exePath = Path.Combine(AppContext.BaseDirectory, "Microsoft.CmdPal.UI.exe");
src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/ExtensionWrapper.cs:124
- Changing the log level from Debug to Error could increase noise if the missing extension is a normal scenario. Please verify that this condition truly warrants an error level log.
Logger.LogError($"Failed to find {ExtensionDisplayName}: {hr}. It may have been uninstalled or deleted.");
zadjii-msft
reviewed
Jun 30, 2025
| @@ -5,82 +5,81 @@ | |||
| using System; | |||
| using System.Runtime.InteropServices; | |||
|
|
|||
| namespace Microsoft.PowerToys.Settings.UI.Helpers | |||
There was a problem hiding this comment.
protip: suppress whitespace on this file
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of the Pull Request
Most of them are safe change but only one line may break something is in TrayIconService.cs
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed