From 06a39f84ee502c795ec32f25bcd03c946300b32d Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Thu, 23 Jun 2022 09:44:44 -0700 Subject: [PATCH] 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: