Skip to content

Commit

Permalink
apply feedback from zadji
Browse files Browse the repository at this point in the history
  • Loading branch information
@carlos-zamora
carlos-zamora committed Aug 18, 2021
1 parent 841cd2a commit 5b4dd33
Show file tree
Hide file tree
Showing 8 changed files with 23 additions and 27 deletions.
6 changes: 3 additions & 3 deletions src/cascadia/TerminalControl/ControlCore.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1340,10 +1340,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// handle ALT key
_terminal->SetBlockSelection(altEnabled);

Core::SelectionExpansion mode = Core::SelectionExpansion::Cell;
Core::SelectionExpansion mode = Core::SelectionExpansion::Char;
if (numberOfClicks == 1)
{
mode = Core::SelectionExpansion::Cell;
mode = Core::SelectionExpansion::Char;
}
else if (numberOfClicks == 2)
{
Expand Down Expand Up @@ -1376,7 +1376,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_terminal->SetSelectionEnd(terminalPosition, mode);
selectionNeedsToBeCopied = true;
}
else if (mode != Core::SelectionExpansion::Cell || shiftEnabled)
else if (mode != Core::SelectionExpansion::Char || shiftEnabled)
{
// If we are handling a double / triple-click or shift+single click
// we establish selection using the selected mode
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalCore/ICoreSettings.idl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ namespace Microsoft.Terminal.Core

enum SelectionExpansion
{
Cell,
Char,
Word,
Line, // Mouse selection only! Not a setting!
Viewport,
Expand Down
22 changes: 9 additions & 13 deletions src/cascadia/TerminalCore/TerminalSelection.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ void Terminal::SetSelectionAnchor(const COORD viewportPos)
_selection = SelectionAnchors{};
_selection->pivot = _ConvertToBufferCell(viewportPos);

_multiClickSelectionMode = SelectionExpansion::Cell;
_multiClickSelectionMode = SelectionExpansion::Char;
SetSelectionEnd(viewportPos);

_selection->start = _selection->pivot;
Expand Down Expand Up @@ -224,7 +224,7 @@ std::pair<COORD, COORD> Terminal::_ExpandSelectionAnchors(std::pair<COORD, COORD
start = _buffer->GetWordStart(start, _wordDelimiters);
end = _buffer->GetWordEnd(end, _wordDelimiters);
break;
case SelectionExpansion::Cell:
case SelectionExpansion::Char:
default:
// no expansion is necessary
break;
Expand All @@ -241,23 +241,17 @@ void Terminal::SetBlockSelection(const bool isEnabled) noexcept
_blockSelection = isEnabled;
}

bool Terminal::MovingStart() const noexcept
{
// true --> we're moving start endpoint ("higher")
// false --> we're moving end endpoint ("lower")
return _selection->start == _selection->pivot ? false : true;
}

void Terminal::UpdateSelection(SelectionDirection direction, SelectionExpansion mode)
{
// 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{ _selection->start == _selection->pivot ? _selection->end : _selection->start };
const bool movingEnd{ _selection->start == _selection->pivot };
auto targetPos{ movingEnd ? _selection->end : _selection->start };

// 2. Perform the movement
switch (mode)
{
case SelectionExpansion::Cell:
case SelectionExpansion::Char:
_MoveByChar(direction, targetPos);
break;
case SelectionExpansion::Word:
Expand Down Expand Up @@ -289,6 +283,7 @@ void Terminal::UpdateSelection(SelectionDirection direction, SelectionExpansion
_scrollOffset -= amtBelowView;
}
_NotifyScrollEvent();
_buffer->GetRenderTarget().TriggerScroll();
}
}

Expand Down Expand Up @@ -397,8 +392,9 @@ void Terminal::_MoveByViewport(SelectionDirection direction, COORD& pos)
case SelectionDirection::Down:
{
const auto viewportHeight{ _mutableViewport.Height() };
const auto mutableBottom{ _mutableViewport.BottomInclusive() };
const auto newY{ base::ClampAdd<short, short>(pos.Y, viewportHeight) };
pos = newY > bufferSize.BottomInclusive() ? COORD{ bufferSize.RightInclusive(), bufferSize.BottomInclusive() } : COORD{ pos.X, newY };
pos = newY > mutableBottom ? COORD{ bufferSize.RightInclusive(), mutableBottom } : COORD{ pos.X, newY };
break;
}
}
Expand All @@ -415,7 +411,7 @@ void Terminal::_MoveByBuffer(SelectionDirection direction, COORD& pos)
break;
case SelectionDirection::Right:
case SelectionDirection::Down:
pos = { bufferSize.RightInclusive(), bufferSize.BottomInclusive() };
pos = { bufferSize.RightInclusive(), _mutableViewport.BottomInclusive() };
break;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalCore/terminalrenderdata.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ void Terminal::SelectNewRegion(const COORD coordStart, const COORD coordEnd)
realCoordEnd.Y -= gsl::narrow<short>(_VisibleStartIndex());

SetSelectionAnchor(realCoordStart);
SetSelectionEnd(realCoordEnd, winrt::Microsoft::Terminal::Core::SelectionExpansion::Cell);
SetSelectionEnd(realCoordEnd, winrt::Microsoft::Terminal::Core::SelectionExpansion::Char);
}

