Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add selection marker overlays for keyboard selection #10865

Merged
merged 12 commits into from Jun 20, 2022
56 changes: 49 additions & 7 deletions src/cascadia/TerminalControl/ControlCore.cpp
Expand Up @@ -421,7 +421,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
auto lock = _terminal->LockForWriting();
_terminal->SelectAll();
_renderer->TriggerSelection();
_updateSelection();
return true;
}

Expand All @@ -430,15 +430,15 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
auto lock = _terminal->LockForWriting();
_terminal->UpdateSelection(updateSlnParams->first, updateSlnParams->second, modifiers);
_renderer->TriggerSelection();
_updateSelection();
return true;
}

// GH#8791 - don't dismiss selection if Windows key was also pressed as a key-combination.
if (!modifiers.IsWinPressed())
{
_terminal->ClearSelection();
_renderer->TriggerSelection();
_updateSelection();
}

// When there is a selection active, escape should clear it and NOT flow through
Expand Down Expand Up @@ -934,6 +934,31 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_terminal->SetSelectionAnchor(position);
}

// Method Description:
// - Retrieves selection metadata from Terminal necessary to draw the
// selection markers.
// - Since all of this needs to be done under lock, it is more performant
// to throw it all in a struct and pass it along.
Control::SelectionMarkerMetadata ControlCore::SelectionMarkerInfo() const
{
auto lock = _terminal->LockForReading();
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
Control::SelectionMarkerMetadata info;

const auto start{ _terminal->SelectionStartForRendering() };
info.StartPos = { start.X, start.Y };

const auto end{ _terminal->SelectionEndForRendering() };
info.EndPos = { end.X, end.Y };

info.MovingEnd = _terminal->MovingEnd();
info.MovingCursor = _terminal->MovingCursor();

const auto bufferSize{ _terminal->GetTextBuffer().GetSize() };
info.FlipStartMarker = _terminal->GetSelectionAnchor().x == bufferSize.Left();
info.FlipEndMarker = _terminal->GetSelectionEnd().x == bufferSize.RightInclusive();
return info;
}

// Method Description:
// - Sets selection's end position to match supplied cursor position, e.g. while mouse dragging.
// Arguments:
Expand All @@ -957,6 +982,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<implementation::UpdateSelectionMarkersEventArgs>(true));
}

// Called when the Terminal wants to set something to the clipboard, i.e.
Expand Down Expand Up @@ -1018,7 +1047,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
if (!_settings->CopyOnSelect())
{
_terminal->ClearSelection();
_renderer->TriggerSelection();
_updateSelection();
}

// send data up for clipboard
Expand All @@ -1034,7 +1063,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
auto lock = _terminal->LockForWriting();
_terminal->SelectAll();
_renderer->TriggerSelection();
_updateSelection();
}

bool ControlCore::ToggleBlockSelection()
Expand All @@ -1044,6 +1073,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
_terminal->SetBlockSelection(!_terminal->IsBlockSelection());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idle thought: maybe we file a follow up to change the shape of the markers when we're in block selection mode? idk.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah. Dustin and I were chatting about that the other day. it would be cool if we used some kind of 'L' shape for that.

Definitely a follow-up and something to mention/show in the spec.

_renderer->TriggerSelection();
// do not update the selection markers!
// if we were showing them, keep it that way.
// otherwise, continue to not show them
return true;
}
return false;
Expand All @@ -1053,7 +1085,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
auto lock = _terminal->LockForWriting();
_terminal->ToggleMarkMode();
_renderer->TriggerSelection();
_updateSelection();
}

bool ControlCore::IsInMarkMode() const
Expand All @@ -1068,7 +1100,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
_terminal->WritePastedText(hstr);
_terminal->ClearSelection();
_renderer->TriggerSelection();
_updateSelection();
_terminal->TrySnapOnInput();
}

Expand Down Expand Up @@ -1389,6 +1421,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<implementation::UpdateSelectionMarkersEventArgs>(true));
}

// Raise a FoundMatch event, which the control will use to notify
Expand Down Expand Up @@ -1589,8 +1625,14 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_terminal->MultiClickSelection(terminalPosition, mode);
selectionNeedsToBeCopied = true;
}
_updateSelection();
}

void ControlCore::_updateSelection()
{
_renderer->TriggerSelection();
const bool clearMarkers{ !_terminal->IsSelectionActive() };
_UpdateSelectionMarkersHandlers(*this, winrt::make<implementation::UpdateSelectionMarkersEventArgs>(clearMarkers));
}

