Skip to content

Commit

Permalink
bugfix: correctly show markers when one cell selected
Browse files Browse the repository at this point in the history
  • Loading branch information
carlos-zamora committed Jun 23, 2022
1 parent 654bb08 commit 06a39f8
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 31 deletions.
3 changes: 1 addition & 2 deletions src/cascadia/TerminalControl/ControlCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<SelectionEndpointTarget>(_terminal->SelectionEndpointTarget());

const auto bufferSize{ _terminal->GetTextBuffer().GetSize() };
info.StartAtLeftBoundary = _terminal->GetSelectionAnchor().x == bufferSize.Left();
Expand Down
10 changes: 8 additions & 2 deletions src/cascadia/TerminalControl/ControlCore.idl
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down
11 changes: 6 additions & 5 deletions src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);
Expand All @@ -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);
}
}
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalCore/Terminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ Terminal::Terminal() :
_markMode{ false },
_quickEditMode{ false },
_selection{ std::nullopt },
_selectionEndpoint{ static_cast<SelectionEndpoint>(0) },
_taskbarState{ 0 },
_taskbarProgress{ 0 },
_trimBlockSelection{ false },
Expand Down
11 changes: 9 additions & 2 deletions src/cascadia/TerminalCore/Terminal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<SelectionExpansion> newExpansionMode = std::nullopt);
Expand All @@ -261,10 +268,9 @@ class Microsoft::Terminal::Core::Terminal final :

using UpdateSelectionParams = std::optional<std::pair<SelectionDirection, SelectionExpansion>>;
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
Expand Down Expand Up @@ -336,6 +342,7 @@ class Microsoft::Terminal::Core::Terminal final :
SelectionExpansion _multiClickSelectionMode;
bool _markMode;
bool _quickEditMode;
SelectionEndpoint _selectionEndpoint;
#pragma endregion

std::unique_ptr<TextBuffer> _mainBuffer;
Expand Down
55 changes: 35 additions & 20 deletions src/cascadia/TerminalCore/TerminalSelection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -303,6 +310,7 @@ void Terminal::ToggleMarkMode()
_markMode = true;
_quickEditMode = false;
_blockSelection = false;
WI_SetAllFlags(_selectionEndpoint, SelectionEndpoint::Start | SelectionEndpoint::End);
}
}

Expand Down Expand Up @@ -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:
Expand All @@ -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<SelectionEndpoint>(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)
Expand Down Expand Up @@ -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<SelectionEndpoint>(0);
WI_SetFlagIf(_selectionEndpoint, SelectionEndpoint::Start, targetStart);
WI_SetFlagIf(_selectionEndpoint, SelectionEndpoint::End, !targetStart);
}

// 4. Scroll (if necessary)
Expand Down Expand Up @@ -586,6 +600,7 @@ void Terminal::ClearSelection()
_selection = std::nullopt;
_markMode = false;
_quickEditMode = false;
_selectionEndpoint = static_cast<SelectionEndpoint>(0);
}

// Method Description:
Expand Down

0 comments on commit 06a39f8

Please sign in to comment.