mention methodtable Ctrl+Q trick in methodshow #35556

simeonschaub · 2020-04-22T17:17:24Z

Thanks to @oxinabox for pointing me to this trick on Slack. I think this deserves to be mentioned in the MethodTable output.

simeonschaub · 2020-05-09T11:09:17Z

Tagging @fredrikekre, since I believed you added this feature in #22007. Could I get a review?

tkf · 2020-05-10T03:49:07Z

This is a very nice thing to have for new users. But wouldn't this write the "tips" message into a file if I type open(io -> show(io, methods(identity)), "file", write=true)? I think "UI" message like this should be implemented in the REPL module (e.g., REPL sets some custom IOContext and it'd be used here instead of isinteractive()).

simeonschaub · 2020-05-10T07:54:38Z

Should I check for haskey(io, :module) instead? Otherwise, I don't think the REPL module sets any custom IOContexts.

tkf · 2020-05-10T22:40:01Z

I don't think :module context is adequate for this. Why not add a new IOContext like this?

diff --git a/stdlib/REPL/src/REPL.jl b/stdlib/REPL/src/REPL.jl
index 30af00a6cd..be3e14538d 100644
--- a/stdlib/REPL/src/REPL.jl
+++ b/stdlib/REPL/src/REPL.jl
@@ -206,6 +206,7 @@ end
 function display(d::REPLDisplay, mime::MIME"text/plain", x)
     io = outstream(d.repl)
     get(io, :color, false) && write(io, answer_color(d.repl))
