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

Campbell color scheme should change the foreground color from #F2F2F2 to #CCCCCC #1363

Closed
rbeesley opened this issue Jun 21, 2019 · 5 comments · Fixed by #1629
Closed

Campbell color scheme should change the foreground color from #F2F2F2 to #CCCCCC #1363

rbeesley opened this issue Jun 21, 2019 · 5 comments · Fixed by #1629
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Meta The product is the management of the products. Resolution-Fix-Available It's available in an Insiders build or a release

Comments

@rbeesley
Copy link
Contributor

Campbell color scheme should change the foreground color from #F2F2F2 to #CCCCCC

The problem with the current Campbell color scheme is that it has no range for the default terminal "white." ESC[m, ESC[97m, ESC[1m, ESC[1;30m, and ESC[1;37m are all the same #F2F2F2 color, "brightWhite." Just changing the foreground from "brightWhite" to "white" means that ESC[m, and ESC[37m are shown as "white" while ESC[1m, ESC[1;30m, and ESC[1;37m are "brightWhite." When you consider what these values mean, it really makes sense that this is how the scheme should be defined. Looking at the intersection of 37m/47m you can see how this really stands out in the current scheme as it breaks the diagonal. With the current Campbell color scheme, you can't even address "white" except as a background color.

Current Campbell:
image

Proposed Campbell:
image

Proposed technical implementation details

{
  "schemes" :
  [
    {
      "background" : "#0C0C0C",
      "black" : "#0C0C0C",
      "blue" : "#0037DA",
      "brightBlack" : "#767676",
      "brightBlue" : "#3B78FF",
      "brightCyan" : "#61D6D6",
      "brightGreen" : "#16C60C",
      "brightPurple" : "#B4009E",
      "brightRed" : "#E74856",
      "brightWhite" : "#F2F2F2",
      "brightYellow" : "#F9F1A5",
      "cyan" : "#3A96DD",
      "foreground" : "#CCCCCC",
      "green" : "#13A10E",
      "name" : "Campbell",
      "purple" : "#881798",
      "red" : "#C50F1F",
      "white" : "#CCCCCC",
      "yellow" : "#C19C00"
    }
  ]
}
@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 Jun 21, 2019
@DHowett-MSFT
Copy link
Contributor

Is this just a fancy way to report #293?

@rbeesley
Copy link
Contributor Author

rbeesley commented Jun 21, 2019

@DHowett-MSFT, no that wasn't my intent. It might be related, but I'll explain more and hopefully I can make it a little more clear.

This is specifically about the color palette of Campbell. The choice was made to make the foreground text #F2F2F2, or "brightWhite." The negative about that choice is that the default terminal color is then set to "brightWhite" for ESC[m, and ESC[37m. As another experiment to understand how things work, I set the "foreground" property to #DCDCDC, this is the average of "white" and "brightBlack." More importantly it is not the same as "black," "brightBlack," "white," or "brightWhite." It's the most average gray I could create without making more extensive changes to the color scheme. With that selected, ESC[m was showing #DCDCDC, but disappointingly ESC[37m was now also showing #DCDCDC and not #CCCCCC.

I believe this is the same issue as where cmd.exe in conhost.exe only had 16 colors instead of the full number of colors available to xterm, supporting colors 0-15, foreground, background, and cursor. ESC[m and ESC[39m I believe would be the same, being default display color as defined by ECMA-48 and I believe referenced in xterm as foreground. As I understand it, because Windows only supported 16 colors, there is some "magic" which tries to map the 16 colors to the extended palette as was used by xterm in WSL. It's not a 1:1 matching. Ideally, foreground, background, and cursorColor would be colors outside that palette of 16, so it'd be 16+3 (or 4 as I think I saw another feature request for having a cursor background color).

I mention this because it seems to affect how ESC[1m works. By setting bold/intensity you're flipping the high bit. ESC[37m is "white," and ESC[1;37m is "brightWhite." The "foreground" property appears to map to ESC[37m.

So, the consequence of how this "magic" happens, if the "foreground" is set to #F2F2F2, it already maps to "brightWhite" so there is no way to make the default/foreground color any more intense. If you change the foreground to #CCCCCC, that matches "white." Therefore when you set the intensity bit for that color, it matches "brightWhite." Without lowering the intensity for "foreground" you can't use any escape sequence to set the foreground to "white." You will only be able to set the background "white" with ESC[47m.

So examining #293, it's possible that enabling the full set of colors would partially resolve this, but for ConPTY consumers which don't have the extended palette, making this adjustment would automatically improve things.

I hope that explains it. I'm still learning this myself so I might not fully understand how it all fits together, but this seems like how everything is working.

@noinkling
Copy link

If it's worth anything, this is a change I also made (before seeing this issue).

@DHowett-MSFT DHowett-MSFT added Area-Settings Issues related to settings and customizability, for console or terminal Issue-Question For questions or discussion Product-Meta The product is the management of the products. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Jun 24, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jun 24, 2019
@DHowett-MSFT DHowett-MSFT added Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. and removed Issue-Question For questions or discussion labels Jun 24, 2019
@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. labels Jun 29, 2019
@DHowett-MSFT
Copy link
Contributor

@rbeesly Thanks for the report and the fix! This was just submitted to the store with the v0.2.1831.0 servicing release. It may take some time for the store to process it.

@DHowett-MSFT DHowett-MSFT added Resolution-Fix-Available It's available in an Insiders build or a release and removed Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. labels Jul 3, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jul 3, 2019
@ghost
Copy link

ghost commented Aug 3, 2019

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

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Meta The product is the management of the products. Resolution-Fix-Available It's available in an Insiders build or a release
Projects
None yet
3 participants