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.

Sign up for GitHub

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

Chunk Selection Expansion for Double/Triple Click Selection #2184

Merged
merged 11 commits into from
Aug 14, 2019
Merged

Chunk Selection Expansion for Double/Triple Click Selection #2184

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
35ca5d6
Chunk selection
carlos-zamora Jul 31, 2019
c7ac572
bugfix: double-click delimiter
carlos-zamora Aug 1, 2019
3035815
I hate the formatter, but it makes my code look better
carlos-zamora Aug 1, 2019
fa0198f
Double/Triple Click testing
carlos-zamora Aug 2, 2019
ceae2b8
MOAR CODE FORMATTING
carlos-zamora Aug 2, 2019
35c06f9
PR change - enum nit
carlos-zamora Aug 13, 2019
08c6e24
Merge branch 'master' into dev/cazamor/chunk-selection
carlos-zamora Aug 13, 2019
8d9e9c3
DHowett-MSFT's suggestions
carlos-zamora Aug 14, 2019
b51d008
code format AGAIN
carlos-zamora Aug 14, 2019
8dd436b
fix x86 failure
carlos-zamora Aug 14, 2019
c4ea3fe
tests: add namespace
carlos-zamora Aug 14, 2019
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
11 changes: 9 additions & 2 deletions src/cascadia/TerminalCore/Terminal.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,13 @@ class Microsoft::Terminal::Core::Terminal final :
SHORT _selectionAnchor_YOffset;
SHORT _endSelectionPosition_YOffset;
std::wstring _wordDelimiters;
enum SelectionExpansionMode
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
{
Cell,
Word,
Line
};
SelectionExpansionMode multiClickSelectionMode;
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved

std::shared_mutex _readWriteLock;

Expand Down Expand Up @@ -214,8 +221,8 @@ class Microsoft::Terminal::Core::Terminal final :
std::vector<SMALL_RECT> _GetSelectionRects() const;
const SHORT _ExpandWideGlyphSelectionLeft(const SHORT xPos, const SHORT yPos) const;
const SHORT _ExpandWideGlyphSelectionRight(const SHORT xPos, const SHORT yPos) const;
void _ExpandDoubleClickSelectionLeft(const COORD position);
void _ExpandDoubleClickSelectionRight(const COORD position);
COORD _ExpandDoubleClickSelectionLeft(const COORD position) const;
COORD _ExpandDoubleClickSelectionRight(const COORD position) const;
const bool _isWordDelimiter(std::wstring_view cellChar) const;
const COORD _ConvertToBufferCell(const COORD viewportPos) const;
#pragma endregion
Expand Down
80 changes: 51 additions & 29 deletions src/cascadia/TerminalCore/TerminalSelection.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,27 @@ std::vector<SMALL_RECT> Terminal::_GetSelectionRects() const
selectionRow.Right = (row == lowerCoord.Y) ? lowerCoord.X : bufferSize.RightInclusive();
}

// expand selection for Double/Triple Click
if (multiClickSelectionMode == selectionExpansionMode::Word)
{
const auto cellChar = _buffer->GetCellDataAt(selectionAnchorWithOffset)->Chars();
if (_selectionAnchor == _endSelectionPosition && _isWordDelimiter(cellChar))
{
// only highlight the cell if you double click a delimiter
}
else
{
selectionRow.Left = _ExpandDoubleClickSelectionLeft({ selectionRow.Left, row }).X;
selectionRow.Right = _ExpandDoubleClickSelectionRight({ selectionRow.Right, row }).X;
}
}
else if (multiClickSelectionMode == selectionExpansionMode::Line)
{
selectionRow.Left = 0;
selectionRow.Right = bufferSize.RightInclusive();
}

// expand selection for Wide Glyphs
selectionRow.Left = _ExpandWideGlyphSelectionLeft(selectionRow.Left, row);
selectionRow.Right = _ExpandWideGlyphSelectionRight(selectionRow.Right, row);

