microsoft / terminal

The new Windows Terminal and the original Windows console host, all in the same place!
MIT License
95.23k stars 8.27k forks source link

Solarized schemes shipped in Terminal Preview are incorrect #1509

Closed TBBle closed 5 years ago

TBBle commented 5 years ago

Environment

Windows build number: Microsoft Windows [Version 10.0.18362.175]
Windows Terminal version: Windows Terminal (Preview) 0.2.1715.0

Steps to reproduce

Note: I already had PowerShell and CMD using Solarized Dark in ConHost, via cmd-colors-solarized.

Expected behavior

The same colours as provided by cmd-colours-solarized, consistent with the main Solarized page screenshots

Actual behavior

The white is too bright. That stood out immediately, so I looked deeper at the pre-defined scheme.

Diagnosis

Comparing the default settings to upstream Solarized The Values and Usage & Development I noted the following differences.

Solarized: Role Tone HEX RGB Terminal: RGB HEX Tone
Dark Foreground base0 #839496 131_148_150 253_246_227 #FDF6E3 base3
Dark Background base03 #002b36 0 _43_54 7_54_66 #073642 base02
Light Foreground base00 #657b83 101_123_131 7_54_66 #073642 base02
Light Background base3 #fdf6e3 253_246_227 253_246_227 #FDF6E3 base3
Red red #dc322f 220_50_47 211_1_2 #D30102 Red (beta1)

The difference in Red is simply that the definition came from an older version of Solarized, and was changed in the 1.0.0beta2 release. This one shows up on the Internet a lot, if you search for #D30102.

The foreground/background differences are a little weird, my guess would be that a modified Light scheme was used (darker font text on the same background) and then some confusion or oversight caused Dark to be defined as 'Light with a swapped FG/BG', leading to an overall-higher brightness from the non-coloured text.

TBBle commented 5 years ago

A 👍 to the team for the "Live update on profile.json save" feature, that made this super-easy to diagnose/test.

TBBle commented 5 years ago

