-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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
Conhost color selection doesn't work correctly with wrapping and DBCS #8572
Comments
Definitely an oversight, probably owing to Color Select being a hidden feature 😄 Legend has it, someone who uses CDB a lot inside the Windows organization added it as well as the “strip leading zeroes on copy” option long before Michael took over the project in ca. 2015. I think it’s cool; my only beef with it (apart from this bug. Excellent find on your part!) is that it actually manipulates the buffer. We have some plans to build on the TextBuffer pattern range support + a new interface to pass to the renderer (ICustomTextEffect?) to make this kind of stuff render-only. Eventually, eventually. |
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
🎉This issue was addressed in #8577, which has now been successfully released as Handy links: |
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 microsoft#8572 and microsoft#8574 are now working correctly. Closes microsoft#8572 Closes microsoft#8574
Environment
Steps to reproduce
Ctrl+4
to highlight the selected text in green.Ctrl+4
to highlight the selected text in green.Expected behavior
When the selection wraps across multiple lines, the green highlighting should also wrap in exactly the same way. When selecting halfway through a double width character, the selection always covers the full width, so I'd expect the green highlighting to work in the same way.
Actual behavior
The wrapped selection changes to a block selection when painting the green background. And only half of the start and end characters in the DBCS range are painted green (I should note that this renders differently in the current openconsole build, but is still doesn't match the initial selection).
The reason for this behaviour is that the color selection code just uses a simple rect spanning the start and end points of the selection, where typically a selection operation should be using the
Selection::GetSelectionRects
method to obtain the affected range (which automatically handles the line wrapping option, and makes sure double width characters are fully covered).The code I'm referring to is here:
terminal/src/host/selectionInput.cpp
Line 720 in d09fdd6
And my recommendation would be to change the line above to use the
GetSelectionRects
method like this:I suppose it's possible the current behaviour is deliberate, but I think it's more likely it was just an oversight when conhost was updated to support wrapped selections.
The text was updated successfully, but these errors were encountered: