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

Miscellaneous bug fixes for Mark Mode #13358

Merged
merged 13 commits into from
Jul 1, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 26 additions & 25 deletions src/cascadia/TerminalControl/ControlCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
auto lock = _terminal->LockForWriting();
_terminal->SelectAll();
_updateSelectionUI();
_updateSelectionUI(true);
return true;
}

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

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

// When there is a selection active, escape should clear it and NOT flow through
Expand Down Expand Up @@ -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<implementation::UpdateSelectionMarkersEventArgs>(true));
_updateSelectionUI(false);
}

// Called when the Terminal wants to set something to the clipboard, i.e.
Expand All @@ -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<CopyFormat>& formats,
bool clearSelection)
const Windows::Foundation::IReference<CopyFormat>& formats)
{
// no selection --> nothing to copy
if (!_terminal->IsSelectionActive())
Expand Down Expand Up @@ -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<CopyToClipboardEventArgs>(winrt::hstring{ textData },
Expand All @@ -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()
Expand All @@ -1086,7 +1084,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
auto lock = _terminal->LockForWriting();
_terminal->ToggleMarkMode();
_updateSelectionUI();
_updateSelectionUI(true);
}

Control::SelectionInteractionMode ControlCore::SelectionMode() const
Expand All @@ -1101,7 +1099,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
_terminal->WritePastedText(hstr);
_terminal->ClearSelection();
_updateSelectionUI();
_updateSelectionUI(false);
_terminal->TrySnapOnInput();
}

Expand Down Expand Up @@ -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<implementation::UpdateSelectionMarkersEventArgs>(true));
_updateSelectionUI(false);
}

// Raise a FoundMatch event, which the control will use to notify
Expand Down Expand Up @@ -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<implementation::UpdateSelectionMarkersEventArgs>(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)
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
{
_renderer->TriggerSelection();
const bool clearMarkers{ !_terminal->IsSelectionActive() };
_UpdateSelectionMarkersHandlers(*this, winrt::make<implementation::UpdateSelectionMarkersEventArgs>(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<implementation::UpdateSelectionMarkersEventArgs>(!showMarkers));
}

void ControlCore::AttachUiaEngine(::Microsoft::Console::Render::IRenderEngine* const pEngine)
Expand Down
5 changes: 3 additions & 2 deletions src/cascadia/TerminalControl/ControlCore.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<CopyFormat>& formats, bool clearSelection = true);
bool CopySelectionToClipboard(bool singleLine, const Windows::Foundation::IReference<CopyFormat>& formats);
void SelectAll();
void ClearSelection();
bool ToggleBlockSelection();
void ToggleMarkMode();
Control::SelectionInteractionMode SelectionMode() const;
Expand Down Expand Up @@ -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);

Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalControl/ControlCore.idl
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
23 changes: 8 additions & 15 deletions src/cascadia/TerminalControl/ControlInteractivity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<CopyFormat>& formats,
bool clearSelection)
const Windows::Foundation::IReference<CopyFormat>& formats)
{
if (_core)
{
Expand All @@ -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;
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

It could be helpful to mention here that CopySelectionToClipboard returns false if the selection is empty, which results in our "unique" right-click behavior (copy if something's selected, paste if it isn't).

const auto successfulCopy = CopySelectionToClipboard(shiftEnabled, nullptr);
_core->ClearSelection();
if (_core->CopyOnSelect() || !successfulCopy)
{
// CopyOnSelect: right click always pastes!
// Otherwise: no selection --> paste
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 1 addition & 2 deletions src/cascadia/TerminalControl/ControlInteractivity.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
#pragma endregion

bool CopySelectionToClipboard(bool singleLine,
const Windows::Foundation::IReference<CopyFormat>& formats,
bool clearSelection = true);
const Windows::Foundation::IReference<CopyFormat>& formats);
void RequestPasteTextFromClipboard();
void SetEndSelectionPoint(const Core::Point pixelPosition);
bool ManglePathsForWsl();
Expand Down
4 changes: 3 additions & 1 deletion src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down