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 21 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: 15 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,20 @@ 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 &&
context.QuickFixes() != 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
83 changes: 83 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 @@ -2922,6 +2930,25 @@ 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());

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.

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

co_await wil::resume_foreground(Dispatcher());

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

// Method Description:
// - Paste text from the Windows Clipboard to the focused terminal
void TerminalPage::_PasteText()
Expand Down Expand Up @@ -4842,6 +4869,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) {
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
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
28 changes: 28 additions & 0 deletions src/cascadia/TerminalControl/ControlCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,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] { ClearQuickFix(); };
_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 @@ -1609,6 +1615,17 @@ 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::ClearQuickFix()
{
_cachedQuickFixes = nullptr;
RefreshQuickFixUI.raise(*this, nullptr);
Copy link
Member

Choose a reason for hiding this comment

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

mildly alarmed that ClearQuickFix seems to refresh the QF UI but UpdateQuickFixes doesn't

}

bool ControlCore::HasSelection() const
{
const auto lock = _terminal->LockForReading();
Expand Down Expand Up @@ -2279,9 +2296,20 @@ namespace winrt::Microsoft::Terminal::Control::implementation

auto context = winrt::make_self<CommandHistoryContext>(std::move(commands));
context->CurrentCommandline(trimmedCurrentCommand);
context->QuickFixes(_cachedQuickFixes);
return *context;
}

bool ControlCore::QuickFixesAvailable() const noexcept
{
return _cachedQuickFixes && _cachedQuickFixes.Size() > 0;
}

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
14 changes: 13 additions & 1 deletion src/cascadia/TerminalControl/ControlCore.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,10 @@ 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;
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved

CommandHistoryContext(std::vector<winrt::hstring>&& history)
CommandHistoryContext(std::vector<winrt::hstring>&& history) :
QuickFixes(winrt::single_threaded_vector<winrt::hstring>())
{
History(winrt::single_threaded_vector<winrt::hstring>(std::move(history)));
}
Expand Down Expand Up @@ -153,6 +155,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
void PersistToPath(const wchar_t* path) const;
void RestoreFromPath(const wchar_t* path) const;

void ClearQuickFix();

#pragma region ICoreState
const size_t TaskbarState() const noexcept;
const size_t TaskbarProgress() const noexcept;
Expand Down Expand Up @@ -241,6 +245,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation

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

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

Expand Down Expand Up @@ -283,6 +289,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<> RefreshQuickFixUI;

til::typed_event<> CloseTerminalRequested;
til::typed_event<> RestartTerminalRequested;
Expand Down Expand Up @@ -352,6 +360,9 @@ 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?

DHowett marked this conversation as resolved.
Show resolved Hide resolved
Windows::Foundation::Collections::IVector<hstring> _cachedQuickFixes{ nullptr };

void _setupDispatcherAndCallbacks();

bool _setFontSizeUnderLock(float fontSize);
Expand All @@ -375,6 +386,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);

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

Expand Down
6 changes: 6 additions & 0 deletions src/cascadia/TerminalControl/ControlCore.idl
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,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 @@ -155,6 +156,7 @@ namespace Microsoft.Terminal.Control

String ReadEntireBuffer();
CommandHistoryContext CommandHistory();
Boolean QuickFixesAvailable { get; };

void AdjustOpacity(Single Opacity, Boolean relative);
void WindowVisibilityChanged(Boolean showOrHide);
Expand All @@ -166,6 +168,8 @@ namespace Microsoft.Terminal.Control
Boolean ShouldShowSelectCommand();
Boolean ShouldShowSelectOutput();

void ClearQuickFix();

// These events are called from some background thread
event Windows.Foundation.TypedEventHandler<Object, TitleChangedEventArgs> TitleChanged;
event Windows.Foundation.TypedEventHandler<Object, Object> WarningBell;
Expand All @@ -174,6 +178,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> RefreshQuickFixUI;

// 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) {}

til::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; };
}
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
}
15 changes: 14 additions & 1 deletion 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 All @@ -312,4 +318,11 @@ Please either install the missing font or choose another one.</value>
<value>invalid</value>
<comment>This brief message is displayed when a regular expression is invalid.</comment>
</data>
</root>
<data name="QuickFixAvailable" xml:space="preserve">
<value>Quick fix available</value>
<comment>"Quick fix" is referencing the same feature as "QuickFixButton.ToolTipService.ToolTip".</comment>
</data>
<data name="QuickFixMenu.[using:Windows.UI.Xaml.Automation]AutomationProperties.Name" xml:space="preserve">
<value>Quick fix</value>
</data>
</root>
Loading
Loading