make print_with_color not default to bold take 2 #18628

KristofferC · 2016-09-22T11:54:20Z

This is take 2 of #18480.

First commit introduces an ENV var for changing the color errors are printed in.
Second commit remove the default boldness in print_with_color. Instead a kwarg bold is added which determines if the result should be printed in bold. For backwards compatability, the :bold key is still left in the text_colors dict. This also sets the default color for errors and warnings to light_red instead of red because on most terminals this is the color it used to be when printing red + bold. This is also the reason for the first commit because for example Jupyter notebook does not support the light versions of the color so IJulia can if it wants set the ENV variable to just be red. Edit: However, 16 color support in notebook is merged, just not in release: Re-factor ANSI color handling jupyter/notebook#1230
Third commit hooks up the test framework to use corresponding ENV variables in a few places. The reason for this is that the blue used is not very visible on many terminals (same problem as with the previous info messages) and it is good to keep the users chosen "colorscheme".

Note, this is not meant to change any input color / boldness, output color / boldness, repl color / boldness etc and not change any of the possible settings that are available now.

KristofferC · 2016-09-22T12:05:06Z

test/misc.jl

+let
+    old_have_color = Base.have_color
+    try
+        eval(Base, :(have_color = true))


Is this acceptable?

isn't something similar done in a withenv in a different test?

this was fine for now, better to test things. we can think about making color not a global elsewhere.

hopefully can restore these from reflog or old commits?

tkelman · 2016-09-23T04:37:04Z

base/client.jl

+
+# Print input and answer in bold.
+input_color()  = text_colors[:bold] * text_colors[repl_color("JULIA_INPUT_COLOR", default_color_input)]
+answer_color() = text_colors[:bold] * text_colors[repl_color("JULIA_ANSWER_COLOR", default_color_answer)]


should we consider moving these functions to the REPL module instead of Base ?

Perhaps. On the other hand it makes sense to have all the ENV color stuff in the same place.

how much of it is more general-purpose beyond REPL usage?

IJulia uses colors for infos, errors and warnings now and thus indirectly uses the corresponding ENV vars.

tkelman · 2016-09-23T10:12:47Z

NEWS.md