void ControlCore::AttachUiaEngine(::Microsoft::Console::Render::IRenderEngine* const pEngine)
Expand Down
3 changes: 3 additions & 0 deletions src/cascadia/TerminalControl/ControlCore.h
Expand Up @@ -159,6 +159,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
bool HasSelection() const;
bool CopyOnSelect() const;
Windows::Foundation::Collections::IVector<winrt::hstring> SelectedText(bool trimTrailingWhitespace) const;
Control::SelectionMarkerMetadata SelectionMarkerInfo() const;
void SetSelectionAnchor(const til::point position);
void SetEndSelectionPoint(const til::point position);

Expand Down Expand Up @@ -214,6 +215,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
TYPED_EVENT(ReceivedOutput, IInspectable, IInspectable);
TYPED_EVENT(FoundMatch, IInspectable, Control::FoundResultsArgs);
TYPED_EVENT(ShowWindowChanged, IInspectable, Control::ShowWindowArgs);
TYPED_EVENT(UpdateSelectionMarkers, IInspectable, Control::UpdateSelectionMarkersEventArgs);
// clang-format on

private:
Expand Down Expand Up @@ -269,6 +271,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
bool _setFontSizeUnderLock(int fontSize);
void _updateFont(const bool initialUpdate = false);
void _refreshSizeUnderLock();
void _updateSelection();

void _sendInputToConnection(std::wstring_view wstr);

Expand Down
12 changes: 12 additions & 0 deletions src/cascadia/TerminalControl/ControlCore.idl
Expand Up @@ -29,6 +29,16 @@ namespace Microsoft.Terminal.Control
All
};

struct SelectionMarkerMetadata
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
{
Microsoft.Terminal.Core.Point StartPos;
Microsoft.Terminal.Core.Point EndPos;
Boolean MovingEnd;
Boolean MovingCursor;
Boolean FlipStartMarker;
Boolean FlipEndMarker;
};

[default_interface] runtimeclass ControlCore : ICoreState
{
ControlCore(IControlSettings settings,
Expand Down Expand Up @@ -90,6 +100,7 @@ namespace Microsoft.Terminal.Control

Boolean HasSelection { get; };
IVector<String> SelectedText(Boolean trimTrailingWhitespace);
SelectionMarkerMetadata SelectionMarkerInfo { get; };

String HoveredUriText { get; };
Windows.Foundation.IReference<Microsoft.Terminal.Core.Point> HoveredCell { get; };
Expand Down Expand Up @@ -125,6 +136,7 @@ namespace Microsoft.Terminal.Control
event Windows.Foundation.TypedEventHandler<Object, Object> ReceivedOutput;
event Windows.Foundation.TypedEventHandler<Object, FoundResultsArgs> FoundMatch;
event Windows.Foundation.TypedEventHandler<Object, ShowWindowArgs> ShowWindowChanged;
event Windows.Foundation.TypedEventHandler<Object, UpdateSelectionMarkersEventArgs> UpdateSelectionMarkers;

};
}
1 change: 1 addition & 0 deletions src/cascadia/TerminalControl/EventArgs.cpp
Expand Up @@ -13,3 +13,4 @@
#include "TransparencyChangedEventArgs.g.cpp"
#include "FoundResultsArgs.g.cpp"
#include "ShowWindowArgs.g.cpp"
#include "UpdateSelectionMarkersEventArgs.g.cpp"
12 changes: 12 additions & 0 deletions src/cascadia/TerminalControl/EventArgs.h
Expand Up @@ -13,6 +13,7 @@
#include "TransparencyChangedEventArgs.g.h"
#include "FoundResultsArgs.g.h"
#include "ShowWindowArgs.g.h"
#include "UpdateSelectionMarkersEventArgs.g.h"

namespace winrt::Microsoft::Terminal::Control::implementation
{
Expand Down Expand Up @@ -157,4 +158,15 @@ namespace winrt::Microsoft::Terminal::Control::implementation

WINRT_PROPERTY(bool, ShowOrHide);
};

struct UpdateSelectionMarkersEventArgs : public UpdateSelectionMarkersEventArgsT<UpdateSelectionMarkersEventArgs>
{
public:
UpdateSelectionMarkersEventArgs(const bool clearMarkers) :
_ClearMarkers(clearMarkers)
{
}

WINRT_PROPERTY(bool, ClearMarkers, false);
};
}
6 changes: 5 additions & 1 deletion src/cascadia/TerminalControl/EventArgs.idl
Expand Up @@ -69,7 +69,6 @@ namespace Microsoft.Terminal.Control
Double Opacity { get; };
}


runtimeclass FoundResultsArgs
{
Boolean FoundMatch { get; };
Expand All @@ -79,4 +78,9 @@ namespace Microsoft.Terminal.Control
{
Boolean ShowOrHide { get; };
}

runtimeclass UpdateSelectionMarkersEventArgs
{
Boolean ClearMarkers { get; };
}
}