From 851d24a2103ff36505f1c26da4b196839221bc28 Mon Sep 17 00:00:00 2001 From: James Holderness Date: Mon, 14 Dec 2020 19:45:49 +0000 Subject: [PATCH] Fix color selection operations in conhost (#8577) In conhost there is a keyboard shortcut that applies colors to the selected range of text, and another shortcut that searches for the selected text, and applies colors to any matching content. The former operation doesn't work correctly when the selection is wrapped, and both have problems when the selected text spans DBCS characters. This PR attempts to fix those issues. The problem with the color section was that it applied the color to a simple rect spanning the start and end points of the selection. I've now updated it to use the `Selection::GetSelectionRects` method, which correctly handles a wrapped range of lines, and makes sure that double width characters are fully covered. The problem with the "find-and-color" operation was in the way it obtained the search text from the selected screen cells. Since it retrieved one cell at a time, and a DBCS character can span two cells, that resulted in some characters being repeated in the search text. I've now corrected that code to take the width of the characters into account. ## Validation Steps Performed I've manually verified that the test cases described in #8572 and #8574 are now working correctly. Closes #8572 Closes #8574 --- src/host/selectionInput.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/host/selectionInput.cpp b/src/host/selectionInput.cpp index afb5831a9824..70b4d4f38420 100644 --- a/src/host/selectionInput.cpp +++ b/src/host/selectionInput.cpp @@ -692,12 +692,13 @@ bool Selection::_HandleColorSelection(const INPUT_KEY_INFO* const pInputKeyInfo) std::wstring str; for (const auto& selectRect : selectionRects) { - auto it = screenInfo.GetTextDataAt(COORD{ selectRect.Left, selectRect.Top }); + auto it = screenInfo.GetCellDataAt(COORD{ selectRect.Left, selectRect.Top }); - for (SHORT i = 0; i < (selectRect.Right - selectRect.Left + 1); ++i) + for (SHORT i = 0; i < (selectRect.Right - selectRect.Left + 1);) { - str.append((*it).begin(), (*it).end()); - it++; + str.append(it->Chars()); + i += gsl::narrow_cast(it->Columns()); + it += it->Columns(); } } @@ -717,7 +718,11 @@ bool Selection::_HandleColorSelection(const INPUT_KEY_INFO* const pInputKeyInfo) } else { - ColorSelection(_srSelectionRect, selectionAttr); + const auto selectionRects = GetSelectionRects(); + for (const auto& selectionRect : selectionRects) + { + ColorSelection(selectionRect, selectionAttr); + } ClearSelection(); }