Skip to content

Commit

Permalink
Add action for Quick Fix and key binding for Suggestions (#17502)
Browse files Browse the repository at this point in the history
Adds a keybinding to open the quick fix menu, if one is available. When
the action is used, we also open up the button (if it was collapsed)
because that looks nice.

The `showSuggestions` action is bound to `ctrl+shift+period` by default
to align with VS' "quick actions" feature and VS Code's "quick fix"
feature. This was chosen over binding to `quickFix` because it's more
helpful. The quick fix button is a route for users that prefer to use
the mouse. If users want to add a keybinding to activate the `quickFix`
button, they can do that now.

This PR also performs a bit of miscellaneous polish from the bug bash.
This includes:
- the suggestions UI now presents quick fixes first
- scrolling may result in the button being drawn in the wrong place
- The bug was tracked down this line:
`TermControl::CursorPositionInDips()` --> `_core.CursorPosition()` -->
`Terminal::GetViewportRelativeCursorPosition()`. The mutable viewport
there does _not_ update when the user scrolls. Thus, the button would be
drawn on the same position _on the screen_ even though we scrolled. To
fix this, I include the `_scrollOffset` in the calculation. The only
other place this function is used is with the suggestions UI, which does
_not_ update the UIs position as we scroll (but if we're interested in
doing that, we can now).

Closes #17377
  • Loading branch information
carlos-zamora authored Jul 17, 2024
1 parent 1feb56e commit 1999366
Show file tree
Hide file tree
Showing 9 changed files with 74 additions and 21 deletions.
36 changes: 23 additions & 13 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1461,7 +1461,19 @@ namespace winrt::TerminalApp::implementation
}

// Aggregate all the commands from the different sources that
// the user selected.
// the user selected. This is the order presented to the user

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);
}
}

// Tasks are all the sendInput commands the user has saved in
// their settings file. Ask the ActionMap for those.
Expand All @@ -1488,18 +1500,6 @@ namespace winrt::TerminalApp::implementation
}
}

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);
}
}

// Open the palette with all these commands in it.
_OpenSuggestions(_GetActiveControl(),
winrt::single_threaded_vector<Command>(std::move(commandsCollection)),
Expand Down Expand Up @@ -1593,4 +1593,14 @@ namespace winrt::TerminalApp::implementation
_ShowAboutDialog();
args.Handled(true);
}

void TerminalPage::_HandleQuickFix(const IInspectable& /*sender*/,
const ActionEventArgs& args)
{
if (const auto& control{ _GetActiveControl() })
{
const auto handled = control.OpenQuickFixMenu();
args.Handled(handled);
}
}
}
39 changes: 35 additions & 4 deletions src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,19 @@ namespace winrt::Microsoft::Terminal::Control::implementation
}
}
});
if constexpr (Feature_QuickFix::IsEnabled())
{
QuickFixMenu().Closed([weakThis = get_weak()](auto&&, auto&&) {
if (auto control{ weakThis.get() }; control && !control->_IsClosing())
{
// Expand the quick fix button if it's collapsed (looks nicer)
if (control->_quickFixButtonCollapsible)
{
VisualStateManager::GoToState(*control, StateCollapsed, false);
}
}
});
}
}

void TermControl::_QuickFixButton_PointerEntered(const IInspectable& /*sender*/, const PointerRoutedEventArgs& /*e*/)
Expand Down Expand Up @@ -3555,8 +3568,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation

if (Feature_QuickFix::IsEnabled())
{
auto quickFixBtn = FindName(L"QuickFixButton").as<Controls::Button>();
quickFixBtn.Height(args.Height() / dpiScale);
QuickFixButton().Height(args.Height() / dpiScale);
QuickFixIcon().FontSize(static_cast<double>(args.Width() / dpiScale));
RefreshQuickFixMenu();
}
Expand Down Expand Up @@ -3892,14 +3904,33 @@ namespace winrt::Microsoft::Terminal::Control::implementation
return std::max(CharacterDimensions().Width * 2.0 / 3.0, GetPadding().Left);
}

bool TermControl::OpenQuickFixMenu()
{
if constexpr (Feature_QuickFix::IsEnabled())
{
if (_core.QuickFixesAvailable())
{
// Expand the quick fix button if it's collapsed (looks nicer)
if (_quickFixButtonCollapsible)
{
VisualStateManager::GoToState(*this, StateNormal, false);
}
auto quickFixBtn = QuickFixButton();
quickFixBtn.Flyout().ShowAt(quickFixBtn);
return true;
}
}
return false;
}

