Cmdpal extension: Powertoys extension for cmdpal #44006
Conversation
michaeljolley
added
the
Product-Command Palette
…uild' into dev/vanzue/cmdpal-pt
vanzue
force-pushed
the
dev/vanzue/cmdpal-pt
branch
from
0581b89 to
4a79a0c
Compare
zadjii-msft
left a comment
zadjii-msft
left a comment
There was a problem hiding this comment.
I haven't reviewed all of the individual providers, but overall this looks right to me
I've got a couple suggestions, but overall I think this feels like the right direction
| private async Task<CommandProviderWrapper?> StartExtensionWithTimeoutAsync(IExtensionWrapper extension) | ||
| { | ||
| Logger.LogDebug($"Starting {extension.PackageFullName}"); | ||
| Logger.LogInfo($"StartExtensionWithTimeoutAsync invoked for {extension.ExtensionUniqueId}"); |
There was a problem hiding this comment.
nit: could probably fold this in with the above LogDebug call
| <AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath> | ||
| <AppendRuntimeIdentifierToOutputPath>false</AppendRuntimeIdentifierToOutputPath> | ||
| <UseWinRT>true</UseWinRT> | ||
| <ErrorOnDuplicatePublishOutputFiles>false</ErrorOnDuplicatePublishOutputFiles> |
There was a problem hiding this comment.
Why are we adding this? This sounds like a good error message
There was a problem hiding this comment.
Ahh, it's used to temporary get around the annoying arm64 cmdpal build failure, will rollback before check in
There was a problem hiding this comment.
YOU FIXED IT?? 🎉
I CHANGED MY MIND!
Can we scope this so that it's only enabled for Debug or non-CI builds? I've been doing x64 builds on my ARM device for the last 6 months and it is not my favorite
We'll leave it on for CI builds because it is a real issue when a clean build happens, but I'm fine keeping it disabled for the inner loop
| <Content Include="Assets\Square44x44Logo.targetsize-24_altform-unplated.png" /> | ||
| <Content Include="Assets\StoreLogo.png" /> | ||
| <Content Include="Assets\Wide310x150Logo.scale-200.png" /> | ||
| <Content Include="Assets\FileLocksmith.png" /> |
There was a problem hiding this comment.
Is there any way we could use the <ContentInclude> property to include all the various icons from wherever they live in the tree, rather than duplicating them all in our Assets/ directory?
| DisplayName="PowerToys Command Palette Extension" | ||
| Description="Expose PowerToys commands to Windows Command Palette" | ||
| BackgroundColor="transparent" |
There was a problem hiding this comment.
these need to be ms-resource's in our resw
| [ | ||
| new CommandItem(new Pages.PowerToysListPage()) | ||
| { | ||
| Title = "PowerToys", |
There was a problem hiding this comment.
okay I won't go through and add "this needs localization" comments everywhere. I might not even block on that (and we can just do that in a follow-up PR, since this is already so big)
| project.fragment.lock.json | ||
| artifacts/ | ||
| **/Properties/launchSettings.json | ||
| !src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.PowerToys/Properties/launchSettings.json |
There was a problem hiding this comment.
nit: we could probablt just make this
| !src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.PowerToys/Properties/launchSettings.json | |
| !src/modules/cmdpal/**/Properties/launchSettings.json |
or something. All those launchSettings.json's are super load bearing
|
|
||
| namespace PowerToysExtension.Commands; | ||
|
|
||
| internal sealed partial class NoOpCommand : InvokableCommand |
There was a problem hiding this comment.
FWIW there is a no-op command in the toolkit already, but it doesn't actually do anything or show up as the command on enter
This here will show No operation (or whatever the title is) in the text on the primary command button
|
|
||
| * Edit src/PackageIdentity/AppxManifest.xml: | ||
| ```xml | ||
| <Applications> | ||
| <Application Id="CmdPalExt.YourExtension" | ||
| Executable="External\Win32\<yourExe>.exe" | ||
| EntryPoint="Windows.FullTrustApplication"> | ||
| <uap:VisualElements DisplayName="Your CmdPal Extension" Square44x44Logo="Assets\Square44x44Logo.png" Description="..." /> | ||
| </Application> | ||
| <!-- Existing PowerToys entries remain --> | ||
| </Applications> | ||
| ``` | ||
|
|
||
| * Embed sparse identity in your Win32 binary (RT_MANIFEST): | ||
| ```xml | ||
| <assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0"> | ||
| <msix xmlns="urn:schemas-microsoft-com:msix.v1" | ||
| packageName="Microsoft.PowerToys.SparseApp" | ||
| applicationId="CmdPalExt.YourExtension" | ||
| publisher="CN=PowerToys Dev Cert, O=Microsoft Corporation, L=..., C=US"/> | ||
| </assembly> | ||
| ``` |
There was a problem hiding this comment.
No, will delete this file : )
| var matched = new List<Tuple<int, ListItem>>(); | ||
| foreach (var item in all) | ||
| { | ||
| var result = StringMatcher.FuzzyMatch(query, item.Title); | ||
| if (result.Success) | ||
| { | ||
| matched.Add(new Tuple<int, ListItem>(result.Score, item)); | ||
| } | ||
| } | ||
|
|
||
| matched.Sort((x, y) => y.Item1.CompareTo(x.Item1)); |
There was a problem hiding this comment.
fwiw I'm pretty sure ListHelpers.FilterList* in the toolkit should do this for you 😉
…uild' into dev/vanzue/cmdpal-pt
Summary of the Pull Request
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed
Runtime memory consumption: ~36 M
MSIX package size: 39M
Looks like a bit big for the size...
Trying to use sparse package instead..