Skip to content
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

[REGR #3956] Terminal incorrectly splits part of escape sequence #4131

Closed
Sidneys1 opened this issue Jan 7, 2020 · 25 comments
Closed

[REGR #3956] Terminal incorrectly splits part of escape sequence #4131

Sidneys1 opened this issue Jan 7, 2020 · 25 comments
Assignees
Labels
Area-VT Virtual Terminal sequence support Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Conpty For console issues specifically related to conpty Resolution-Duplicate There's another issue on the tracker that's pretty much the same thing. Severity-Blocking We won't ship a release like this! No-siree.

Comments

@Sidneys1
Copy link

Sidneys1 commented Jan 7, 2020

Environment

Windows build number: 10.0.19041.0
Windows Terminal version (if applicable): Dev build @ 4882917
    Fix flipped sense in TerminalDispatch::CursorPosition (#4113)

Any other software?

PowerShell: 7.0.0-rc.1 and PowerShell 6.1.3
oh-my-posh: 2.0.342

Steps to reproduce

# Start powershell

# Install posh-git from the gallery
Install-Module posh-git -Scope CurrentUser

# Install oh-my-posh from the gallery, enter Y or A at the prompt
Install-Module oh-my-posh -Scope CurrentUser

# Load the oh-my-posh module
Import-Module oh-my-posh

# Set the theme to Paradox
Set-Theme Paradox

Expected behavior

Prompt is printed cleanly (as seen in the store release v0.7.3451.0):

image

Actual behavior

Right-most character of prompt is cut off (see timestamp) and part of an escape sequence has bled back to the beginning of the line (see "97ms"):

image

The existing prompt corrects itself after resizing the window, but subsequent prompts repeat the behavior:

image

This happens at any buffer size

Related Issues

Possibly related to #4037 (seems to be splitting an escape sequence) and/or #635 (although the "snap back" behavior is not exhibited)?

@ghost 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 Jan 7, 2020
@DHowett-MSFT
Copy link
Contributor

So, this regressed between 0.7 and 0.8? That's not great. Neither 4037 nor 635 should have changed behavior since 0.7. This may have been related to #4005.

@DHowett-MSFT DHowett-MSFT added the Severity-Blocking We won't ship a release like this! No-siree. label Jan 7, 2020
@DHowett-MSFT
Copy link
Contributor

If you set your startup project to Host.EXE (this is just traditional conhost), does the issue still reproduce?

@Sidneys1
Copy link
Author

Sidneys1 commented Jan 7, 2020 via email

@Sidneys1
Copy link
Author

Sidneys1 commented Jan 8, 2020

If you set your startup project to Host.EXE (this is just traditional conhost), does the issue still reproduce?

It does not appear to, even when changing the font to match (Cascadia Code PL):

image

@Sidneys1
Copy link
Author

Sidneys1 commented Jan 8, 2020

Additionally, an older build (e294e66) on a different computer does not exhibit the same behavior. There was a different font being used, but copying the font to the computer with the problem and using it did not fix the problem...

I'm running git bisect now to see where things go wrong :)

@Sidneys1
Copy link
Author

Sidneys1 commented Jan 8, 2020

Bisect confirmed the faulty commit is #3956 :( Lots of changes to sift through...

Edit: and it looks like the source branch is deleted, so I can't bisect the individual commits there :(

@zadjii-msft
Copy link
Member

@Sidneys1 by any chance, does #4125 fix this for you?

git fetch origin pull/4125/head:pull/4125 && git checkout pull/4125

should check out the PR

@Sidneys1
Copy link
Author

Sidneys1 commented Jan 8, 2020

@Sidneys1 by any chance, does #4125 fix this for you?

...yes.... and also no. It fixes the original issue, and introduces a lot more:

WxA2FFV785

Looks like a lot of characters are being duplicated when typing, and clear put the prompt at the bottom of the visible buffer instead of the top...

@Sidneys1
Copy link
Author

Sidneys1 commented Jan 8, 2020

As you can see though the duplication is only visual, the commands run as typed.

@zadjii-msft
Copy link
Member

Woah that's maybe not so good 😅 @miniksa for visibility

@DHowett-MSFT DHowett-MSFT changed the title Terminal incorrectly splits part of escape sequence [REGR #3956] Terminal incorrectly splits part of escape sequence Jan 8, 2020
@miniksa
Copy link
Member

miniksa commented Jan 8, 2020

Hnnn ok.

@miniksa miniksa self-assigned this Jan 8, 2020
@miniksa
Copy link
Member

miniksa commented Jan 8, 2020

My biggest problem right now is now I have to go learn how the hell to install oh-my-posh. The repro steps leave something to be desired...

@DHowett-MSFT
Copy link
Contributor

Hey now- we got a bisect out of it, which is a heck of a lot more useful than most bug reports. Don't be too hard on them! 😄

@miniksa
Copy link
Member

miniksa commented Jan 8, 2020

Yeah, true. Sorry, Sidneys1. I'm still just salty about how much of a pain in the ass StateMachine::ProcessString is turning out to be.

@miniksa
Copy link
Member

miniksa commented Jan 8, 2020

Bisect confirmed the faulty commit is #3956 :( Lots of changes to sift through...

Edit: and it looks like the source branch is deleted, so I can't bisect the individual commits there :(

I restored dev/miniksa/gardening3 in the mean time. I'm chasing down what I think is the problem though.

@miniksa miniksa added Area-VT Virtual Terminal sequence support Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Conpty For console issues specifically related to conpty labels Jan 8, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jan 8, 2020
@miniksa miniksa removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Jan 8, 2020
@Sidneys1
Copy link
Author

Sidneys1 commented Jan 8, 2020

My biggest problem right now is now I have to go learn how the hell to install oh-my-posh. The repro steps leave something to be desired...

All, good :) Your best bet is:

# Start powershell
Install-Module oh-my-posh -Scope CurrentUser
# Enter Y or A at the prompt
Import-Module oh-my-posh
Set-Theme Paradox

@miniksa
Copy link
Member

miniksa commented Jan 8, 2020

Thanks. However, I just launched straight up Powershell in my 4116 branch and it's totally toasted so I'm going to guess it's related.

@Sidneys1
Copy link
Author

Sidneys1 commented Jan 8, 2020

Updated the ticket itself with better reproduction steps.

Eventually if I have time I'll isolate the exact sequence that triggers the bug so you can reproduce without oh-my-posh.

@miniksa
Copy link
Member

miniksa commented Jan 8, 2020

I hope to figure it out today. Stay tuned.

@miniksa
Copy link
Member

miniksa commented Jan 8, 2020

master merged into 4116 seems to resolve both visible behaviors:

  1. the messed up right edge
  2. the reprinting of the text over and over.

I'm going to guess #4107 whose fix was committed yesterday was also contributing here.

@miniksa
Copy link
Member

miniksa commented Jan 8, 2020

@Sidneys1, everything seems fine now that I merged master into dev/miniksa/4116. So I think I might resolve this as a duplicate to 4116/4125.

@Sidneys1
Copy link
Author

Sidneys1 commented Jan 8, 2020

Confirmed that dev/miniska/4116 @ 1ff0037 fixes both problems for me.

@miniksa
Copy link
Member

miniksa commented Jan 8, 2020

Excellent, thanks. Sorry about that.

@miniksa miniksa closed this as completed Jan 8, 2020
@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Jan 8, 2020
@miniksa
Copy link
Member

miniksa commented Jan 8, 2020

/dup #4116

@ghost
Copy link

ghost commented Jan 8, 2020

Hi! We've identified this issue as a duplicate of another one that already exists on this Issue Tracker. This specific instance is being closed in favor of tracking the concern over on the referenced thread. Thanks for your report!

@ghost ghost added Resolution-Duplicate There's another issue on the tracker that's pretty much the same thing. and removed Needs-Tag-Fix Doesn't match tag requirements labels Jan 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-VT Virtual Terminal sequence support Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Conpty For console issues specifically related to conpty Resolution-Duplicate There's another issue on the tracker that's pretty much the same thing. Severity-Blocking We won't ship a release like this! No-siree.
Projects
None yet
Development

No branches or pull requests

4 participants