Skip to content

Commit

Permalink
Adjust 'ANSI 8' color to be more visible against background. Fixes mi…
Browse files Browse the repository at this point in the history 
…crosoft#180
  • Loading branch information
Mike MacCana committed May 16, 2018
1 parent a997313 commit 8d75ff1
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions tools/ColorTool/schemes/solarized_dark.itermcolors
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,9 @@
<key>Ansi 8 Color</key>
<dict>
<key>Blue Component</key>
<real>0.15170273184776306</real>
<real>0.39215686274509803</real>
<key>Green Component</key>
<real>0.11783610284328461</real>
<real>0.30196078431372547</real>
<key>Red Component</key>
<real>0.0</real>
</dict>
Expand Down

9 comments on commit 8d75ff1

@miniksa
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I thought we said that we should make like solarized_dark_for_powershell.itermcolors with the adjustment and keep this one preserving the exact same values as other platforms.

I expected a new file, not a change to this one.

@mikemaccana
Copy link
Owner

@mikemaccana mikemaccana commented on 8d75ff1 May 16, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@miniksa I understand - wanting to distinguish that this is a changed version. I'd suggest actually not shipping solarised_dark at all (since it isn't usable as-is) and shipping an existing modification instead (with the name of the mod). How does that sound? Happy to send a new PR.

@miniksa
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dunno. @zadjii-msft, do you have an opinion here. I don't really super care if he changes the base one. Do you?

@mikemaccana
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally off topic: how did you know that 1;30m was ANSI 8? I worked it out using the importer on https://terminal.sexy, but is there a better way to determine it?

@miniksa
Copy link

@miniksa miniksa commented on 8d75ff1 May 16, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are 16 ansi colors. 0-7 are the dark shades and 8-15 are the bright shades.
Doing 1;30m is the same as doing 90m.
https://en.wikipedia.org/wiki/ANSI_escape_code#3/4_bit

I mean, literally 30 = 0, 31 = 1, 32 = 2, etc. 1;30/90 = 8, 1;31/91 = 9, etc.

@zadjii-msft
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also:

  • 30-37 sets the foreground to 0-7,
  • 40-47 sets the background to 0-7,
  • 90-97 sets the foreground to 8-15,
  • 100-117 sets the background to 8-15,
  • 1 bolds (brightens) a 3x or 4x.

@zadjii-msft
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Back on topic...

I'd probably prefer that if we're changing the default solarized_dark scheme, at very least we pick colors from their official list:

SOLARIZED HEX     16/8 TERMCOL  XTERM/HEX   L*A*B      RGB         HSB
--------- ------- ---- -------  ----------- ---------- ----------- -----------
base03    #002b36  8/4 brblack  234 #1c1c1c 15 -12 -12   0  43  54 193 100  21
base02    #073642  0/4 black    235 #262626 20 -12 -12   7  54  66 192  90  26
base01    #586e75 10/7 brgreen  240 #585858 45 -07 -07  88 110 117 194  25  46
base00    #657b83 11/7 bryellow 241 #626262 50 -07 -07 101 123 131 195  23  51
base0     #839496 12/6 brblue   244 #808080 60 -06 -03 131 148 150 186  13  59
base1     #93a1a1 14/4 brcyan   245 #8a8a8a 65 -05 -02 147 161 161 180   9  63
base2     #eee8d5  7/7 white    254 #e4e4e4 92 -00  10 238 232 213  44  11  93
base3     #fdf6e3 15/7 brwhite  230 #ffffd7 97  00  10 253 246 227  44  10  99

... Even though apparently we weren't doing that before...
Maybe we go with the value here?
https://github.com/mbadolato/iTerm2-Color-Schemes/blob/master/schemes/Solarized%20Dark%20-%20Patched.itermcolors#L131-L139

@mikemaccana
Copy link
Owner

@mikemaccana mikemaccana commented on 8d75ff1 May 16, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we did pick colors from their official list, wouldn't we either not solve the bug (using the same color, which has low contrast), or significantly alter the output from the original intention by using a different color from their list?

@miniksa
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll let @zadjii-msft make the final call here. Color tool is his pet project. I could go either way.

But it might take us a few more days, @mikemaccana. We have a few high priority things to accomplish this week and musing about colors unfortunately falls lower. Just want you to know that we're not ignoring you, just getting pulled away to other things.

Please sign in to comment.