Refactor stacktrace processing and display pipeline for more clarity and less redundancy #55841
Conversation
|
Could the first entry be removed? Saying that an error starts at |
This PR wouldn't be the spot for a change like that. I'm personally all for removing frames that aren't useful, though formally a stacktrace needs to show where an error was thrown from, and the error function is just a convenience to trigger an error. So it wouldn't be a default even if possible. That said, there are bigger fish to fry when it comes to stacktrace lengths, and movement on that is... slow. |
|
Just a small suggestion for your consideration: one positive aspect of the first of the two current styles is how it is explicit about which lines are repeated; the style proposed in this PR attempts to capture this information, but I find it a bit more ambiguous, perhaps because of the asymetry between the begin and the end markers of the blocks: julia> foo1(100)
ERROR:
Stacktrace:
[1] error()
@ Base ./error.jl:53
┌┌ [2] foo1(n::Int64)
││ @ Main ./REPL[2]:1
││ [3] foo2(n::Int64)
││ @ Main ./REPL[3]:1
│└───── repeated 9 more times
│┌ [22] foo3(n::Int64)
││ @ Main ./REPL[4]:1
││ [23] foo2(n::Int64)
││ @ Main ./REPL[3]:1
│└───── repeated 9 more times
└────── repeated 1 more time
┌ [82] foo1(n::Int64)
│ @ Main ./REPL[2]:1
│ [83] foo2(n::Int64)
│ @ Main ./REPL[3]:1
└────── repeated 9 more times
[102] foo1(n::Int64)
@ Main ./REPL[2]:1
[103] top-level scope
@ REPL[14]:1Perhaps adding a block-ending marker in a similar fashion to the starting one could help? julia> foo1(100)
ERROR:
Stacktrace:
[1] error()
@ Base ./error.jl:53
┌┌ [2] foo1(n::Int64)
││ @ Main ./REPL[2]:1
││ [3] foo2(n::Int64)
│├ @ Main ./REPL[3]:1
│╰───── repeated 9 more times
│┌ [22] foo3(n::Int64)
││ @ Main ./REPL[4]:1
││ [23] foo2(n::Int64)
├├ @ Main ./REPL[3]:1
│╰───── repeated 9 more times
╰────── repeated 1 more time
┌ [82] foo1(n::Int64)
│ @ Main ./REPL[2]:1
│ [83] foo2(n::Int64)
├ @ Main ./REPL[3]:1
╰────── repeated 9 more times
[102] foo1(n::Int64)
@ Main ./REPL[2]:1
[103] top-level scope
@ REPL[14]:1In case this seems too busy, perhaps the block delimiters could be distinguished from the "arrow" pointing to the "repeated x times" line by using a different emphasis, e.g.: julia> foo1(100)
ERROR:
Stacktrace:
[1] error()
@ Base ./error.jl:53
┏┏ [2] foo1(n::Int64)
┃┃ @ Main ./REPL[2]:1
┃┃ [3] foo2(n::Int64)
┃┡ @ Main ./REPL[3]:1
┃╰───── repeated 9 more times
┃┏ [22] foo3(n::Int64)
┃┃ @ Main ./REPL[4]:1
┃┃ [23] foo2(n::Int64)
┡┡ @ Main ./REPL[3]:1
│╰───── repeated 9 more times
╰────── repeated 1 more time
┏ [82] foo1(n::Int64)
┃ @ Main ./REPL[2]:1
┃ [83] foo2(n::Int64)
┡ @ Main ./REPL[3]:1
╰────── repeated 9 more times
[102] foo1(n::Int64)
@ Main ./REPL[2]:1
[103] top-level scope
@ REPL[14]:1WDYT? |
|
Yeah I take your point... Could work! |
But While I sympathise, making a special case for just this seems odd to me. |
|
@waldyrious how do you feel about one of these? Adding the end marker inside complicates things a bit, but maybe the bold region is enough of an indicator? (bold start tick, thin start tick, no start tick) 
|
The current stacktrace display pipeline has 2 different ways to show repetition:
And each of these uses two separate printing paths, leading to other inconsistencies, like sometimes top-level scope is shown and sometimes it isn't, and sometimes the frame counter includes the hidden frames and sometimes it doesn't.
I merged the two pathways and display the cycles more nicely using line characters, and extracted some of the filtering into separate functions. I also added a comment to document the stacktrace processing pipeline.
Keeping as a draft to look at CI results and get feedback.