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

RFC: print error messages in REPL with colors and structure #18228

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions base/REPL.jl
Original file line number Diff line number Diff line change
Expand Up @@ -111,15 +111,13 @@ function ip_matches_func(ip, func::Symbol)
end

function display_error(io::IO, er, bt)
Base.with_output_color(:red, io) do io
print(io, "ERROR: ")
# remove REPL-related frames from interactive printing
eval_ind = findlast(addr->ip_matches_func(addr, :eval), bt)
if eval_ind != 0
bt = bt[1:eval_ind-1]
end
Base.showerror(IOContext(io, :limit => true), er, bt)
print_with_color(:red, io, "ERROR: ")
# remove REPL-related frames from interactive printing
eval_ind = findlast(addr->Base.REPL.ip_matches_func(addr, :eval), bt)
if eval_ind != 0
bt = bt[1:eval_ind-1]
end
showerror(IOContext(IOContext(io, :hascolor => true), :limit => true), er, bt)
end

immutable REPLDisplay{R<:AbstractREPL} <: Display
Expand Down Expand Up @@ -167,6 +165,8 @@ function print_response(errio::IO, val::ANY, bt, show_value::Bool, have_color::B
catch err
if bt !== nothing
println(errio, "SYSTEM: show(lasterr) caused an error")
println(errio, err)
Base.show_backtrace(errio, bt)
break
end
val = err
Expand Down
4 changes: 4 additions & 0 deletions base/client.jl
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const text_colors = AnyDict(
:white => "\033[1m\033[37m",
:normal => "\033[0m",
:bold => "\033[1m",
:nothing => "",
)

for i in 0:255
Expand All @@ -35,6 +36,7 @@ const available_text_colors_docstring =
"""Dictionary of color codes for the terminal.

Available colors are: $available_text_colors_docstring as well as the integers 0 to 255 inclusive.
Printing with the color `:nothing` will print the string without modifications.
"""
text_colors

Expand All @@ -61,6 +63,8 @@ warn_color() = repl_color("JULIA_WARN_COLOR", default_color_warn)
info_color() = repl_color("JULIA_INFO_COLOR", default_color_info)
input_color() = text_colors[repl_color("JULIA_INPUT_COLOR", default_color_input)]
answer_color() = text_colors[repl_color("JULIA_ANSWER_COLOR", default_color_answer)]
stackframe_lineinfo_color() = repl_color("JULIA_STACKFRAME_LINEINFO_COLOR", :bold)
stackframe_function_color() = repl_color("JULIA_STACKFRAME_FUNCTION_COLOR", :bold)

function repl_cmd(cmd, out)
shell = shell_split(get(ENV,"JULIA_SHELL",get(ENV,"SHELL","/bin/sh")))
Expand Down
23 changes: 17 additions & 6 deletions base/replutil.jl
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,13 @@ end

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

Expand Down Expand Up @@ -559,15 +563,22 @@ function show_method_candidates(io::IO, ex::MethodError, kwargs::Vector=Any[])
end
end

function show_trace_entry(io, frame, n)
function show_trace_entry(io, frame, n; prefix = " in ")
print(io, "\n")
show(io, frame, full_path=true)
show(io, frame, full_path=true; prefix = prefix)
n > 1 && print(io, " (repeats ", n, " times)")
end

function show_backtrace(io::IO, t::Vector)
process_entry(last_frame, n) =
show_trace_entry(io, last_frame, n)
n_frames = 0
frame_counter = 0
process_backtrace((a,b) -> n_frames += 1, t)
n_frames != 0 && print(io, "\n\nStacktrace:")
process_entry = (last_frame, n) -> begin
frame_counter += 1
n_spaces_align = ndigits(n_frames) - ndigits(frame_counter) + 1
show_trace_entry(io, last_frame, n, prefix = string(" "^n_spaces_align, "[", frame_counter, "] "))
end
process_backtrace(process_entry, t)
end

Expand Down
42 changes: 24 additions & 18 deletions base/show.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1030,31 +1030,37 @@ end

function show_lambda_types(io::IO, li::Core.MethodInstance)
# print a method signature tuple for a lambda definition
if li.specTypes === Tuple
print(io, li.def.name, "(...)")
return
end

sig = li.specTypes.parameters
ft = sig[1]
if ft <: Function && isempty(ft.parameters) &&
isdefined(ft.name.module, ft.name.mt.name) &&
ft == typeof(getfield(ft.name.module, ft.name.mt.name))
print(io, ft.name.mt.name)
elseif isa(ft, DataType) && ft.name === Type.name && isleaftype(ft)
f = ft.parameters[1]
print(io, f)
else
print(io, "(::", ft, ")")
local sig
returned_from_do = false
Base.with_output_color(get(io, :hascolor, false) ? stackframe_function_color() : :nothing, io) do io
if li.specTypes === Tuple
print(io, li.def.name, "(...)")
returned_from_do = true
return
end
sig = li.specTypes.parameters
ft = sig[1]
if ft <: Function && isempty(ft.parameters) &&
isdefined(ft.name.module, ft.name.mt.name) &&
ft == typeof(getfield(ft.name.module, ft.name.mt.name))
print(io, ft.name.mt.name)
elseif isa(ft, DataType) && ft.name === Type.name && isleaftype(ft)
f = ft.parameters[1]
print(io, f)
else
print(io, "(::", ft, ")")
end
end
returned_from_do && return
first = true
print(io, '(')
print_style = get(io, :hascolor, false) ? :bold : :nothing
print_with_color(print_style, io, "(")
for i = 2:length(sig) # fixme (iter): `eachindex` with offset?
first || print(io, ", ")
first = false
print(io, "::", sig[i])
end
print(io, ')')
print_with_color(print_style, io, ")")
nothing
end

Expand Down
20 changes: 12 additions & 8 deletions base/stacktraces.jl
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ function show_spec_linfo(io::IO, frame::StackFrame)
if frame.func === empty_sym
@printf(io, "ip:%#x", frame.pointer)
else
print(io, frame.func)
print_with_color(get(io, :hascolor, false) ? Base.stackframe_function_color() : :nothing, io, string(frame.func))
end
else
linfo = get(frame.linfo)
Expand All @@ -200,16 +200,20 @@ function show_spec_linfo(io::IO, frame::StackFrame)
end
end

function show(io::IO, frame::StackFrame; full_path::Bool=false)
print(io, " in ")
function show(io::IO, frame::StackFrame; full_path::Bool=false,
prefix = " in ")
print(io, prefix)
show_spec_linfo(io, frame)
if frame.file !== empty_sym
file_info = full_path ? string(frame.file) : basename(string(frame.file))
print(io, " at ", file_info, ":")
if frame.line >= 0
print(io, frame.line)
else
print(io, "?")
print(io, " at ")
Base.with_output_color(get(io, :hascolor, false) ? Base.stackframe_lineinfo_color() : :nothing, io) do io
print(io, file_info, ":")
if frame.line >= 0
print(io, frame.line)
else
print(io, "?")
end
end
end
if frame.inlined
Expand Down
2 changes: 1 addition & 1 deletion base/util.jl
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ function with_output_color(f::Function, color::Union{Int, Symbol}, io::IO, args.
have_color && print(buf, get(text_colors, color, color_normal))
try f(IOContext(buf, io), args...)
finally
have_color && print(buf, color_normal)
have_color && color != :nothing && print(buf, color_normal)
print(io, String(take!(buf)))
end
end
Expand Down
6 changes: 3 additions & 3 deletions test/cmdlineargs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -328,12 +328,12 @@ end
for precomp in ("yes", "no")
bt = readstring(pipeline(ignorestatus(`$(Base.julia_cmd()) --startup-file=no --precompiled=$precomp
-E 'include("____nonexistent_file")'`), stderr=catcmd))
@test contains(bt, "in include_from_node1")
@test contains(bt, "include_from_node1")
if is_windows() && Sys.WORD_SIZE == 32 && precomp == "yes"
# fixme, issue #17251
@test_broken contains(bt, "in include_from_node1(::String) at $(joinpath(".","loading.jl"))")
@test_broken contains(bt, "include_from_node1(::String) at $(joinpath(".","loading.jl"))")
else
@test contains(bt, "in include_from_node1(::String) at $(joinpath(".","loading.jl"))")
@test contains(bt, "include_from_node1(::String) at $(joinpath(".","loading.jl"))")
end
lno = match(r"at \.[\/\\]loading\.jl:(\d+)", bt)
@test length(lno.captures) == 1
Expand Down
4 changes: 2 additions & 2 deletions test/compile.jl
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ try
end
""")

t = redirected_stderr("ERROR: LoadError: Declaring __precompile__(false) is not allowed in files that are being precompiled.\n in __precompile__")
t = redirected_stderr("ERROR: LoadError: Declaring __precompile__(false) is not allowed in files that are being precompiled.\n\nStacktrace:\n [1] __precompile__")
try
Base.compilecache("Baz") # from __precompile__(false)
error("__precompile__ disabled test failed")
Expand Down Expand Up @@ -306,7 +306,7 @@ try
error("break me")
end
""")
t = redirected_stderr("ERROR: LoadError: break me\n in error")
t = redirected_stderr("ERROR: LoadError: break me\n\nStacktrace:\n [1] error")
try
Base.require(:FooBar)
error("\"LoadError: break me\" test failed")
Expand Down