Clear-Host fails to clear the first line #5039

brantburnett · 2020-03-20T15:00:51Z

Environment

Windows build number: 10.0.19041.0
Windows Terminal version: 0.10.761.0

Any other software?
PowerShell Core 7.0.0
NodeJS 12.16.1
coronavirus-tracker-cli 0.6.0 (`npm install -g coronavirus-tracker-cli@0.6.0`)

Steps to reproduce

Open PowerShell Core
Run corona to get output
Run cls to clear the screen

Expected behavior

Screen should be fully cleared.

Actual behavior

The first line retains some characters. This is probably related to the color formatting being used by the CLI. The characters retained are also not from the line that was originally the topmost line.

Output before clearing:

Output after clearing:

Note: A second cls finishes clearing the screen.

The text was updated successfully, but these errors were encountered:

DHowett-MSFT · 2020-03-24T01:02:11Z

This is really strange. Would you mind running a quick test for me?

Can you do this from Powershell Core outside of Terminal? To reproduce the same conditions as Terminal runs in, you will need to run this first (it will delete and temporarily disable your scrollback, but this is an important part of the repro!)

$Host.UI.RawUI.BufferSize = $Host.UI.RawUI.WindowSize

brantburnett · 2020-03-24T13:42:18Z

@DHowett

I performed the following steps (version of coranavirus-tracker-cli on npmjs.com has some issues with upstream API sources, so I pulled latest master):

Pulled latest source from https://github.com/sagarkarira/coronavirus-tracker-cli (b5f2f7e26bb98df0e47f7ebd3dcb6899d30d9409)
npm install
npm ./bin/index.js followed by cls in Windows Terminal to confirm the problem is still present
Switched to running PowerShell Core 7.0.0 directly, and ran $Host.UI.RawUI.BufferSize = $Host.UI.RawUI.WindowSize
npm ./bin/index.js followed by cls

The problem did not appear when running directly in PowerShell Core, only in Windows Terminal hosted PowerShell Core.

DHowett-MSFT · 2020-03-27T01:29:33Z

Yeah, alright, I can definitely reproduce this. Gotta fix for v1.

Notes to future debuggers:

When I run clear in powershell, this is what I get out of the pty.

