Skip to content

Commit e96f6e6

Browse files
carlos-zamoracarlos-zamora
committed
PRE-MERGE #14635 Don't copy before pasting in copyOnSelect mode
2 parents 2d177d8 + ca658ba commit e96f6e6

File tree

2 files changed

+196
-3
lines changed

2 files changed

+196
-3
lines changed

‎src/cascadia/TerminalControl/ControlInteractivity.cpp

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -255,10 +255,21 @@ namespace winrt::Microsoft::Terminal::Control::implementation
255255
}
256256
else if (WI_IsFlagSet(buttonState, MouseButtonState::IsRightButtonDown))
257257
{
258-
// Try to copy the text and clear the selection
259-
const auto successfulCopy = CopySelectionToClipboard(shiftEnabled, nullptr);
258+
const auto copyOnSelect{ _core->CopyOnSelect() };
259+
bool successfulCopy = false;
260+
261+
// Don't try to copy if we're in copyOnSelect mode and have already
262+
// copied this selection. GH#14464 demonstrates a scenario where the
263+
// buffer contents might have changed since the selection was made,
264+
// and copying here would cause weirdness.
265+
if (_selectionNeedsToBeCopied || !copyOnSelect)
266+
{
267+
// Try to copy the text and clear the selection
268+
successfulCopy = CopySelectionToClipboard(shiftEnabled, nullptr);
269+
}
260270
_core->ClearSelection();
261-
if (_core->CopyOnSelect() || !successfulCopy)
271+
272+
if (copyOnSelect || !successfulCopy)
262273
{
263274
// CopyOnSelect: right click always pastes!
264275
// Otherwise: no selection --> paste

‎src/cascadia/UnitTests_Control/ControlInteractivityTests.cpp

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ namespace ControlUnitTests
4343
TEST_METHOD(GetMouseEventsInTest);
4444
TEST_METHOD(AltBufferClampMouse);
4545

46+
TEST_METHOD(CopyOnSelectSimple);
47+
TEST_METHOD(CopyOnSelectAltBuffer);
48+
4649
TEST_CLASS_SETUP(ClassSetup)
4750
{
4851
winrt::init_apartment(winrt::apartment_type::single_threaded);
@@ -966,4 +969,183 @@ namespace ControlUnitTests
966969
cursorPosition1.to_core_point());
967970
VERIFY_ARE_EQUAL(0u, expectedOutput.size(), L"Validate we drained all the expected output");
968971
}
972+
973+
void ControlInteractivityTests::CopyOnSelectSimple()
974+
{
975+
auto [settings, conn] = _createSettingsAndConnection();
976+
settings->CopyOnSelect(true);
977+
auto [core, interactivity] = _createCoreAndInteractivity(*settings, *conn);
978+
_standardInit(core, interactivity);
979+
auto& term{ *core->_terminal };
980+
981+
// Output some text
982+
for (auto i = 0; i < core->ViewHeight() / 2; ++i)
983+
{
984+
conn->WriteInput(winrt::hstring{ fmt::format(L"line {}\r\n", i) });
985+
}
986+
987+
// A callback for checking Copy events. expectedCopyContents=nullopt
988+
// indicates that we're not expecting any copy events.
989+
std::optional<std::wstring> expectedCopyContents{ std::nullopt };
990+
core->CopyToClipboard([&](auto&&, auto& args) {
991+
VERIFY_IS_TRUE(expectedCopyContents.has_value());
992+
const std::wstring expected{ expectedCopyContents->c_str() };
993+
const std::wstring actual{ args.Text().c_str() };
994+
VERIFY_ARE_EQUAL(expected, actual);
995+
});
996+
bool expectedPaste = false;
997+
interactivity->PasteFromClipboard([&](auto&&, auto&&) {
998+
VERIFY_IS_TRUE(expectedPaste);
999+
});
1000+
1001+
const auto originalViewport{ term.GetViewport() };
1002+
VERIFY_ARE_EQUAL(originalViewport.Width(), 30);
1003+
1004+
// For this test, don't use any modifiers
1005+
const auto modifiers = ControlKeyStates();
1006+
const auto leftMouseDown{ Control::MouseButtonState::IsLeftButtonDown };
1007+
const auto rightMouseDown{ Control::MouseButtonState::IsRightButtonDown };
1008+
const Control::MouseButtonState noMouseDown{};
1009+
const til::size fontSize{ 9, 21 };
1010+
const til::point terminalPosition0{ 0, 5 };
1011+
const til::point terminalPosition1{ 5, 5 };
1012+
const auto cursorPosition0{ terminalPosition0 * fontSize };
1013+
const auto cursorPosition1{ terminalPosition1 * fontSize };
1014+
1015+
Log::Comment(L" --- Click on the terminal at 0,5 ---");
1016+
1017+
interactivity->PointerPressed(leftMouseDown,
1018+
WM_LBUTTONDOWN, //pointerUpdateKind
1019+
0, // timestamp
1020+
modifiers,
1021+
cursorPosition0.to_core_point());
1022+
VERIFY_IS_FALSE(core->HasSelection());
1023+
1024+
Log::Comment(L" --- Drag to 5,5 ---");
1025+
1026+
interactivity->PointerMoved(leftMouseDown,
1027+
WM_LBUTTONDOWN, //pointerUpdateKind
1028+
modifiers,
1029+
true, // focused,
1030+
cursorPosition1.to_core_point(),
1031+
true); // pointerPressedInBounds
1032+
VERIFY_IS_TRUE(core->HasSelection());
1033+
// expectedCopyContents being nullopt here will ensure that we don't send the copy event till the pointer released
1034+
1035+
Log::Comment(L" --- Release the mouse --- ");
1036+
expectedCopyContents = L"line 5";
1037+
interactivity->PointerReleased(noMouseDown,
1038+
WM_LBUTTONUP, //pointerUpdateKind
1039+
modifiers,
1040+
cursorPosition1.to_core_point());
1041+
VERIFY_IS_TRUE(core->HasSelection());
1042+
1043+
Log::Comment(L" --- Right-click to paste --- ");
1044+
// Note from GH#14464: we don't want to copy _again_ at this point. The
1045+
// copy occured when the selection was made, we shouldn't stealth-update
1046+
// the clipboard again.
1047+
expectedCopyContents = std::nullopt;
1048+
expectedPaste = true;
1049+
interactivity->PointerPressed(rightMouseDown,
1050+
WM_RBUTTONDOWN, //pointerUpdateKind
1051+
0, // timestamp
1052+
modifiers,
1053+
cursorPosition1.to_core_point());
1054+
VERIFY_IS_FALSE(core->HasSelection());
1055+
}
1056+
1057+
void ControlInteractivityTests::CopyOnSelectAltBuffer()
1058+
{
1059+
// This test was inspired by GH#14464. Ultimately, it's similar to the
1060+
// CopyOnSelectSimple, just with an alt buffer, and outputting text
1061+
// after the selection was made.
1062+
auto [settings, conn] = _createSettingsAndConnection();
1063+
settings->CopyOnSelect(true);
1064+
auto [core, interactivity] = _createCoreAndInteractivity(*settings, *conn);
1065+
_standardInit(core, interactivity);
1066+
auto& term{ *core->_terminal };
1067+
1068+
Log::Comment(L" --- Switch to alt buffer ---");
1069+
term.Write(L"\x1b[?1049h");
1070+
auto returnToMain = wil::scope_exit([&]() { term.Write(L"\x1b[?1049h"); });
1071+
1072+
// Output some text
1073+
auto i = 0;
1074+
for (i = 0; i < core->ViewHeight() - 1; ++i)
1075+
{
1076+
conn->WriteInput(winrt::hstring{ fmt::format(L"line {}\r\n", i) });
1077+
}
1078+
1079+
// A callback for checking Copy events. expectedCopyContents=nullopt
1080+
// indicates that we're not expecting any copy events.
1081+
std::optional<std::wstring> expectedCopyContents{ std::nullopt };
1082+
core->CopyToClipboard([&](auto&&, auto& args) {
1083+
VERIFY_IS_TRUE(expectedCopyContents.has_value());
1084+
const std::wstring expected{ expectedCopyContents->c_str() };
1085+
const std::wstring actual{ args.Text().c_str() };
1086+
VERIFY_ARE_EQUAL(expected, actual);
1087+
});
1088+
bool expectedPaste = false;
1089+
interactivity->PasteFromClipboard([&](auto&&, auto&&) {
1090+
VERIFY_IS_TRUE(expectedPaste);
1091+
});
1092+
1093+
const auto originalViewport{ term.GetViewport() };
1094+
VERIFY_ARE_EQUAL(originalViewport.Width(), 30);
1095+
1096+
// For this test, don't use any modifiers
1097+
const auto modifiers = ControlKeyStates();
1098+
const auto leftMouseDown{ Control::MouseButtonState::IsLeftButtonDown };
1099+
const auto rightMouseDown{ Control::MouseButtonState::IsRightButtonDown };
1100+
const Control::MouseButtonState noMouseDown{};
1101+
const til::size fontSize{ 9, 21 };
1102+
const til::point terminalPosition0{ 0, 5 };
1103+
const til::point terminalPosition1{ 5, 5 };
1104+
const auto cursorPosition0{ terminalPosition0 * fontSize };
1105+
const auto cursorPosition1{ terminalPosition1 * fontSize };
1106+
1107+
Log::Comment(L" --- Click on the terminal at 0,5 ---");
1108+
1109+
interactivity->PointerPressed(leftMouseDown,
1110+
WM_LBUTTONDOWN, //pointerUpdateKind
1111+
0, // timestamp
1112+
modifiers,
1113+
cursorPosition0.to_core_point());
1114+
VERIFY_IS_FALSE(core->HasSelection());
1115+
1116+
Log::Comment(L" --- Drag to 5,5 ---");
1117+
1118+
interactivity->PointerMoved(leftMouseDown,
1119+
WM_LBUTTONDOWN, //pointerUpdateKind
1120+
modifiers,
1121+
true, // focused,
1122+
cursorPosition1.to_core_point(),
1123+
true); // pointerPressedInBounds
1124+
VERIFY_IS_TRUE(core->HasSelection());
1125+
// expectedCopyContents being nullopt here will ensure that we don't send the copy event till the pointer released
1126+
1127+
Log::Comment(L" --- Release the mouse --- ");
1128+
expectedCopyContents = L"line 5";
1129+
interactivity->PointerReleased(noMouseDown,
1130+
WM_LBUTTONUP, //pointerUpdateKind
1131+
modifiers,
1132+
cursorPosition1.to_core_point());
1133+
VERIFY_IS_TRUE(core->HasSelection());
1134+
1135+
// Output some text
1136+
for (; i < 5; ++i)
1137+
{
1138+
conn->WriteInput(winrt::hstring{ fmt::format(L"line {}\r\n", i) });
1139+
}
1140+
1141+
Log::Comment(L" --- Right-click to paste --- ");
1142+
expectedCopyContents = std::nullopt;
1143+
expectedPaste = true;
1144+
interactivity->PointerPressed(rightMouseDown,
1145+
WM_RBUTTONDOWN, //pointerUpdateKind
1146+
0, // timestamp
1147+
modifiers,
1148+
cursorPosition1.to_core_point());
1149+
VERIFY_IS_FALSE(core->HasSelection());
1150+
}
9691151
}

0 commit comments

Comments
 (0)