Skip to content
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.

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

Add Quick Fix UI and support for custom CommandNotFound OSC #16848

Merged
merged 26 commits into from
Jun 29, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
12100c5
Add support for custom CommandNotFound OSC
carlos-zamora Jan 22, 2024
c8d13d4
[Broken] Properly search through WinGet's catalog
carlos-zamora Jan 23, 2024
534c2d9
add debug code & dismiss button
carlos-zamora Mar 8, 2024
c1e1eab
remove info bar code; fix branch
carlos-zamora Mar 26, 2024
68ed03d
use suggestions UI instead
carlos-zamora Mar 26, 2024
c2417bb
add a lil polish
carlos-zamora Mar 26, 2024
6a361ef
address feedback; winget integration still in progress
carlos-zamora Mar 27, 2024
a2e57b4
Merge branch 'main' into dev/cazamor/cmdNotFound
carlos-zamora Apr 11, 2024
cfaa09d
moar progress! (useTransitions -> crash!)
carlos-zamora Apr 23, 2024
88b64cd
fix design
carlos-zamora Apr 25, 2024
48a36be
Merge branch 'main' into dev/cazamor/cmdNotFound
carlos-zamora Apr 25, 2024
d8c8807
feature flag; fix scroll bug; clear QF on enter
carlos-zamora Apr 26, 2024
8c0d7c6
clear quick fix on interaction
carlos-zamora Apr 29, 2024
ec1fbc2
Merge branch 'main' into dev/cazamor/cmdNotFound
carlos-zamora Jun 3, 2024
767692c
remove feature flag and WinMD refs; add QuickFixesAvailable API; addr…
carlos-zamora Jun 3, 2024
d4b6904
improve clearing functionality; fix build; x:Load; try to fix sizing
carlos-zamora Jun 5, 2024
4e5d07d
accessibility!
carlos-zamora Jun 5, 2024
27d464c
adjust button sizing for better visibility
carlos-zamora Jun 5, 2024
0f801a9
make the icon not dumb
carlos-zamora Jun 5, 2024
a545325
codeformat
carlos-zamora Jun 5, 2024
755f121
remove the winmd we're not using anymore
carlos-zamora Jun 5, 2024
a0aa2d0
re-add feature flag; address Leonard's comments
carlos-zamora Jun 12, 2024
1ffbae1
Merge branch 'main' into dev/cazamor/cmdNotFound
carlos-zamora Jun 12, 2024
0c5d880
Apply suggestions from code review
DHowett Jun 28, 2024
ef6af81
Merge remote-tracking branch 'origin/main' into dev/cazamor/cmdNotFound
DHowett Jun 28, 2024
db7f464
Adjust for renderer changes on main
DHowett Jun 28, 2024
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
12 changes: 11 additions & 1 deletion src/cascadia/TerminalApp/AppActionHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1308,7 +1308,7 @@ namespace winrt::TerminalApp::implementation
// requires context from the control)
// then get that here.
const bool shouldGetContext = realArgs.UseCommandline() ||
WI_IsFlagSet(source, SuggestionsSource::CommandHistory);
WI_IsAnyFlagSet(source, SuggestionsSource::CommandHistory | SuggestionsSource::WinGetCommandNotFound);
if (shouldGetContext)
{
if (const auto& control{ _GetActiveControl() })
Expand Down Expand Up @@ -1348,6 +1348,16 @@ namespace winrt::TerminalApp::implementation
}
}

