Skip to content

Commit

Permalink
Use WinGet API to improve Quick Fix results (microsoft#17614)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request
Improves Quick Fix's suggestions to use WinGet API and actually query
winget for packages based on the missing command.

To interact with the WinGet API, we need the
`Microsoft.WindowsPackageManager.ComInterop` NuGet package.
`Microsoft.WindowsPackageManager.ComInterop.Additional.targets` is used
to copy over the winmd into CascadiaPackage. The build variable
`TerminalWinGetInterop` is used to import the package properly.

`WindowsPackageManagerFactory` is used as a centralized way to generate
the winget objects. Long-term, we may need to do manual activation for
elevated sessions, which this class can easily be extended to support.
In the meantime, we'll just use the normal `winrt::create_instance` on
all sessions.

In `TerminalPage`, we conduct the search asynchronously when a missing
command was found. Search results are limited to 20 packages. We try to
retrieve packages with the following filters set, then fallback into the
next step:
1. `PackageMatchField::Command`,
`PackageFieldMatchOption::StartsWithCaseInsensitive`
2. `PackageMatchField::Name`,
`PackageFieldMatchOption::ContainsCaseInsensitive`
3. `PackageMatchField::Moniker`,
`PackageFieldMatchOption::ContainsCaseInsensitive`

This aligns with the Microsoft.WinGet.CommandNotFound PowerShell module
([link to relevant
code](https://github.com/microsoft/winget-command-not-found/blob/9bc83617b94f6dc88e1fc9599e1c859bc3adf96f/src/WinGetCommandNotFoundFeedbackPredictor.cs#L165-L202)).

Closes microsoft#17378
Closes microsoft#17631
Support for elevated sessions tracked in microsoft#17677

## References
-
https://github.com/microsoft/winget-cli/blob/master/src/Microsoft.Management.Deployment/PackageManager.idl:
winget object documentation

## Validation Steps Performed
- [X] unelevated sessions --> winget query performed and presented
- [X] elevated sessions --> nothing happens (got rid of `winget install
{}` suggestion)
  • Loading branch information
carlos-zamora authored Aug 23, 2024
1 parent 1de142b commit dbbc581
Show file tree
Hide file tree
Showing 19 changed files with 216 additions and 20 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT License. See LICENSE in the project root for license information. -->
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<PropertyGroup>
<Native-Platform Condition="'$(Platform)' == 'Win32'">x86</Native-Platform>
<Native-Platform Condition="'$(Platform)' != 'Win32'">$(Platform)</Native-Platform>
</PropertyGroup>
<ItemGroup>
<Reference Include="$(WinGetPackageRoot)\lib\Microsoft.Management.Deployment.winmd">
<IsWinMDFile>true</IsWinMDFile>
</Reference>
</ItemGroup>
<Target Name="_FixWinGetWinmdPackaging" BeforeTargets="_ComputeAppxPackagePayload">
<ItemGroup>
<PackagingOutputs Include="$(WinGetPackageRoot)\lib\Microsoft.Management.Deployment.winmd">
<OutputGroup>CustomOutputGroupForPackaging</OutputGroup>
<ProjectName>$(ProjectName)</ProjectName>
<TargetPath>Microsoft.Management.Deployment.winmd</TargetPath>
</PackagingOutputs>
</ItemGroup>
</Target>
</Project>
1 change: 1 addition & 0 deletions dep/nuget/packages.config
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
<package id="Microsoft.UI.Xaml" version="2.8.4" targetFramework="native" />
<package id="Microsoft.Web.WebView2" version="1.0.1661.34" targetFramework="native" />
<package id="Microsoft.Windows.ImplementationLibrary" version="1.0.240122.1" targetFramework="native" developmentDependency="true" />
<package id="Microsoft.WindowsPackageManager.ComInterop" version="1.8.1911" targetFramework="native" developmentDependency="true" />

<!-- Managed packages -->
<package id="Appium.WebDriver" version="3.0.0.2" targetFramework="net45" />
Expand Down
14 changes: 14 additions & 0 deletions src/cascadia/TerminalApp/SuggestionsControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,20 @@ namespace winrt::TerminalApp::implementation
// ever allow non-sendInput actions.
DispatchCommandRequested.raise(*this, actionPaletteItem.Command());

if (const auto& sendInputCmd = actionPaletteItem.Command().ActionAndArgs().Args().try_as<SendInputArgs>())
{
if (til::starts_with(sendInputCmd.Input(), L"winget"))
{
TraceLoggingWrite(
g_hTerminalAppProvider,
"QuickFixSuggestionUsed",
TraceLoggingDescription("Event emitted when a winget suggestion from is used"),
TraceLoggingValue("SuggestionsUI", "Source"),
TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES),
TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage));
}
}

TraceLoggingWrite(
g_hTerminalAppProvider, // handle to TerminalApp tracelogging provider
"SuggestionsControlDispatchedAction",
Expand Down
4 changes: 3 additions & 1 deletion src/cascadia/TerminalApp/TerminalAppLib.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
<PropertyGroup Label="NuGet Dependencies">
<TerminalCppWinrt>true</TerminalCppWinrt>
<TerminalMUX>true</TerminalMUX>
<TerminalWinGetInterop>true</TerminalWinGetInterop>
</PropertyGroup>
<Import Project="..\..\..\common.openconsole.props" Condition="'$(OpenConsoleDir)'==''" />
<Import Project="$(OpenConsoleDir)src\common.nugetversions.props" />
Expand Down Expand Up @@ -177,6 +178,7 @@
<ClInclude Include="SuggestionsControl.h">
<DependentUpon>SuggestionsControl.xaml</DependentUpon>
</ClInclude>
<ClInclude Include="WindowsPackageManagerFactory.h" />
</ItemGroup>
<!-- ========================= Cpp Files ======================== -->
<ItemGroup>
Expand Down Expand Up @@ -361,7 +363,7 @@
</Midl>
<Midl Include="FilteredCommand.idl" />
<Midl Include="IPaneContent.idl" />
<Midl Include="TerminalPaneContent.idl" >
<Midl Include="TerminalPaneContent.idl">
<DependentUpon>TaskPaneContent.xaml</DependentUpon>
<SubType>Code</SubType>
</Midl>
Expand Down
82 changes: 78 additions & 4 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "LaunchPositionRequest.g.cpp"

using namespace winrt;
using namespace winrt::Microsoft::Management::Deployment;
using namespace winrt::Microsoft::Terminal::Control;
using namespace winrt::Microsoft::Terminal::Settings::Model;
using namespace winrt::Microsoft::Terminal::TerminalConnection;
Expand Down Expand Up @@ -3001,18 +3002,82 @@ namespace winrt::TerminalApp::implementation
ShowWindowChanged.raise(*this, args);
}

winrt::fire_and_forget TerminalPage::_SearchMissingCommandHandler(const IInspectable /*sender*/, const Microsoft::Terminal::Control::SearchMissingCommandEventArgs args)
Windows::Foundation::IAsyncOperation<IVectorView<MatchResult>> TerminalPage::_FindPackageAsync(hstring query)
{
assert(!Dispatcher().HasThreadAccess());
const PackageManager packageManager = WindowsPackageManagerFactory::CreatePackageManager();
PackageCatalogReference catalogRef{
packageManager.GetPredefinedPackageCatalog(PredefinedPackageCatalog::OpenWindowsCatalog)
};
catalogRef.PackageCatalogBackgroundUpdateInterval(std::chrono::hours(24));

ConnectResult connectResult{ nullptr };
for (int retries = 0;;)
{
connectResult = catalogRef.Connect();
if (connectResult.Status() == ConnectResultStatus::Ok)
{
break;
}

if (++retries == 3)
{
co_return nullptr;
}
}

PackageCatalog catalog = connectResult.PackageCatalog();
// clang-format off
static constexpr std::array<WinGetSearchParams, 3> searches{ {
{ .Field = PackageMatchField::Command, .MatchOption = PackageFieldMatchOption::StartsWithCaseInsensitive },
{ .Field = PackageMatchField::Name, .MatchOption = PackageFieldMatchOption::ContainsCaseInsensitive },
{ .Field = PackageMatchField::Moniker, .MatchOption = PackageFieldMatchOption::ContainsCaseInsensitive } } };
// clang-format on

PackageMatchFilter filter = WindowsPackageManagerFactory::CreatePackageMatchFilter();
filter.Value(query);

FindPackagesOptions options = WindowsPackageManagerFactory::CreateFindPackagesOptions();
options.Filters().Append(filter);
options.ResultLimit(20);

IVectorView<MatchResult> pkgList;
for (const auto& search : searches)
{
filter.Field(search.Field);
filter.Option(search.MatchOption);

const auto result = co_await catalog.FindPackagesAsync(options);
pkgList = result.Matches();
if (pkgList.Size() > 0)
{
break;
}
}
co_return pkgList;
}

Windows::Foundation::IAsyncAction TerminalPage::_SearchMissingCommandHandler(const IInspectable /*sender*/, const Microsoft::Terminal::Control::SearchMissingCommandEventArgs args)
{
if (!Feature_QuickFix::IsEnabled())
{
co_return;
}
co_await winrt::resume_background();

// no packages were found, nothing to suggest
const auto pkgList = co_await _FindPackageAsync(args.MissingCommand());
if (!pkgList || pkgList.Size() == 0)
{
co_return;
}

std::vector<hstring> suggestions;
suggestions.reserve(1);
suggestions.emplace_back(fmt::format(FMT_COMPILE(L"winget install {}"), args.MissingCommand()));
suggestions.reserve(pkgList.Size());
for (const auto& pkg : pkgList)
{
// --id and --source ensure we don't collide with another package catalog
suggestions.emplace_back(fmt::format(FMT_COMPILE(L"winget install --id {} -s winget"), pkg.CatalogPackage().Id()));
}

co_await wil::resume_foreground(Dispatcher());

Expand Down Expand Up @@ -5006,6 +5071,14 @@ namespace winrt::TerminalApp::implementation
{
ctrl.ClearQuickFix();
}

TraceLoggingWrite(
g_hTerminalAppProvider,
"QuickFixSuggestionUsed",
TraceLoggingDescription("Event emitted when a winget suggestion from is used"),
TraceLoggingValue("QuickFixMenu", "Source"),
TraceLoggingKeyword(MICROSOFT_KEYWORD_MEASURES),
TelemetryPrivacyDataTag(PDT_ProductAndServiceUsage));
}
};
};
Expand All @@ -5027,6 +5100,7 @@ namespace winrt::TerminalApp::implementation

