Change default warn color to yellow #18453

KristofferC · 2016-09-12T11:39:20Z

Change the warning color to what I believe is a better default (to yellow from red).

Reasons

Red is a bit harsh for a warning.
Googling around for warning messages it seems that the vast majority uses yellow for warnings. Following what appears to be a standard makes sense.
Having a visual difference between warnings and errors seems sensible.

xterm default

gnome-terminal default (ubuntu default)

gnome-terminal white background (this one is not so nice...)

powershell

win cmd

cc @twadleigh

tkelman · 2016-09-12T15:04:32Z

guess the only question is why was it set to red from the beginning?

KristofferC · 2016-09-12T15:14:52Z

Hah! Found it 68f63db#diff-2b2ebeef35e91b1fff15d49dc1d526b5R50. It got changed from yellow to red 3 ½ years ago.

@StefanKarpinski didn't like the yellow. Apparently it was red for a while then yellow then red again... lol

ViralBShah · 2016-09-12T15:19:33Z

Yes, Red for errors, yellow for warning goes nicely with traffic light colors.

nalimilan · 2016-09-12T15:24:53Z

Definitely a good idea to differentiate warnings from errors. I've had students who thought (in R) that a package wasn't installed correctly just because an innocent warning was printed in red.

TotalVerb · 2016-09-12T22:46:01Z

To make everyone happy, can we use 256-colour orange if it's available, and fall back to yellow otherwise? Or is this possibly infeasible for portability?

StefanKarpinski · 2016-09-12T23:19:51Z

I don't really have an issue with yellow for warnings. This is why we really should ship our own terminal of some kind on Windows. On UNIX we're always going to need to deal with people's terminals, but xterm-265 should be possible on most UNIX systems.

DNF2 · 2016-09-13T07:47:21Z

My impression is that most hardcore SW developers use dark terminal/editor backgrounds. But I hope you will at least give some consideration to users who prefer light backgrounds. Yellow on white (and also cyan on white) are not great combinations. I certainly understand that it is hard, bordering on impossible, to choose colours that look good for both light and dark backgrounds.

I also have a (non-empirical, gut-level) hunch that people who use light backgrounds are less likely to tweak their setup.

KristofferC · 2016-09-13T07:51:23Z

Yellow on white is not so bad. The problem is that we are printing everything in bold all the time:

ViralBShah · 2016-09-13T07:56:29Z

The solution to the white terminal is to have two profiles - light and dark and make it easy to switch.

DNF2 · 2016-09-13T07:57:57Z

Actually, I see that bold yellow looks ok for me, but the gnome-terminal version was bad. And honestly, I don't really think the colour below is yellow, it's more like beige.

KristofferC · 2016-09-13T07:58:51Z

Well, it is defined to be yellow at least.. http://pueblo.sourceforge.net/doc/manual/ansi_color_codes.html

KristofferC · 2016-09-13T07:59:41Z

In version 2.50 and later, bold is interpreted as ‘high-intensity’, which was the original ANSI definition. (High-intensity colors are brighter than low-intensity ones, which are the default.)

I guess some terminals does that which is why the bold yellow looks so bad on white.

KristofferC · 2016-09-13T08:09:08Z

@TotalVerb AFAIU, detecting if a terminal supports 256 colors is tricky because terminals lie through their teeth. I can however give you #18473 so you can easily set orange yourself.

ViralBShah · 2016-09-13T08:24:32Z

I kinda prefer orange over yellow.

KristofferC · 2016-09-13T08:30:27Z

And after #18473 you can easily set that with an env var. What we are talking about here is the default color and for that orange is out of the question due to not being supported in many default / unconfigured terminals.

KristofferC · 2016-09-17T10:25:34Z

Added news entry, anyone opposed of merging?

kshyatt · 2016-09-21T03:36:58Z

Bump.

KristofferC · 2016-10-08T18:20:38Z

From the 9 thumbs up, 0 thumbs down and the triviality of reverting, I will merge this at the end of the weekend if no one has objections.

tkelman · 2016-10-08T20:17:18Z

That yellow will be pretty bad on light terminals. Is determining the background color any more doable than detecting 256 color support? We could also do yellow on Windows and orange elsewhere since I get the impression far fewer people use light background terminals on Windows, and it's the only place where 256 color support isn't a safe assumption.

KristofferC · 2016-10-08T21:41:39Z

Guess I will focus on the non boldness in the colorized messages then which is what is causing this to look bad on white backgrounds.

KristofferC · 2016-12-16T10:16:58Z

Rebased + hooked up pkg + test to use the warn color function.

I tried the online merge resolution tool and it sort of messed up my branch... Anyway the total diff of these commit seems ok so should be fine to squash.

tkelman

so now this prints the lead "WARNING" in bold but the rest of the content not bold, right?

MichaelHatherly · 2016-12-16T12:54:36Z

base/test.jl

@@ -140,7 +140,7 @@ type Broken <: Result
    orig_expr
 end
 function Base.show(io::IO, t::Broken)
-    print_with_color(:yellow, io, "Test Broken\n"; bold = true)
+    print_with_color(:Base.warn_color(), io, "Test Broken\n"; bold = true)


Is the : meant to be prefixed to Base here, or just a typo?

typo (which I thought I fixed but maybe I forgot to commit the file).

KristofferC · 2016-12-16T14:01:14Z

Yes @tkelman.

tkelman · 2016-12-27T15:02:52Z

NEWS.md

@@ -60,9 +60,9 @@ Library improvements
    Therefore, light versions of the colors are now supported.
    For the available colors see the help entry on `print_with_color`.

-  * The default color for info messages has been changed from blue to cyan ([#18442]).


we should put this link back (and add 18453 too)

Change default warn color to yellow

b698e53

KristofferC force-pushed the kc/warn_color branch from 0253ede to d8897d2 Compare September 17, 2016 10:24

add news entry

806db05

KristofferC force-pushed the kc/warn_color branch from d8897d2 to 806db05 Compare September 17, 2016 10:25

tkelman mentioned this pull request Oct 31, 2016

make print_with_color not default to bold take 2 #18628

Merged

KristofferC added 5 commits December 16, 2016 09:24

Merge branch 'master' into kc/warn_color

49f56e0

das

6a8a668

foo

88cb352

Update client.jl

442fdff

Update NEWS.md

245abd3

tkelman approved these changes Dec 16, 2016

View reviewed changes

MichaelHatherly reviewed Dec 16, 2016

View reviewed changes

fix typo

35faf2a

Sacha0 approved these changes Dec 16, 2016

View reviewed changes

tkelman merged commit 363ecad into master Dec 17, 2016

tkelman deleted the kc/warn_color branch December 17, 2016 06:45

tkelman reviewed Dec 27, 2016

View reviewed changes

tkelman added the needs news A NEWS entry is required for this change label Dec 31, 2016

tkelman mentioned this pull request Dec 31, 2016

Try make REPL error easier to visually parse #19569

Merged

Sacha0 removed the needs news A NEWS entry is required for this change label May 12, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change default warn color to yellow #18453

Change default warn color to yellow #18453

KristofferC commented Sep 12, 2016 •

edited

Loading

tkelman commented Sep 12, 2016

KristofferC commented Sep 12, 2016 •

edited

Loading

ViralBShah commented Sep 12, 2016

nalimilan commented Sep 12, 2016

TotalVerb commented Sep 12, 2016

StefanKarpinski commented Sep 12, 2016

DNF2 commented Sep 13, 2016

KristofferC commented Sep 13, 2016

ViralBShah commented Sep 13, 2016

DNF2 commented Sep 13, 2016

KristofferC commented Sep 13, 2016

KristofferC commented Sep 13, 2016

KristofferC commented Sep 13, 2016 •

edited

Loading

ViralBShah commented Sep 13, 2016

KristofferC commented Sep 13, 2016

KristofferC commented Sep 17, 2016

kshyatt commented Sep 21, 2016

KristofferC commented Oct 8, 2016

tkelman commented Oct 8, 2016

KristofferC commented Oct 8, 2016

KristofferC commented Dec 16, 2016

tkelman left a comment

MichaelHatherly Dec 16, 2016

KristofferC Dec 16, 2016

KristofferC commented Dec 16, 2016

tkelman Dec 27, 2016

Change default warn color to yellow #18453

Change default warn color to yellow #18453

Conversation

KristofferC commented Sep 12, 2016 • edited Loading

tkelman commented Sep 12, 2016

KristofferC commented Sep 12, 2016 • edited Loading

ViralBShah commented Sep 12, 2016

nalimilan commented Sep 12, 2016

TotalVerb commented Sep 12, 2016

StefanKarpinski commented Sep 12, 2016

DNF2 commented Sep 13, 2016

KristofferC commented Sep 13, 2016

ViralBShah commented Sep 13, 2016

DNF2 commented Sep 13, 2016

KristofferC commented Sep 13, 2016

KristofferC commented Sep 13, 2016

KristofferC commented Sep 13, 2016 • edited Loading

ViralBShah commented Sep 13, 2016

KristofferC commented Sep 13, 2016

KristofferC commented Sep 17, 2016

kshyatt commented Sep 21, 2016

KristofferC commented Oct 8, 2016

tkelman commented Oct 8, 2016

KristofferC commented Oct 8, 2016

KristofferC commented Dec 16, 2016

tkelman left a comment

Choose a reason for hiding this comment

MichaelHatherly Dec 16, 2016

Choose a reason for hiding this comment

KristofferC Dec 16, 2016

Choose a reason for hiding this comment

KristofferC commented Dec 16, 2016

tkelman Dec 27, 2016

Choose a reason for hiding this comment

KristofferC commented Sep 12, 2016 •

edited

Loading

KristofferC commented Sep 12, 2016 •

edited

Loading

KristofferC commented Sep 13, 2016 •

edited

Loading