␛[m␛[25l␛[94m␍␊␛[H(␛[m␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␍␊␛[K␛[1;2H␛[?25h␛[97mdhowett-sl␛[94m)␣␛[95m~␣␛[92m%␣

Weirdly enough, all those ESC [ K are coming through, but we don't end up clearing the line the prompt ends up on.

DHowett-MSFT · 2020-03-27T01:51:13Z

␛[H(␛[m␍␊ -- we're moving home, then cr, lf, and emitting one fewer EL than the entire screen. This seems to be related to the VT renderer when we circle the buffer.

DHowett-MSFT · 2020-03-31T19:41:45Z

Alright, this is also doable with some alt buffer trickery. It looks like we're not clearing the top line when [something something scrollback] happens.

From PowerShell 7:

"`e[?1049h`e#8" + ("`n" * $Host.UI.RawUI.WindowSize.Height) + "`e[?1049l"

enter the alt buffer
emit DECALN to fill the screen with EEEEEE...
push some of them off the screen with enough newlines to fill the screen height
exit the alt buffer

Critically, when we redraw PowerShell 7.0.0 we don't EL the rest of that line (!)

(note the missing ␛[K which should follow the 0)

DHowett-MSFT · 2020-03-31T19:55:01Z

VtEngine_TraceInvalidateScroll  scrollDelta: "(X:0, Y:-1)"
VtEngine_TraceInvalidate        invalidated: "(L:0, T:0, R:171, B:41) [W:171, H:41]"
VtEngine_TraceStartPaint        cursorMoved: true
                                invalidated: [whole screen],
                                lastView:    "(L:0, T:0, R:171, B:41) [W:171, H:41]"
                                quickReturn: false
                                scrollDelta: "(X:0, Y:-1)
VtEngine_TraceLastText          lastText:    "(X:0, Y:40)
VtEngine_TraceMoveCursor        cursorPos:   "(X:0, Y:40)
                                lastText:    "(X:0, Y:40)"
VtEngine_TraceString            seq:         ^J
VtEngine_TraceMoveCursor        cursorPos:   "(X:0, Y:0)"
                                lastText:    "(X:0, Y:40)"
VtEngine_TraceString            seq:         ^[[H
VtEngine_TraceString            seq:         PowerShellSPC7.0.0
VtEngine_TraceMoveCursor        cursorPos:   "(X:0, Y:1)"
                                lastText:    "(X:16, Y:0)"
VtEngine_TraceString            seq:         ^M^J
VtEngine_TraceString            seq:         CopyrightSPC(c)SPCMicrosoftSPCCorporation.SPCAllSPCrightsSPCreserved.

that scroll delta bit scares me

DHowett-MSFT · 2020-03-31T20:19:05Z

Okay, nailed it down. This is happening because _newBottomLine is true, even though we're rendering at 0,0 and we shouldn't care about the new bottom line.
Perhaps we had an assumption that when something scrolled, it was always triggering a draw into one of the new bottom lines.

This concept is invalidated by a full screen scroll-and-redraw.

We need to determine whether we're writing to the "new" bottom line before we optimize around its contents already being empty. 😄

Open PR #5181 touches the code that sets _newBottomLine.

DHowett-MSFT · 2020-03-31T20:21:48Z

Moving the cursor clears the _newBottomLine state, but we only move the cursor after we do all the determinations about whether to clear the lines based on its state.

zadjii-msft · 2020-04-01T21:00:38Z

Okay so #5181 almost fixes this too:

Note the one mysterious 'E' immediately following the prompt that's left untouched.

If you leave more spaces in the middle of the line, you'll get more 'E's

zadjii-msft · 2020-04-02T14:46:12Z

Oh my goodness gracious. You wanna know something crazy? This won't repro without PSReadline as well.
With:

Without:

The trick is that the 'E's will only appear in spaces after the end of one run of text, before the next one starts. PSReadline will color the input as blue-on-default, but pure powershell won't. I've incorporated this into the test I'm building for #5113.

DHowett-MSFT · 2020-04-02T15:39:11Z

I’m think that’s less because of PSReadline and more because your top row is fully printed in every column, actually. If you go down one line before running that command, what happens?

zadjii-msft · 2020-04-02T16:12:06Z

psreadline:

no psreadline looks the same but apparently attempting to take a SS on my PC causes System to eat all the CPU, so I can't take another. I'll make sure the test covers this too

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

ghost · 2020-04-22T17:14:09Z

🎉This issue was addressed in #5181, which has now been successfully released as Windows Terminal Preview v0.11.1121.0.:tada:

Handy links:

ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Mar 20, 2020

ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Mar 24, 2020

ghost added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Mar 24, 2020

DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Mar 27, 2020

DHowett-MSFT added this to the Terminal v1.0 milestone Mar 27, 2020

DHowett-MSFT mentioned this issue Mar 27, 2020

Vertical scrolling duplicates last ran command #4812

Closed

DHowett-MSFT added Product-Conpty For console issues specifically related to conpty and removed Product-Terminal The new Windows Terminal. labels Mar 27, 2020

DHowett-MSFT removed the Needs-Attention The core contributors need to come back around and look at this ASAP. label Mar 27, 2020

zadjii-msft assigned zadjii-msft and DHowett-MSFT and unassigned zadjii-msft Mar 30, 2020

DHowett-MSFT assigned zadjii-msft and unassigned DHowett-MSFT Mar 31, 2020

zadjii-msft mentioned this issue Apr 2, 2020

Fix copying wrapped lines by implementing better scrolling #5181

Merged

6 tasks

DHowett-MSFT added the Priority-2 A description (P2) label Apr 3, 2020

zadjii-msft added a commit that referenced this issue Apr 3, 2020

add a test for #5039

1c9a16f

zadjii-msft added a commit that referenced this issue Apr 3, 2020

This is the test case that's actaully broken in #5039

c7e5a45

zadjii-msft added a commit that referenced this issue Apr 3, 2020

finish the test for #5039

015844b

ghost added the In-PR This issue has a related PR label Apr 3, 2020

ghost closed this as completed in #5181 Apr 9, 2020

ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Apr 9, 2020

This issue was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clear-Host fails to clear the first line #5039

Clear-Host fails to clear the first line #5039

brantburnett commented Mar 20, 2020

DHowett-MSFT commented Mar 24, 2020

brantburnett commented Mar 24, 2020

DHowett-MSFT commented Mar 27, 2020

DHowett-MSFT commented Mar 27, 2020

DHowett-MSFT commented Mar 31, 2020

DHowett-MSFT commented Mar 31, 2020

DHowett-MSFT commented Mar 31, 2020

DHowett-MSFT commented Mar 31, 2020

zadjii-msft commented Apr 1, 2020 •

edited

Loading

zadjii-msft commented Apr 2, 2020

DHowett-MSFT commented Apr 2, 2020

zadjii-msft commented Apr 2, 2020

ghost commented Apr 22, 2020

Clear-Host fails to clear the first line #5039

Clear-Host fails to clear the first line #5039

Comments

brantburnett commented Mar 20, 2020

Environment

Steps to reproduce

Expected behavior

Actual behavior

DHowett-MSFT commented Mar 24, 2020

brantburnett commented Mar 24, 2020

DHowett-MSFT commented Mar 27, 2020

DHowett-MSFT commented Mar 27, 2020

DHowett-MSFT commented Mar 31, 2020

DHowett-MSFT commented Mar 31, 2020

DHowett-MSFT commented Mar 31, 2020

DHowett-MSFT commented Mar 31, 2020

zadjii-msft commented Apr 1, 2020 • edited Loading

zadjii-msft commented Apr 2, 2020

DHowett-MSFT commented Apr 2, 2020

zadjii-msft commented Apr 2, 2020

ghost commented Apr 22, 2020

zadjii-msft commented Apr 1, 2020 •

edited

Loading