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 1 commit
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
Prev Previous commit
Next Next commit
feature flag; fix scroll bug; clear QF on enter
  • Loading branch information
carlos-zamora committed Apr 26, 2024
commit d8c8807ee7722404846f69be2e6b25e215a4be51
155 changes: 4 additions & 151 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2908,160 +2908,14 @@ namespace winrt::TerminalApp::implementation
{
assert(!Dispatcher().HasThreadAccess());

#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)
if (!Feature_QuickFix::IsEnabled())
{
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";
}
co_return;
}

// 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"";
}
#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));
}
suggestions.reserve(1);
suggestions.emplace_back(fmt::format(L"winget install {}", args.MissingCommand()));
Copy link
Member

Choose a reason for hiding this comment

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

📝 this doesn't actually do the winget lookup for args.MissingCommand, but we are including the winget winmd?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops. Yeah, we're blocked by winget. Internally tracking here.

Removed any references to the WinMD since we're not actually using it.

carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved

co_await wil::resume_foreground(Dispatcher());

Expand All @@ -3072,7 +2926,6 @@ namespace winrt::TerminalApp::implementation
}
term.UpdateWinGetSuggestions(single_threaded_vector<hstring>(std::move(suggestions)));
term.ShowQuickFixMenu();
#endif
}

// Method Description:
Expand Down
8 changes: 8 additions & 0 deletions src/cascadia/TerminalControl/ControlCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
auto pfnSearchMissingCommand = [this](auto&& PH1) { _terminalSearchMissingCommand(std::forward<decltype(PH1)>(PH1)); };
_terminal->SetSearchMissingCommandCallback(pfnSearchMissingCommand);

auto pfnClearQuickFix = [this] { _terminalClearQuickFix(); };
_terminal->SetClearQuickFixCallback(pfnClearQuickFix);

// MSFT 33353327: Initialize the renderer in the ctor instead of Initialize().
// We need the renderer to be ready to accept new engines before the SwapChainPanel is ready to go.
// If we wait, a screen reader may try to get the AutomationPeer (aka the UIA Engine), and we won't be able to attach
Expand Down Expand Up @@ -1622,6 +1625,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
SearchMissingCommand.raise(*this, make<implementation::SearchMissingCommandEventArgs>(hstring{ missingCommand }));
Copy link
Member

Choose a reason for hiding this comment

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

📝 this goes up on the BG thread, then comes back down into us in UpdateQuickFixes on the UI thread

}

void ControlCore::_terminalClearQuickFix()
{
ClearQuickFix.raise(*this, nullptr);
}

bool ControlCore::HasSelection() const
{
const auto lock = _terminal->LockForReading();
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalControl/ControlCore.h
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
til::typed_event<IInspectable, Control::OpenHyperlinkEventArgs> OpenHyperlink;
til::typed_event<IInspectable, Control::CompletionsChangedEventArgs> CompletionsChanged;
til::typed_event<IInspectable, Control::SearchMissingCommandEventArgs> SearchMissingCommand;
til::typed_event<> ClearQuickFix;

til::typed_event<> CloseTerminalRequested;
til::typed_event<> RestartTerminalRequested;
Expand Down Expand Up @@ -383,6 +384,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
const int velocity,
const std::chrono::microseconds duration);
void _terminalSearchMissingCommand(std::wstring_view missingCommand);
void _terminalClearQuickFix();

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

Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalControl/ControlCore.idl
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ namespace Microsoft.Terminal.Control
event Windows.Foundation.TypedEventHandler<Object, Object> RendererEnteredErrorState;
event Windows.Foundation.TypedEventHandler<Object, ShowWindowArgs> ShowWindowChanged;
event Windows.Foundation.TypedEventHandler<Object, SearchMissingCommandEventArgs> SearchMissingCommand;
event Windows.Foundation.TypedEventHandler<Object, Object> ClearQuickFix;

// These events are always called from the UI thread (bugs aside)
event Windows.Foundation.TypedEventHandler<Object, FontSizeChangedArgs> FontSizeChanged;
Expand Down
56 changes: 42 additions & 14 deletions src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation

_revokers.PasteFromClipboard = _interactivity.PasteFromClipboard(winrt::auto_revoke, { get_weak(), &TermControl::_bubblePasteFromClipboard });

_revokers.ClearQuickFix = _core.ClearQuickFix(winrt::auto_revoke, { get_weak(), &TermControl::_clearQuickFix });

// Initialize the terminal only once the swapchainpanel is loaded - that
// way, we'll be able to query the real pixel size it got on layout
_layoutUpdatedRevoker = SwapChainPanel().LayoutUpdated(winrt::auto_revoke, [this](auto /*s*/, auto /*e*/) {
Expand Down Expand Up @@ -337,9 +339,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
}
});

auto quickFixBtn{ QuickFixButton() };
quickFixBtn.PointerEntered({ get_weak(), &TermControl::QuickFixButton_PointerEntered });
quickFixBtn.PointerExited({ get_weak(), &TermControl::QuickFixButton_PointerExited });
if (Feature_QuickFix::IsEnabled())
{
auto quickFixBtn{ QuickFixButton() };
quickFixBtn.PointerEntered({ get_weak(), &TermControl::QuickFixButton_PointerEntered });
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
quickFixBtn.PointerExited({ get_weak(), &TermControl::QuickFixButton_PointerExited });
}
}

