From cdbe030eac32dad1005c65bffffe4f6fdaee2b47 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Tue, 21 Jun 2022 14:57:42 -0700 Subject: [PATCH 01/11] Miscellaneous bug fixes for Mark Mode --- src/cascadia/TerminalControl/ControlCore.cpp | 8 ++++++-- src/cascadia/TerminalSettingsModel/defaults.json | 1 + 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 19d66932227..bbb923095f8 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -1044,7 +1044,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation bgColor) : ""; - if (!_settings->CopyOnSelect()) + if (!_settings->CopyOnSelect() || IsInMarkMode()) { _terminal->ClearSelection(); _updateSelection(); @@ -1625,7 +1625,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation _terminal->MultiClickSelection(terminalPosition, mode); selectionNeedsToBeCopied = true; } - _updateSelection(); + _renderer->TriggerSelection(); + + // this is used for mouse selection, + // so hide the markers + _UpdateSelectionMarkersHandlers(*this, winrt::make(true)); } void ControlCore::_updateSelection() diff --git a/src/cascadia/TerminalSettingsModel/defaults.json b/src/cascadia/TerminalSettingsModel/defaults.json index e6d68bb2af1..6067aa6130e 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" }, From 654bb08a4c51852d6382479eb1fdc3a424eecac6 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Wed, 22 Jun 2022 12:32:16 -0700 Subject: [PATCH 02/11] fix copyOnSelect interactions with keyboard selection --- src/cascadia/TerminalControl/ControlCore.cpp | 12 ++++++-- src/cascadia/TerminalControl/ControlCore.h | 3 +- .../TerminalControl/ControlInteractivity.cpp | 30 ++++++++++++++----- .../TerminalControl/ControlInteractivity.h | 3 +- src/cascadia/TerminalCore/Terminal.cpp | 1 + src/cascadia/TerminalCore/Terminal.hpp | 2 ++ .../TerminalCore/TerminalSelection.cpp | 11 ++++++- 7 files changed, 49 insertions(+), 13 deletions(-) diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index bbb923095f8..2dc3d08a02c 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -998,13 +998,14 @@ 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 // if we should defer which formats are copied to the global setting + // - clearSelection: if true, clear the selection. Used for CopyOnSelect. bool ControlCore::CopySelectionToClipboard(bool singleLine, - const Windows::Foundation::IReference& formats) + const Windows::Foundation::IReference& formats, + bool clearSelection) { // no selection --> nothing to copy if (!_terminal->IsSelectionActive()) @@ -1044,7 +1045,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation bgColor) : ""; - if (!_settings->CopyOnSelect() || IsInMarkMode()) + if (clearSelection) { _terminal->ClearSelection(); _updateSelection(); @@ -1093,6 +1094,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation return _terminal->IsInMarkMode(); } + bool ControlCore::IsInQuickEditMode() const + { + return _terminal->IsInQuickEditMode(); + } + // Method Description: // - Pre-process text pasted (presumably from the clipboard) // before sending it over the terminal's connection. diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h index e11bd19772a..701dc2d3118 100644 --- a/src/cascadia/TerminalControl/ControlCore.h +++ b/src/cascadia/TerminalControl/ControlCore.h @@ -80,11 +80,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation void SendInput(const winrt::hstring& wstr); void PasteText(const winrt::hstring& hstr); - bool CopySelectionToClipboard(bool singleLine, const Windows::Foundation::IReference& formats); + bool CopySelectionToClipboard(bool singleLine, const Windows::Foundation::IReference& formats, bool clearSelection = false); void SelectAll(); bool ToggleBlockSelection(); void ToggleMarkMode(); bool IsInMarkMode() const; + bool IsInQuickEditMode() const; void GotFocus(); void LostFocus(); diff --git a/src/cascadia/TerminalControl/ControlInteractivity.cpp b/src/cascadia/TerminalControl/ControlInteractivity.cpp index 85f92cba396..c6fffa99832 100644 --- a/src/cascadia/TerminalControl/ControlInteractivity.cpp +++ b/src/cascadia/TerminalControl/ControlInteractivity.cpp @@ -142,13 +142,14 @@ 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 // if we should defer which formats are copied to the global setting + // - clearSelection: if true, clear the selection after copying it. Used for CopyOnSelect. bool ControlInteractivity::CopySelectionToClipboard(bool singleLine, - const Windows::Foundation::IReference& formats) + const Windows::Foundation::IReference& formats, + bool clearSelection) { if (_core) { @@ -164,7 +165,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation // Mark the current selection as copied _selectionNeedsToBeCopied = false; - return _core->CopySelectionToClipboard(singleLine, formats); + return _core->CopySelectionToClipboard(singleLine, formats, clearSelection); } return false; @@ -257,15 +258,27 @@ namespace winrt::Microsoft::Terminal::Control::implementation } else if (WI_IsFlagSet(buttonState, MouseButtonState::IsRightButtonDown)) { - // CopyOnSelect right click always pastes - if (_core->CopyOnSelect() || !_core->HasSelection()) + if (_core->CopyOnSelect()) { + // CopyOnSelect: + // 1. keyboard selection? --> copy the new content first + // 2. right click always pastes! + if (_core->IsInQuickEditMode() || _core->IsInMarkMode()) + { + CopySelectionToClipboard(shiftEnabled, nullptr); + } RequestPasteTextFromClipboard(); } - else + else if (_core->HasSelection()) { + // copy selected text CopySelectionToClipboard(shiftEnabled, nullptr); } + else + { + // no selection --> paste + RequestPasteTextFromClipboard(); + } } } @@ -383,7 +396,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation isLeftMouseRelease && _selectionNeedsToBeCopied) { - CopySelectionToClipboard(false, nullptr); + // IMPORTANT! + // Set clearSelection to false here! + // Otherwise, the selection will be cleared immediately after you make it. + CopySelectionToClipboard(false, nullptr, /*clearSelection*/ false); } _singleClickTouchdownPos = std::nullopt; diff --git a/src/cascadia/TerminalControl/ControlInteractivity.h b/src/cascadia/TerminalControl/ControlInteractivity.h index 3d47ad7c0c9..8f5cc6d2415 100644 --- a/src/cascadia/TerminalControl/ControlInteractivity.h +++ b/src/cascadia/TerminalControl/ControlInteractivity.h @@ -80,7 +80,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation #pragma endregion bool CopySelectionToClipboard(bool singleLine, - const Windows::Foundation::IReference& formats); + const Windows::Foundation::IReference& formats, + bool clearSelection = true); void RequestPasteTextFromClipboard(); void SetEndSelectionPoint(const Core::Point pixelPosition); bool ManglePathsForWsl(); diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index fe63670a6a5..42072a0f0a6 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -46,6 +46,7 @@ Terminal::Terminal() : _altGrAliasing{ true }, _blockSelection{ false }, _markMode{ false }, + _quickEditMode{ false }, _selection{ std::nullopt }, _taskbarState{ 0 }, _taskbarProgress{ 0 }, diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index 092dcf7c185..a56356b5572 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -255,6 +255,7 @@ class Microsoft::Terminal::Core::Terminal final : void SetBlockSelection(const bool isEnabled) noexcept; void UpdateSelection(SelectionDirection direction, SelectionExpansion mode, ControlKeyStates mods); void SelectAll(); + bool IsInQuickEditMode() const; bool IsInMarkMode() const; void ToggleMarkMode(); @@ -334,6 +335,7 @@ class Microsoft::Terminal::Core::Terminal final : std::wstring _wordDelimiters; SelectionExpansion _multiClickSelectionMode; bool _markMode; + bool _quickEditMode; #pragma endregion std::unique_ptr _mainBuffer; diff --git a/src/cascadia/TerminalCore/TerminalSelection.cpp b/src/cascadia/TerminalCore/TerminalSelection.cpp index 98729c789b1..d37c46ad353 100644 --- a/src/cascadia/TerminalCore/TerminalSelection.cpp +++ b/src/cascadia/TerminalCore/TerminalSelection.cpp @@ -273,6 +273,11 @@ void Terminal::SetBlockSelection(const bool isEnabled) noexcept _blockSelection = isEnabled; } +bool Terminal::IsInQuickEditMode() const +{ + return _quickEditMode; +} + bool Terminal::IsInMarkMode() const { return _markMode; @@ -296,6 +301,7 @@ void Terminal::ToggleMarkMode() _selection->end = cursorPos; _selection->pivot = cursorPos; _markMode = true; + _quickEditMode = false; _blockSelection = false; } } @@ -393,7 +399,8 @@ void Terminal::UpdateSelection(SelectionDirection direction, SelectionExpansion break; } - // 3. Actually modify the selection + // 3. Actually modify the selection state + _quickEditMode = !_markMode; if (_markMode && !mods.IsShiftPressed()) { // [Mark Mode] + shift unpressed --> move all three (i.e. just use arrow keys) @@ -404,6 +411,7 @@ void Terminal::UpdateSelection(SelectionDirection direction, SelectionExpansion else { // [Mark Mode] + shift --> updating a standard selection + // This is also standard quick-edit modification // NOTE: targetStart doesn't matter here auto targetStart = false; std::tie(_selection->start, _selection->end) = _PivotSelection(targetPos, targetStart); @@ -577,6 +585,7 @@ void Terminal::ClearSelection() { _selection = std::nullopt; _markMode = false; + _quickEditMode = false; } // Method Description: From 0b5434583e0de69792af90d6e87e559e8108ea19 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Wed, 22 Jun 2022 13:56:49 -0700 Subject: [PATCH 03/11] Fix a few method signatures --- src/cascadia/TerminalControl/ControlCore.h | 2 +- src/cascadia/TerminalCore/Terminal.hpp | 2 +- src/cascadia/TerminalCore/TerminalSelection.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h index 701dc2d3118..2988d57a263 100644 --- a/src/cascadia/TerminalControl/ControlCore.h +++ b/src/cascadia/TerminalControl/ControlCore.h @@ -80,7 +80,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation void SendInput(const winrt::hstring& wstr); void PasteText(const winrt::hstring& hstr); - bool CopySelectionToClipboard(bool singleLine, const Windows::Foundation::IReference& formats, bool clearSelection = false); + bool CopySelectionToClipboard(bool singleLine, const Windows::Foundation::IReference& formats, bool clearSelection = true); void SelectAll(); bool ToggleBlockSelection(); void ToggleMarkMode(); diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index a56356b5572..8a2b507977b 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -255,7 +255,7 @@ class Microsoft::Terminal::Core::Terminal final : void SetBlockSelection(const bool isEnabled) noexcept; void UpdateSelection(SelectionDirection direction, SelectionExpansion mode, ControlKeyStates mods); void SelectAll(); - bool IsInQuickEditMode() const; + const bool IsInQuickEditMode() const noexcept; bool IsInMarkMode() const; void ToggleMarkMode(); diff --git a/src/cascadia/TerminalCore/TerminalSelection.cpp b/src/cascadia/TerminalCore/TerminalSelection.cpp index d37c46ad353..466c66a8ad6 100644 --- a/src/cascadia/TerminalCore/TerminalSelection.cpp +++ b/src/cascadia/TerminalCore/TerminalSelection.cpp @@ -273,7 +273,7 @@ void Terminal::SetBlockSelection(const bool isEnabled) noexcept _blockSelection = isEnabled; } -bool Terminal::IsInQuickEditMode() const +const bool Terminal::IsInQuickEditMode() const noexcept { return _quickEditMode; } From 06a39f84ee502c795ec32f25bcd03c946300b32d Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Thu, 23 Jun 2022 09:44:44 -0700 Subject: [PATCH 04/11] bugfix: correctly show markers when one cell selected --- src/cascadia/TerminalControl/ControlCore.cpp | 3 +- src/cascadia/TerminalControl/ControlCore.idl | 10 +++- src/cascadia/TerminalControl/TermControl.cpp | 11 ++-- src/cascadia/TerminalCore/Terminal.cpp | 1 + src/cascadia/TerminalCore/Terminal.hpp | 11 +++- .../TerminalCore/TerminalSelection.cpp | 55 ++++++++++++------- 6 files changed, 60 insertions(+), 31 deletions(-) diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 2dc3d08a02c..7c40f51d20b 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -950,8 +950,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(); diff --git a/src/cascadia/TerminalControl/ControlCore.idl b/src/cascadia/TerminalControl/ControlCore.idl index 1dc39842f97..12b2736002e 100644 --- a/src/cascadia/TerminalControl/ControlCore.idl +++ b/src/cascadia/TerminalControl/ControlCore.idl @@ -29,12 +29,18 @@ namespace Microsoft.Terminal.Control All }; + [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; }; diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index fb44871fdd6..9128aff0790 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -2830,9 +2830,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 +2842,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 +2852,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 42072a0f0a6..bd9f01a6990 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -48,6 +48,7 @@ Terminal::Terminal() : _markMode{ false }, _quickEditMode{ false }, _selection{ std::nullopt }, + _selectionEndpoint{ static_cast(0) }, _taskbarState{ 0 }, _taskbarProgress{ 0 }, _trimBlockSelection{ false }, diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index a56356b5572..0b299ae98c5 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -249,6 +249,13 @@ class Microsoft::Terminal::Core::Terminal final : 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); @@ -261,10 +268,9 @@ class Microsoft::Terminal::Core::Terminal final : 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 @@ -336,6 +342,7 @@ class Microsoft::Terminal::Core::Terminal final : SelectionExpansion _multiClickSelectionMode; bool _markMode; bool _quickEditMode; + SelectionEndpoint _selectionEndpoint; #pragma endregion std::unique_ptr _mainBuffer; diff --git a/src/cascadia/TerminalCore/TerminalSelection.cpp b/src/cascadia/TerminalCore/TerminalSelection.cpp index d37c46ad353..1884a20fbbf 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: @@ -303,6 +310,7 @@ void Terminal::ToggleMarkMode() _markMode = true; _quickEditMode = false; _blockSelection = false; + WI_SetAllFlags(_selectionEndpoint, SelectionEndpoint::Start | SelectionEndpoint::End); } } @@ -354,21 +362,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: @@ -378,9 +371,25 @@ 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 (_markMode && !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. Perform the movement switch (mode) @@ -412,9 +421,14 @@ void Terminal::UpdateSelection(SelectionDirection direction, SelectionExpansion { // [Mark Mode] + shift --> updating a standard selection // This is also standard quick-edit modification - // NOTE: targetStart doesn't matter here - auto targetStart = false; + bool targetStart; std::tie(_selection->start, _selection->end) = _PivotSelection(targetPos, targetStart); + + // IMPORTANT! Pivoting the selection here might've changed which endpoint we're targetting. + // So let's set the targetted endpoint again. + _selectionEndpoint = static_cast(0); + WI_SetFlagIf(_selectionEndpoint, SelectionEndpoint::Start, targetStart); + WI_SetFlagIf(_selectionEndpoint, SelectionEndpoint::End, !targetStart); } // 4. Scroll (if necessary) @@ -586,6 +600,7 @@ void Terminal::ClearSelection() _selection = std::nullopt; _markMode = false; _quickEditMode = false; + _selectionEndpoint = static_cast(0); } // Method Description: From 408fca2a190332f71126d0127d1b071b6db25734 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Thu, 23 Jun 2022 09:50:54 -0700 Subject: [PATCH 05/11] spell fix --- src/cascadia/TerminalCore/TerminalSelection.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cascadia/TerminalCore/TerminalSelection.cpp b/src/cascadia/TerminalCore/TerminalSelection.cpp index 33a2f24fec9..30dfecc150f 100644 --- a/src/cascadia/TerminalCore/TerminalSelection.cpp +++ b/src/cascadia/TerminalCore/TerminalSelection.cpp @@ -424,8 +424,8 @@ void Terminal::UpdateSelection(SelectionDirection direction, SelectionExpansion bool targetStart; std::tie(_selection->start, _selection->end) = _PivotSelection(targetPos, targetStart); - // IMPORTANT! Pivoting the selection here might've changed which endpoint we're targetting. - // So let's set the targetted endpoint again. + // IMPORTANT! Pivoting the selection here might have changed which endpoint we're targeting. + // So let's set the targeted endpoint again. _selectionEndpoint = static_cast(0); WI_SetFlagIf(_selectionEndpoint, SelectionEndpoint::Start, targetStart); WI_SetFlagIf(_selectionEndpoint, SelectionEndpoint::End, !targetStart); From 32ca43770a4789249ae897166482ed0fba2fb3be Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Thu, 23 Jun 2022 17:35:23 -0700 Subject: [PATCH 06/11] combine mark mode and quick edit mode --- src/cascadia/TerminalControl/ControlCore.cpp | 12 +++------ src/cascadia/TerminalControl/ControlCore.h | 3 +-- src/cascadia/TerminalControl/ControlCore.idl | 9 ++++++- .../TerminalControl/ControlInteractivity.cpp | 2 +- src/cascadia/TerminalControl/TermControl.cpp | 4 +-- src/cascadia/TerminalCore/Terminal.cpp | 5 ++-- src/cascadia/TerminalCore/Terminal.hpp | 13 +++++++--- .../TerminalCore/TerminalSelection.cpp | 25 +++++++------------ 8 files changed, 36 insertions(+), 37 deletions(-) diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 7c40f51d20b..f1463889136 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -417,7 +417,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation vkey != VK_SNAPSHOT && keyDown) { - if (_terminal->IsInMarkMode() && modifiers.IsCtrlPressed() && vkey == 'A') + const auto isInMarkMode = static_cast(_terminal->SelectionMode()) == Control::SelectionInteractionMode::Mark; + if (isInMarkMode && modifiers.IsCtrlPressed() && vkey == 'A') { auto lock = _terminal->LockForWriting(); _terminal->SelectAll(); @@ -1088,14 +1089,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation _updateSelection(); } - bool ControlCore::IsInMarkMode() const + Control::SelectionInteractionMode ControlCore::SelectionMode() const { - return _terminal->IsInMarkMode(); - } - - bool ControlCore::IsInQuickEditMode() const - { - return _terminal->IsInQuickEditMode(); + return static_cast(_terminal->SelectionMode()); } // Method Description: diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h index 2988d57a263..0a6c22500a5 100644 --- a/src/cascadia/TerminalControl/ControlCore.h +++ b/src/cascadia/TerminalControl/ControlCore.h @@ -84,8 +84,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation void SelectAll(); bool ToggleBlockSelection(); void ToggleMarkMode(); - bool IsInMarkMode() const; - bool IsInQuickEditMode() const; + Control::SelectionInteractionMode SelectionMode() const; void GotFocus(); void LostFocus(); diff --git a/src/cascadia/TerminalControl/ControlCore.idl b/src/cascadia/TerminalControl/ControlCore.idl index 12b2736002e..0f0d81ba1bf 100644 --- a/src/cascadia/TerminalControl/ControlCore.idl +++ b/src/cascadia/TerminalControl/ControlCore.idl @@ -29,6 +29,13 @@ namespace Microsoft.Terminal.Control All }; + enum SelectionInteractionMode + { + Mouse, + Keyboard, + Mark + }; + [flags] enum SelectionEndpointTarget { @@ -84,7 +91,6 @@ namespace Microsoft.Terminal.Control Boolean ToggleBlockSelection(); void ToggleMarkMode(); void ClearBuffer(ClearBufferType clearType); - Boolean IsInMarkMode(); void SetHoveredCell(Microsoft.Terminal.Core.Point terminalPosition); void ClearHoveredCell(); @@ -107,6 +113,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 c6fffa99832..c1289ce80ee 100644 --- a/src/cascadia/TerminalControl/ControlInteractivity.cpp +++ b/src/cascadia/TerminalControl/ControlInteractivity.cpp @@ -263,7 +263,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation // CopyOnSelect: // 1. keyboard selection? --> copy the new content first // 2. right click always pastes! - if (_core->IsInQuickEditMode() || _core->IsInMarkMode()) + if (_core->SelectionMode() > SelectionInteractionMode::Keyboard) { CopySelectionToClipboard(shiftEnabled, nullptr); } diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 9128aff0790..fe6662e0dc2 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(); } diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index bd9f01a6990..bb1db5f2b3d 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -45,8 +45,7 @@ Terminal::Terminal() : _snapOnInput{ true }, _altGrAliasing{ true }, _blockSelection{ false }, - _markMode{ false }, - _quickEditMode{ false }, + _selectionMode{ 0 }, _selection{ std::nullopt }, _selectionEndpoint{ static_cast(0) }, _taskbarState{ 0 }, @@ -1371,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 790ae8b07e2..b98fe41859b 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -233,6 +233,13 @@ class Microsoft::Terminal::Core::Terminal final : #pragma region TextSelection // These methods are defined in TerminalSelection.cpp + enum class SelectionInteractionMode + { + Mouse, + Keyboard, + Mark + }; + enum class SelectionDirection { Left, @@ -262,8 +269,7 @@ class Microsoft::Terminal::Core::Terminal final : void SetBlockSelection(const bool isEnabled) noexcept; void UpdateSelection(SelectionDirection direction, SelectionExpansion mode, ControlKeyStates mods); void SelectAll(); - const bool IsInQuickEditMode() const noexcept; - bool IsInMarkMode() const; + SelectionInteractionMode SelectionMode() const noexcept; void ToggleMarkMode(); using UpdateSelectionParams = std::optional>; @@ -340,8 +346,7 @@ class Microsoft::Terminal::Core::Terminal final : bool _blockSelection; std::wstring _wordDelimiters; SelectionExpansion _multiClickSelectionMode; - bool _markMode; - bool _quickEditMode; + SelectionInteractionMode _selectionMode; SelectionEndpoint _selectionEndpoint; #pragma endregion diff --git a/src/cascadia/TerminalCore/TerminalSelection.cpp b/src/cascadia/TerminalCore/TerminalSelection.cpp index b57aa8cb270..5187e08f79e 100644 --- a/src/cascadia/TerminalCore/TerminalSelection.cpp +++ b/src/cascadia/TerminalCore/TerminalSelection.cpp @@ -280,19 +280,14 @@ void Terminal::SetBlockSelection(const bool isEnabled) noexcept _blockSelection = isEnabled; } -const bool Terminal::IsInQuickEditMode() const noexcept +Terminal::SelectionInteractionMode Terminal::SelectionMode() const noexcept { - return _quickEditMode; -} - -bool Terminal::IsInMarkMode() const -{ - return _markMode; + return _selectionMode; } void Terminal::ToggleMarkMode() { - if (_markMode) + if (_selectionMode == SelectionInteractionMode::Mark) { // Exit Mark Mode ClearSelection(); @@ -307,8 +302,7 @@ void Terminal::ToggleMarkMode() _selection->start = cursorPos; _selection->end = cursorPos; _selection->pivot = cursorPos; - _markMode = true; - _quickEditMode = false; + _selectionMode = SelectionInteractionMode::Mark; _blockSelection = false; WI_SetAllFlags(_selectionEndpoint, SelectionEndpoint::Start | SelectionEndpoint::End); } @@ -316,7 +310,7 @@ void Terminal::ToggleMarkMode() 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()) { @@ -377,7 +371,7 @@ void Terminal::UpdateSelection(SelectionDirection direction, SelectionExpansion // [Quick Edit] // - just move "end" (or "start" if "pivot" == "end") _selectionEndpoint = static_cast(0); - if (_markMode && !mods.IsShiftPressed()) + if (_selectionMode == SelectionInteractionMode::Mark && !mods.IsShiftPressed()) { WI_SetAllFlags(_selectionEndpoint, SelectionEndpoint::Start | SelectionEndpoint::End); } @@ -412,8 +406,8 @@ void Terminal::UpdateSelection(SelectionDirection direction, SelectionExpansion targetPos = std::min(targetPos, _GetMutableViewport().BottomRightInclusive()); // 3. Actually modify the selection state - _quickEditMode = !_markMode; - if (_markMode && !mods.IsShiftPressed()) + _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; @@ -601,8 +595,7 @@ void Terminal::_MoveByBuffer(SelectionDirection direction, til::point& pos) void Terminal::ClearSelection() { _selection = std::nullopt; - _markMode = false; - _quickEditMode = false; + _selectionMode = SelectionInteractionMode::Mouse; _selectionEndpoint = static_cast(0); } From 0c5a1e955931a69d34b1f560b2c476148bda51ee Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Thu, 23 Jun 2022 17:37:25 -0700 Subject: [PATCH 07/11] fix clamping the selection at top/bottom properly --- src/cascadia/TerminalCore/TerminalSelection.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/cascadia/TerminalCore/TerminalSelection.cpp b/src/cascadia/TerminalCore/TerminalSelection.cpp index 5187e08f79e..f2bf4c69001 100644 --- a/src/cascadia/TerminalCore/TerminalSelection.cpp +++ b/src/cascadia/TerminalCore/TerminalSelection.cpp @@ -385,7 +385,7 @@ void Terminal::UpdateSelection(SelectionDirection direction, SelectionExpansion } 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: @@ -402,9 +402,6 @@ 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 state _selectionMode = std::max(_selectionMode, SelectionInteractionMode::Keyboard); if (_selectionMode == SelectionInteractionMode::Mark && !mods.IsShiftPressed()) @@ -471,13 +468,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; } } From c78818a587badcea162b932c6e17618388f4e80e Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Fri, 24 Jun 2022 09:51:16 -0700 Subject: [PATCH 08/11] add toggleBlockSelection to schema --- doc/cascadia/profiles.schema.json | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/cascadia/profiles.schema.json b/doc/cascadia/profiles.schema.json index 391095c379f..2179a4c294f 100644 --- a/doc/cascadia/profiles.schema.json +++ b/doc/cascadia/profiles.schema.json @@ -350,6 +350,7 @@ "switchToTab", "tabSearch", "toggleAlwaysOnTop", + "toggleBlockSelection", "toggleFocusMode", "selectAll", "setFocusMode", From ce6c87e446f6f01cdeeb25021268002e6473fe8a Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Thu, 30 Jun 2022 14:04:46 -0700 Subject: [PATCH 09/11] address Dustin's feedback --- src/cascadia/TerminalControl/ControlCore.cpp | 18 ++++++++-------- src/cascadia/TerminalControl/ControlCore.h | 2 +- src/cascadia/TerminalControl/ControlCore.idl | 1 + .../TerminalControl/ControlInteractivity.cpp | 21 +++++++------------ src/cascadia/TerminalCore/Terminal.cpp | 2 +- src/cascadia/TerminalCore/Terminal.hpp | 1 + .../TerminalCore/TerminalSelection.cpp | 7 +++---- 7 files changed, 23 insertions(+), 29 deletions(-) diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index f1463889136..19b93a9eadb 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -417,12 +417,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation vkey != VK_SNAPSHOT && keyDown) { - const auto isInMarkMode = static_cast(_terminal->SelectionMode()) == Control::SelectionInteractionMode::Mark; + 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; } @@ -431,7 +431,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { auto lock = _terminal->LockForWriting(); _terminal->UpdateSelection(updateSlnParams->first, updateSlnParams->second, modifiers); - _updateSelection(); + _updateSelectionUI(); return true; } @@ -439,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 @@ -1048,7 +1048,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation if (clearSelection) { _terminal->ClearSelection(); - _updateSelection(); + _updateSelectionUI(); } // send data up for clipboard @@ -1064,7 +1064,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { auto lock = _terminal->LockForWriting(); _terminal->SelectAll(); - _updateSelection(); + _updateSelectionUI(); } bool ControlCore::ToggleBlockSelection() @@ -1086,7 +1086,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { auto lock = _terminal->LockForWriting(); _terminal->ToggleMarkMode(); - _updateSelection(); + _updateSelectionUI(); } Control::SelectionInteractionMode ControlCore::SelectionMode() const @@ -1101,7 +1101,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { _terminal->WritePastedText(hstr); _terminal->ClearSelection(); - _updateSelection(); + _updateSelectionUI(); _terminal->TrySnapOnInput(); } @@ -1633,7 +1633,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation _UpdateSelectionMarkersHandlers(*this, winrt::make(true)); } - void ControlCore::_updateSelection() + void ControlCore::_updateSelectionUI() { _renderer->TriggerSelection(); const bool clearMarkers{ !_terminal->IsSelectionActive() }; diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h index 0a6c22500a5..b2ef0a6f6d8 100644 --- a/src/cascadia/TerminalControl/ControlCore.h +++ b/src/cascadia/TerminalControl/ControlCore.h @@ -271,7 +271,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 0f0d81ba1bf..23959171bdd 100644 --- a/src/cascadia/TerminalControl/ControlCore.idl +++ b/src/cascadia/TerminalControl/ControlCore.idl @@ -31,6 +31,7 @@ namespace Microsoft.Terminal.Control enum SelectionInteractionMode { + None, Mouse, Keyboard, Mark diff --git a/src/cascadia/TerminalControl/ControlInteractivity.cpp b/src/cascadia/TerminalControl/ControlInteractivity.cpp index c1289ce80ee..a1c6e34f523 100644 --- a/src/cascadia/TerminalControl/ControlInteractivity.cpp +++ b/src/cascadia/TerminalControl/ControlInteractivity.cpp @@ -258,25 +258,18 @@ namespace winrt::Microsoft::Terminal::Control::implementation } else if (WI_IsFlagSet(buttonState, MouseButtonState::IsRightButtonDown)) { - if (_core->CopyOnSelect()) - { - // CopyOnSelect: - // 1. keyboard selection? --> copy the new content first - // 2. right click always pastes! - if (_core->SelectionMode() > SelectionInteractionMode::Keyboard) - { - CopySelectionToClipboard(shiftEnabled, nullptr); - } - RequestPasteTextFromClipboard(); - } - else if (_core->HasSelection()) + // CopySelectionToClipboard() clears the selection. + // So we need to keep track of the state before copying it. + const auto initiallyHadSelection = _core->HasSelection(); + if (initiallyHadSelection) { // copy selected text CopySelectionToClipboard(shiftEnabled, nullptr); } - else + if (_core->CopyOnSelect() || !initiallyHadSelection) { - // no selection --> paste + // CopyOnSelect: right click always pastes! + // Otherwise: no selection --> paste RequestPasteTextFromClipboard(); } } diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index bb1db5f2b3d..2e78808f8e2 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -45,7 +45,7 @@ Terminal::Terminal() : _snapOnInput{ true }, _altGrAliasing{ true }, _blockSelection{ false }, - _selectionMode{ 0 }, + _selectionMode{ SelectionInteractionMode::None }, _selection{ std::nullopt }, _selectionEndpoint{ static_cast(0) }, _taskbarState{ 0 }, diff --git a/src/cascadia/TerminalCore/Terminal.hpp b/src/cascadia/TerminalCore/Terminal.hpp index b98fe41859b..7f08390cbd2 100644 --- a/src/cascadia/TerminalCore/Terminal.hpp +++ b/src/cascadia/TerminalCore/Terminal.hpp @@ -235,6 +235,7 @@ class Microsoft::Terminal::Core::Terminal final : // These methods are defined in TerminalSelection.cpp enum class SelectionInteractionMode { + None, Mouse, Keyboard, Mark diff --git a/src/cascadia/TerminalCore/TerminalSelection.cpp b/src/cascadia/TerminalCore/TerminalSelection.cpp index f2bf4c69001..55d685c4a7e 100644 --- a/src/cascadia/TerminalCore/TerminalSelection.cpp +++ b/src/cascadia/TerminalCore/TerminalSelection.cpp @@ -415,14 +415,13 @@ void Terminal::UpdateSelection(SelectionDirection direction, SelectionExpansion { // [Mark Mode] + shift --> updating a standard selection // This is also standard quick-edit modification - bool targetStart; + 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. - _selectionEndpoint = static_cast(0); - WI_SetFlagIf(_selectionEndpoint, SelectionEndpoint::Start, targetStart); - WI_SetFlagIf(_selectionEndpoint, SelectionEndpoint::End, !targetStart); + WI_UpdateFlag(_selectionEndpoint, SelectionEndpoint::Start, targetStart); + WI_UpdateFlag(_selectionEndpoint, SelectionEndpoint::End, !targetStart); } // 4. Scroll (if necessary) From 1a84bc6b50c858b8044e0df55b866a85edc8e7bb Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Thu, 30 Jun 2022 16:10:31 -0700 Subject: [PATCH 10/11] split copy and clear sln; simplify logic --- src/cascadia/TerminalControl/ControlCore.cpp | 51 ++++++++++--------- src/cascadia/TerminalControl/ControlCore.h | 5 +- src/cascadia/TerminalControl/ControlCore.idl | 1 + .../TerminalControl/ControlInteractivity.cpp | 23 +++------ .../TerminalControl/ControlInteractivity.h | 3 +- src/cascadia/TerminalControl/TermControl.cpp | 4 +- 6 files changed, 42 insertions(+), 45 deletions(-) diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 19b93a9eadb..d250a0751ab 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -422,7 +422,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { auto lock = _terminal->LockForWriting(); _terminal->SelectAll(); - _updateSelectionUI(); + _updateSelectionUI(true); return true; } @@ -431,7 +431,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { auto lock = _terminal->LockForWriting(); _terminal->UpdateSelection(updateSlnParams->first, updateSlnParams->second, modifiers); - _updateSelectionUI(); + _updateSelectionUI(true); return true; } @@ -439,7 +439,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation if (!modifiers.IsWinPressed()) { _terminal->ClearSelection(); - _updateSelectionUI(); + _updateSelectionUI(false); } // When there is a selection active, escape should clear it and NOT flow through @@ -981,11 +981,10 @@ 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(false); } // Called when the Terminal wants to set something to the clipboard, i.e. @@ -1002,10 +1001,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation // - singleLine: collapse all of the text to one line // - formats: which formats to copy (defined by action's CopyFormatting arg). nullptr // if we should defer which formats are copied to the global setting - // - clearSelection: if true, clear the selection. Used for CopyOnSelect. bool ControlCore::CopySelectionToClipboard(bool singleLine, - const Windows::Foundation::IReference& formats, - bool clearSelection) + const Windows::Foundation::IReference& formats) { // no selection --> nothing to copy if (!_terminal->IsSelectionActive()) @@ -1045,12 +1042,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation bgColor) : ""; - if (clearSelection) - { - _terminal->ClearSelection(); - _updateSelectionUI(); - } - // send data up for clipboard _CopyToClipboardHandlers(*this, winrt::make(winrt::hstring{ textData }, @@ -1064,7 +1055,14 @@ namespace winrt::Microsoft::Terminal::Control::implementation { auto lock = _terminal->LockForWriting(); _terminal->SelectAll(); - _updateSelectionUI(); + _updateSelectionUI(true); + } + + void ControlCore::ClearSelection() + { + auto lock = _terminal->LockForWriting(); + _terminal->ClearSelection(); + _updateSelectionUI(false); } bool ControlCore::ToggleBlockSelection() @@ -1086,7 +1084,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { auto lock = _terminal->LockForWriting(); _terminal->ToggleMarkMode(); - _updateSelectionUI(); + _updateSelectionUI(true); } Control::SelectionInteractionMode ControlCore::SelectionMode() const @@ -1101,7 +1099,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { _terminal->WritePastedText(hstr); _terminal->ClearSelection(); - _updateSelectionUI(); + _updateSelectionUI(false); _terminal->TrySnapOnInput(); } @@ -1421,11 +1419,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation { _terminal->SetBlockSelection(false); search.Select(); - _renderer->TriggerSelection(); // this is used for search, // so hide the markers - _UpdateSelectionMarkersHandlers(*this, winrt::make(true)); + _updateSelectionUI(false); } // Raise a FoundMatch event, which the control will use to notify @@ -1626,18 +1623,22 @@ namespace winrt::Microsoft::Terminal::Control::implementation _terminal->MultiClickSelection(terminalPosition, mode); selectionNeedsToBeCopied = true; } - _renderer->TriggerSelection(); - // this is used for mouse selection, // so hide the markers - _UpdateSelectionMarkersHandlers(*this, winrt::make(true)); + _updateSelectionUI(false); } - void ControlCore::_updateSelectionUI() + // Method Description: + // - Updates the renderer's representation of the selection as well as the selection marker overlay in TermControl + // Arguments: + // - tryShowMarkers: if true, show the selection marker overlay. Otherwise hide it. + void ControlCore::_updateSelectionUI(bool tryShowMarkers) { _renderer->TriggerSelection(); - const bool clearMarkers{ !_terminal->IsSelectionActive() }; - _UpdateSelectionMarkersHandlers(*this, winrt::make(clearMarkers)); + // if show markers and there's a selection, show the markers. + // otherwise, hide the markers (i.e. mouse selection, search, etc...) + const bool showMarkers{ _terminal->IsSelectionActive() && tryShowMarkers }; + _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 b2ef0a6f6d8..85de51ecea1 100644 --- a/src/cascadia/TerminalControl/ControlCore.h +++ b/src/cascadia/TerminalControl/ControlCore.h @@ -80,8 +80,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation void SendInput(const winrt::hstring& wstr); void PasteText(const winrt::hstring& hstr); - bool CopySelectionToClipboard(bool singleLine, const Windows::Foundation::IReference& formats, bool clearSelection = true); + bool CopySelectionToClipboard(bool singleLine, const Windows::Foundation::IReference& formats); void SelectAll(); + void ClearSelection(); bool ToggleBlockSelection(); void ToggleMarkMode(); Control::SelectionInteractionMode SelectionMode() const; @@ -271,7 +272,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation bool _setFontSizeUnderLock(int fontSize); void _updateFont(const bool initialUpdate = false); void _refreshSizeUnderLock(); - void _updateSelectionUI(); + void _updateSelectionUI(bool tryShowMarkers); void _sendInputToConnection(std::wstring_view wstr); diff --git a/src/cascadia/TerminalControl/ControlCore.idl b/src/cascadia/TerminalControl/ControlCore.idl index 23959171bdd..a1d348d8a26 100644 --- a/src/cascadia/TerminalControl/ControlCore.idl +++ b/src/cascadia/TerminalControl/ControlCore.idl @@ -89,6 +89,7 @@ namespace Microsoft.Terminal.Control void SendInput(String text); void PasteText(String text); void SelectAll(); + void ClearSelection(); Boolean ToggleBlockSelection(); void ToggleMarkMode(); void ClearBuffer(ClearBufferType clearType); diff --git a/src/cascadia/TerminalControl/ControlInteractivity.cpp b/src/cascadia/TerminalControl/ControlInteractivity.cpp index a1c6e34f523..75b8cf5b614 100644 --- a/src/cascadia/TerminalControl/ControlInteractivity.cpp +++ b/src/cascadia/TerminalControl/ControlInteractivity.cpp @@ -146,10 +146,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation // - singleLine: collapse all of the text to one line // - formats: which formats to copy (defined by action's CopyFormatting arg). nullptr // if we should defer which formats are copied to the global setting - // - clearSelection: if true, clear the selection after copying it. Used for CopyOnSelect. bool ControlInteractivity::CopySelectionToClipboard(bool singleLine, - const Windows::Foundation::IReference& formats, - bool clearSelection) + const Windows::Foundation::IReference& formats) { if (_core) { @@ -165,7 +163,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation // Mark the current selection as copied _selectionNeedsToBeCopied = false; - return _core->CopySelectionToClipboard(singleLine, formats, clearSelection); + return _core->CopySelectionToClipboard(singleLine, formats); } return false; @@ -258,15 +256,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation } else if (WI_IsFlagSet(buttonState, MouseButtonState::IsRightButtonDown)) { - // CopySelectionToClipboard() clears the selection. - // So we need to keep track of the state before copying it. - const auto initiallyHadSelection = _core->HasSelection(); - if (initiallyHadSelection) - { - // copy selected text - CopySelectionToClipboard(shiftEnabled, nullptr); - } - if (_core->CopyOnSelect() || !initiallyHadSelection) + // 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 @@ -390,9 +383,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation _selectionNeedsToBeCopied) { // IMPORTANT! - // Set clearSelection to false here! + // DO NOT clear the selection here! // Otherwise, the selection will be cleared immediately after you make it. - CopySelectionToClipboard(false, nullptr, /*clearSelection*/ false); + CopySelectionToClipboard(false, nullptr); } _singleClickTouchdownPos = std::nullopt; diff --git a/src/cascadia/TerminalControl/ControlInteractivity.h b/src/cascadia/TerminalControl/ControlInteractivity.h index 8f5cc6d2415..3d47ad7c0c9 100644 --- a/src/cascadia/TerminalControl/ControlInteractivity.h +++ b/src/cascadia/TerminalControl/ControlInteractivity.h @@ -80,8 +80,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation #pragma endregion bool CopySelectionToClipboard(bool singleLine, - const Windows::Foundation::IReference& formats, - bool clearSelection = true); + const Windows::Foundation::IReference& formats); void RequestPasteTextFromClipboard(); void SetEndSelectionPoint(const Core::Point pixelPosition); bool ManglePathsForWsl(); diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index fe6662e0dc2..1e5dcdc9f1a 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -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: From b8b2d765f16a3ab291674a826c5635a479d3bca7 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Thu, 30 Jun 2022 16:48:55 -0700 Subject: [PATCH 11/11] remove the bool from _updateSelectionUI --- src/cascadia/TerminalControl/ControlCore.cpp | 38 ++++++++----------- src/cascadia/TerminalControl/ControlCore.h | 2 +- .../TerminalCore/TerminalSelection.cpp | 4 +- 3 files changed, 20 insertions(+), 24 deletions(-) diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index d250a0751ab..3472bf1016e 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -422,7 +422,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { auto lock = _terminal->LockForWriting(); _terminal->SelectAll(); - _updateSelectionUI(true); + _updateSelectionUI(); return true; } @@ -431,7 +431,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { auto lock = _terminal->LockForWriting(); _terminal->UpdateSelection(updateSlnParams->first, updateSlnParams->second, modifiers); - _updateSelectionUI(true); + _updateSelectionUI(); return true; } @@ -439,7 +439,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation if (!modifiers.IsWinPressed()) { _terminal->ClearSelection(); - _updateSelectionUI(false); + _updateSelectionUI(); } // When there is a selection active, escape should clear it and NOT flow through @@ -981,10 +981,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation // save location (for rendering) + render _terminal->SetSelectionEnd(terminalPosition); - - // this is used for mouse dragging, - // so hide the markers - _updateSelectionUI(false); + _updateSelectionUI(); } // Called when the Terminal wants to set something to the clipboard, i.e. @@ -1055,14 +1052,14 @@ namespace winrt::Microsoft::Terminal::Control::implementation { auto lock = _terminal->LockForWriting(); _terminal->SelectAll(); - _updateSelectionUI(true); + _updateSelectionUI(); } void ControlCore::ClearSelection() { auto lock = _terminal->LockForWriting(); _terminal->ClearSelection(); - _updateSelectionUI(false); + _updateSelectionUI(); } bool ControlCore::ToggleBlockSelection() @@ -1084,7 +1081,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { auto lock = _terminal->LockForWriting(); _terminal->ToggleMarkMode(); - _updateSelectionUI(true); + _updateSelectionUI(); } Control::SelectionInteractionMode ControlCore::SelectionMode() const @@ -1099,7 +1096,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { _terminal->WritePastedText(hstr); _terminal->ClearSelection(); - _updateSelectionUI(false); + _updateSelectionUI(); _terminal->TrySnapOnInput(); } @@ -1421,8 +1418,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation search.Select(); // this is used for search, - // so hide the markers - _updateSelectionUI(false); + // 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)); } // Raise a FoundMatch event, which the control will use to notify @@ -1623,21 +1622,16 @@ namespace winrt::Microsoft::Terminal::Control::implementation _terminal->MultiClickSelection(terminalPosition, mode); selectionNeedsToBeCopied = true; } - // this is used for mouse selection, - // so hide the markers - _updateSelectionUI(false); + _updateSelectionUI(); } // Method Description: // - Updates the renderer's representation of the selection as well as the selection marker overlay in TermControl - // Arguments: - // - tryShowMarkers: if true, show the selection marker overlay. Otherwise hide it. - void ControlCore::_updateSelectionUI(bool tryShowMarkers) + void ControlCore::_updateSelectionUI() { _renderer->TriggerSelection(); - // if show markers and there's a selection, show the markers. - // otherwise, hide the markers (i.e. mouse selection, search, etc...) - const bool showMarkers{ _terminal->IsSelectionActive() && tryShowMarkers }; + // 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)); } diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h index 85de51ecea1..5f021c10a07 100644 --- a/src/cascadia/TerminalControl/ControlCore.h +++ b/src/cascadia/TerminalControl/ControlCore.h @@ -272,7 +272,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation bool _setFontSizeUnderLock(int fontSize); void _updateFont(const bool initialUpdate = false); void _refreshSizeUnderLock(); - void _updateSelectionUI(bool tryShowMarkers); + void _updateSelectionUI(); void _sendInputToConnection(std::wstring_view wstr); diff --git a/src/cascadia/TerminalCore/TerminalSelection.cpp b/src/cascadia/TerminalCore/TerminalSelection.cpp index 55d685c4a7e..6deddbd936d 100644 --- a/src/cascadia/TerminalCore/TerminalSelection.cpp +++ b/src/cascadia/TerminalCore/TerminalSelection.cpp @@ -215,6 +215,7 @@ void Terminal::SetSelectionEnd(const til::point viewportPos, std::optionalstart, _selection->end) = expandedAnchors; } + _selectionMode = SelectionInteractionMode::Mouse; } // Method Description: @@ -450,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) @@ -594,7 +596,7 @@ void Terminal::_MoveByBuffer(SelectionDirection direction, til::point& pos) void Terminal::ClearSelection() { _selection = std::nullopt; - _selectionMode = SelectionInteractionMode::Mouse; + _selectionMode = SelectionInteractionMode::None; _selectionEndpoint = static_cast(0); }