Expand Down Expand Up @@ -142,16 +163,24 @@ void Terminal::DoubleClickSelection(const COORD position)
if (_isWordDelimiter(cellChar))
{
SetSelectionAnchor(position);
multiClickSelectionMode = selectionExpansionMode::Word;
return;
}

// scan leftwards until delimiter is found and
// set selection anchor to one right of that spot
_ExpandDoubleClickSelectionLeft(position);
_selectionAnchor = _ExpandDoubleClickSelectionLeft(positionWithOffsets);
THROW_IF_FAILED(ShortSub(_selectionAnchor.Y, gsl::narrow<SHORT>(_ViewStartIndex()), &_selectionAnchor.Y));
_selectionAnchor_YOffset = gsl::narrow<SHORT>(_ViewStartIndex());

// scan rightwards until delimiter is found and
// set endSelectionPosition to one left of that spot
_ExpandDoubleClickSelectionRight(position);
_endSelectionPosition = _ExpandDoubleClickSelectionRight(positionWithOffsets);
THROW_IF_FAILED(ShortSub(_endSelectionPosition.Y, gsl::narrow<SHORT>(_ViewStartIndex()), &_endSelectionPosition.Y));
_endSelectionPosition_YOffset = gsl::narrow<SHORT>(_ViewStartIndex());

_selectionActive = true;
multiClickSelectionMode = selectionExpansionMode::Word;
}

// Method Description:
Expand All @@ -162,6 +191,8 @@ void Terminal::TripleClickSelection(const COORD position)
{
SetSelectionAnchor({ 0, position.Y });
SetEndSelectionPosition({ _buffer->GetSize().RightInclusive(), position.Y });

multiClickSelectionMode = selectionExpansionMode::Line;
}

// Method Description:
Expand All @@ -181,6 +212,8 @@ void Terminal::SetSelectionAnchor(const COORD position)

_selectionActive = true;
SetEndSelectionPosition(position);

multiClickSelectionMode = selectionExpansionMode::Cell;
}

// Method Description:
Expand Down Expand Up @@ -253,16 +286,10 @@ const std::wstring Terminal::RetrieveSelectedTextFromBuffer(bool trimTrailingWhi
// Arguments:
// - position: viewport coordinate for selection
// Return Value:
// - update _selectionAnchor to new expanded location
void Terminal::_ExpandDoubleClickSelectionLeft(const COORD position)
// - updated copy of "position" to new expanded location (with vertical offset)
COORD Terminal::_ExpandDoubleClickSelectionLeft(const COORD position) const
{
// don't change the value if at/outside the boundary
if (position.X <= 0 || position.X >= _buffer->GetSize().RightInclusive())
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
{
return;
}

COORD positionWithOffsets = _ConvertToBufferCell(position);
COORD positionWithOffsets = position;
const auto bufferViewport = _buffer->GetSize();
auto cellChar = _buffer->GetCellDataAt(positionWithOffsets)->Chars();
while (positionWithOffsets.X != 0 && !_isWordDelimiter(cellChar))
Expand All @@ -271,33 +298,24 @@ void Terminal::_ExpandDoubleClickSelectionLeft(const COORD position)
cellChar = _buffer->GetCellDataAt(positionWithOffsets)->Chars();
}

if (positionWithOffsets.X != 0 || _isWordDelimiter(cellChar))
if (positionWithOffsets.X != 0 && _isWordDelimiter(cellChar))
{
// move off of delimiter to highlight properly
bufferViewport.IncrementInBounds(positionWithOffsets);
}

THROW_IF_FAILED(ShortSub(positionWithOffsets.Y, gsl::narrow<SHORT>(_ViewStartIndex()), &positionWithOffsets.Y));
_selectionAnchor = positionWithOffsets;
_selectionAnchor_YOffset = gsl::narrow<SHORT>(_ViewStartIndex());
_selectionActive = true;
return positionWithOffsets;
}