const std::wstring_view Terminal::GetConsoleTitle() const noexcept
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsModel/ActionArgs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1703,7 +1703,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
{
UpdateSelectionArgs() = default;
ACTION_ARG(Core::SelectionDirection, Direction, Core::SelectionDirection::None);
ACTION_ARG(Core::SelectionExpansion, Mode, Core::SelectionExpansion::Cell);
ACTION_ARG(Core::SelectionExpansion, Mode, Core::SelectionExpansion::Char);
static constexpr std::string_view DirectionKey{ "direction" };
static constexpr std::string_view ModeKey{ "mode" };

Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ JSON_ENUM_MAPPER(::winrt::Microsoft::Terminal::Core::SelectionDirection)
JSON_ENUM_MAPPER(::winrt::Microsoft::Terminal::Core::SelectionExpansion)
{
static constexpr std::array<pair_type, 4> mappings = {
pair_type{ "cell", ValueType::Cell },
pair_type{ "char", ValueType::Char },
pair_type{ "word", ValueType::Word },
pair_type{ "view", ValueType::Viewport },
pair_type{ "buffer", ValueType::Buffer }
Expand Down
8 changes: 4 additions & 4 deletions src/cascadia/TerminalSettingsModel/defaults.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,10 +359,10 @@
{ "command": "paste", "keys": "shift+insert" },

// Keyboard Selection
{ "command": {"action": "updateSelection", "direction": "left", "mode": "cell" }, "keys": "shift+left" },
{ "command": {"action": "updateSelection", "direction": "right", "mode": "cell" }, "keys": "shift+right" },
{ "command": {"action": "updateSelection", "direction": "up", "mode": "cell" }, "keys": "shift+up" },
{ "command": {"action": "updateSelection", "direction": "down", "mode": "cell" }, "keys": "shift+down" },
{ "command": {"action": "updateSelection", "direction": "left", "mode": "char" }, "keys": "shift+left" },
{ "command": {"action": "updateSelection", "direction": "right", "mode": "char" }, "keys": "shift+right" },
{ "command": {"action": "updateSelection", "direction": "up", "mode": "char" }, "keys": "shift+up" },
{ "command": {"action": "updateSelection", "direction": "down", "mode": "char" }, "keys": "shift+down" },
{ "command": {"action": "updateSelection", "direction": "left", "mode": "word" }, "keys": "ctrl+shift+left" },
{ "command": {"action": "updateSelection", "direction": "right", "mode": "word" }, "keys": "ctrl+shift+right" },
{ "command": {"action": "updateSelection", "direction": "left", "mode": "view" }, "keys": "shift+home" },
Expand Down
6 changes: 3 additions & 3 deletions src/cascadia/UnitTests_TerminalCore/SelectionTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,7 @@ namespace TerminalCoreUnitTests
// buffer: doubleClickMe dragThroughHere
// ^ ^
// start finish
term.SetSelectionEnd({ 21, 10 }, SelectionExpansion::Cell);
term.SetSelectionEnd({ 21, 10 }, SelectionExpansion::Char);

// Validate selection area: "doubleClickMe drag" selected
ValidateSingleRowSelection(term, SMALL_RECT({ 4, 10, 21, 10 }));
Expand Down Expand Up @@ -825,7 +825,7 @@ namespace TerminalCoreUnitTests

// Step 4: Shift+Click at (5,10)
{
term.SetSelectionEnd({ 5, 10 }, SelectionExpansion::Cell);
term.SetSelectionEnd({ 5, 10 }, SelectionExpansion::Char);

// Validate selection area
// NOTE: Pivot should still be (10, 10)
Expand All @@ -834,7 +834,7 @@ namespace TerminalCoreUnitTests

// Step 5: Shift+Click back at (20,10)
{
term.SetSelectionEnd({ 20, 10 }, SelectionExpansion::Cell);
term.SetSelectionEnd({ 20, 10 }, SelectionExpansion::Char);

// Validate selection area
// NOTE: Pivot should still be (10, 10)
Expand Down

0 comments on commit 5b4dd33

Please sign in to comment.