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

Swap brightBlack/black in the Solarized color schemes #6985

Merged
merged 1 commit into from
Jul 20, 2020

Conversation

DHowett
Copy link
Member

@DHowett DHowett commented Jul 20, 2020

Original notes from @M-Pixel:

Console applications assume that backgrounds are black, and that
lightBlack/DarkGrey are lighter than black/Black. This
assumption is accounted for by all color schemes in defaults.json,
except for the Solarized themes.

The Solarized Dark theme, in particular, makes -Parameters invisible
against the background in PowerShell, which is obviously an unacceptable
usability flaw.

This change makes black and background to the same (which is common
throughout the color schemes), and makes brightBlack (DarkGray in
.NET) lighter than black (which is obviously more correct given the
meanings of those words).

Out of the box, we ship a pretty bad behavior.

If I look at all of the existing shipped color schemes--and that
includes things like Tango and One Half--we are universally following a
background == black rule.

If I consult gnome-terminal or xterm, they do the same thing; Xterm by
default, gnome-terminal for solarized. The background generally matches
color index 0 across all their dark schemes. Konsole and
lxterminal disagree and map background to 0 intense for Solarized.

I want to put our Solarized schemes on a deprecation path, but
unfortunately we still need to ship something for users who we're
going to strand on them.

I'm going to have to swallow my bitter and say that yes, we should
probably just change the index mapping and go with something that works
right out of the box while we figure out how to do perceptual color
nudging and eventually remove bad defaults (like Solarized).

From #6618.

Closes #4047.
Closes #6618.

@ghost ghost added Issue-Question For questions or discussion Product-Terminal The new Windows Terminal. labels Jul 20, 2020
@DHowett
Copy link
Member Author

DHowett commented Jul 20, 2020

I'll note that this is not much better, but the colors are at least ... uh, readable. Sorta.

image

image

However, it is better than it was:

image

image

@DHowett DHowett merged commit 04f5ee7 into master Jul 20, 2020
@DHowett DHowett deleted the dev/duhowett/solar branch July 20, 2020 20:50
DHowett added a commit that referenced this pull request Jul 20, 2020
Original notes from @M-Pixel:

> Console applications assume that backgrounds are black, and that
> `lightBlack`/`DarkGrey` are lighter than `black`/`Black`.  This
> assumption is accounted for by all color schemes in `defaults.json`,
> except for the Solarized themes.
>
> The Solarized Dark theme, in particular, makes `-Parameters` invisible
> against the background in PowerShell, which is obviously an unacceptable
> usability flaw.
>
> This change makes `black` and `background` to the same (which is common
> throughout the color schemes), and makes `brightBlack` (`DarkGray` in
> .NET) lighter than black (which is obviously more correct given the
> meanings of those words).

Out of the box, we ship a pretty bad behavior.

If I look at all of the existing shipped color schemes--and that
includes things like Tango and One Half--we are universally following a
`background` == `black` rule.

If I consult gnome-terminal or xterm, they do the same thing; Xterm by
default, gnome-terminal for solarized. The background generally matches
color index `0` across all their **dark** schemes. Konsole and
lxterminal disagree and map background to `0 intense` for Solarized.

I want to put our Solarized schemes on a deprecation path, but
unfortunately we still need to ship _something_ for users who we're
going to strand on them.

I'm going to have to swallow my bitter and say that yes, we should
probably just change the index mapping and go with something that works
right out of the box while we figure out how to do perceptual color
nudging and eventually remove bad defaults (like Solarized).

From #6618.

Fixes #4047.
Closes #6618.

(cherry picked from commit 04f5ee7)
DHowett added a commit that referenced this pull request Jul 20, 2020
Original notes from @M-Pixel:

> Console applications assume that backgrounds are black, and that
> `lightBlack`/`DarkGrey` are lighter than `black`/`Black`.  This
> assumption is accounted for by all color schemes in `defaults.json`,
> except for the Solarized themes.
>
> The Solarized Dark theme, in particular, makes `-Parameters` invisible
> against the background in PowerShell, which is obviously an unacceptable
> usability flaw.
>
> This change makes `black` and `background` to the same (which is common
> throughout the color schemes), and makes `brightBlack` (`DarkGray` in
> .NET) lighter than black (which is obviously more correct given the
> meanings of those words).

Out of the box, we ship a pretty bad behavior.

If I look at all of the existing shipped color schemes--and that
includes things like Tango and One Half--we are universally following a
`background` == `black` rule.