// Method Description:
// - expand the double click selection to the right (stopped by delimiter)
// Arguments:
// - position: viewport coordinate for selection
// Return Value:
// - update _endSelectionPosition to new expanded location
void Terminal::_ExpandDoubleClickSelectionRight(const COORD position)
// - updated copy of "position" to new expanded location (with vertical offset)
COORD Terminal::_ExpandDoubleClickSelectionRight(const COORD position) const
{
// don't change the value if at/outside the boundary
if (position.X <= 0 || position.X >= _buffer->GetSize().RightInclusive())
{
return;
}

COORD positionWithOffsets = _ConvertToBufferCell(position);
COORD positionWithOffsets = position;
const auto bufferViewport = _buffer->GetSize();
auto cellChar = _buffer->GetCellDataAt(positionWithOffsets)->Chars();
while (positionWithOffsets.X != _buffer->GetSize().RightInclusive() && !_isWordDelimiter(cellChar))
Expand All @@ -306,15 +324,13 @@ void Terminal::_ExpandDoubleClickSelectionRight(const COORD position)
cellChar = _buffer->GetCellDataAt(positionWithOffsets)->Chars();
}

if (positionWithOffsets.X != bufferViewport.RightInclusive() || _isWordDelimiter(cellChar))
if (positionWithOffsets.X != bufferViewport.RightInclusive() && _isWordDelimiter(cellChar))
{
// move off of delimiter to highlight properly
bufferViewport.DecrementInBounds(positionWithOffsets);
}

THROW_IF_FAILED(ShortSub(positionWithOffsets.Y, gsl::narrow<SHORT>(_ViewStartIndex()), &positionWithOffsets.Y));
_endSelectionPosition = positionWithOffsets;
_endSelectionPosition_YOffset = gsl::narrow<SHORT>(_ViewStartIndex());
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
return positionWithOffsets;
}