item.Text(qf);
item.Click(makeCallback(qf));
ToolTipService::SetToolTip(item, box_value(qf));
menu.Items().Append(item);
}
}
Expand Down
11 changes: 10 additions & 1 deletion src/cascadia/TerminalApp/TerminalPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
#include "LaunchPositionRequest.g.h"
#include "Toast.h"

#include "WindowsPackageManagerFactory.h"

#define DECLARE_ACTION_HANDLER(action) void _Handle##action(const IInspectable& sender, const Microsoft::Terminal::Settings::Model::ActionEventArgs& args);

namespace TerminalAppLocalTests
Expand Down Expand Up @@ -87,6 +89,12 @@ namespace winrt::TerminalApp::implementation
til::property<winrt::Microsoft::Terminal::Settings::Model::LaunchPosition> Position;
};

struct WinGetSearchParams
{
winrt::Microsoft::Management::Deployment::PackageMatchField Field;
winrt::Microsoft::Management::Deployment::PackageFieldMatchOption MatchOption;
};

struct TerminalPage : TerminalPageT<TerminalPage>
{
public:
Expand Down Expand Up @@ -530,7 +538,8 @@ namespace winrt::TerminalApp::implementation
void _OpenSuggestions(const Microsoft::Terminal::Control::TermControl& sender, Windows::Foundation::Collections::IVector<winrt::Microsoft::Terminal::Settings::Model::Command> commandsCollection, winrt::TerminalApp::SuggestionsMode mode, winrt::hstring filterText);

void _ShowWindowChangedHandler(const IInspectable sender, const winrt::Microsoft::Terminal::Control::ShowWindowArgs args);
winrt::fire_and_forget _SearchMissingCommandHandler(const IInspectable sender, const winrt::Microsoft::Terminal::Control::SearchMissingCommandEventArgs args);
Windows::Foundation::IAsyncAction _SearchMissingCommandHandler(const IInspectable sender, const winrt::Microsoft::Terminal::Control::SearchMissingCommandEventArgs args);
Windows::Foundation::IAsyncOperation<Windows::Foundation::Collections::IVectorView<winrt::Microsoft::Management::Deployment::MatchResult>> _FindPackageAsync(hstring query);

winrt::fire_and_forget _windowPropertyChanged(const IInspectable& sender, const winrt::Windows::UI::Xaml::Data::PropertyChangedEventArgs& args);

Expand Down
61 changes: 61 additions & 0 deletions src/cascadia/TerminalApp/WindowsPackageManagerFactory.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.
//
// Module Name:
// - WindowsPackageManagerFactory.h
//
// Abstract:
// - This factory is designed to create production-level instances of WinGet objects.
// Elevated sessions require manual activation of WinGet objects and are not currently supported,
// while non-elevated sessions can use the standard WinRT activation system.
// Author:
// - Carlos Zamora (carlos-zamora) 23-Jul-2024

#pragma once

#include <winrt/Microsoft.Management.Deployment.h>

namespace winrt::TerminalApp::implementation
{
struct WindowsPackageManagerFactory
{
public:
static winrt::Microsoft::Management::Deployment::PackageManager CreatePackageManager()
{
return winrt::create_instance<winrt::Microsoft::Management::Deployment::PackageManager>(PackageManagerGuid, CLSCTX_ALL);
}

static winrt::Microsoft::Management::Deployment::FindPackagesOptions CreateFindPackagesOptions()
{
return winrt::create_instance<winrt::Microsoft::Management::Deployment::FindPackagesOptions>(FindPackageOptionsGuid, CLSCTX_ALL);
}

static winrt::Microsoft::Management::Deployment::CreateCompositePackageCatalogOptions CreateCreateCompositePackageCatalogOptions()
{
return winrt::create_instance<winrt::Microsoft::Management::Deployment::CreateCompositePackageCatalogOptions>(CreateCompositePackageCatalogOptionsGuid, CLSCTX_ALL);
}

static winrt::Microsoft::Management::Deployment::InstallOptions CreateInstallOptions()
{
return winrt::create_instance<winrt::Microsoft::Management::Deployment::InstallOptions>(InstallOptionsGuid, CLSCTX_ALL);
}

static winrt::Microsoft::Management::Deployment::UninstallOptions CreateUninstallOptions()
{
return winrt::create_instance<winrt::Microsoft::Management::Deployment::UninstallOptions>(UninstallOptionsGuid, CLSCTX_ALL);
}

static winrt::Microsoft::Management::Deployment::PackageMatchFilter CreatePackageMatchFilter()
{
return winrt::create_instance<winrt::Microsoft::Management::Deployment::PackageMatchFilter>(PackageMatchFilterGuid, CLSCTX_ALL);
}

private:
static constexpr winrt::guid PackageManagerGuid{ 0xC53A4F16, 0x787E, 0x42A4, { 0xB3, 0x04, 0x29, 0xEF, 0xFB, 0x4B, 0xF5, 0x97 } };
static constexpr winrt::guid FindPackageOptionsGuid{ 0x572DED96, 0x9C60, 0x4526, { 0x8F, 0x92, 0xEE, 0x7D, 0x91, 0xD3, 0x8C, 0x1A } };
static constexpr winrt::guid CreateCompositePackageCatalogOptionsGuid{ 0x526534B8, 0x7E46, 0x47C8, { 0x84, 0x16, 0xB1, 0x68, 0x5C, 0x32, 0x7D, 0x37 } };
static constexpr winrt::guid InstallOptionsGuid{ 0x1095F097, 0xEB96, 0x453B, { 0xB4, 0xE6, 0x16, 0x13, 0x63, 0x7F, 0x3B, 0x14 } };
static constexpr winrt::guid UninstallOptionsGuid{ 0xE1D9A11E, 0x9F85, 0x4D87, { 0x9C, 0x17, 0x2B, 0x93, 0x14, 0x3A, 0xDB, 0x8D } };
static constexpr winrt::guid PackageMatchFilterGuid{ 0xD02C9DAF, 0x99DC, 0x429C, { 0xB5, 0x03, 0x4E, 0x50, 0x4E, 0x4A, 0xB0, 0x00 } };
};
}
6 changes: 3 additions & 3 deletions src/cascadia/TerminalControl/ControlCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
auto pfnCompletionsChanged = [=](auto&& menuJson, auto&& replaceLength) { _terminalCompletionsChanged(menuJson, replaceLength); };
_terminal->CompletionsChangedCallback(pfnCompletionsChanged);

auto pfnSearchMissingCommand = [this](auto&& PH1) { _terminalSearchMissingCommand(std::forward<decltype(PH1)>(PH1)); };
auto pfnSearchMissingCommand = [this](auto&& PH1, auto&& PH2) { _terminalSearchMissingCommand(std::forward<decltype(PH1)>(PH1), std::forward<decltype(PH2)>(PH2)); };
_terminal->SetSearchMissingCommandCallback(pfnSearchMissingCommand);

auto pfnClearQuickFix = [this] { ClearQuickFix(); };
Expand Down Expand Up @@ -1629,9 +1629,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_midiAudio.PlayNote(reinterpret_cast<HWND>(_owningHwnd), noteNumber, velocity, std::chrono::duration_cast<std::chrono::milliseconds>(duration));
}