void TermControl::RefreshQuickFixMenu()
{
if (!Feature_QuickFix::IsEnabled())
{
return;
}

auto quickFixBtn = FindName(L"QuickFixButton").as<Controls::Button>();
auto quickFixBtn = QuickFixButton();
if (!_core.QuickFixesAvailable())
{
quickFixBtn.Visibility(Visibility::Collapsed);
Expand All @@ -3920,7 +3951,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
const auto viewportBufferPosition = rd->GetViewport();
const auto cursorBufferPosition = rd->GetCursorPosition();
rd->UnlockConsole();
if (cursorBufferPosition.y < viewportBufferPosition.Top() || cursorBufferPosition.y > viewportBufferPosition.BottomExclusive())
if (cursorBufferPosition.y < viewportBufferPosition.Top() || cursorBufferPosition.y > viewportBufferPosition.BottomInclusive())
{
quickFixBtn.Visibility(Visibility::Collapsed);
return;
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 @@ -182,6 +182,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
void RawWriteString(const winrt::hstring& text);

void ShowContextMenu();
bool OpenQuickFixMenu();
void RefreshQuickFixMenu();
void ClearQuickFix();

Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalControl/TermControl.idl
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ namespace Microsoft.Terminal.Control
Double QuickFixButtonCollapsedWidth { get; };

void ShowContextMenu();
Boolean OpenQuickFixMenu();
void RefreshQuickFixMenu();
void ClearQuickFix();

Expand Down
7 changes: 4 additions & 3 deletions src/cascadia/TerminalCore/Terminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1597,12 +1597,13 @@ void Terminal::ColorSelection(const TextAttribute& attr, winrt::Microsoft::Termi
}

// Method Description:
// - Returns the position of the cursor relative to the active viewport
// - Returns the position of the cursor relative to the visible viewport
til::point Terminal::GetViewportRelativeCursorPosition() const noexcept
{
const auto absoluteCursorPosition{ GetCursorPosition() };
const auto viewport{ _GetMutableViewport() };
return absoluteCursorPosition - viewport.Origin();
const auto mutableViewport{ _GetMutableViewport() };
const auto relativeCursorPos = absoluteCursorPosition - mutableViewport.Origin();
return { relativeCursorPos.x, relativeCursorPos.y + _scrollOffset };
}

void Terminal::PreviewText(std::wstring_view input)
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ static constexpr std::string_view RestartConnectionKey{ "restartConnection" };
static constexpr std::string_view ToggleBroadcastInputKey{ "toggleBroadcastInput" };
static constexpr std::string_view OpenScratchpadKey{ "experimental.openScratchpad" };
static constexpr std::string_view OpenAboutKey{ "openAbout" };
static constexpr std::string_view QuickFixKey{ "quickFix" };

static constexpr std::string_view ActionKey{ "action" };

Expand Down Expand Up @@ -438,6 +439,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
{ ShortcutAction::ToggleBroadcastInput, RS_(L"ToggleBroadcastInputCommandKey") },
{ ShortcutAction::OpenScratchpad, RS_(L"OpenScratchpadKey") },
{ ShortcutAction::OpenAbout, RS_(L"OpenAboutCommandKey") },
{ ShortcutAction::QuickFix, RS_(L"QuickFixCommandKey") },
};
}();

Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalSettingsModel/AllShortcutActions.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@
ON_ALL_ACTIONS(RestartConnection) \
ON_ALL_ACTIONS(ToggleBroadcastInput) \
ON_ALL_ACTIONS(OpenScratchpad) \
ON_ALL_ACTIONS(OpenAbout)
ON_ALL_ACTIONS(OpenAbout) \
ON_ALL_ACTIONS(QuickFix)

#define ALL_SHORTCUT_ACTIONS_WITH_ARGS \
ON_ALL_ACTIONS_WITH_ARGS(AdjustFontSize) \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -734,4 +734,7 @@
<data name="SaveSnippetNamePrefix" xml:space="preserve">
<value>Save Snippet</value>
</data>
<data name="QuickFixCommandKey" xml:space="preserve">
<value>Open quick fix menu</value>
</data>
</root>
3 changes: 3 additions & 0 deletions src/cascadia/TerminalSettingsModel/defaults.json
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,8 @@
{ "command": "restoreLastClosed", "id": "Terminal.RestoreLastClosed" },
{ "command": "openAbout", "id": "Terminal.OpenAboutDialog" },
{ "command": "experimental.openTasks", "id": "Terminal.OpenTasks" },
{ "command": "quickFix", "id": "Terminal.QuickFix" },
{ "command": { "action": "showSuggestions", "source": "all"}, "id": "Terminal.Suggestions" },

// Tab Management
// "command": "closeTab" is unbound by default.
Expand Down Expand Up @@ -639,6 +641,7 @@
{ "keys":"ctrl+shift+p", "id": "Terminal.ToggleCommandPalette" },
{ "keys":"win+sc(41)", "id": "Terminal.QuakeMode" },
{ "keys": "alt+space", "id": "Terminal.OpenSystemMenu" },
{ "keys": "ctrl+shift+period", "id": "Terminal.Suggestions" },

// Tab Management
// "command": "closeTab" is unbound by default.
Expand Down

0 comments on commit 1999366

Please sign in to comment.