// Method Description:
Expand All @@ -336,6 +352,12 @@ const bool Terminal::_isWordDelimiter(std::wstring_view cellChar) const
// - the corresponding location on the buffer
const COORD Terminal::_ConvertToBufferCell(const COORD viewportPos) const
{
// Invalid position if outside of boundaries
if (viewportPos.X < 0 || viewportPos.X > _buffer->GetSize().RightInclusive())
{
THROW_HR(E_INVALIDARG);
Copy link
Contributor

Choose a reason for hiding this comment

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

y new except

Copy link
Member Author

Choose a reason for hiding this comment

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

Theoretically, we should never hit this case. But, if somebody somehow did a double click OUTSIDE of the terminal, I think it makes sense to clamp it to the nearest valid buffer cell. So how does this sound?

positionWithOffsets.X = std::clamp(viewportPos.X, static_cast<SHORT>(0), _buffer->GetSize().RightInclusive());
positionWithOffsets.Y = std::clamp(viewportPos.Y, static_cast<SHORT>(0), _buffer->GetSize().BottomInclusive());

Note that this is explicitly should not be clamped to the viewport to allow for selections outside of the region we can see. This mainly affects scroll interactions (i.e.: auto scroll, scrolling into an existing selection, etc...).

}

COORD positionWithOffsets = viewportPos;
THROW_IF_FAILED(ShortSub(viewportPos.Y, gsl::narrow<SHORT>(_scrollOffset), &positionWithOffsets.Y));
THROW_IF_FAILED(ShortAdd(positionWithOffsets.Y, gsl::narrow<SHORT>(_ViewStartIndex()), &positionWithOffsets.Y));
Expand Down
59 changes: 59 additions & 0 deletions src/cascadia/UnitTests_TerminalCore/MockTermSettings.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
#pragma once

#include "precomp.h"
#include <WexTestClass.h>

#include "DefaultSettings.h"

#include "winrt/Microsoft.Terminal.Settings.h"

using namespace winrt::Microsoft::Terminal::Settings;

namespace TerminalCoreUnitTests
{
class MockTermSettings : public winrt::implements<MockTermSettings, ICoreSettings>
{
public:
MockTermSettings(int32_t historySize, int32_t initialRows, int32_t initialCols) :
_historySize(historySize),
_initialRows(initialRows),
_initialCols(initialCols)
{
}

// property getters - all implemented
int32_t HistorySize() { return _historySize; }
int32_t InitialRows() { return _initialRows; }
int32_t InitialCols() { return _initialCols; }
uint32_t DefaultForeground() { return COLOR_WHITE; }
uint32_t DefaultBackground() { return COLOR_BLACK; }
bool SnapOnInput() { return false; }
uint32_t CursorColor() { return COLOR_WHITE; }
CursorStyle CursorShape() const noexcept { return CursorStyle::Vintage; }
uint32_t CursorHeight() { return 42UL; }
winrt::hstring WordDelimiters() { return winrt::to_hstring(DEFAULT_WORD_DELIMITERS.c_str()); }

// other implemented methods
uint32_t GetColorTableEntry(int32_t) const { return 123; }

// property setters - all unimplemented
void HistorySize(int32_t) {}
void InitialRows(int32_t) {}
void InitialCols(int32_t) {}
void DefaultForeground(uint32_t) {}
void DefaultBackground(uint32_t) {}
void SnapOnInput(bool) {}
void CursorColor(uint32_t) {}
void CursorShape(CursorStyle const&) noexcept {}
void CursorHeight(uint32_t) {}
void WordDelimiters(winrt::hstring) {}

// other unimplemented methods
void SetColorTableEntry(int32_t /* index */, uint32_t /* value */) {}

private:
int32_t _historySize;
int32_t _initialRows;
int32_t _initialCols;
};
}
50 changes: 1 addition & 49 deletions src/cascadia/UnitTests_TerminalCore/ScreenSizeLimitsTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,64 +4,16 @@
#include "precomp.h"
#include <WexTestClass.h>

#include "DefaultSettings.h"
#include "../cascadia/TerminalCore/Terminal.hpp"
#include "../cascadia/UnitTests_TerminalCore/MockTermSettings.h"
carlos-zamora marked this conversation as resolved.
Show resolved Hide resolved
#include "../renderer/inc/DummyRenderTarget.hpp"
#include "consoletaeftemplates.hpp"

#include "winrt/Microsoft.Terminal.Settings.h"

using namespace winrt::Microsoft::Terminal::Settings;
using namespace Microsoft::Terminal::Core;

namespace TerminalCoreUnitTests
{
class MockTermSettings : public winrt::implements<MockTermSettings, ICoreSettings>
{
public:
MockTermSettings(int32_t historySize, int32_t initialRows, int32_t initialCols) :
_historySize(historySize),
_initialRows(initialRows),
_initialCols(initialCols)
{
}

// property getters - all implemented
int32_t HistorySize() { return _historySize; }
int32_t InitialRows() { return _initialRows; }
int32_t InitialCols() { return _initialCols; }
uint32_t DefaultForeground() { return COLOR_WHITE; }
uint32_t DefaultBackground() { return COLOR_BLACK; }
bool SnapOnInput() { return false; }
uint32_t CursorColor() { return COLOR_WHITE; }
CursorStyle CursorShape() const noexcept { return CursorStyle::Vintage; }
uint32_t CursorHeight() { return 42UL; }
winrt::hstring WordDelimiters() { return winrt::to_hstring(DEFAULT_WORD_DELIMITERS.c_str()); }

// other implemented methods
uint32_t GetColorTableEntry(int32_t) const { return 123; }

// property setters - all unimplemented
void HistorySize(int32_t) {}
void InitialRows(int32_t) {}
void InitialCols(int32_t) {}
void DefaultForeground(uint32_t) {}
void DefaultBackground(uint32_t) {}
void SnapOnInput(bool) {}
void CursorColor(uint32_t) {}
void CursorShape(CursorStyle const&) noexcept {}
void CursorHeight(uint32_t) {}
void WordDelimiters(winrt::hstring) {}

// other unimplemented methods
void SetColorTableEntry(int32_t /* index */, uint32_t /* value */) {}

private:
int32_t _historySize;
int32_t _initialRows;
int32_t _initialCols;
};

#define WCS(x) WCSHELPER(x)
#define WCSHELPER(x) L#x

Expand Down
Loading