+    io = IOContext(io, :interactive => true)
     if isdefined(d.repl, :options) && isdefined(d.repl.options, :iocontext)
         # this can override the :limit property set initially
         io = foldl(IOContext, d.repl.options.iocontext,

tkf · 2020-05-11T22:32:32Z

If we go with this direction, should we add :interactive to this list?

julia/base/show.jl

Lines 266 to 283 in 65869b1

    
            - `:compact`: Boolean specifying that small values should be printed more compactly, e.g. 
        
              that numbers should be printed with fewer digits. This is set when printing array 
        
              elements. 
        
            - `:limit`: Boolean specifying that containers should be truncated, e.g. showing `…` in 
        
              place of most elements. 
        
            - `:displaysize`: A `Tuple{Int,Int}` giving the size in rows and columns to use for text 
        
              output. This can be used to override the display size for called functions, but to 
        
              get the size of the screen use the `displaysize` function. 
        
            - `:typeinfo`: a `Type` characterizing the information already printed 
        
              concerning the type of the object about to be displayed. This is mainly useful when 
        
              displaying a collection of objects of the same type, so that redundant type information 
        
              can be avoided (e.g. `[Float16(0)]` can be shown as "Float16[0.0]" instead 
        
              of "Float16[Float16(0.0)]" : while displaying the elements of the array, the `:typeinfo` 
        
              property will be set to `Float16`). 
        
            - `:color`: Boolean specifying whether ANSI color/escape codes are supported/expected. 
        
              By default, this is determined by whether `io` is a compatible terminal and by any 
        
              `--color` command-line flag when `julia` was launched.

simeonschaub · 2020-05-11T22:40:51Z

Yes, I think that would be helpful. Should this all go into this PR or would it make more sense to have a dedicated PR for this?

tkf · 2020-05-11T22:45:40Z

This patch introduces a new convention in REPL.jl so I think it makes sense to add the documentation here.

simeonschaub · 2020-05-12T14:52:21Z

Perhaps REPL or isREPL is a better name for this, since this trick doesn't work in Jupyter Notebooks, for example, but there isinteractive() still returns true?

tkf · 2020-05-12T23:20:14Z

Ah, that's a very good point. Yes, :interactive context is useless for this.

I wonder if we should just directly overload display(d::REPLDisplay, mime::MIME"text/plain", x) or add some overloadable hint mechanism.

A bit hacky solution may be something like

function display(d::REPLDisplay, mime::MIME"text/plain", x)
    backup = copy(Base.LAST_SHOWN_LINE_INFOS)
    empty!(Base.LAST_SHOWN_LINE_INFOS)

    ...

    if isempty(Base.LAST_SHOWN_LINE_INFOS)
        append!(Base.LAST_SHOWN_LINE_INFOS, backup)
    else
        print(io, the_hint_message)
    end
end

tkf · 2020-05-12T23:41:43Z

Hmm... Actually, maybe LAST_SHOWN_LINE_INFOS itself should be in IOContext? It's not nice to communicate via a mutating global state. I think it'd be better to do something like

infos = []
io = IOContext(io, :LAST_SHOWN_LINE_INFOS => infos)
show(io, "text/plain", x)
d.repl.last_shown_line_infos = infos

in display.

simeonschaub · 2020-05-15T07:20:02Z

I don't really get, why we would need Base.LAST_SHOWN_LINE_INFOS here. Does this tell us anything about whether the output is a REPL? Is there anything wrong with just using the approach here and renaming :interactive to :REPL?

tkf · 2020-05-15T20:03:24Z

Does this tell us anything about whether the output is a REPL?

No, it doesn't. My suggestion was to do it the other way around. If we implement this in display(d::REPLDisplay, mime::MIME"text/plain", x), we know that the output is the REPL because of d::REPLDisplay. However, now we need to know if the method table is printed or not. If we do empty!(Base.LAST_SHOWN_LINE_INFOS) before calling show and then if it becomes non-empty after that, we know that a method table is printed.

(However, this is correct only if there is only one Task. It's much better to do this without the global variable Base.LAST_SHOWN_LINE_INFOS now that threading (or even @async) becomes more common. So that's why I'm mentioning that using IOContext is even better.)

I think this is the right way to do it. As you pointed out, this is very specific to REPL. So, I think REPL is the best place to implement it.

simeonschaub · 2020-05-25T21:51:15Z

@tkf I got the same error locally, I just wanted to see whether it was just on my machine. Do you have any idea why it would fail? I don't really know where to start looking into this.

stdlib/REPL/src/REPL.jl

tkf

Isn't it simpler to always use io = IOContext(io, :LAST_SHOWN_LINE_INFOS => infos)?

Also, current code seems to not update d.repl.last_shown_line_infos after the second time?

stdlib/REPL/src/REPL.jl

simeonschaub · 2020-05-26T21:09:55Z

Yes, that makes sense. Originally, I had structured it a bit differently, but doesn't really matter either way.

Co-authored-by: Takafumi Arakaki <takafumi.a@gmail.com>

stdlib/REPL/src/REPL.jl

Co-authored-by: Takafumi Arakaki <takafumi.a@gmail.com>

simeonschaub · 2020-06-03T20:27:20Z

Bump. Is this good to go?

tkf · 2020-06-03T20:39:38Z

It's is rather a radical change in the "philosophy" of how display works on REPL. So it'd be nice if other devs can have a look at it.

ping @StefanKarpinski

StefanKarpinski · 2020-06-04T16:30:34Z

I don't think I'm the person for this particular review. @JeffBezanson, @stevengj and @vtjnash have more of a history and familiarity with the display system.

vtjnash

Ah, this is rather clever! It's unusual, but I can't see any reason that'd be a problem.

vtjnash · 2020-06-04T18:22:36Z

base/errorshow.jl

+    if haskey(io, :LAST_SHOWN_LINE_INFOS)
+        resize!(io[:LAST_SHOWN_LINE_INFOS], 0)
+    end


Suggested change

if haskey(io, :LAST_SHOWN_LINE_INFOS)

resize!(io[:LAST_SHOWN_LINE_INFOS], 0)

end

Is it OK to skip resize! here? What happens when multiple stack traces are shown (via nested errors)? Would IOContext be reset appropriately?

vtjnash · 2020-06-04T18:22:51Z

base/methodshow.jl

@@ -373,6 +374,7 @@ show(io::IO, mime::MIME"text/html", mt::Core.MethodTable) = show(io, mime, Metho

 # pretty-printing of AbstractVector{Method}
 function show(io::IO, mime::MIME"text/plain", mt::AbstractVector{Method})
+    LAST_SHOWN_LINE_INFOS = get(io, :LAST_SHOWN_LINE_INFOS, Tuple{String,Int}[])
    resize!(LAST_SHOWN_LINE_INFOS, 0)


Suggested change

resize!(LAST_SHOWN_LINE_INFOS, 0)

Don't you need resize! for something like this?

struct TwoMethodTables methods1 methods2 end function Base.show(io::IO, ::MIME"text/plain", two::TwoMethodTables) println(io, "TwoMethodTables:") show(io, MIME"text/plain"(), two.methods1) show(io, MIME"text/plain"(), two.methods2) end

Currently, I think Ctrl+Q works for numbering in methods2. If we remove resize!, it'd refer to the numbering in methods1 (and also in methods2 shifted by length(methods)).

vtjnash · 2020-06-04T18:22:58Z

base/methodshow.jl

@@ -240,6 +240,7 @@ function show_method_table(io::IO, ms::MethodList, max::Int=-1, header::Bool=tru
    end
    n = rest = 0
    local last
+    LAST_SHOWN_LINE_INFOS = get(io, :LAST_SHOWN_LINE_INFOS, Tuple{String,Int}[])

    resize!(LAST_SHOWN_LINE_INFOS, 0)


Suggested change

resize!(LAST_SHOWN_LINE_INFOS, 0)

stdlib/REPL/src/REPL.jl

Co-authored-by: Jameson Nash <vtjnash@gmail.com>

simeonschaub · 2020-06-13T18:55:24Z

Bumping this again. I thought the resize! was still necessary too, so printing multiple stack traces still works correctly.

tkf · 2020-06-14T17:35:06Z

I merged this as @vtjnash is OK with it at a high-level picture. I'm keeping resize! since both @simeonschaub and I think that's necessary.

@vtjnash Let us know if the code around resize! still has to be fixed.

It seems to be better to merge now so that there is more chance to play with the UI in the 1.6-DEV cycle. It's also better to merge this before #36134.

(The test failures look like some network glitches.)

Co-authored-by: Takafumi Arakaki <takafumi.a@gmail.com> Co-authored-by: Jameson Nash <vtjnash@gmail.com>

simeonschaub force-pushed the repl_trick_for_methodtable branch from c9b0be8 to e407c53 Compare April 22, 2020 17:20

simeonschaub force-pushed the repl_trick_for_methodtable branch from e407c53 to 4dd8acd Compare May 11, 2020 21:58

tkf added display and printing Aesthetics and correctness of printed representations of objects. REPL Julia's REPL (Read Eval Print Loop) labels May 11, 2020

tkf requested a review from KristofferC May 11, 2020 22:31

simeonschaub added 3 commits May 25, 2020 20:36

mention methodtable Ctrl+Q trick in methodshow

716f9d8

set custom interactive option when in REPL

3971667

implement @tkf's suggestion

2a32e13

simeonschaub force-pushed the repl_trick_for_methodtable branch from 4dd8acd to 2a32e13 Compare May 25, 2020 21:35

simeonschaub commented May 25, 2020

View reviewed changes

stdlib/REPL/src/REPL.jl Outdated Show resolved Hide resolved

tkf reviewed May 26, 2020

View reviewed changes

stdlib/REPL/src/REPL.jl Show resolved Hide resolved

fix build failure

d4269ff

tkf reviewed May 26, 2020

View reviewed changes

stdlib/REPL/src/REPL.jl Outdated Show resolved Hide resolved

stdlib/REPL/src/REPL.jl Show resolved Hide resolved

Apply suggestions from code review

19d57b3

Co-authored-by: Takafumi Arakaki <takafumi.a@gmail.com>

tkf reviewed May 26, 2020

View reviewed changes

stdlib/REPL/src/REPL.jl Outdated Show resolved Hide resolved

Update stdlib/REPL/src/REPL.jl

0ca2803

Co-authored-by: Takafumi Arakaki <takafumi.a@gmail.com>

tkf approved these changes May 26, 2020

View reviewed changes

tkf requested a review from StefanKarpinski June 3, 2020 20:38

tkf requested review from JeffBezanson, stevengj and vtjnash and removed request for StefanKarpinski and KristofferC June 4, 2020 16:42

vtjnash approved these changes Jun 4, 2020

View reviewed changes

Update stdlib/REPL/src/REPL.jl

c3dd3d4

Co-authored-by: Jameson Nash <vtjnash@gmail.com>

simeonschaub mentioned this pull request Jun 13, 2020

include ClearStacktrace.jl in Base #36134

Merged

tkf merged commit 13b07fc into JuliaLang:master Jun 14, 2020

tkf mentioned this pull request Jun 18, 2020

Ctrl+Q stopped working for stack traces etc. #36340

Closed

tkf mentioned this pull request Jul 1, 2020

Fix Ctrl+Q for stack traces #36344

Merged

simeonschaub added a commit to simeonschaub/julia that referenced this pull request Aug 11, 2020

mention methodtable Ctrl+Q trick in methodshow (JuliaLang#35556)

cfb3e10

Co-authored-by: Takafumi Arakaki <takafumi.a@gmail.com> Co-authored-by: Jameson Nash <vtjnash@gmail.com>

NHDaly mentioned this pull request Dec 20, 2020

The new stacktrace printing is missing "repeated N times" info for some StackOverflows #37587

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mention methodtable Ctrl+Q trick in methodshow #35556

mention methodtable Ctrl+Q trick in methodshow #35556

simeonschaub commented Apr 22, 2020 •

edited

Loading

simeonschaub commented May 9, 2020

tkf commented May 10, 2020

simeonschaub commented May 10, 2020

tkf commented May 10, 2020

tkf commented May 11, 2020

simeonschaub commented May 11, 2020

tkf commented May 11, 2020

simeonschaub commented May 12, 2020

tkf commented May 12, 2020 •

edited

Loading

tkf commented May 12, 2020

simeonschaub commented May 15, 2020

tkf commented May 15, 2020

simeonschaub commented May 25, 2020

tkf left a comment

simeonschaub commented May 26, 2020

simeonschaub commented Jun 3, 2020

tkf commented Jun 3, 2020

StefanKarpinski commented Jun 4, 2020

vtjnash left a comment

vtjnash Jun 4, 2020

tkf Jun 4, 2020

vtjnash Jun 4, 2020

tkf Jun 4, 2020

vtjnash Jun 4, 2020

simeonschaub commented Jun 13, 2020

tkf commented Jun 14, 2020

	if haskey(io, :LAST_SHOWN_LINE_INFOS)
	resize!(io[:LAST_SHOWN_LINE_INFOS], 0)
	end

mention methodtable Ctrl+Q trick in methodshow #35556

mention methodtable Ctrl+Q trick in methodshow #35556

Conversation

simeonschaub commented Apr 22, 2020 • edited Loading

simeonschaub commented May 9, 2020

tkf commented May 10, 2020

simeonschaub commented May 10, 2020

tkf commented May 10, 2020

tkf commented May 11, 2020

simeonschaub commented May 11, 2020

tkf commented May 11, 2020

simeonschaub commented May 12, 2020

tkf commented May 12, 2020 • edited Loading

tkf commented May 12, 2020

simeonschaub commented May 15, 2020

tkf commented May 15, 2020

simeonschaub commented May 25, 2020

tkf left a comment

Choose a reason for hiding this comment

simeonschaub commented May 26, 2020

simeonschaub commented Jun 3, 2020

tkf commented Jun 3, 2020

StefanKarpinski commented Jun 4, 2020

vtjnash left a comment

Choose a reason for hiding this comment

vtjnash Jun 4, 2020

Choose a reason for hiding this comment

tkf Jun 4, 2020

Choose a reason for hiding this comment

vtjnash Jun 4, 2020

Choose a reason for hiding this comment

tkf Jun 4, 2020

Choose a reason for hiding this comment

vtjnash Jun 4, 2020

Choose a reason for hiding this comment

simeonschaub commented Jun 13, 2020

tkf commented Jun 14, 2020

simeonschaub commented Apr 22, 2020 •

edited

Loading

tkf commented May 12, 2020 •

edited

Loading