Skip to content

Commit

Permalink
Make Mark Mode keybindings precede custom keybindings (#13659)
Browse files Browse the repository at this point in the history 
## Summary of the Pull Request
This PR moves the key handling for mark mode into a helper method that is then called before an action/key binding is attempted. 

## References
Epic: #4993 
Closes #13533

## Validation Steps Performed
- Add custom keybinding to "down" arrow key
- in mark mode --> selection updates appropriately
- out of mark mode --> keybinding executed
  • Loading branch information
@carlos-zamora
carlos-zamora authored Aug 22, 2022
1 parent 2dedc9a commit 70313db
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 59 deletions.
114 changes: 67 additions & 47 deletions src/cascadia/TerminalControl/ControlCore.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,72 @@ namespace winrt::Microsoft::Terminal::Control::implementation
return _terminal->SendCharEvent(ch, scanCode, modifiers);
}

bool ControlCore::_shouldTryUpdateSelection(const WORD vkey)
{
// GH#6423 - don't update selection if the key that was pressed was a
// modifier key. We'll wait for a real keystroke to dismiss the
// GH #7395 - don't update selection when taking PrintScreen
// selection.
return HasSelection() && !KeyEvent::IsModifierKey(vkey) && vkey != VK_SNAPSHOT;
}

bool ControlCore::TryMarkModeKeybinding(const WORD vkey,
const ::Microsoft::Terminal::Core::ControlKeyStates mods)
{
if (_shouldTryUpdateSelection(vkey) && _terminal->SelectionMode() == ::Terminal::SelectionInteractionMode::Mark)
{
if (vkey == 'A' && !mods.IsAltPressed() && !mods.IsShiftPressed() && mods.IsCtrlPressed())
{
// Ctrl + A --> Select all
auto lock = _terminal->LockForWriting();
_terminal->SelectAll();
_updateSelectionUI();
return true;
}
else if (vkey == VK_TAB && !mods.IsAltPressed() && !mods.IsCtrlPressed() && _settings->DetectURLs())
{
// [Shift +] Tab --> next/previous hyperlink
auto lock = _terminal->LockForWriting();
const auto direction = mods.IsShiftPressed() ? ::Terminal::SearchDirection::Backward : ::Terminal::SearchDirection::Forward;
_terminal->SelectHyperlink(direction);
_updateSelectionUI();
return true;
}
else if (vkey == VK_RETURN && mods.IsCtrlPressed() && _terminal->IsTargetingUrl())
{
// Ctrl + Enter --> Open URL
auto lock = _terminal->LockForReading();
const auto uri = _terminal->GetHyperlinkAtBufferPosition(_terminal->GetSelectionAnchor());
_OpenHyperlinkHandlers(*this, winrt::make<OpenHyperlinkEventArgs>(winrt::hstring{ uri }));
return true;
}
else if (vkey == VK_RETURN && !mods.IsCtrlPressed() && !mods.IsAltPressed())
{
// [Shift +] Enter --> copy text
// Don't lock here! CopySelectionToClipboard already locks for you!
CopySelectionToClipboard(mods.IsShiftPressed(), nullptr);
_terminal->ClearSelection();
_updateSelectionUI();
return true;
}
else if (vkey == VK_ESCAPE)
{
_terminal->ClearSelection();
_updateSelectionUI();
return true;
}
else if (const auto updateSlnParams{ _terminal->ConvertKeyEventToUpdateSelectionParams(mods, vkey) })
{
// try to update the selection
auto lock = _terminal->LockForWriting();
_terminal->UpdateSelection(updateSlnParams->first, updateSlnParams->second, mods);
_updateSelectionUI();
return true;
}
}
return false;
}

// Method Description:
// - Send this particular key event to the terminal.
// See Terminal::SendKeyEvent for more information.
Expand All @@ -406,57 +472,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
const bool keyDown)
{
// Update the selection, if it's present
// GH#6423 - don't dismiss selection if the key that was pressed was a
// modifier key. We'll wait for a real keystroke to dismiss the
// GH #7395 - don't dismiss selection when taking PrintScreen
// selection.
// GH#8522, GH#3758 - Only modify the selection on key _down_. If we
// modify on key up, then there's chance that we'll immediately dismiss
// a selection created by an action bound to a keydown.
if (HasSelection() &&
!KeyEvent::IsModifierKey(vkey) &&
vkey != VK_SNAPSHOT &&
keyDown)
if (_shouldTryUpdateSelection(vkey) && keyDown)
{
const auto isInMarkMode = _terminal->SelectionMode() == ::Terminal::SelectionInteractionMode::Mark;
if (isInMarkMode)
{
if (modifiers.IsCtrlPressed() && vkey == 'A')
{
// Ctrl + A --> Select all
auto lock = _terminal->LockForWriting();
_terminal->SelectAll();
_updateSelectionUI();
return true;
}
else if (vkey == VK_TAB && _settings->DetectURLs())
{
// [Shift +] Tab --> next/previous hyperlink
auto lock = _terminal->LockForWriting();
const auto direction = modifiers.IsShiftPressed() ? ::Terminal::SearchDirection::Backward : ::Terminal::SearchDirection::Forward;
_terminal->SelectHyperlink(direction);
_updateSelectionUI();
return true;
}
else if (vkey == VK_RETURN && modifiers.IsCtrlPressed() && _terminal->IsTargetingUrl())
{
// Ctrl + Enter --> Open URL
auto lock = _terminal->LockForReading();
const auto uri = _terminal->GetHyperlinkAtBufferPosition(_terminal->GetSelectionAnchor());
_OpenHyperlinkHandlers(*this, winrt::make<OpenHyperlinkEventArgs>(winrt::hstring{ uri }));
return true;
}
else if (vkey == VK_RETURN)
{
// [Shift +] Enter --> copy text
// Don't lock here! CopySelectionToClipboard already locks for you!
CopySelectionToClipboard(modifiers.IsShiftPressed(), nullptr);
_terminal->ClearSelection();
_updateSelectionUI();
return true;
}
}

// try to update the selection
if (const auto updateSlnParams{ _terminal->ConvertKeyEventToUpdateSelectionParams(modifiers, vkey) })
{
Expand Down
3 changes: 3 additions & 0 deletions src/cascadia/TerminalControl/ControlCore.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
void ToggleMarkMode();
Control::SelectionInteractionMode SelectionMode() const;
bool SwitchSelectionEndpoint();
bool TryMarkModeKeybinding(const WORD vkey,
const ::Microsoft::Terminal::Core::ControlKeyStates modifiers);

void GotFocus();
void LostFocus();
Expand Down Expand Up @@ -275,6 +277,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
void _updateFont(const bool initialUpdate = false);
void _refreshSizeUnderLock();
void _updateSelectionUI();
bool _shouldTryUpdateSelection(const WORD vkey);

void _sendInputToConnection(std::wstring_view wstr);

Expand Down
20 changes: 9 additions & 11 deletions src/cascadia/TerminalControl/ControlCore.idl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ namespace Microsoft.Terminal.Control
// !! LOAD BEARING !! If you make this a struct with Booleans (like they
// make the most sense as), then the app will crash trying to toss one of
// these across the process boundary. I haven't the damndest idea why.
[flags]
enum MouseButtonState
{
[flags] enum MouseButtonState {
IsLeftButtonDown = 0x1,
IsMiddleButtonDown = 0x2,
IsRightButtonDown = 0x4
Expand All @@ -37,9 +35,7 @@ namespace Microsoft.Terminal.Control
Mark
};

[flags]
enum SelectionEndpointTarget
{
[flags] enum SelectionEndpointTarget {
Start = 0x1,
End = 0x2
};
Expand Down Expand Up @@ -79,13 +75,15 @@ namespace Microsoft.Terminal.Control
Double Opacity { get; };
Boolean UseAcrylic { get; };

Boolean TryMarkModeKeybinding(Int16 vkey,
Microsoft.Terminal.Core.ControlKeyStates modifiers);
Boolean TrySendKeyEvent(Int16 vkey,
Int16 scanCode,
Microsoft.Terminal.Core.ControlKeyStates modifiers,
Boolean keyDown);
Int16 scanCode,
Microsoft.Terminal.Core.ControlKeyStates modifiers,
Boolean keyDown);
Boolean SendCharEvent(Char ch,
Int16 scanCode,
Microsoft.Terminal.Core.ControlKeyStates modifiers);
Int16 scanCode,
Microsoft.Terminal.Core.ControlKeyStates modifiers);
void SendInput(String text);
void PasteText(String text);
void SelectAll();
Expand Down
12 changes: 11 additions & 1 deletion src/cascadia/TerminalControl/TermControl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1109,7 +1109,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// keybindings on the keyUp, then we'll still send the keydown to the
// connected terminal application, and something like ctrl+shift+T will
// emit a ^T to the pipe.
if (!modifiers.IsAltGrPressed() && keyDown && _TryHandleKeyBinding(vkey, scanCode, modifiers))
if (!modifiers.IsAltGrPressed() &&
keyDown &&
_TryHandleKeyBinding(vkey, scanCode, modifiers))
{
e.Handled(true);
return;
Expand All @@ -1135,6 +1137,14 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// - modifiers: The ControlKeyStates representing the modifier key states.
bool TermControl::_TryHandleKeyBinding(const WORD vkey, const WORD scanCode, ::Microsoft::Terminal::Core::ControlKeyStates modifiers) const
{
// Mark mode has a specific set of pre-defined key bindings.
// If we're in mark mode, we should be prioritizing those over
// the custom defined key bindings.
if (_core.TryMarkModeKeybinding(vkey, modifiers))
{
return true;
}

// TODO: GH#5000
// The Core owning the keybindings is weird. That's for sure. In the
// future, we may want to pass the keybindings into the control
Expand Down

0 comments on commit 70313db

Please sign in to comment.