void ControlCore::_terminalSearchMissingCommand(std::wstring_view missingCommand)
void ControlCore::_terminalSearchMissingCommand(std::wstring_view missingCommand, const til::CoordType& bufferRow)
{
SearchMissingCommand.raise(*this, make<implementation::SearchMissingCommandEventArgs>(hstring{ missingCommand }));
SearchMissingCommand.raise(*this, make<implementation::SearchMissingCommandEventArgs>(hstring{ missingCommand }, bufferRow));
}

void ControlCore::ClearQuickFix()
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalControl/ControlCore.h
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
void _terminalPlayMidiNote(const int noteNumber,
const int velocity,
const std::chrono::microseconds duration);
void _terminalSearchMissingCommand(std::wstring_view missingCommand);
void _terminalSearchMissingCommand(std::wstring_view missingCommand, const til::CoordType& bufferRow);

winrt::fire_and_forget _terminalCompletionsChanged(std::wstring_view menuJson, unsigned int replaceLength);

Expand Down
6 changes: 4 additions & 2 deletions src/cascadia/TerminalControl/EventArgs.h
Original file line number Diff line number Diff line change
Expand Up @@ -216,10 +216,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
struct SearchMissingCommandEventArgs : public SearchMissingCommandEventArgsT<SearchMissingCommandEventArgs>
{
public:
SearchMissingCommandEventArgs(const winrt::hstring& missingCommand) :
MissingCommand(missingCommand) {}
SearchMissingCommandEventArgs(const winrt::hstring& missingCommand, const til::CoordType& bufferRow) :
MissingCommand(missingCommand),
BufferRow(bufferRow) {}

til::property<winrt::hstring> MissingCommand;
til::property<til::CoordType> BufferRow;
};
}

Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalControl/EventArgs.idl
Original file line number Diff line number Diff line change
Expand Up @@ -130,5 +130,6 @@ namespace Microsoft.Terminal.Control
runtimeclass SearchMissingCommandEventArgs
{
String MissingCommand { get; };
Int32 BufferRow { get; };
}
}
8 changes: 4 additions & 4 deletions src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4059,18 +4059,17 @@ namespace winrt::Microsoft::Terminal::Control::implementation
const auto rd = get_self<ControlCore>(_core)->GetRenderData();
rd->LockConsole();
const auto viewportBufferPosition = rd->GetViewport();
const auto cursorBufferPosition = rd->GetCursorPosition();
rd->UnlockConsole();
if (cursorBufferPosition.y < viewportBufferPosition.Top() || cursorBufferPosition.y > viewportBufferPosition.BottomInclusive())
if (_quickFixBufferPos < viewportBufferPosition.Top() || _quickFixBufferPos > viewportBufferPosition.BottomInclusive())
{
quickFixBtn.Visibility(Visibility::Collapsed);
return;
}