+    a keyword argument `bold::Bool` which determines whether to print in bold or not.
+    On some terminals, printing a color in non bold results in slightly darker colors being printed than when printing in bold.
+    Therefore, light versions of the colors are now supported.
+    For the available colors see the help entry on `print_with_color.


missing closing backtick

still missing

tkelman · 2016-09-23T10:15:30Z

base/client.jl

+    :magenta       => "\033[35m",
+    :cyan          => "\033[36m",
+    :white         => "\033[37m",
+    :light_black   => "\033[90m",


how does that work?

I could either call it gray or keep the light_COLOR pattern like I did here.

I don't think I've ever heard gray called light black, but I guess it works. throw a comment in?

tkelman · 2016-09-23T10:18:31Z

base/test.jl

 end
 function Base.show(io::IO, t::Broken)
-    print_with_color(:yellow, io, "Test Broken\n")
+    print_with_color(:yellow, io, "Test Broken\n"; bold = true)


the other PR that changes warnings to yellow should think about making these use the configurable warning color

was that merged before this? lost track

answering my own question: not yet #18453

tkelman · 2016-09-23T10:21:42Z

doc/manual/interacting-with-julia.rst

-You can also customize the color used to render warning and informational messages by
+You can also customize the color used to render error, warning and informational messages by
 setting the appropriate environment variable. For instance, to render warning messages in yellow and
 informational messages in cyan you can add the following to your ``juliarc.jl`` file::


... and error messages in magenta

StefanKarpinski · 2016-10-27T17:14:16Z

This seems to just need a quick rebase and merge.

tkelman · 2016-10-28T15:37:12Z

test/misc.jl

-    for (n,crc) in [(0,0x00000000),(1,0xa016d052),(2,0x03f89f52),(3,0xf130f21e),(4,0x29308cf4),(5,0x53518fab),(6,0x4f4dfbab),(7,0xbd3a64dc),(8,0x46891f81),(9,0x5a14b9f9),(10,0xb219db69),(11,0xd232a91f),(12,0x51a15563),(13,0x9f92de41),(14,0x4d8ae017),(15,0xc8b74611),(16,0xa0de6714),(17,0x672c992a),(18,0xe8206eb6),(19,0xc52fd285),(20,0x327b0397),(21,0x318263dd),(22,0x08485ccd),(23,0xea44d29e),(24,0xf6c0cb13),(25,0x3969bba2),(26,0x6a8810ec),(27,0x75b3d0df),(28,0x82d535b1),(29,0xbdf7fc12),(30,0x1f836b7d),(31,0xd29f33af),(32,0x8e4acb3e),(33,0x1cbee2d1),(34,0xb25f7132),(35,0xb0fa484c),(36,0xb9d262b4),(37,0x3207fe27),(38,0xa024d7ac),(39,0x49a2e7c5),(40,0x0e2c157f),(41,0x25f7427f),(42,0x368c6adc),(43,0x75efd4a5),(44,0xa84c5c31),(45,0x0fc817b2),(46,0x8d99a881),(47,0x5cc3c078),(48,0x9983d5e2),(49,0x9267c2db),(50,0xc96d4745),(51,0x058d8df3),(52,0x453f9cf3),(53,0xb714ade1),(54,0x55d3c2bc),(55,0x495710d0),(56,0x3bddf494),(57,0x4f2577d0),(58,0xdae0f604),(59,0x3c57c632),(60,0xfe39bbb0),(61,0x6f5d1d41),(62,0x7d996665),(63,0x68c738dc),(64,0x8dfea7ae)]
-        @test Base.crc32c(UInt8[1:n;]) == crc
-    end
-end


yeah, git lied to me :P

https://www.youtube.com/watch?v=-4rFl43xG4k&feature=youtu.be&t=1m19s

made minor edits to address my own review

tkelman · 2016-11-01T18:46:41Z

wasn't this previously adding some new tests at the end of test/misc.jl? @KristofferC did you intend to remove those, or was it a rebase issue again?

KristofferC · 2016-12-10T17:02:08Z

Rebased and added test

tkelman · 2016-12-10T18:18:00Z

Need to convert the doc additions to markdown

KristofferC · 2016-12-10T19:26:58Z

Thanks, forgot about that.

tkelman · 2016-12-10T19:31:56Z

doc/src/manual/interacting-with-julia.md

-The available color keys in `Base.text_colors` are `:black`, `:red`, `:green`, `:yellow`, `:blue`,
-`:magenta`, `:cyan`, `:white`, `:normal`, and `:bold` as well as the integers 0 to 255 for terminals
-with 256 color support. Similarly, you can change the colors for the help and shell prompts and
+The available color keys can be seen by typing ``Base.text_colors`` in the help mode of the REPL.


single backticks in markdown

tkelman · 2016-12-10T19:32:25Z

doc/src/manual/interacting-with-julia.md

 ```julia
 ENV["JULIA_WARN_COLOR"] = :yellow
 ENV["JULIA_INFO_COLOR"] = :cyan
+ENV["JULIA_ERROR_COLOR"] = :magenta


this undid 5a9b1d7

KristofferC · 2016-12-10T19:37:02Z

Apologies, hopefully fixed.

KristofferC · 2016-12-11T12:05:23Z

Squashed a bit and rebased.

tkelman · 2016-12-11T13:06:04Z

base/client.jl

-default_color_warn = :red
-default_color_error = :red
+default_color_warn = :light_red
+default_color_error = :light_red


did the pr always include this change?

Yes, see the second bullet in the first post for this PR.

tkelman · 2016-12-11T13:11:41Z

base/show.jl

 end

-emphasize(io, str::AbstractString) = have_color ? print_with_color(:red, io, str) : print(io, uppercase(str))
+emphasize(io, str::AbstractString) = have_color ? print_with_color(Base.error_color(), io, str; bold = true) : print(io, uppercase(str))