If I consult gnome-terminal or xterm, they do the same thing; Xterm by
default, gnome-terminal for solarized. The background generally matches
color index `0` across all their **dark** schemes. Konsole and
lxterminal disagree and map background to `0 intense` for Solarized.

I want to put our Solarized schemes on a deprecation path, but
unfortunately we still need to ship _something_ for users who we're
going to strand on them.

I'm going to have to swallow my bitter and say that yes, we should
probably just change the index mapping and go with something that works
right out of the box while we figure out how to do perceptual color
nudging and eventually remove bad defaults (like Solarized).

From #6618.

Fixes #4047.
Closes #6618.

(cherry picked from commit 04f5ee7)
@ghost
Copy link

ghost commented Jul 21, 2020

🎉Windows Terminal v1.1.2021.0 has been released which incorporates this pull request.:tada:

Handy links:

@ghost
Copy link

ghost commented Jul 22, 2020

🎉Windows Terminal v1.1.2021.0 has been released which incorporates this pull request.:tada:

Handy links:

@ghost
Copy link

ghost commented Jul 22, 2020

🎉Windows Terminal Preview v1.2.2022.0 has been released which incorporates this pull request.:tada:

Handy links:

@winstliu
Copy link
Member

Just curious -- what website/app did you use to generate those contrast ratios?

@DHowett
Copy link
Member Author

DHowett commented Jul 23, 2020

I used “Accessibility Insights for Windows”!

@TBBle
Copy link

TBBle commented Jul 24, 2020

I just saw this change... I think it's faulty. It's also going to be more-wrong in any application that is configured to use Solarized via the ANSI/terminal codes, as the Solarized Dark background base03 is canonically brblack, index 8.

I'd suggest, if you want to go this way, to adopt the cmd-colours variation of Solarized, which swaps all the bright/non-bright attributes. Using it requires modifying colour mappings for 16-colour Solarized themes, but at least it's a consistent swap rather than individual replacement, and already a known quantity for people coming from Conhost-based Solarized setups.


(Side-note: I was confused because this commit appears in the list of commits to master since 1.2.2022.0 was tagged, linked from the release. Is the git tag incorrect compared to the release?)


Results with canonical Solarized colours:
image

Results on Terminal 1.2.2022.0 with newly-modified Solarized Dark:
image