// draw the button in the gutter
const auto& cursorPosInDips = CursorPositionInDips();
const auto& quickFixBtnPosInDips = _toPosInDips({ 0, _quickFixBufferPos });
Controls::Canvas::SetLeft(quickFixBtn, -termPadding.Left);
Controls::Canvas::SetTop(quickFixBtn, cursorPosInDips.Y - termPadding.Top);
Controls::Canvas::SetTop(quickFixBtn, quickFixBtnPosInDips.y - termPadding.Top);
quickFixBtn.Visibility(Visibility::Visible);

if (auto automationPeer{ FrameworkElementAutomationPeer::FromElement(*this) })
Expand All @@ -4085,6 +4084,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation

void TermControl::_bubbleSearchMissingCommand(const IInspectable& /*sender*/, const Control::SearchMissingCommandEventArgs& args)
{
_quickFixBufferPos = args.BufferRow();
SearchMissingCommand.raise(*this, args);
}

Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalControl/TermControl.h
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
bool _initializedTerminal{ false };
bool _quickFixButtonCollapsible{ false };
bool _quickFixesAvailable{ false };
til::CoordType _quickFixBufferPos{};

std::shared_ptr<ThrottledFuncLeading> _playWarningBell;

Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalCore/Terminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1233,7 +1233,7 @@ void Microsoft::Terminal::Core::Terminal::CompletionsChangedCallback(std::functi
_pfnCompletionsChanged.swap(pfn);
}

void Microsoft::Terminal::Core::Terminal::SetSearchMissingCommandCallback(std::function<void(std::wstring_view)> pfn) noexcept
void Microsoft::Terminal::Core::Terminal::SetSearchMissingCommandCallback(std::function<void(std::wstring_view, const til::CoordType)> pfn) noexcept
{
_pfnSearchMissingCommand.swap(pfn);
}
Expand Down
Loading

0 comments on commit dbbc581

Please sign in to comment.