From 2cd2351debee8a3149c27d9a6bd452c5c767a180 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Thu, 30 Jun 2022 18:07:07 -0700 Subject: [PATCH] Miscellaneous bug fixes for Mark Mode (#13358) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary of the Pull Request 1. [copy on select] when manually copying text (i.e. kbd or right-click) while in mark/quick-edit mode, we now dismiss the selection. 2. `Enter` is now bound to copy by default. - This works very well with mark mode and provides a more consistent behavior with conhost's selection experience overall. - "why not hardcode `Enter` as a way to copy when in mark mode?" - In an effort to make this as configurable as possible, I decided to make it a configurable keybinding, but am open to suggestions. 3. selection markers a. we now hide the selection markers when multi-clicking the terminal. b. selection markers are now properly shown when a single cell selection exists - Prior to this PR, any single cell selection would display both markers. Now, we actually track which endpoint we're moving and display the appropriate one. 4. ensures that when you use keyboard selection to move past the top/bottom of the scroll area, we clamp it to the origin/bottom-right respectively. The fix is also better here in that it provides consistent behavior across all of the `_MoveByX` functions. 5. adds `toggleBlockSelection` to the schema ## References #13053 ## Validation Steps Performed Did a whole flowchart of expected behavior with copy on select: - enable `copyOnSelect` - make a selection with the mouse - ✅ right-click should copy the text --> clear the selection --> paste - use keyboard selection to quick-edit the existing selection - ✅ `copy` action should clear the selection - ✅ right-click should copy the text --> clear the selection --> paste Played with selection markers a bit in mark mode and quick edit mode. Markers are updating appropriately. --- doc/cascadia/profiles.schema.json | 1 + src/cascadia/TerminalControl/ControlCore.cpp | 58 ++++++------- src/cascadia/TerminalControl/ControlCore.h | 5 +- src/cascadia/TerminalControl/ControlCore.idl | 21 ++++- .../TerminalControl/ControlInteractivity.cpp | 16 ++-- src/cascadia/TerminalControl/TermControl.cpp | 19 ++-- src/cascadia/TerminalCore/Terminal.cpp | 5 +- src/cascadia/TerminalCore/Terminal.hpp | 23 ++++- .../TerminalCore/TerminalSelection.cpp | 86 +++++++++++-------- .../TerminalSettingsModel/defaults.json | 1 + 10 files changed, 146 insertions(+), 89 deletions(-) diff --git a/doc/cascadia/profiles.schema.json b/doc/cascadia/profiles.schema.json index 391095c379..2179a4c294 100644 --- a/doc/cascadia/profiles.schema.json +++ b/doc/cascadia/profiles.schema.json @@ -350,6 +350,7 @@ "switchToTab", "tabSearch", "toggleAlwaysOnTop", + "toggleBlockSelection", "toggleFocusMode", "selectAll", "setFocusMode", diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 19d6693222..3472bf1016 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -417,11 +417,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation vkey != VK_SNAPSHOT && keyDown) { - if (_terminal->IsInMarkMode() && modifiers.IsCtrlPressed() && vkey == 'A') + const auto isInMarkMode = _terminal->SelectionMode() == ::Microsoft::Terminal::Core::Terminal::SelectionInteractionMode::Mark; + if (isInMarkMode && modifiers.IsCtrlPressed() && vkey == 'A') { auto lock = _terminal->LockForWriting(); _terminal->SelectAll(); - _updateSelection(); + _updateSelectionUI(); return true; } @@ -430,7 +431,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { auto lock = _terminal->LockForWriting(); _terminal->UpdateSelection(updateSlnParams->first, updateSlnParams->second, modifiers); - _updateSelection(); + _updateSelectionUI(); return true; } @@ -438,7 +439,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation if (!modifiers.IsWinPressed()) { _terminal->ClearSelection(); - _updateSelection(); + _updateSelectionUI(); } // When there is a selection active, escape should clear it and NOT flow through @@ -950,8 +951,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation const auto end{ _terminal->SelectionEndForRendering() }; info.EndPos = { end.X, end.Y }; - info.MovingEnd = _terminal->MovingEnd(); - info.MovingCursor = _terminal->MovingCursor(); + info.Endpoint = static_cast(_terminal->SelectionEndpointTarget()); const auto bufferSize{ _terminal->GetTextBuffer().GetSize() }; info.StartAtLeftBoundary = _terminal->GetSelectionAnchor().x == bufferSize.Left(); @@ -981,11 +981,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation // save location (for rendering) + render _terminal->SetSelectionEnd(terminalPosition); - _renderer->TriggerSelection(); - - // this is used for mouse dragging, - // so hide the markers - _UpdateSelectionMarkersHandlers(*this, winrt::make(true)); + _updateSelectionUI(); } // Called when the Terminal wants to set something to the clipboard, i.e. @@ -998,7 +994,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation // Method Description: // - Given a copy-able selection, get the selected text from the buffer and send it to the // Windows Clipboard (CascadiaWin32:main.cpp). - // - CopyOnSelect does NOT clear the selection // Arguments: // - singleLine: collapse all of the text to one line // - formats: which formats to copy (defined by action's CopyFormatting arg). nullptr @@ -1044,12 +1039,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation bgColor) : ""; - if (!_settings->CopyOnSelect()) - { - _terminal->ClearSelection(); - _updateSelection(); - } - // send data up for clipboard _CopyToClipboardHandlers(*this, winrt::make(winrt::hstring{ textData }, @@ -1063,7 +1052,14 @@ namespace winrt::Microsoft::Terminal::Control::implementation { auto lock = _terminal->LockForWriting(); _terminal->SelectAll(); - _updateSelection(); + _updateSelectionUI(); + } + + void ControlCore::ClearSelection() + { + auto lock = _terminal->LockForWriting(); + _terminal->ClearSelection(); + _updateSelectionUI(); } bool ControlCore::ToggleBlockSelection() @@ -1085,12 +1081,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation { auto lock = _terminal->LockForWriting(); _terminal->ToggleMarkMode(); - _updateSelection(); + _updateSelectionUI(); } - bool ControlCore::IsInMarkMode() const + Control::SelectionInteractionMode ControlCore::SelectionMode() const { - return _terminal->IsInMarkMode(); + return static_cast(_terminal->SelectionMode()); } // Method Description: @@ -1100,7 +1096,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { _terminal->WritePastedText(hstr); _terminal->ClearSelection(); - _updateSelection(); + _updateSelectionUI(); _terminal->TrySnapOnInput(); } @@ -1420,10 +1416,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation { _terminal->SetBlockSelection(false); search.Select(); - _renderer->TriggerSelection(); // this is used for search, - // so hide the markers + // DO NOT call _updateSelectionUI() here. + // We don't want to show the markers so manually tell it to clear it. + _renderer->TriggerSelection(); _UpdateSelectionMarkersHandlers(*this, winrt::make(true)); } @@ -1625,14 +1622,17 @@ namespace winrt::Microsoft::Terminal::Control::implementation _terminal->MultiClickSelection(terminalPosition, mode); selectionNeedsToBeCopied = true; } - _updateSelection(); + _updateSelectionUI(); } - void ControlCore::_updateSelection() + // Method Description: + // - Updates the renderer's representation of the selection as well as the selection marker overlay in TermControl + void ControlCore::_updateSelectionUI() { _renderer->TriggerSelection(); - const bool clearMarkers{ !_terminal->IsSelectionActive() }; - _UpdateSelectionMarkersHandlers(*this, winrt::make(clearMarkers)); + // only show the markers if we're doing a keyboard selection or in mark mode + const bool showMarkers{ _terminal->SelectionMode() >= ::Microsoft::Terminal::Core::Terminal::SelectionInteractionMode::Keyboard }; + _UpdateSelectionMarkersHandlers(*this, winrt::make(!showMarkers)); } void ControlCore::AttachUiaEngine(::Microsoft::Console::Render::IRenderEngine* const pEngine) diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h index e11bd19772..5f021c10a0 100644 --- a/src/cascadia/TerminalControl/ControlCore.h +++ b/src/cascadia/TerminalControl/ControlCore.h @@ -82,9 +82,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation void PasteText(const winrt::hstring& hstr); bool CopySelectionToClipboard(bool singleLine, const Windows::Foundation::IReference& formats); void SelectAll(); + void ClearSelection(); bool ToggleBlockSelection(); void ToggleMarkMode(); - bool IsInMarkMode() const; + Control::SelectionInteractionMode SelectionMode() const; void GotFocus(); void LostFocus(); @@ -271,7 +272,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation bool _setFontSizeUnderLock(int fontSize); void _updateFont(const bool initialUpdate = false); void _refreshSizeUnderLock(); - void _updateSelection(); + void _updateSelectionUI(); void _sendInputToConnection(std::wstring_view wstr); diff --git a/src/cascadia/TerminalControl/ControlCore.idl b/src/cascadia/TerminalControl/ControlCore.idl index 1dc39842f9..a1d348d8a2 100644 --- a/src/cascadia/TerminalControl/ControlCore.idl +++ b/src/cascadia/TerminalControl/ControlCore.idl @@ -29,12 +29,26 @@ namespace Microsoft.Terminal.Control All }; + enum SelectionInteractionMode + { + None, + Mouse, + Keyboard, + Mark + }; + + [flags] + enum SelectionEndpointTarget + { + Start = 0x1, + End = 0x2 + }; + struct SelectionData { Microsoft.Terminal.Core.Point StartPos; Microsoft.Terminal.Core.Point EndPos; - Boolean MovingEnd; - Boolean MovingCursor; + SelectionEndpointTarget Endpoint; Boolean StartAtLeftBoundary; Boolean EndAtRightBoundary; }; @@ -75,10 +89,10 @@ namespace Microsoft.Terminal.Control void SendInput(String text); void PasteText(String text); void SelectAll(); + void ClearSelection(); Boolean ToggleBlockSelection(); void ToggleMarkMode(); void ClearBuffer(ClearBufferType clearType); - Boolean IsInMarkMode(); void SetHoveredCell(Microsoft.Terminal.Core.Point terminalPosition); void ClearHoveredCell(); @@ -101,6 +115,7 @@ namespace Microsoft.Terminal.Control Boolean HasSelection { get; }; IVector SelectedText(Boolean trimTrailingWhitespace); SelectionData SelectionInfo { get; }; + SelectionInteractionMode SelectionMode(); String HoveredUriText { get; }; Windows.Foundation.IReference HoveredCell { get; }; diff --git a/src/cascadia/TerminalControl/ControlInteractivity.cpp b/src/cascadia/TerminalControl/ControlInteractivity.cpp index 85f92cba39..75b8cf5b61 100644 --- a/src/cascadia/TerminalControl/ControlInteractivity.cpp +++ b/src/cascadia/TerminalControl/ControlInteractivity.cpp @@ -142,7 +142,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation // Method Description: // - Given a copy-able selection, get the selected text from the buffer and send it to the // Windows Clipboard (CascadiaWin32:main.cpp). - // - CopyOnSelect does NOT clear the selection // Arguments: // - singleLine: collapse all of the text to one line // - formats: which formats to copy (defined by action's CopyFormatting arg). nullptr @@ -257,15 +256,15 @@ namespace winrt::Microsoft::Terminal::Control::implementation } else if (WI_IsFlagSet(buttonState, MouseButtonState::IsRightButtonDown)) { - // CopyOnSelect right click always pastes - if (_core->CopyOnSelect() || !_core->HasSelection()) + // Try to copy the text and clear the selection + const auto successfulCopy = CopySelectionToClipboard(shiftEnabled, nullptr); + _core->ClearSelection(); + if (_core->CopyOnSelect() || !successfulCopy) { + // CopyOnSelect: right click always pastes! + // Otherwise: no selection --> paste RequestPasteTextFromClipboard(); } - else - { - CopySelectionToClipboard(shiftEnabled, nullptr); - } } } @@ -383,6 +382,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation isLeftMouseRelease && _selectionNeedsToBeCopied) { + // IMPORTANT! + // DO NOT clear the selection here! + // Otherwise, the selection will be cleared immediately after you make it. CopySelectionToClipboard(false, nullptr); } diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index fb44871fdd..1e5dcdc9f1 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -1188,7 +1188,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { // Manually show the cursor when a key is pressed. Restarting // the timer prevents flickering. - _core.CursorOn(!_core.IsInMarkMode()); + _core.CursorOn(_core.SelectionMode() != SelectionInteractionMode::Mark); _cursorTimer->Start(); } @@ -1659,7 +1659,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation if (_cursorTimer) { // When the terminal focuses, show the cursor immediately - _core.CursorOn(!_core.IsInMarkMode()); + _core.CursorOn(_core.SelectionMode() != SelectionInteractionMode::Mark); _cursorTimer->Start(); } @@ -1902,7 +1902,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation return false; } - return _interactivity.CopySelectionToClipboard(singleLine, formats); + const auto successfulCopy = _interactivity.CopySelectionToClipboard(singleLine, formats); + _core.ClearSelection(); + return successfulCopy; } // Method Description: @@ -2830,9 +2832,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation // show/update selection markers // figure out which endpoint to move, get it and the relevant icon (hide the other icon) - const auto selectionAnchor{ markerData.MovingEnd ? markerData.EndPos : markerData.StartPos }; - const auto& marker{ markerData.MovingEnd ? SelectionEndMarker() : SelectionStartMarker() }; - const auto& otherMarker{ markerData.MovingEnd ? SelectionStartMarker() : SelectionEndMarker() }; + const auto movingEnd{ WI_IsFlagSet(markerData.Endpoint, SelectionEndpointTarget::End) }; + const auto selectionAnchor{ movingEnd ? markerData.EndPos : markerData.StartPos }; + const auto& marker{ movingEnd ? SelectionEndMarker() : SelectionStartMarker() }; + const auto& otherMarker{ movingEnd ? SelectionStartMarker() : SelectionEndMarker() }; if (selectionAnchor.Y < 0 || selectionAnchor.Y >= _core.ViewHeight()) { // if the endpoint is outside of the viewport, @@ -2841,7 +2844,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation otherMarker.Visibility(Visibility::Collapsed); co_return; } - else if (markerData.MovingCursor) + else if (WI_AreAllFlagsSet(markerData.Endpoint, SelectionEndpointTarget::Start | SelectionEndpointTarget::End)) { // display both markers displayMarker(true); @@ -2851,7 +2854,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { // display one marker, // but hide the other - displayMarker(markerData.MovingEnd); + displayMarker(movingEnd); otherMarker.Visibility(Visibility::Collapsed); } } diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index fe63670a6a..2e78808f8e 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -45,8 +45,9 @@ Terminal::Terminal() : _snapOnInput{ true }, _altGrAliasing{ true }, _blockSelection{ false }, - _markMode{ false }, + _selectionMode{ SelectionInteractionMode::None }, _selection{ std::nullopt }, + _selectionEndpoint{ static_cast(0) }, _taskbarState{ 0 }, _taskbarProgress{ 0 }, _trimBlockSelection{ false }, @@ -1369,7 +1370,7 @@ void Terminal::SetCursorOn(const bool isOn) bool Terminal::IsCursorBlinkingAllowed() const noexcept { const auto& cursor = _activeBuffer().GetCursor(); - return !_markMode && cursor.IsBlinkingAllowed(); + return _selectionMode != SelectionInteractionMode::Mark && cursor.IsBlinkingAllowed(); } // Method Description: diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index 092dcf7c18..7f08390cbd 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -233,6 +233,14 @@ public: #pragma region TextSelection // These methods are defined in TerminalSelection.cpp + enum class SelectionInteractionMode + { + None, + Mouse, + Keyboard, + Mark + }; + enum class SelectionDirection { Left, @@ -249,21 +257,27 @@ public: Viewport, Buffer }; + + enum class SelectionEndpoint + { + Start = 0x1, + End = 0x2 + }; + void MultiClickSelection(const til::point viewportPos, SelectionExpansion expansionMode); void SetSelectionAnchor(const til::point position); void SetSelectionEnd(const til::point position, std::optional newExpansionMode = std::nullopt); void SetBlockSelection(const bool isEnabled) noexcept; void UpdateSelection(SelectionDirection direction, SelectionExpansion mode, ControlKeyStates mods); void SelectAll(); - bool IsInMarkMode() const; + SelectionInteractionMode SelectionMode() const noexcept; void ToggleMarkMode(); using UpdateSelectionParams = std::optional>; UpdateSelectionParams ConvertKeyEventToUpdateSelectionParams(const ControlKeyStates mods, const WORD vkey) const; - bool MovingEnd() const noexcept; - bool MovingCursor() const noexcept; til::point SelectionStartForRendering() const; til::point SelectionEndForRendering() const; + const SelectionEndpoint SelectionEndpointTarget() const noexcept; const TextBuffer::TextAndColor RetrieveSelectedTextFromBuffer(bool trimTrailingWhitespace); #pragma endregion @@ -333,7 +347,8 @@ private: bool _blockSelection; std::wstring _wordDelimiters; SelectionExpansion _multiClickSelectionMode; - bool _markMode; + SelectionInteractionMode _selectionMode; + SelectionEndpoint _selectionEndpoint; #pragma endregion std::unique_ptr _mainBuffer; diff --git a/src/cascadia/TerminalCore/TerminalSelection.cpp b/src/cascadia/TerminalCore/TerminalSelection.cpp index cf5c8dafbe..b2626ec024 100644 --- a/src/cascadia/TerminalCore/TerminalSelection.cpp +++ b/src/cascadia/TerminalCore/TerminalSelection.cpp @@ -7,6 +7,8 @@ using namespace Microsoft::Terminal::Core; +DEFINE_ENUM_FLAG_OPERATORS(Terminal::SelectionEndpoint); + /* Selection Pivot Description: * The pivot helps properly update the selection when a user moves a selection over itself * See SelectionTest::DoubleClickDrag_Left for an example of the functionality mentioned here @@ -120,6 +122,11 @@ til::point Terminal::SelectionEndForRendering() const return til::point{ pos }; } +const Terminal::SelectionEndpoint Terminal::SelectionEndpointTarget() const noexcept +{ + return _selectionEndpoint; +} + // Method Description: // - Checks if selection is active // Return Value: @@ -208,6 +215,7 @@ void Terminal::SetSelectionEnd(const til::point viewportPos, std::optionalstart, _selection->end) = expandedAnchors; } + _selectionMode = SelectionInteractionMode::Mouse; } // Method Description: @@ -273,14 +281,14 @@ void Terminal::SetBlockSelection(const bool isEnabled) noexcept _blockSelection = isEnabled; } -bool Terminal::IsInMarkMode() const +Terminal::SelectionInteractionMode Terminal::SelectionMode() const noexcept { - return _markMode; + return _selectionMode; } void Terminal::ToggleMarkMode() { - if (_markMode) + if (_selectionMode == SelectionInteractionMode::Mark) { // Exit Mark Mode ClearSelection(); @@ -295,14 +303,15 @@ void Terminal::ToggleMarkMode() _selection->start = cursorPos; _selection->end = cursorPos; _selection->pivot = cursorPos; - _markMode = true; + _selectionMode = SelectionInteractionMode::Mark; _blockSelection = false; + WI_SetAllFlags(_selectionEndpoint, SelectionEndpoint::Start | SelectionEndpoint::End); } } Terminal::UpdateSelectionParams Terminal::ConvertKeyEventToUpdateSelectionParams(const ControlKeyStates mods, const WORD vkey) const { - if ((_markMode || mods.IsShiftPressed()) && !mods.IsAltPressed()) + if ((_selectionMode == SelectionInteractionMode::Mark || mods.IsShiftPressed()) && !mods.IsAltPressed()) { if (mods.IsCtrlPressed()) { @@ -348,21 +357,6 @@ Terminal::UpdateSelectionParams Terminal::ConvertKeyEventToUpdateSelectionParams return std::nullopt; } -bool Terminal::MovingEnd() const noexcept -{ - // true --> we're moving end endpoint ("lower") - // false --> we're moving start endpoint ("higher") - return _selection->start == _selection->pivot; -} - -bool Terminal::MovingCursor() const noexcept -{ - // Relevant for keyboard selection: - // true --> the selection is just a "cursor"; we're moving everything together with arrow keys - // false --> otherwise - return _selection->start == _selection->pivot && _selection->pivot == _selection->end; -} - // Method Description: // - updates the selection endpoints based on a direction and expansion mode. Primarily used for keyboard selection. // Arguments: @@ -372,11 +366,27 @@ bool Terminal::MovingCursor() const noexcept void Terminal::UpdateSelection(SelectionDirection direction, SelectionExpansion mode, ControlKeyStates mods) { // 1. Figure out which endpoint to update - // One of the endpoints is the pivot, - // signifying that the other endpoint is the one we want to move. - auto targetPos{ MovingEnd() ? _selection->end : _selection->start }; + // [Mark Mode] + // - shift pressed --> only move "end" (or "start" if "pivot" == "end") + // - otherwise --> move both "start" and "end" (moving cursor) + // [Quick Edit] + // - just move "end" (or "start" if "pivot" == "end") + _selectionEndpoint = static_cast(0); + if (_selectionMode == SelectionInteractionMode::Mark && !mods.IsShiftPressed()) + { + WI_SetAllFlags(_selectionEndpoint, SelectionEndpoint::Start | SelectionEndpoint::End); + } + else if (_selection->start == _selection->pivot) + { + WI_SetFlag(_selectionEndpoint, SelectionEndpoint::End); + } + else if (_selection->end == _selection->pivot) + { + WI_SetFlag(_selectionEndpoint, SelectionEndpoint::Start); + } + auto targetPos{ WI_IsFlagSet(_selectionEndpoint, SelectionEndpoint::Start) ? _selection->start : _selection->end }; - // 2.A) Perform the movement + // 2 Perform the movement switch (mode) { case SelectionExpansion::Char: @@ -393,11 +403,9 @@ void Terminal::UpdateSelection(SelectionDirection direction, SelectionExpansion break; } - // 2.B) Clamp the movement to the mutable viewport - targetPos = std::min(targetPos, _GetMutableViewport().BottomRightInclusive()); - - // 3. Actually modify the selection - if (_markMode && !mods.IsShiftPressed()) + // 3. Actually modify the selection state + _selectionMode = std::max(_selectionMode, SelectionInteractionMode::Keyboard); + if (_selectionMode == SelectionInteractionMode::Mark && !mods.IsShiftPressed()) { // [Mark Mode] + shift unpressed --> move all three (i.e. just use arrow keys) _selection->start = targetPos; @@ -407,9 +415,14 @@ void Terminal::UpdateSelection(SelectionDirection direction, SelectionExpansion else { // [Mark Mode] + shift --> updating a standard selection - // NOTE: targetStart doesn't matter here - auto targetStart = false; + // This is also standard quick-edit modification + bool targetStart = false; std::tie(_selection->start, _selection->end) = _PivotSelection(targetPos, targetStart); + + // IMPORTANT! Pivoting the selection here might have changed which endpoint we're targeting. + // So let's set the targeted endpoint again. + WI_UpdateFlag(_selectionEndpoint, SelectionEndpoint::Start, targetStart); + WI_UpdateFlag(_selectionEndpoint, SelectionEndpoint::End, !targetStart); } // 4. Scroll (if necessary) @@ -438,6 +451,7 @@ void Terminal::SelectAll() _selection->start = bufferSize.Origin(); _selection->end = { bufferSize.RightInclusive(), _GetMutableViewport().BottomInclusive() }; _selection->pivot = _selection->end; + _selectionMode = SelectionInteractionMode::Keyboard; } void Terminal::_MoveByChar(SelectionDirection direction, til::point& pos) @@ -455,13 +469,16 @@ void Terminal::_MoveByChar(SelectionDirection direction, til::point& pos) case SelectionDirection::Up: { const auto bufferSize{ _activeBuffer().GetSize() }; - pos = { pos.X, std::clamp(pos.Y - 1, bufferSize.Top(), bufferSize.BottomInclusive()) }; + const auto newY{ pos.Y - 1 }; + pos = newY < bufferSize.Top() ? bufferSize.Origin() : til::point{ pos.X, newY }; break; } case SelectionDirection::Down: { const auto bufferSize{ _activeBuffer().GetSize() }; - pos = { pos.X, std::clamp(pos.Y + 1, bufferSize.Top(), bufferSize.BottomInclusive()) }; + const auto mutableBottom{ _GetMutableViewport().BottomInclusive() }; + const auto newY{ pos.Y + 1 }; + pos = newY > mutableBottom ? til::point{ bufferSize.RightInclusive(), mutableBottom } : til::point{ pos.X, newY }; break; } } @@ -579,7 +596,8 @@ void Terminal::_MoveByBuffer(SelectionDirection direction, til::point& pos) void Terminal::ClearSelection() { _selection = std::nullopt; - _markMode = false; + _selectionMode = SelectionInteractionMode::None; + _selectionEndpoint = static_cast(0); } // Method Description: diff --git a/src/cascadia/TerminalSettingsModel/defaults.json b/src/cascadia/TerminalSettingsModel/defaults.json index e6d68bb2af..6067aa6130 100644 --- a/src/cascadia/TerminalSettingsModel/defaults.json +++ b/src/cascadia/TerminalSettingsModel/defaults.json @@ -382,6 +382,7 @@ // Clipboard Integration { "command": { "action": "copy", "singleLine": false }, "keys": "ctrl+shift+c" }, { "command": { "action": "copy", "singleLine": false }, "keys": "ctrl+insert" }, + { "command": { "action": "copy", "singleLine": false }, "keys": "enter" }, { "command": "paste", "keys": "ctrl+shift+v" }, { "command": "paste", "keys": "shift+insert" }, { "command": "selectAll", "keys": "ctrl+shift+a" },