void TermControl::QuickFixButton_PointerEntered(const IInspectable& /*sender*/, const PointerRoutedEventArgs& /*e*/)
Expand Down Expand Up @@ -827,10 +832,13 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// update. If we enabled scrollbar marks, then great, when we handle
// that message, we'll redraw them.

// update the position of the quick fix menu (in case we changed the padding)
if (QuickFixButton().Visibility() == Visibility::Visible)
if (Feature_QuickFix::IsEnabled())
{
ShowQuickFixMenu();
// update the position of the quick fix menu (in case we changed the padding)
if (_quickFixesAvailable)
{
ShowQuickFixMenu();
}
}
}

Expand Down Expand Up @@ -2333,7 +2341,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_updateSelectionMarkers(nullptr, winrt::make<UpdateSelectionMarkersEventArgs>(false));
}

if (QuickFixButton().Visibility() == Visibility::Visible)
if (_quickFixesAvailable)
{
ShowQuickFixMenu();
}
Expand Down Expand Up @@ -3514,12 +3522,15 @@ namespace winrt::Microsoft::Terminal::Control::implementation
scaleMarker(SelectionStartMarker());
scaleMarker(SelectionEndMarker());

auto quickFixBtn = QuickFixButton();
quickFixBtn.Height(args.Height() / dpiScale);
QuickFixIcon().FontSize(std::min(static_cast<double>(args.Width() / dpiScale), GetPadding().Left));
if (quickFixBtn.Visibility() == Visibility::Visible)
if (Feature_QuickFix::IsEnabled())
{
ShowQuickFixMenu();
auto quickFixBtn = QuickFixButton();
quickFixBtn.Height(args.Height() / dpiScale);
QuickFixIcon().FontSize(std::min(static_cast<double>(args.Width() / dpiScale), GetPadding().Left));
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
if (quickFixBtn.Visibility() == Visibility::Visible)
{
ShowQuickFixMenu();
}
}
}

Expand Down Expand Up @@ -3820,21 +3831,38 @@ namespace winrt::Microsoft::Terminal::Control::implementation
void TermControl::ShowQuickFixMenu()
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
{
auto quickFixBtn{ QuickFixButton() };
_quickFixesAvailable = true;

// If the gutter is narrow, display the collapsed version
const auto& termPadding = GetPadding();

_quickFixButtonCollapsible = termPadding.Left < CharacterDimensions().Width;
VisualStateManager::GoToState(*this, !_quickFixButtonCollapsible ? StateNormal : StateCollapsed, false);

// draw the button in the gutter
Controls::Canvas::SetLeft(quickFixBtn, -termPadding.Left);
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.BottomExclusive())
{
quickFixBtn.Visibility(Visibility::Collapsed);
return;
}

// draw the button in the gutter
const auto& cursorPosInDips = CursorPositionInDips();
Controls::Canvas::SetLeft(quickFixBtn, -termPadding.Left);
Controls::Canvas::SetTop(quickFixBtn, cursorPosInDips.Y);
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
quickFixBtn.Visibility(Visibility::Visible);
}

void TermControl::_clearQuickFix(const IInspectable& /*sender*/, const IInspectable& /*args*/)
{
_quickFixesAvailable = false;
QuickFixButton().Visibility(Visibility::Collapsed);
}

void TermControl::_PasteCommandHandler(const IInspectable& /*sender*/,
const IInspectable& /*args*/)
{
Expand Down
3 changes: 3 additions & 0 deletions src/cascadia/TerminalControl/TermControl.h
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
bool _focused{ false };
bool _initializedTerminal{ false };
bool _quickFixButtonCollapsible{ false };
bool _quickFixesAvailable{ false };

std::shared_ptr<ThrottledFuncLeading> _playWarningBell;

Expand Down Expand Up @@ -402,6 +403,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation

void _contextMenuHandler(IInspectable sender, Control::ContextMenuRequestedEventArgs args);
void _showContextMenuAt(const til::point& controlRelativePos);
void _clearQuickFix(const IInspectable& sender, const IInspectable& args);

void _PasteCommandHandler(const IInspectable& sender, const IInspectable& args);
void _CopyCommandHandler(const IInspectable& sender, const IInspectable& args);
Expand Down Expand Up @@ -433,6 +435,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
Control::ControlCore::CompletionsChanged_revoker CompletionsChanged;
Control::ControlCore::RestartTerminalRequested_revoker RestartTerminalRequested;
Control::ControlCore::SearchMissingCommand_revoker SearchMissingCommand;
Control::ControlCore::ClearQuickFix_revoker ClearQuickFix;

// These are set up in _InitializeTerminal
Control::ControlCore::RendererWarning_revoker RendererWarning;
Expand Down
10 changes: 1 addition & 9 deletions src/cascadia/TerminalControl/TermControl.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -1376,15 +1376,7 @@
</Grid>

<VisualStateManager.VisualStateGroups>
<VisualStateGroup x:Name="CommonStates">
<VisualStateGroup.Transitions>
<VisualTransition GeneratedDuration="0:0:0.5"
From="Collapsed"
To="Normal" />
<VisualTransition GeneratedDuration="0:0:0.5"
From="Normal"
To="Collapsed" />
</VisualStateGroup.Transitions>
<VisualStateGroup x:Name="QuickFixButtonStates">
<VisualState x:Name="Normal" />
<VisualState x:Name="Collapsed">
<VisualState.Setters>
Expand Down
Loading
Loading