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

Remove enforcal of printing exceptions as red #36015

Closed
KristofferC opened this issue May 24, 2020 · 5 comments · Fixed by #36024
Closed

Remove enforcal of printing exceptions as red #36015

KristofferC opened this issue May 24, 2020 · 5 comments · Fixed by #36024
Labels
error handling Handling of exceptions by Julia or the user

Comments

@KristofferC
Copy link
Member

The code here

julia/base/errorshow.jl

Lines 87 to 95 in 0413ef0

function showerror(io::IO, ex, bt; backtrace=true)
try
with_output_color(get(io, :color, false) ? error_color() : :nothing, io) do io
showerror(io, ex)
end
finally
backtrace && show_backtrace(io, bt)
end
end

tries its best to ensure that the showerror code an exception implement gets printed in red. This can mess up custom coloring that the exception itself tries to do in showerror. For example, here is some code that defines an exception text using colors:

julia> struct CustomException end

julia> function Base.showerror(io::IO, ce::CustomException)
           printstyled(io, "This is in red\n"; color=:light_red)
           println(io, "This is in white")
       end

Trying with the normal two arg showerror and things look ok. But when the error would actually be shown in REPL, the second line would have "magically" also turned red, ruining the formatting.

bild

If an exception wants to print its text in red, it can just implement that with printstyled.

@KristofferC KristofferC added the error handling Handling of exceptions by Julia or the user label May 24, 2020
@JeffBezanson
Copy link
Member

I guess just printing ERROR: in red is enough to call attention to it, so I'm fine with that.

@kimikage
Copy link
Contributor

Although I don't feel strongly about it, the fully red messages are somewhat useful when the log is slightly long, and the color is no longer helpful when the log is "very" long.

The coloration is auxiliary function, so I don't think designers of exceptions should specify the colors actively. If desired, you can specify the :normal color

function Base.showerror(io::IO, ce::CustomException)
    printstyled(io, "This is in red\n"; color=:light_red)
    printstyled(io, "This is in white(?)\n"; color=:normal)
end

@KristofferC
Copy link
Member Author

The coloration is auxiliary function, so I don't think designers of exceptions should specify the colors actively.

Why not? Colors can be very useful to improve the clarity of error messages.

If desired, you can specify the :normal color

I know how to work around it. I am saying that trying to color the whole exception message in red is just a bad idea because you don't know how the one that made the error message wants to format it.

@kimikage
Copy link
Contributor

kimikage commented May 25, 2020

Why not?

Encouraging the designers to customize the color of error messages spoils the simple rule of "red means error".
Unfortunately, I don't think many users read error messages as carefully as you do. 😅

@KristofferC
Copy link
Member Author

KristofferC commented May 25, 2020

Unfortunately, I don't think many users read error messages as carefully as you do. sweat_smile

That seems like an argument to make them better so that people will start reading them? The context this comes up in was when doing error messages for a TOML parser:

bild

compare that with

bild

Also, remember that the Julia error messages used to look like

bild

c42f added a commit that referenced this issue Jul 10, 2020
The expanded stacktrace printing introduced in #36134 and the fact that
we no longer print errors in red (#36015) makes it harder to distinguish
distinct exceptions in the stack.

Add a newline for this, and print the "caused by" in red. Also remove
exception numbering, which was arguably not very helpful.
c42f added a commit that referenced this issue Jul 14, 2020
The expanded stacktrace printing introduced in #36134 and the fact that
we no longer print errors in red (#36015) makes it harder to distinguish
distinct exceptions in the stack.

Add a newline for this, and print the "caused by" in error_color(). Also
remove exception numbering which was arguably not very helpful.
c42f added a commit that referenced this issue Jul 14, 2020
The expanded stacktrace printing introduced in #36134 and the fact that
we no longer print errors in red (#36015) makes it harder to distinguish
distinct exceptions in the stack.

Add a newline for this, and print the "caused by" in error_color(). Also
remove exception numbering which was arguably not very helpful.
simeonschaub pushed a commit to simeonschaub/julia that referenced this issue Aug 11, 2020
The expanded stacktrace printing introduced in JuliaLang#36134 and the fact that
we no longer print errors in red (JuliaLang#36015) makes it harder to distinguish
distinct exceptions in the stack.

Add a newline for this, and print the "caused by" in error_color(). Also
remove exception numbering which was arguably not very helpful.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error handling Handling of exceptions by Julia or the user
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants