-
Notifications
You must be signed in to change notification settings - Fork 8.5k
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
Implement Keyboard Selection #10824
Implement Keyboard Selection #10824
Changes from 9 commits
428dec9
bcbf01e
a0e4f2d
8a64613
724cd2c
bf0cf5f
b13b8a7
118d1cd
30009eb
b784262
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,6 +33,74 @@ constexpr const auto TsfRedrawInterval = std::chrono::milliseconds(100); | |
// The minimum delay between updating the locations of regex patterns | ||
constexpr const auto UpdatePatternLocationsInterval = std::chrono::milliseconds(500); | ||
|
||
static constexpr std::optional<Terminal::SelectionDirection> ConvertVKeyToSelectionDirection(WORD vkey) | ||
{ | ||
switch (vkey) | ||
{ | ||
case VK_LEFT: | ||
case VK_HOME: | ||
return Terminal::SelectionDirection::Left; | ||
case VK_RIGHT: | ||
case VK_END: | ||
return Terminal::SelectionDirection::Right; | ||
case VK_UP: | ||
case VK_PRIOR: | ||
return Terminal::SelectionDirection::Up; | ||
case VK_DOWN: | ||
case VK_NEXT: | ||
return Terminal::SelectionDirection::Down; | ||
default: | ||
return std::nullopt; | ||
} | ||
} | ||
carlos-zamora marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
static constexpr std::optional<std::tuple<Terminal::SelectionDirection, Terminal::SelectionExpansion>> ConvertKeyEventToUpdateSelectionParams(const ControlKeyStates mods, const WORD vkey) | ||
{ | ||
if (mods.IsShiftPressed() && !mods.IsAltPressed()) | ||
{ | ||
if (const auto dir{ ConvertVKeyToSelectionDirection(vkey) }) | ||
{ | ||
if (mods.IsCtrlPressed()) | ||
{ | ||
switch (vkey) | ||
{ | ||
case VK_LEFT: | ||
case VK_RIGHT: | ||
// Move by word | ||
return std::make_tuple(*dir, Terminal::SelectionExpansion::Word); | ||
case VK_HOME: | ||
case VK_END: | ||
// Move by buffer | ||
return std::make_tuple(*dir, Terminal::SelectionExpansion::Buffer); | ||
default: | ||
__fallthrough; | ||
} | ||
} | ||
else | ||
{ | ||
switch (vkey) | ||
{ | ||
case VK_HOME: | ||
case VK_END: | ||
case VK_PRIOR: | ||
case VK_NEXT: | ||
// Move by viewport | ||
return std::make_tuple(*dir, Terminal::SelectionExpansion::Viewport); | ||
case VK_LEFT: | ||
case VK_RIGHT: | ||
case VK_UP: | ||
case VK_DOWN: | ||
// Move by character | ||
return std::make_tuple(*dir, Terminal::SelectionExpansion::Char); | ||
default: | ||
__fallthrough; | ||
} | ||
} | ||
} | ||
} | ||
return std::nullopt; | ||
} | ||
|
||
namespace winrt::Microsoft::Terminal::Control::implementation | ||
{ | ||
// Helper static function to ensure that all ambiguous-width glyphs are reported as narrow. | ||
|
@@ -359,28 +427,38 @@ namespace winrt::Microsoft::Terminal::Control::implementation | |
const ControlKeyStates modifiers, | ||
const bool keyDown) | ||
{ | ||
// When there is a selection active, escape should clear it and NOT flow through | ||
// to the terminal. With any other keypress, it should clear the selection AND | ||
// flow through to the terminal. | ||
// 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 dismiss the selection on key _down_. If we | ||
// dismiss on key up, then there's chance that we'll immediately dismiss | ||
// 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. <showerthought> If we could handle this modification on key up, then could we do something like bind This is of course, crazy, but something to think about There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. HECK IT DOESN"T EVEN MATTER a user could just bind markmost to shift+left, and that would start a selection at the cursor, and the subsequent keypresses would extend it without hitting the keybinding, which is actually just like conhost. Brilliant 🥂 |
||
// a selection created by an action bound to a keydown. | ||
if (HasSelection() && | ||
!KeyEvent::IsModifierKey(vkey) && | ||
vkey != VK_SNAPSHOT && | ||
keyDown) | ||
{ | ||
// try to update the selection | ||
if (const auto updateSlnParams{ ConvertKeyEventToUpdateSelectionParams(modifiers, vkey) }) | ||
{ | ||
auto lock = _terminal->LockForWriting(); | ||
_terminal->UpdateSelection(std::get<0>(*updateSlnParams), std::get<1>(*updateSlnParams)); | ||
carlos-zamora marked this conversation as resolved.
Show resolved
Hide resolved
|
||
_renderer->TriggerSelection(); | ||
return true; | ||
} | ||
|
||
// GH#8791 - don't dismiss selection if Windows key was also pressed as a key-combination. | ||
if (!modifiers.IsWinPressed()) | ||
{ | ||
_terminal->ClearSelection(); | ||
_renderer->TriggerSelection(); | ||
} | ||
|
||
// When there is a selection active, escape should clear it and NOT flow through | ||
// to the terminal. With any other keypress, it should clear the selection AND | ||
// flow through to the terminal. | ||
if (vkey == VK_ESCAPE) | ||
{ | ||
return true; | ||
|
@@ -1422,18 +1500,18 @@ namespace winrt::Microsoft::Terminal::Control::implementation | |
// handle ALT key | ||
_terminal->SetBlockSelection(altEnabled); | ||
|
||
::Terminal::SelectionExpansionMode mode = ::Terminal::SelectionExpansionMode::Cell; | ||
::Terminal::SelectionExpansion mode = ::Terminal::SelectionExpansion::Char; | ||
if (numberOfClicks == 1) | ||
{ | ||
mode = ::Terminal::SelectionExpansionMode::Cell; | ||
mode = ::Terminal::SelectionExpansion::Char; | ||
} | ||
else if (numberOfClicks == 2) | ||
{ | ||
mode = ::Terminal::SelectionExpansionMode::Word; | ||
mode = ::Terminal::SelectionExpansion::Word; | ||
} | ||
else if (numberOfClicks == 3) | ||
{ | ||
mode = ::Terminal::SelectionExpansionMode::Line; | ||
mode = ::Terminal::SelectionExpansion::Line; | ||
} | ||
|
||
// Update the selection appropriately | ||
|
@@ -1458,7 +1536,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation | |
_terminal->SetSelectionEnd(terminalPosition, mode); | ||
selectionNeedsToBeCopied = true; | ||
} | ||
else if (mode != ::Terminal::SelectionExpansionMode::Cell || shiftEnabled) | ||
else if (mode != ::Terminal::SelectionExpansion::Char || shiftEnabled) | ||
{ | ||
// If we are handling a double / triple-click or shift+single click | ||
// we establish selection using the selected mode | ||
|
@@ -1557,5 +1635,4 @@ namespace winrt::Microsoft::Terminal::Control::implementation | |
|
||
return hstring(ss.str()); | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(The amount of arguments our
TextBuffer
member functions have is worrying ngl. 😄)Why does "accessibility mode" mean "become exclusive" here? Don't you mean "bool exclusivePosition"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeeeeaaaaah. So, a11y and selection have a lot of similarities and very small/important differences. Some highlights include...
I just chose to name the parameter
accessibilityMode
to make it clear what and when we're only doing some extra work for a11y.