There might be something else fundamentally-odd happening here. Using `Out-Colors.ps`` from cmd-colors-solarized in Terminal: image versus my existing PowerShell setup: image with the following profile:

        {
            "acrylicOpacity" : 0.5,
            "closeOnExit" : true,
            "colorScheme" : "Solarized Dark",
            "commandline" : "powershell.exe",
            "cursorColor" : "#839496",
            "cursorShape" : "bar",
            "fontFace" : "Fira Code Retina",
            "fontSize" : 12,
            "guid" : "{61c54bbd-c2c6-5271-96e7-009a87ff44bf}",
            "historySize" : 9001,
            "icon" : "ms-appx:///ProfileIcons/{61c54bbd-c2c6-5271-96e7-009a87ff44bf}.png",
            "name" : "Windows PowerShell",
            "padding" : "0, 0, 0, 0",
            "snapOnInput" : true,
            "startingDirectory" : "%USERPROFILE%",
            "useAcrylic" : false
        },

and scheme

        {
            "background" : "#002B36",
            "black" : "#073642",
            "blue" : "#268BD2",
            "brightBlack" : "#002B36",
            "brightBlue" : "#839496",
            "brightCyan" : "#93A1A1",
            "brightGreen" : "#586E75",
            "brightPurple" : "#6C71C4",
            "brightRed" : "#CB4B16",
            "brightWhite" : "#FDF6E3",
            "brightYellow" : "#657B83",
            "cyan" : "#2AA198",
            "foreground" : "#839496",
            "green" : "#859900",
            "name" : "Solarized Dark",
            "purple" : "#D33682",
            "red" : "#DC322F",
            "white" : "#EEE8D5",
            "yellow" : "#B58900"
        },

It might be an issue in Out-Colors.ps1's name/value mapping, but I'm definitely seeing different PSReadLine colouring results and $HOST.UI.RawUI.ForegroundColor etc oddities too.

DHowett-MSFT commented 5 years ago

There’s an outstanding design choice (or issue, depending on how you look at it) where terminal applications cannot emit colors 0 and 7 because they get converted into “background” and “foreground”. This is done for maximum compatibility with existing Windows console applications. It’ll probably be the cause of at least a couple solarized rendering bugs. #293

TBBle commented 5 years ago

That would explain the difference in border for the output. The actual colour difference (dark/light mismatch mostly) is possibly due to the cmd-colours-solarized choice of mappings between ANSI/TERMCOL and PowerShell colours, which are clearly different than how Terminal maps its themes (defined in terms of TERMCOL).

TBBle commented 5 years ago

Confirmed, with the following mapping (simply swapping bright and non-bright), the colours are the same as cmd-colours-solarized:

    {
      "background": "#002B36",
      "brightBlack": "#073642",
      "brightBlue": "#268BD2",
      "black": "#002B36",
      "blue": "#839496",
      "cyan": "#93A1A1",
      "green": "#586E75",
      "purple": "#6C71C4",
      "red": "#CB4B16",
      "white": "#FDF6E3",
      "yellow": "#657B83",
      "brightCyan": "#2AA198",
      "foreground": "#839496",
      "brightGreen": "#859900",
      "name": "Solarized Dark cmd-colors",
      "brightPurple": "#D33682",
      "brightRed": "#DC322F",
      "brightWhite": "#EEE8D5",
      "brightYellow": "#B58900"
    },

Confirmed with the following script, which also checks the ANSI codes.

# Check the mappings per https://github.com/neilpa/cmd-colors-solarized
# in the various ways they can be applied as foreground colours

$Pallet = [ordered]@{
    "base03" = [System.Tuple]::Create("0;30m", "Black")
    "base02" = [System.Tuple]::Create("1;30m", "DarkGray")
    "base01" = [System.Tuple]::Create("0;32m", "DarkGreen")
    "base00" = [System.Tuple]::Create("0;33m", "DarkYellow")
    "base0"  = [System.Tuple]::Create("0;34m", "DarkBlue")
    "base1"  = [System.Tuple]::Create("0;36m", "DarkCyan")
    "base2"  = [System.Tuple]::Create("0;37m", "Gray")
    "base3"  = [System.Tuple]::Create("1;37m", "White")
    "yellow" = [System.Tuple]::Create("1;33m", "Yellow")
    "orange" = [System.Tuple]::Create("0;31m", "DarkRed")
    "red"    = [System.Tuple]::Create("1;31m", "Red")
    "magenta"= [System.Tuple]::Create("1;35m", "Magenta")
    "violet" = [System.Tuple]::Create("0;35m", "DarkMagenta")
    "blue"   = [System.Tuple]::Create("1;34m", "Blue")
    "cyan"   = [System.Tuple]::Create("1;36m", "Cyan")
    "green"  = [System.Tuple]::Create("1;32m", "Green")
}

Write-Host ("{0,-15} | {1,-15} | {2,-15} |" -f "Color", "ANSI", "Write-Host")

$Pallet.keys | `
    ForEach-Object {
        $Color = $_
        $ANSI = $Pallette[$_].Item1
        $HostColour = $Pallette[$_].Item2

        Write-Host ("{0,-15}" -f $Color) -NoNewline
        Write-Host (" | ") -NoNewline
        Write-Host ("{0,-27}" -f "$([char]0x1b)[${ANSI}ESC[${ANSI}$([char]0x1b)[39m" ) -NoNewline
        Write-Host (" | ") -NoNewline
        Write-Host ("{0,-15}" -f $HostColour ) -ForegroundColor $HostColour -NoNewline
        Write-Host (" |")
    }

Write-Host

So really, the colour mismatch is probably a ticket to file against cmd-colors-solarized, to include the relevant pallet in its distribution. A quick look around, e.g. putty-colors-solarized, suggests that mappings for unix-hosted systems are probably consistent with the stock mappings in Terminal, not the cmd-colours-solarized mapping, at least for 8-colours-plus-bold setups.

So none of my comments here are really issues, just the original report's colour issues.

It's all very complex. Hopefully over time Terminal will help make this a bit simpler and more consistent.

antoineco commented 5 years ago

I agree with the two main concerns expressed in the issue description:

as defined at https://github.com/altercation/solarized#the-values

Background colors are also off:

Not sure if that's related to https://github.com/microsoft/terminal/issues/1509#issuecomment-504860548, but I don't think arbitrary foreground and background colors should be chosen.

The resulting scheme should be identical to this:

DHowett-MSFT commented 5 years ago

We should fix this.

DHowett-MSFT commented 5 years ago

Thanks for the report! 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.

MorgusLethe commented 5 years ago

Hi, thanks for all your work. Just to be clear, what exactly do I have to do to get Solarized Light to look good when I have Debian open? I can't seem to find a quick answer.

I expected that this will happen just by setting Solarized Light as the color scheme, but it does not look good to me, the folder names appear as badly readable lightblue on green background. Is this expected? image

antoineco commented 5 years ago

@MorgusLethe that's because the colored output of ls does not have Solarized-friendly colors by default. This can be changed using different "dircolors". Take a look at https://github.com/seebi/dircolors-solarized and pick the dircolors-ansi-light variant.

If you are new to Solarized, you will find out that a few extra terminal configurations like this one are sometimes necessary to display colors correctly (tmux, mutt, git, ...).