Skip to content

Commit

Permalink
Fix copying wrapped lines by implementing better scrolling (#5181)
Browse files Browse the repository at this point in the history
Now that the Terminal is doing a better job of actually marking which
lines were and were not wrapped, we're not always copying lines as
"wrapped" when they should be. We're more correctly marking lines as not
wrapped, when previously we'd leave them marked wrapped.

The real problem is here in the `ScrollFrame` method - we'd manually
newline the cursor to make the terminal's viewport shift down to a new
line. If we had to scroll the viewport for a _wrapped_ line, this would
cause the Terminal to mark that line as broken, because conpty would
emit an extra `\n` that didn't actually exist.

This more correctly implements `ScrollFrame`. Now, well move where we
"thought" the cursor was, so when we get to the next `PaintBufferLine`,
if the cursor needs to newline for the next line, it'll newline, but if
we're in the middle of a wrapped line, we'll just keep printing the
wrapped line.

A couple follow up bugs were found to be caused by the same bad logic.
See #5039 and #5161 for more details on the investigations there.

## References

* #4741 RwR, which probably made this worse
* #5122, which I branched off of 
* #1245, #357 - a pair of other conpty wrapped lines bugs
* #5228 - A followup issue for this PR

## PR Checklist
* [x] Closes #5113
* [x] Closes #5180 (by fixing DECRST 25)
* [x] Closes #5039
* [x] Closes #5161 (by ensuring we only `removeSpaces` on the actual
  bottom line)
* [x] I work here
* [x] Tests added/passed
* [n/a] Requires documentation to be updated

## Validation Steps Performed

* Checked the cases from #1245, #357 to validate that they still work
* Added more and more tests for these scenarios, and then I added MORE
  tests
* The entire team played with this in selfhost builds
  • Loading branch information
zadjii-msft committed Apr 9, 2020
1 parent 5622f23 commit 6fabc4a
Show file tree
Hide file tree
Showing 12 changed files with 1,136 additions and 86 deletions.
3 changes: 3 additions & 0 deletions .github/actions/spell-check/patterns/patterns.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,6 @@ https://(?:(?:www\.|)youtube\.com|youtu.be)/[-a-zA-Z0-9?&=]*
Scro\&ll
# selectionInput.cpp
:\\windows\\syste\b
TestUtils::VerifyExpectedString\(tb, L"[^"]+"
hostSm\.ProcessString\(L"[^"]+"
\b([A-Za-z])\1{3,}\b
10 changes: 3 additions & 7 deletions .github/actions/spell-check/whitelist/alphabet.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ ABCDEFGHIJ
abcdefghijk
abcdefghijklmnop
ABCDEFGHIJKLMNOPQRST
ABCDEFGHIJKLMNOPQRSTQ
abcdefghijklmnopqrstuvwxyz
QQQQQ
QQQQQQQQQ
QQQQQQQQQQ
ABCDEFGHIJPQRST
ABCDEFGHIJPQRSTQ
QQQQQQQQQQABCDEFGHIJ
QQQQQQQQQQABCDEFGHIJKLMNOPQRSTQQQQQQQQQ
QQQQQQQQQQABCDEFGHIJKLMNOPQRSTQQQQQQQQQQ
Expand All @@ -19,9 +19,6 @@ qrstuvwxyz
qwerty
QWERTYUIOP
qwertyuiopasdfg
TTTTTTTTTTTTTTTTTTTTTTTTTT
VVVVVVVVVVVVVVVV
yyyy
ZAAZZ
ZABBZ
ZBAZZ
Expand All @@ -31,4 +28,3 @@ ZYXWVUT
ZZBBZ
ZZZBB
ZZZBZ
ZZZZZ
1 change: 1 addition & 0 deletions .github/actions/spell-check/whitelist/whitelist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2893,6 +2893,7 @@ ZCtrl
zd
zh
ZM
zsh
zu
zxcvbnm
zy
Loading

0 comments on commit 6fabc4a

Please sign in to comment.