the bold kwarg doesn't exist yet if this is the first commit, right?

You are right. I moved it to the second commit.

…ia#18628

…ia#18628 (#293)

…ia#18628 (JuliaLang#293)

KristofferC added this to the 0.6.0 milestone Sep 22, 2016

KristofferC added the REPL Julia's REPL (Read Eval Print Loop) label Sep 22, 2016

KristofferC commented Sep 22, 2016

View reviewed changes

tkelman reviewed Sep 23, 2016

View reviewed changes

vtjnash assigned tkelman Oct 27, 2016

KristofferC force-pushed the kc/little_less_bold_2 branch 3 times, most recently from 4088cd8 to e16e7c1 Compare October 28, 2016 15:29

tkelman previously requested changes Oct 28, 2016

View reviewed changes

KristofferC force-pushed the kc/little_less_bold_2 branch from e16e7c1 to 9ecb48a Compare October 28, 2016 16:02

tkelman added the needs tests Unit tests are required for this change label Nov 5, 2016

KristofferC force-pushed the kc/little_less_bold_2 branch from 2cb5cc8 to 9329ab7 Compare December 10, 2016 17:01

tkelman removed the needs tests Unit tests are required for this change label Dec 10, 2016

tkelman reviewed Dec 10, 2016

View reviewed changes

KristofferC force-pushed the kc/little_less_bold_2 branch from 866edcd to 6977a6d Compare December 10, 2016 19:36

KristofferC force-pushed the kc/little_less_bold_2 branch 2 times, most recently from f12b7d9 to 6c8585d Compare December 11, 2016 12:04

tkelman reviewed Dec 11, 2016

View reviewed changes

introduce JULIA_ERROR_COLOR to control how errors are printed

2fc856d

KristofferC added 2 commits December 11, 2016 18:15

no longer print bold by default in print_with_color

c247d3f

hook up test macro to ENV color settings

6d6265b

KristofferC force-pushed the kc/little_less_bold_2 branch from 6c8585d to 6d6265b Compare December 11, 2016 17:17

tkelman merged commit 466f754 into master Dec 15, 2016

tkelman deleted the kc/little_less_bold_2 branch December 15, 2016 17:54

stevengj mentioned this pull request Dec 22, 2016

with_output_color writes control codes to non-terminal streams #19682

Closed

stevengj added a commit to JuliaLang/Compat.jl that referenced this pull request Dec 22, 2016

fix test script for changes in warn() escape codes from JuliaLang/jul…

0a80ef3

…ia#18628

stevengj mentioned this pull request Dec 22, 2016

fix test script for changes in warn() escape codes JuliaLang/Compat.jl#293

Merged

tkelman pushed a commit to JuliaLang/Compat.jl that referenced this pull request Dec 22, 2016

fix test script for changes in warn() escape codes from JuliaLang/jul…

202d41c

…ia#18628 (#293)

dpsanders pushed a commit to dpsanders/Compat.jl that referenced this pull request Feb 1, 2017

fix test script for changes in warn() escape codes from JuliaLang/jul…

1457225

…ia#18628 (JuliaLang#293)

apieum mentioned this pull request Nov 9, 2017

[LoadError] function print_with_color does not accept keyword arguments JuliaLang/Compat.jl#410

Closed

Uh oh!

make print_with_color not default to bold take 2 #18628

make print_with_color not default to bold take 2 #18628

Uh oh!

Conversation

KristofferC commented Sep 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KristofferC Sep 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

StefanKarpinski commented Oct 27, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tkelman commented Nov 1, 2016

Uh oh!

KristofferC commented Dec 10, 2016

Uh oh!

tkelman commented Dec 10, 2016

Uh oh!

KristofferC commented Dec 10, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KristofferC commented Dec 10, 2016

Uh oh!

KristofferC commented Dec 11, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

KristofferC commented Sep 22, 2016 •

edited

Loading

KristofferC Sep 22, 2016 •

edited

Loading

KristofferC commented Dec 10, 2016 •

edited

Loading