Skip to content

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

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

Merged
merged 26 commits into from
Jun 29, 2024
Merged
Show file tree
Hide file tree
Changes from 13 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
17 changes: 17 additions & 0 deletions src/cascadia/CascadiaPackage/CascadiaPackage.wapproj
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,23 @@
</ItemGroup>
</Target>

<!-- We need to include some additional files in the package for things like WinGet's Command Not Found -->
<Target Name="IncludeAdditionalFilesInPackage">
<PropertyGroup>
<WinGetAdditionalPackageFileRoot>$(SolutionDir)\src\cascadia\TerminalControl</WinGetAdditionalPackageFileRoot>
</PropertyGroup>
<ItemGroup>
<WinGetAdditionalPackageFile Include="$(WinGetAdditionalPackageFileRoot)\Microsoft.Management.Deployment.winmd">
<PackagePath>Microsoft.Management.Deployment.winmd</PackagePath>
</WinGetAdditionalPackageFile>
</ItemGroup>
<ItemGroup>
<AppxPackagePayload Include="%(WinGetAdditionalPackageFile.Identity))">
<TargetPath>%(WinGetAdditionalPackageFile.PackagePath)</TargetPath>
</AppxPackagePayload>
</ItemGroup>
</Target>

<!-- This is required to get the package dependency in the AppXManifest. -->
<Import Project="$(OpenConsoleDir)src\common.nugetversions.targets" />

Expand Down
16 changes: 14 additions & 2 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1340,7 +1340,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::QuickFixes);
if (shouldGetContext)
{
if (const auto& control{ _GetActiveControl() })
Expand Down Expand Up @@ -1373,7 +1373,19 @@ namespace winrt::TerminalApp::implementation
if (WI_IsFlagSet(source, SuggestionsSource::CommandHistory) &&
context != nullptr)
{
const auto recentCommands = Command::HistoryToCommands(context.History(), currentCommandline, false);
// \ue81c --> History icon
const auto recentCommands = Command::HistoryToCommands(context.History(), currentCommandline, false, hstring{ L"\ue81c" });
for (const auto& t : recentCommands)
{
commandsCollection.push_back(t);
}
}

if (WI_IsFlagSet(source, SuggestionsSource::QuickFixes) &&
context != nullptr)
{
// \ue74c --> OEM icon
const auto recentCommands = Command::HistoryToCommands(context.QuickFixes(), hstring{ L"" }, false, hstring{ L"\ue74c" });
for (const auto& t : recentCommands)
{
commandsCollection.push_back(t);
Expand Down
88 changes: 88 additions & 0 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1671,6 +1671,8 @@ namespace winrt::TerminalApp::implementation

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

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

// Don't even register for the event if the feature is compiled off.
if constexpr (Feature_ShellCompletions::IsEnabled())
{
Expand All @@ -1689,6 +1691,12 @@ namespace winrt::TerminalApp::implementation
page->_PopulateContextMenu(weakTerm.get(), sender.try_as<MUX::Controls::CommandBarFlyout>(), true);
}
});
term.QuickFixMenu().Opening([weak = get_weak(), weakTerm](auto&& sender, auto&& /*args*/) {
if (const auto& page{ weak.get() })
{
page->_PopulateQuickFixMenu(weakTerm.get(), sender.try_as<Controls::MenuFlyout>());
}
});
}

// Method Description:
Expand Down Expand Up @@ -2896,6 +2904,30 @@ namespace winrt::TerminalApp::implementation
ShowWindowChanged.raise(*this, args);
}

winrt::fire_and_forget TerminalPage::_SearchMissingCommandHandler(const IInspectable /*sender*/, const Microsoft::Terminal::Control::SearchMissingCommandEventArgs args)
{
assert(!Dispatcher().HasThreadAccess());

if (!Feature_QuickFix::IsEnabled())
{
co_return;
}

std::vector<hstring> suggestions;
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.


co_await wil::resume_foreground(Dispatcher());

auto term = _GetActiveControl();
if (!term)
{
co_return;
}
term.UpdateWinGetSuggestions(single_threaded_vector<hstring>(std::move(suggestions)));
term.ShowQuickFixMenu();
}

// Method Description:
// - Paste text from the Windows Clipboard to the focused terminal
void TerminalPage::_PasteText()
Expand Down Expand Up @@ -4814,6 +4846,62 @@ namespace winrt::TerminalApp::implementation
makeItem(RS_(L"TabClose"), L"\xE711", ActionAndArgs{ ShortcutAction::CloseTab, CloseTabArgs{ _GetFocusedTabIndex().value() } });
}