if (WI_IsFlagSet(source, SuggestionsSource::WinGetCommandNotFound) &&
context != nullptr)
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
{
const auto recentCommands = Command::ToSendInputCommands(context.WinGetSuggestions());
for (const auto& t : recentCommands)
{
commandsCollection.push_back(t);
}
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zadjii-msft is this kinda what you meant by adding the new suggestions to the context? If that's the case, we should rename the context to something else. Maybe ShellIntegrationContext?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly what I was thinking. My quick recommendations:

  • I'd probably not name the member WinGetSuggestions, but instead QuickFixes. I'd think these should come through just the same as any other "quick fix" we add to the control (i.e. like a VT-driven one).
  • the same thing but with the flag in SuggestionsSource - that pobably also needs to get added to the JSON_FLAG_MAPPER in TerminalSettingsSerializationHelpers.h, as quickFixes
  • we probably want to pass a \uEA80 icon in here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oofda I see the comment about the OEM icon below now. Well, similar idea I guess.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh huh, maybe we do want to give the winget suggestions a different icon. That's actually a really fun idea.

// Open the palette with all these commands in it.
_OpenSuggestions(_GetActiveControl(),
winrt::single_threaded_vector<Command>(std::move(commandsCollection)),
Expand Down
Binary file not shown.
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/Resources/en-US/Resources.resw
Original file line number Diff line number Diff line change
Expand Up @@ -898,4 +898,4 @@
<data name="RestartConnectionToolTip" xml:space="preserve">
<value>Restart the active pane connection</value>
</data>
</root>
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
</root>
5 changes: 5 additions & 0 deletions src/cascadia/TerminalApp/TerminalAppLib.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,11 @@
<ResolvedFrom>CppWinRTImplicitlyExpandTargetPlatform</ResolvedFrom>
<IsSystemReference>True</IsSystemReference>
</Reference>
<Reference Include="$(OpenConsoleDir)src\cascadia\TerminalApp\Microsoft.Management.Deployment.winmd">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but also, just adding this here, is that enough to actually let our package know what dll these types are implemented in? Or do we need to do something to make sure the winget package (that presumably implements these) is actually in our package graph?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No clue! This and this comment are gonna be my next step (figuring out how to actually interact with WinGet).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a thought: stage this as two PRs.

  • this first one: plumbing, store the "quick fix" in the buffer, plumb into sxnui. But only store a blind winget install foo, don't actually do the package lookup.
    • probably just hide this behind velocity into canary only for the time being
  • a second PR that enlightens the suggestion to include real winget results for foo.

<IsWinMDFile>true</IsWinMDFile>
<Private>false</Private>
<CopyLocalSatelliteAssemblies>false</CopyLocalSatelliteAssemblies>
</Reference>
</ItemGroup>
<!-- ====================== Compiler & Linker Flags ===================== -->
<ItemDefinitionGroup>
Expand Down
189 changes: 189 additions & 0 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "RequestReceiveContentArgs.g.cpp"

#include <filesystem>
#include <winrt/Microsoft.Management.Deployment.h>
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved

#include <inc/WindowingBehavior.h>
#include <LibraryResources.h>
Expand All @@ -26,6 +27,7 @@
#include "Utils.h"

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 @@ -1670,6 +1672,8 @@ namespace winrt::TerminalApp::implementation

term.ShowWindowChanged({ get_weak(), &TerminalPage::_ShowWindowChangedHandler });

//term.SearchMissingCommand({ get_weak(), &TerminalPage::_SearchMissingCommandHandler });

carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
// Don't even register for the event if the feature is compiled off.
if constexpr (Feature_ShellCompletions::IsEnabled())
{
Expand Down Expand Up @@ -2952,6 +2956,191 @@ namespace winrt::TerminalApp::implementation
ShowWindowChanged.raise(*this, args);
}

void TerminalPage::_SearchMissingCommandHandler(const IInspectable sender, const winrt::Microsoft::Terminal::Control::SearchMissingCommandEventArgs args)
{
#if 0
static constexpr CLSID CLSID_PackageManager = { 0xC53A4F16, 0x787E, 0x42A4, 0xB3, 0x04, 0x29, 0xEF, 0xFB, 0x4B, 0xF5, 0x97 }; //C53A4F16-787E-42A4-B304-29EFFB4BF597
static constexpr CLSID CLSID_FindPackagesOptions = { 0x572DED96, 0x9C60, 0x4526, { 0x8F, 0x92, 0xEE, 0x7D, 0x91, 0xD3, 0x8C, 0x1A } }; //572DED96-9C60-4526-8F92-EE7D91D38C1A
static constexpr CLSID CLSID_PackageMatchFilter = { 0xD02C9DAF, 0x99DC, 0x429C, { 0xB5, 0x03, 0x4E, 0x50, 0x4E, 0x4A, 0xB0, 0x00 } }; //D02C9DAF-99DC-429C-B503-4E504E4AB000

static constexpr unsigned int maxSuggestions = 5;
bool tooManySuggestions = false;

// TODO CARLOS: this is where we fail! "Class not registered" error
PackageManager pkgManager = winrt::create_instance<PackageManager>(CLSID_PackageManager, CLSCTX_ALL);
auto catalogRef = pkgManager.GetPredefinedPackageCatalog(PredefinedPackageCatalog::OpenWindowsCatalog);
auto connectResult = catalogRef.Connect();
int retryCount = 0;
while (connectResult.Status() != ConnectResultStatus::Ok && retryCount < 3)
{
connectResult = catalogRef.Connect();
++retryCount;
}
if (connectResult.Status() != ConnectResultStatus::Ok)
{
return;
}
auto catalog = connectResult.PackageCatalog();

// Perform the query (search by command)
auto packageMatchFilter = winrt::create_instance<PackageMatchFilter>(CLSID_PackageMatchFilter, CLSCTX_ALL);
auto findPackagesOptions = winrt::create_instance<FindPackagesOptions>(CLSID_FindPackagesOptions, CLSCTX_ALL);

// Helper lambda to apply a filter to the query
auto applyPackageMatchFilter = [&packageMatchFilter, &findPackagesOptions](PackageMatchField field, PackageFieldMatchOption matchOption, hstring query) {
// Configure filter
packageMatchFilter.Field(field);
packageMatchFilter.Option(matchOption);
packageMatchFilter.Value(query);

// Apply filter
findPackagesOptions.ResultLimit(maxSuggestions + 1u);
findPackagesOptions.Filters().Clear();
findPackagesOptions.Filters().Append(packageMatchFilter);
};

// Helper lambda to retrieve the best matching package(s) from the query's result
auto tryGetBestMatchingPackage = [&tooManySuggestions](IVectorView<MatchResult> matches) {
std::vector<CatalogPackage> results;
results.reserve(std::min(matches.Size(), maxSuggestions));
if (matches.Size() == 1)
{
// One match --> return the package
results.emplace_back(matches.GetAt(0).CatalogPackage());
}
else if (matches.Size() > 1)
{
// Multiple matches --> display top 5 matches (prioritize best matches first)
std::queue<CatalogPackage> bestExactMatches, secondaryMatches, tertiaryMatches;
for (auto match : matches)
{
switch (match.MatchCriteria().Option())
{
case PackageFieldMatchOption::EqualsCaseInsensitive:
case PackageFieldMatchOption::Equals:
bestExactMatches.push(match.CatalogPackage());
break;
case PackageFieldMatchOption::StartsWithCaseInsensitive:
secondaryMatches.push(match.CatalogPackage());
break;
case PackageFieldMatchOption::ContainsCaseInsensitive:
tertiaryMatches.push(match.CatalogPackage());
break;
}
}

// Now return the top maxSuggestions
while (results.size() < maxSuggestions)
{
if (bestExactMatches.size() > 0)
{
results.emplace_back(bestExactMatches.front());
bestExactMatches.pop();
}
else if (secondaryMatches.size() > 0)
{
results.emplace_back(secondaryMatches.front());
secondaryMatches.pop();
}
else if (tertiaryMatches.size() > 0)
{
results.emplace_back(tertiaryMatches.front());
tertiaryMatches.pop();
}
else
{
break;
}
}
}
tooManySuggestions = matches.Size() > maxSuggestions;
return results;
};

// Search by command
auto missingCmd = args.MissingCommand();
std::wstring searchOption = L"command";
applyPackageMatchFilter(PackageMatchField::Command, PackageFieldMatchOption::StartsWithCaseInsensitive, missingCmd);
auto findPackagesResult = catalog.FindPackages(findPackagesOptions);
auto matches = findPackagesResult.Matches();
auto pkgList = tryGetBestMatchingPackage(matches);
if (pkgList.empty())
{
// No matches found --> search by name
applyPackageMatchFilter(PackageMatchField::Name, PackageFieldMatchOption::ContainsCaseInsensitive, missingCmd);

findPackagesResult = catalog.FindPackages(findPackagesOptions);
matches = findPackagesResult.Matches();
pkgList = tryGetBestMatchingPackage(matches);
searchOption = L"name";

if (pkgList.empty())
{
// No matches found --> search by moniker
applyPackageMatchFilter(PackageMatchField::Moniker, PackageFieldMatchOption::ContainsCaseInsensitive, missingCmd);

// Perform the query (search by name)
findPackagesResult = catalog.FindPackages(findPackagesOptions);
matches = findPackagesResult.Matches();
pkgList = tryGetBestMatchingPackage(matches);
searchOption = L"moniker";
}
}

// Display packages in UI
if (!pkgList.empty())
{
std::vector<std::wstring> suggestions;
suggestions.reserve(pkgList.size());
for (auto pkg : pkgList)
{
suggestions.emplace_back(fmt::format(L"winget install --id {}", pkg.Id()));
}

std::wstring footer = tooManySuggestions ?
fmt::format(L"winget search --{} {}", searchOption, missingCmd) :
L"";

// TODO CARLOS: no more info bar; replace!
//ShowCommandNotFoundInfoBar(suggestions, footer);
}
#elif defined(DEBUG) || defined(_DEBUG) || defined(DBG)
//const bool tooManySuggestions = false;
//const std::wstring searchOption = L"command";
const std::wstring missingCmd = args.MissingCommand().data();
std::vector<std::wstring> pkgList = { L"pkg1", L"pkg2", L"pkg3" };
std::vector<hstring> suggestions;
suggestions.reserve(pkgList.size());
for (auto pkg : pkgList)
{
suggestions.emplace_back(fmt::format(L"winget install --id {}", pkg));
}

// This will come in on a background (not-UI, not output) thread.

// Parse the json string into a collection of actions
try
{
//auto commandsCollection = Command::ParsePowerShellMenuComplete(args.MenuJson(),
// args.ReplacementLength());

auto suggestionsWinRT = winrt::single_threaded_vector<hstring>(std::move(suggestions));
auto commandsCollection = Command::ToSendInputCommands(suggestionsWinRT);

//auto weakThis{ get_weak() };
//Dispatcher().RunAsync(CoreDispatcherPriority::Normal, [weakThis, commandsCollection, sender]() {
// // On the UI thread...
// if (const auto& page{ weakThis.get() })
// {
// // Open the Suggestions UI with the commands from the control
// page->_OpenSuggestions(sender.try_as<TermControl>(), commandsCollection, SuggestionsMode::Menu, L"");
// }
//});
}
CATCH_LOG();
#endif
}

// Method Description:
// - Paste text from the Windows Clipboard to the focused terminal
void TerminalPage::_PasteText()
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalApp/TerminalPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,8 @@ namespace winrt::TerminalApp::implementation

void _ShowWindowChangedHandler(const IInspectable sender, const winrt::Microsoft::Terminal::Control::ShowWindowArgs args);

void _SearchMissingCommandHandler(const IInspectable sender, const winrt::Microsoft::Terminal::Control::SearchMissingCommandEventArgs args);

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

void _onTabDragStarting(const winrt::Microsoft::UI::Xaml::Controls::TabView& sender, const winrt::Microsoft::UI::Xaml::Controls::TabViewTabDragStartingEventArgs& e);
Expand Down
Loading
Loading