The gain is that $ESC[48;5;0m is now the same as $ESC[40m, i.e. hiding #293. The loss is that $ESC[48;5;0m and $ESC[48;5;8m (and hence $ESC[40m and $ESC[100m are reversed which means your dark background has the wrong brightness relationship to your dark foreground.

Why were you testing with #073642 (base02) on #002b36 (base03) BTW? Those are both background colours in Solarized, so you should never actually see that combination. If this is the problem we're trying to solve, then indeed just punting Solarized from the codebase is probably the best option, since it always requires further configuration than just switching a colour palette.

My Terminal-colour test script, the Solarized-specific part is at the bottom.

# Based on https://github.com/fikovnik/bin-scripts/blob/master/color-test.sh
#   Author: Giles Orr
#   URL: http://gilesorr.com/bashprompt/howto/c350.html
#   License: GNU Free Documentation License, Version 1.1 or any later version published by the Free Software Foundation;
#            http://gilesorr.com/bashprompt/howto/a1004.html
#   Termcolor and Solarized outputs (the last three sections) by Paul "TBBle" Hampson
# and https://www.reddit.com/r/PowerShell/comments/b06gtw/til_that_powershell_can_do_colors/

$NL=[char]13
$ESC=[char]27
$TAB=[char]9

$T='gYw'   # The test text

Write-Host "$NL                 40m     41m     42m     43m     44m     45m     46m     47m";

$FGList = '    m', '   1m', '  30m', '1;30m', '  31m', '1;31m', '  32m', '1;32m', '  33m', '1;33m', '  34m', '1;34m', '  35m', '1;35m', '  36m', '1;36m', '  37m', '1;37m'

foreach ($FGs in $FGList) {
  $FG=$FGs -replace ' ',''
  Write-Host -NoNewline " $FGs $ESC[$FG  $T  "
  foreach ($BG in "40m", "41m", "42m", "43m", "44m", "45m", "46m", "47m") {
    Write-Host -NoNewline " $ESC[$FG$ESC[$BG  $T  $ESC[0m";
  }
  Write-Host
}
Write-Host

#Write-Host "$ESC[0mCOLOR_NC (No color)"
#Write-Host "$ESC[1;37mCOLOR_WHITE$TAB$ESC[0;30mCOLOR_BLACK$ESC[0m"
#Write-Host "$ESC[0;34mCOLOR_BLUE$TAB$ESC[1;34mCOLOR_LIGHT_BLUE$ESC[0m"
#Write-Host "$ESC[0;32mCOLOR_GREEN$TAB$ESC[1;32mCOLOR_LIGHT_GREEN$ESC[0m"
#Write-Host "$ESC[0;36mCOLOR_CYAN$TAB$ESC[1;36mCOLOR_LIGHT_CYAN$ESC[0m"
#Write-Host "$ESC[0;31mCOLOR_RED$TAB$ESC[1;31mCOLOR_LIGHT_RED$ESC[0m"
#Write-Host "$ESC[0;35mCOLOR_PURPLE$TAB$ESC[1;35mCOLOR_LIGHT_PURPLE$ESC[0m"
#Write-Host "$ESC[0;33mCOLOR_YELLOW$TAB$ESC[1;33mCOLOR_LIGHT_YELLOW$ESC[0m"
#Write-Host "$ESC[1;30mCOLOR_GRAY$TAB$ESC[0;37mCOLOR_LIGHT_GRAY$ESC[0m"
#Write-Host

Write-Host "Termcolors FG"
Write-Host "$ESC[0;30mblack$TAB$ESC[1;30mbrblack$ESC[0m"
Write-Host "$ESC[0;31mred$TAB$ESC[1;31mbrred$ESC[0m"
Write-Host "$ESC[0;32mgreen$TAB$ESC[1;32mbrgreen$ESC[0m"
Write-Host "$ESC[0;33myellow$TAB$ESC[1;33mbryellow$ESC[0m"
Write-Host "$ESC[0;34mblue$TAB$ESC[1;34mbrblue$ESC[0m"
Write-Host "$ESC[0;35mmagenta$TAB$ESC[1;35mbrmagenta$ESC[0m"
Write-Host "$ESC[0;36mcyan$TAB$ESC[1;36mbrcyan$ESC[0m"
Write-Host "$ESC[0;37mwhite$TAB$ESC[1;37mbrwhite$ESC[0m"
Write-Host

Write-Host "Solarized FG (aixterm)"
#Write-Host "base03$TAB$ESC[38;2;0;43;54mRGB$TAB$ESC[38;5;8mfg$TAB$ESC[90mbrblack$ESC[0m$TAB$TAB<== DARK BG"
#Write-Host "base02$TAB$ESC[38;2;7;54;66mRGB$TAB$ESC[38;5;0mfg$TAB$ESC[30mblack$ESC[0m$TAB$TAB<== DARK BG HIGHLIGHT"
Write-Host "base01$TAB$ESC[38;2;88;110;117mRGB$TAB$ESC[38;5;10mfg$TAB$ESC[92mbrgreen$ESC[0m$TAB$TAB<== DARK FG SECONDARY, LIGHT FG HIGHLIGHT"
Write-Host "base00$TAB$ESC[38;2;101;123;131mRGB$TAB$ESC[38;5;11mfg$TAB$ESC[93mbryellow$ESC[0m$TAB<== LIGHT FG"
Write-Host "base0$TAB$ESC[38;2;131;148;150mRGB$TAB$ESC[38;5;12mfg$TAB$ESC[94mbrblue$ESC[0m$TAB$TAB<== DARK FG"
Write-Host "base1$TAB$ESC[38;2;147;161;161mRGB$TAB$ESC[38;5;14mfg$TAB$ESC[96mbrcyan$ESC[0m$TAB$TAB<== DARK FG HIGHLIGHT, LIGHT FG SECONDARY"
#Write-Host "base2$TAB$ESC[38;2;238;232;213mRGB$TAB$ESC[38;5;7mfg$TAB$ESC[37mwhite$ESC[0m$TAB$TAB<== LIGHT BG HIGHLIGHT"
#Write-Host "base3$TAB$ESC[38;2;253;246;227mRGB$TAB$ESC[38;5;15mfg$TAB$ESC[97mbrwhite$ESC[0m$TAB$TAB<== LIGHT BG"
Write-Host "yellow$TAB$ESC[38;2;181;137;0mRGB$TAB$ESC[38;5;3mfg$TAB$ESC[33myellow$ESC[0m"
Write-Host "orange$TAB$ESC[38;2;203;75;22mRGB$TAB$ESC[38;5;9mfg$TAB$ESC[91mbrred$ESC[0m"
Write-Host "red$TAB$ESC[38;2;220;50;47mRGB$TAB$ESC[38;5;1mfg$TAB$ESC[31mred$ESC[0m"
Write-Host "magenta$TAB$ESC[38;2;211;54;130mRGB$TAB$ESC[38;5;5mfg$TAB$ESC[35mmagenta$ESC[0m"
Write-Host "violet$TAB$ESC[38;2;108;113;196mRGB$TAB$ESC[38;5;13mfg$TAB$ESC[95mbrmagenta$ESC[0m"
Write-Host "blue$TAB$ESC[38;2;38;139;210mRGB$TAB$ESC[38;5;4mfg$TAB$ESC[34mblue$ESC[0m"
Write-Host "cyan$TAB$ESC[38;2;42;161;152mRGB$TAB$ESC[38;5;6mfg$TAB$ESC[36mcyan$ESC[0m"
Write-Host "green$TAB$ESC[38;2;133;153;0mRGB$TAB$ESC[38;5;2mfg$TAB$ESC[32mgreen$ESC[0m"
Write-Host

Write-Host "Solarized BG (aixterm)"
Write-Host "base03$TAB$ESC[48;2;0;43;54mRGB$TAB$ESC[48;5;8mbg$TAB$ESC[100mbrblack$ESC[0m$TAB$TAB<== DARK BG"
Write-Host "base02$TAB$ESC[48;2;7;54;66mRGB$TAB$ESC[48;5;0mbg$TAB$ESC[40mblack$ESC[0m$TAB$TAB<== DARK BG HIGHLIGHT"
#Write-Host "base01$TAB$ESC[48;2;88;110;117mRGB$TAB$ESC[48;5;10mbg$TAB$ESC[102mbrgreen$ESC[0m$TAB$TAB<== DARK FG SECONDARY, LIGHT FG HIGHLIGHT"
#Write-Host "base00$TAB$ESC[48;2;101;123;131mRGB$TAB$ESC[48;5;11mbg$TAB$ESC[103mbryellow$ESC[0m$TAB<== LIGHT FG"
#Write-Host "base0$TAB$ESC[48;2;131;148;150mRGB$TAB$ESC[48;5;12mbg$TAB$ESC[104mbrblue$ESC[0m$TAB$TAB<== DARK FG"
#Write-Host "base1$TAB$ESC[48;2;147;161;161mRGB$TAB$ESC[48;5;14mbg$TAB$ESC[106mbrcyan$ESC[0m$TAB$TAB<== DARK FG HIGHLIGHT, LIGHT FG SECONDARY"
Write-Host "base2$TAB$ESC[48;2;238;232;213mRGB$TAB$ESC[48;5;7mbg$TAB$ESC[47mwhite$ESC[0m$TAB$TAB<== LIGHT BG HIGHLIGHT"
Write-Host "base3$TAB$ESC[48;2;253;246;227mRGB$TAB$ESC[48;5;15mbg$TAB$ESC[107mbrwhite$ESC[0m$TAB$TAB<== LIGHT BG"
#Write-Host "yellow$TAB$ESC[48;2;181;137;0mRGB$TAB$ESC[48;5;3mbg$TAB$ESC[43myellow$ESC[0m"
#Write-Host "orange$TAB$ESC[48;2;203;75;22mRGB$TAB$ESC[48;5;9mbg$TAB$ESC[101mbrred$ESC[0m"
#Write-Host "red$TAB$ESC[48;2;220;50;47mRGB$TAB$ESC[48;5;1mbg$TAB$ESC[41mred$ESC[0m"
#Write-Host "magenta$TAB$ESC[48;2;211;54;130mRGB$TAB$ESC[48;5;5mbg$TAB$ESC[45mmagenta$ESC[0m"
#Write-Host "violet$TAB$ESC[48;2;108;113;196mRGB$TAB$ESC[48;5;13mbg$TAB$ESC[105mbrmagenta$ESC[0m"
#Write-Host "blue$TAB$ESC[48;2;38;139;210mRGB$TAB$ESC[48;5;4mbg$TAB$ESC[44mblue$ESC[0m"
#Write-Host "cyan$TAB$ESC[48;2;42;161;152mRGB$TAB$ESC[48;5;6mbg$TAB$ESC[46mcyan$ESC[0m"
#Write-Host "green$TAB$ESC[48;2;133;153;0mRGB$TAB$ESC[48;5;2mbg$TAB$ESC[42mgreen$ESC[0m"

@DHowett
Copy link
Member Author

DHowett commented Jul 24, 2020

So, a couple notes outside of the discussion about Solarized:


You're right that we're not following Solarized's "canonical" color mappings... but neither is gnome-terminal. Generally, though, I'm concerned about this:

"your dark background has the wrong brightness relationship ..."

That's fair and well-specified from a Solarized standpoint, but any other terminal would consider the default Solarized mappings to have the wrong brightness relationship between "black" and "bright black". Nobody has strictly defined that "bright black" must indeed be brighter than "black", but it's right there in the name.

I'm checking base02 on base03 because we do see that color combination in the wild. PowerShell's default configuration prints 90 on 49 (or 90 on 40 in traditional consoles predating Windows 10). If we use cmd-colors-solarized or the official upstream mapping, that means printing one base color on another base color.

Personally I'd rather yank all reference to Solarized from this repo forever than move color indices back and forth for all time, but that doesn't mean I'm not amenable to discussing it 😄

@TBBle
Copy link

TBBle commented Jul 24, 2020

As far as the releases go, it might be less confusing if the Release page gives the "commits to release-1.2 since..." instead of "commits to master since..." it does now. I'm not sure if that's under your control, but I've seen it in other repos using release-branches. I would have assumed it was automatic...

I was unaware of the PowerShell-specific colour hack, and now that I try my script (have a shell version of it too) in MSYS2 bash and WSL2 Ubuntu bash (which I meant to do at the time, but got distracted with upgrading my vim config...), the output is perfect with canonical Solarized Dark, and shows the same swappage with the new-default Terminal scheme, as you'd expect.

It just happens that PowerShell is the main thing I use where Solarized is implemented using the 16-colour palette instead of RGB values, so it was the first place I went to enjoy the resolution of #293. -_-


It's absolutely fair that Solarized isn't really interested in the colour naming. 8 of the sixteen colours in the palette are tones of grey, despite their names, and the other 8 are colours. The names names of the colours suggest 12 colours and four shades, so clearly it's never going to map cleanly.

I agree it's reasonably out-of-purview for this app to maintain upstream support for Solarized, given it requires more than just a palette swap, and an important shell generates "brightblack on background" output by default. It'd be easy enough to add the relevant config block to the Solarized docs, for example.


If nothing else, this has reminded me that I'm still using the cmd-colors-solarized PowerShell config, and I need to go invert my mappings so my errors stop being orange.

@ArthVader
Copy link

ArthVader commented Aug 10, 2020

I noticed on the Microsoft App Store you all mentioned that this is an open source project and encourage community participation. I'm not sure if this is wasting your time posting comments like this here but I extremely appreciate you all trying to fix the Solarized Light and Dark themes (even if some of you might not particularly like the themes at all). I have a really bad medical condition that induces severe migraines when I look at something with too much contrast. Using Solarized, along with other visual tools, has been one of the only themes to seriously help me over the last 7 years that I have been using it. It might sound counterintuitive but I feel that for many a theme that makes things stand out less is kind of a blessing. I feared you all were going to remove Solarized from the default themes and try to make the users just figure out a custom Solarized theme, but if it is not trivial for you all to get working, I can't imagine it being easy for a common user to resolve. I read up through other pull requests regarding this issue and can definitely tell it has been frustrating to handle and I just wanted to express gratitude for the work you are all putting forward to resolve this.

@zadjii-msft
Copy link
Member

@ArthVader Thank you so much for sharing your story. It's been definitely appreciated here on the dev team - we haven't really heard any requests from people who want schemes to have less contrast, so it's good to be reminded to balance both the people who are currently happy with Solarized against the vocal people who are unhappy with it. I don't think Solarized is going to go anywhere, but hopefully we can make it just a little more usable ☺️

@TBBle
Copy link

TBBle commented Aug 22, 2020

Following up on my previous comment: For anyone who's bouncing off issues with Solarized, PowerShell, and Windows Terminal, I've documented by current setup at https://github.com/TBBle/powershell-colours-solarized-canon, and particularly that I effectively revert this change, in order to get a correctly-configured Solarized display in PowerShell. Barring #6807, which is only a minor annoyance for me, since I usually work in Solarized Dark, so I just lose background highlights.

@TBBle
Copy link

TBBle commented Sep 10, 2020

And again, since this is the last place I'm aware of this being discussed, I just came across Selenized, which is a rework of Solarized. Although I'm personally not planning on switching to it (it doesn't have the neat magic-switch properties of Solarized), it does have four distinct variants and the large advantage of mapping to terminal colours in a way that is rational out-of-the-box.

So it might be a good candidate for Windows Terminal to support in place of (or as well as) the current some-colour-swapped-Solarized schemes, for people who like that style of colour scheme but not fiddling with their console application configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Question For questions or discussion Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OperatorColor and ParameterColor are same color as background on Solarized Dark
6 participants