void TerminalPage::_PopulateQuickFixMenu(const TermControl& control,
const Controls::MenuFlyout& menu)
{
if (!control || !menu)
{
return;
}

// Helper lambda for dispatching a SendInput ActionAndArgs onto the
// ShortcutActionDispatch. Used below to wire up each menu entry to the
// respective action. Then clear the quick fix menu.
auto weak = get_weak();
auto makeCallback = [weak](const hstring& suggestion) {
return [weak, suggestion](auto&&, auto&&) {
if (auto page{ weak.get() })
{
const auto actionAndArgs = ActionAndArgs{ ShortcutAction::SendInput, SendInputArgs{ hstring{ L"\u0003" } + suggestion } };
page->_actionDispatch->DoAction(actionAndArgs);
if (auto ctrl = page->_GetActiveControl())
{
ctrl.ClearQuickFix();
}
}
};
};

auto makeItem = [&menu, &makeCallback](const winrt::hstring& label,
const winrt::hstring& icon,
const winrt::hstring& suggestion) {
MenuFlyoutItem item{};

if (!icon.empty())
{
auto iconElement = UI::IconPathConverter::IconWUX(icon);
Automation::AutomationProperties::SetAccessibilityView(iconElement, Automation::Peers::AccessibilityView::Raw);
item.Icon(iconElement);
}

item.Text(label);
item.Click(makeCallback(suggestion));
menu.Items().Append(item);
};

// Wire up each item to the action that should be performed. By actually
// connecting these to actions, we ensure the implementation is
// consistent. This also leaves room for customizing this menu with
// actions in the future.

menu.Items().Clear();
const auto quickFixes = control.CommandHistory().QuickFixes();
for (const auto& qf : quickFixes)
{
makeItem(qf, L"\ue74c", qf);
}
}

// Handler for our WindowProperties's PropertyChanged event. We'll use this
// to pop the "Identify Window" toast when the user renames our window.
winrt::fire_and_forget TerminalPage::_windowPropertyChanged(const IInspectable& /*sender*/,
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 @@ -520,6 +520,7 @@ 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);

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

Expand All @@ -537,6 +538,7 @@ namespace winrt::TerminalApp::implementation
void _sendDraggedTabToWindow(const winrt::hstring& windowId, const uint32_t tabIndex, std::optional<til::point> dragPoint);

void _PopulateContextMenu(const Microsoft::Terminal::Control::TermControl& control, const Microsoft::UI::Xaml::Controls::CommandBarFlyout& sender, const bool withSelection);
void _PopulateQuickFixMenu(const Microsoft::Terminal::Control::TermControl& control, const Windows::UI::Xaml::Controls::MenuFlyout& sender);
winrt::Windows::UI::Xaml::Controls::MenuFlyout _CreateRunAsAdminFlyout(int profileIndex);

winrt::Microsoft::Terminal::Control::TermControl _senderOrActiveControl(const winrt::Windows::Foundation::IInspectable& sender);
Expand Down
27 changes: 27 additions & 0 deletions src/cascadia/TerminalControl/ControlCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <unicode.hpp>
#include <utils.hpp>
#include <WinUser.h>
//#include <winrt/Microsoft.Management.Deployment.h>

#include "EventArgs.h"
#include "../../renderer/atlas/AtlasEngine.h"
Expand All @@ -22,6 +23,7 @@
#include "ControlCore.g.cpp"
#include "SelectionColor.g.cpp"

//using namespace winrt::Microsoft::Management::Deployment;
using namespace ::Microsoft::Console::Types;
using namespace ::Microsoft::Console::VirtualTerminal;
using namespace ::Microsoft::Terminal::Core;
Expand Down Expand Up @@ -110,6 +112,12 @@ 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)); };
_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 @@ -1612,6 +1620,16 @@ 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)
{
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 Expand Up @@ -2258,9 +2276,18 @@ namespace winrt::Microsoft::Terminal::Control::implementation
auto context = winrt::make_self<CommandHistoryContext>(std::move(commands));
context->CurrentCommandline(winrt::hstring{ _terminal->CurrentCommand() });

// TODO CARLOS: should we delete this after a new command is run? Or delete it after a suggestion is used? Or just after the next winget suggestion (current impl)?
// No clue which we should do. Thoughts?
context->QuickFixes(_cachedQuickFixes);

return *context;
}

void ControlCore::UpdateQuickFixes(const Windows::Foundation::Collections::IVector<hstring>& quickFixes)
Copy link
Member

Choose a reason for hiding this comment

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

this is more of a Set than an Update. Update is like "do it on your own". Set is more like "i will give it to you"

{
_cachedQuickFixes = quickFixes;
}

