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

CMD color Not Working with Scroll #11867

Closed
MikeMcCollister opened this issue Dec 2, 2021 · 11 comments
Closed

CMD color Not Working with Scroll #11867

MikeMcCollister opened this issue Dec 2, 2021 · 11 comments
Labels
Area-Output Related to output processing (inserting text into buffer, retrieving buffer text, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Priority-2 A description (P2) Product-Conpty For console issues specifically related to conpty Product-Terminal The new Windows Terminal. Resolution-Fix-Available It's available in an Insiders build or a release

Comments

@MikeMcCollister
Copy link

Windows Terminal version

1.11.2921.0

Windows build number

10.0.22000

Other Software

No response

Steps to reproduce

  1. Open Windows Terminal with a command prompt
  2. type "cd "
  3. type "color 1e"
  4. Now the background is blue and the text is yellow.
  5. type "dir /s"

Expected Behavior

After the "dir /s" I expect the background to stay blue.

Actual Behavior

After the "dir /s" the background of the text stays blue but the non-text area goes black (or the color as specified by the Windows Terminal preferences).

@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 Dec 2, 2021
@zadjii-msft
Copy link
Member

Huh. I wonder if this regressed in #5792, or if this always existed in conpty.

the color 1e call is going to change all the attributes currently in the buffer. For conpty, that's only what's currently in the screen.

So then, when the new lines get scrolled into conpty, are those:

  • initialized with the 1e attributes which then get sent to the Terminal and mistakenly interpreted as the default?
  • initialized with the "default" attributes, not the 1e ones?

@zadjii-msft
Copy link
Member

image

Looking at just a plain old conhost, looks like when we're at the bottom of the buffer, we restore back to the default colors. This may have never worked in conhost at the bottom of the buffer, it's just obvious now that conpty's buffer is only the size of the viewport

@j4james
Copy link
Collaborator

j4james commented Dec 2, 2021

So then, when the new lines get scrolled into conpty, are those:

  • initialized with the 1e attributes which then get sent to the Terminal and mistakenly interpreted as the default?

  • initialized with the "default" attributes, not the 1e ones?

In VT mode, new lines when scrolling are meant to be initialized with the active color attributes. This would typically be the 1e colors for a regular cmd prompt, but if you've got a fancy prompt which changes the colors, the active attributes will likely have changed by the time it scrolls.

As far as I can tell this works in conhost, even when the buffer is set to the same size as the screen (this was implemented in PR #3100). So I suspect this is probably a conpty issue and/or a bug in the WT VT implementation.

@j4james
Copy link
Collaborator

j4james commented Dec 2, 2021

I think it's possible that #8823 might be a related problem. In both cases, it looks like you can "fix" the issue by setting the Windows Terminal history size to 0.

@zadjii-msft zadjii-msft added Area-Output Related to output processing (inserting text into buffer, retrieving buffer text, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Conpty For console issues specifically related to conpty Product-Terminal The new Windows Terminal. labels Feb 25, 2022
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Feb 25, 2022
@zadjii-msft
Copy link
Member

Might be related to #12567, or at least worth looking at when we look at that one.

@zadjii-msft zadjii-msft added this to the Terminal v1.14 milestone Feb 25, 2022
@j4james
Copy link
Collaborator

j4james commented Feb 26, 2022

@zadjii-msft FYI, I wasn't able to reproduce this with the latest version, so I did a git bisect, and it looks to me like it was fixed by PR #11982. That PR also fixed issues #12049 and #8823, which are essentially the same thing.

As I understand it, the problem in all these cases is that conpty is sending through an LF control to scroll the screen, but the Windows Terminal linefeed implementation doesn't fill that new line with the correct color. Since PR #11982, though, conpty is now also outputting a bunch of spaces for the new line (with the correct attributes), so the broken linefeed implementation is no longer an issue.

This isn't ideal though - it means the conpty scrolling is probably less efficient than it could be. I'm also concerned that this got fixed by accident, and that maybe there are still edge cases where things will break - I have no clue how that conpty code actually works. But at least for now I think it's better than it was.

@MikeMcCollister
Copy link
Author

I just tried this on the preview version 1.13.10395.0. There is, however, too much black space to the right of the window; but it works much better.

image

Mike

@j4james
Copy link
Collaborator

j4james commented Feb 27, 2022

There is, however, too much black space to the right

I think what you're seeing is the space reserved for the scrollbar. If you set the scrollbar visibility to hidden, and the window padding to 0, the background color should extend all the way to the edge.

@MikeMcCollister
Copy link
Author

There is, however, too much black space to the right

I think what you're seeing is the space reserved for the scrollbar. If you set the scrollbar visibility to hidden, and the window padding to 0, the background color should extend all the way to the edge.

Yes, room for the scrollbar. However, on the stable version of Windows Terminal (1.11.3471.0) on Windows 10, the scroll bar is very skinny but when scrolling it gets larger and uses up the space on the right.

image

On the preview version of Windows Terminal (1.13.10395.0) on Windows 10, the scroll bar is also very skinny but when scrolling it gets a little larger but does not take up the space that it should.

image

Mike

@j4james
Copy link
Collaborator

j4james commented Mar 1, 2022

On the preview version of Windows Terminal (1.13.10395.0) on Windows 10, the scroll bar is also very skinny but when scrolling it gets a little larger but does not take up the space that it should.

OK, I'm seeing that too. That looks like a bug to me. I know there have been some complaints about the narrowness of the new scrollbar, so if that gets reverted this problem might just go away, but you may want to raise an issue for it anyway if it bothers you.

@zadjii-msft
Copy link
Member

Well cool. Thanks for the confirmation, I'm glad this got fixed by accident! I'm sure there's always more conpty optimizations to make, in fact I think we're working on some similar ones to fix #8341.

The scrollbar thing we're (more or less) tracking in #12400

@zadjii-msft zadjii-msft added the Resolution-Fix-Available It's available in an Insiders build or a release label Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Output Related to output processing (inserting text into buffer, retrieving buffer text, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Priority-2 A description (P2) Product-Conpty For console issues specifically related to conpty Product-Terminal The new Windows Terminal. Resolution-Fix-Available It's available in an Insiders build or a release
Projects
None yet
Development

No branches or pull requests

3 participants