Core::Scheme ControlCore::ColorScheme() const noexcept
{
Core::Scheme s;
Expand Down
7 changes: 7 additions & 0 deletions src/cascadia/TerminalControl/ControlCore.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
til::property<Windows::Foundation::Collections::IVector<winrt::hstring>> History;
til::property<winrt::hstring> CurrentCommandline;
til::property<Windows::Foundation::Collections::IVector<winrt::hstring>> QuickFixes;

CommandHistoryContext(std::vector<winrt::hstring>&& history)
{
Expand Down Expand Up @@ -242,6 +243,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation

hstring ReadEntireBuffer() const;
Control::CommandHistoryContext CommandHistory() const;
void UpdateQuickFixes(const Windows::Foundation::Collections::IVector<hstring>& quickFixes);

void AdjustOpacity(const float opacity, const bool relative);

Expand Down Expand Up @@ -284,6 +286,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
til::typed_event<IInspectable, Control::UpdateSelectionMarkersEventArgs> UpdateSelectionMarkers;
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 @@ -354,6 +358,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
til::point _contextMenuBufferPosition{ 0, 0 };

Windows::Foundation::Collections::IVector<int32_t> _cachedSearchResultRows{ nullptr };
Copy link
Member

Choose a reason for hiding this comment

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

what is this?

Copy link
Member

Choose a reason for hiding this comment

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

it looks like a line that was removed in #17132. bad conflict resolution?

Windows::Foundation::Collections::IVector<hstring> _cachedQuickFixes{ nullptr };

void _setupDispatcherAndCallbacks();

Expand All @@ -378,6 +383,8 @@ 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 _terminalClearQuickFix();

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

Expand Down
3 changes: 3 additions & 0 deletions src/cascadia/TerminalControl/ControlCore.idl
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ namespace Microsoft.Terminal.Control
{
IVector<String> History { get; };
String CurrentCommandline { get; };
IVector<String> QuickFixes { get; };
};

[default_interface] runtimeclass ControlCore : ICoreState
Expand Down Expand Up @@ -174,6 +175,8 @@ namespace Microsoft.Terminal.Control
event Windows.Foundation.TypedEventHandler<Object, Object> TaskbarProgressChanged;
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
1 change: 1 addition & 0 deletions src/cascadia/TerminalControl/EventArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@
#include "KeySentEventArgs.g.cpp"
#include "CharSentEventArgs.g.cpp"
#include "StringSentEventArgs.g.cpp"
#include "SearchMissingCommandEventArgs.g.cpp"
10 changes: 10 additions & 0 deletions src/cascadia/TerminalControl/EventArgs.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "KeySentEventArgs.g.h"
#include "CharSentEventArgs.g.h"
#include "StringSentEventArgs.g.h"
#include "SearchMissingCommandEventArgs.g.h"

namespace winrt::Microsoft::Terminal::Control::implementation
{
Expand Down Expand Up @@ -211,6 +212,15 @@ namespace winrt::Microsoft::Terminal::Control::implementation

WINRT_PROPERTY(winrt::hstring, Text);
};

struct SearchMissingCommandEventArgs : public SearchMissingCommandEventArgsT<SearchMissingCommandEventArgs>
{
public:
SearchMissingCommandEventArgs(const winrt::hstring& missingCommand) :
_MissingCommand(missingCommand) {}

WINRT_PROPERTY(winrt::hstring, MissingCommand);
};
}

namespace winrt::Microsoft::Terminal::Control::factory_implementation
Expand Down
5 changes: 5 additions & 0 deletions src/cascadia/TerminalControl/EventArgs.idl
Original file line number Diff line number Diff line change
Expand Up @@ -119,4 +119,9 @@ namespace Microsoft.Terminal.Control
{
String Text { get; };
}

runtimeclass SearchMissingCommandEventArgs
{
String MissingCommand { get; };
}
}
Binary file not shown.
6 changes: 6 additions & 0 deletions src/cascadia/TerminalControl/Resources/en-US/Resources.resw
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,12 @@ Please either install the missing font or choose another one.</value>
<value>Select output</value>
<comment>The tooltip for a button for selecting all of a command's output</comment>
</data>
<data name="QuickFixButton.ToolTipService.ToolTip" xml:space="preserve">
<value>Quick fix</value>
</data>
<data name="QuickFixButton.[using:Windows.UI.Xaml.Automation]AutomationProperties.Name" xml:space="preserve">
<value>Quick fix</value>
</data>
<data name="SessionRestoreMessage" xml:space="preserve">
<value>Restored</value>
<comment>"Restored" as in "This content was restored"</comment>
Expand Down
Loading
Loading