Fix Ctrl+Q for stack traces #36344

simeonschaub · 2020-06-18T15:02:31Z

This should be a more general fix than #36341. It currently silently gets stuck while generating precompile statements on my machine, but not sure if that's reproducible.

close #36340

tkf · 2020-06-18T15:40:02Z

Maybe try debug_output = stdout?

julia/contrib/generate_precompile.jl

Line 69 in bf3ed57

debug_output = devnull # or stdout

stdlib/REPL/src/REPL.jl

simeonschaub · 2020-06-18T16:53:17Z

@tkf What do you think should be done about the helpmode case? The relevant code is in docview.jl, but the problem is that io is not passed through as an argument and some of those functions are documented, so I don't know if we can just easily change them.

tkf · 2020-06-18T17:01:39Z

I guess we need #34949 to fix help mode? This PR as-is is still a big usability improvement/fix so I think we should just merge this.

Do you think it's possible to write a few tests for this?

simeonschaub · 2020-06-18T17:09:35Z

It's probably possible to write a test here, but I'm not really familiar with how REPLTests.fake_repl works, so I would appreciate any pointers from people who have written those existing REPL tests. Last time I looked into it, I didn't get very far.

tkf · 2020-06-18T22:59:45Z

I think it'd be easier to unit-test this than doing an integration test. It's a bit cumbersome to setup a REPL instance, though:

julia> term = REPL.TTYTerminal("dumb",IOBuffer(),IOBuffer(),IOBuffer());

julia> repl = REPL.LineEditREPL(term, false);

julia> repl.specialdisplay = REPL.REPLDisplay(repl);

julia> REPL.print_response(repl, (:hello, false), true, false)

julia> String(take!(term.out_stream))
":hello\n"

simeonschaub · 2020-06-19T12:14:22Z

I guess we need #34949 to fix help mode? This PR as-is is still a big usability improvement/fix so I think we should just merge this.

#36289 doesn't really buy us a lot here, because methods actually gets called in e.g. Docs.@doc Base._in, which in turn calls REPL.lookup_doc, so fixing this would mean having to change how parts of the Docs module work.

tkf · 2020-06-19T20:00:38Z

Ah, I see. So Docs.@doc macro creates a string before it is printed again. Yeah, it sounds like a non-trivial problem.

tkf

I think it's better to set IOContext outside outstream. This function sounds like an accessor function and having a side-effect is not great. For example, it'd mean some innocent use like

julia/stdlib/REPL/src/REPL.jl

Line 982 in 5142abf

println(outstream(repl))

would clear out last_shown_line_infos. I think it'd be cleaner if we can keep outstream as a simple accessor. We've already discussed this in #36344 (comment) but do you think the change I suggested here work? It's just moving what we were doing in display to print_response.

stdlib/REPL/src/REPL.jl

KristofferC · 2020-06-26T14:11:59Z

Bump.

simeonschaub

Sorry, my laptop crashed, that's why it took me some time to restore the changes I made. I refactored most of the logic behind the hint message into with_methodtable_hint to avoid duplicating this code. I also realized that LAST_SHOWN_LINE_INFOS should probably be lowercase now, since it's no longer a global variable.

simeonschaub · 2020-06-28T16:20:09Z

stdlib/REPL/test/replcompletions.jl

@@ -110,7 +110,7 @@ end

 let s = "using REP"
    c, r = test_complete(s)
-    @test count(isequal("REPL"), c) == 1
+    @test count(isequal("REPL"), c) == 2


I have no clue what's going on here...

Weird, this now breaks the CI test, but was needed locally on my machine for tests to pass 🤔.

Weird, this now breaks the CI test, but was needed locally on my machine for tests to pass

Yeah, it's a bug in the REPL tests.

Does CI run the tests with different options or why isn't this bug present there?

#32377 has some more info.

Ah, thanks for the pointer!

base/methodshow.jl

staticfloat · 2020-06-30T23:08:37Z

Just a note that this is somehow freezing up the buildbots :(

simeonschaub · 2020-07-01T08:50:23Z

Are they actually frozen? To me, it looked like they finished, but somehow didn't report back correctly to GitHub. Might have been because GitHub was down for a while?

tkf · 2020-07-01T09:09:43Z

Yeah it looks like the problem is on GitHub side. For example, tester_linux64 is finished but package_linux64 still says it's pending.

staticfloat · 2020-07-01T21:27:18Z

Are they actually frozen? To me, it looked like they finished, but somehow didn't report back correctly to GitHub.

Yes they're freezing, I go in manually and end them. Here's one that was been running for 21 hours before I killed it: https://build.julialang.org/#/builders/77/builds/1247

simeonschaub · 2020-07-01T21:53:43Z

Do you think that's related to this PR? That's kind of the annoying thing about changing something in REPL – You change some small thing and suddenly some other thing gets silently broken/stuck. 😄

staticfloat · 2020-07-01T22:27:10Z

Yes, it's been consistent with this PR, unfortunately. Here's another one: https://build.julialang.org/#/builders/77/builds/1281

That being said, the latest version of this seems to have made it through the package_* builders for win64, so maybe you fixed it? I'm only seeing it on Windows, perhaps there's something particularly weird about that platform's handling of this?

simeonschaub · 2020-07-02T08:20:36Z

Weird... I don't see how any of these changes could have fixed this.

simeonschaub · 2020-07-02T08:56:58Z

I've rebased onto the latest master, let's hope the Windows build still goes through. 😁

simeonschaub · 2020-07-03T08:44:14Z

@staticfloat All tests now passed, so it seems like this should be ok now. @tkf Would you be willing to review?

tkf

LGTM!

tkf · 2020-07-03T08:54:44Z

stdlib/REPL/src/REPL.jl

@@ -495,6 +484,22 @@ function complete_line(c::LatexCompletions, s)
    return unique!(map(completion_text, ret)), partial[range], should_complete
 end

+with_methodtable_hint(f, repl) = f(outstream(repl))
+function with_methodtable_hint(f, repl::LineEditREPL)


👍 I think this is a clean approach!

tkf · 2020-07-03T08:59:43Z

@KristofferC Do you want to review it again?

If no objections, let's merge this in a few days.

simeonschaub · 2020-07-07T08:31:45Z

Bump.

tkf · 2020-07-08T05:54:47Z

Thanks for the fix! (and the ping)

@KristofferC

* alternative fix for JuliaLang#36340 * refactor method hint mechanism * make LAST_SHOWN_LINE_INFOS lowercase * implement style suggestion from @KristofferC * fix use of LAST_SHOWN_LINE_INFOS on latest master

tkf reviewed Jun 18, 2020

View reviewed changes

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

simeonschaub commented Jun 18, 2020

View reviewed changes

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

simeonschaub force-pushed the fix_36272_2 branch 2 times, most recently from a07d52b to ffd4fb3 Compare June 18, 2020 16:49

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

simeonschaub mentioned this pull request Jun 19, 2020

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

Closed

tkf reviewed Jun 19, 2020

View reviewed changes

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

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

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

tkf added the needs tests Unit tests are required for this change label Jun 19, 2020

tkf changed the title ~~alternative fix for #36340~~ Fix Ctrl+Q for stack traces Jun 19, 2020

KristofferC added this to the 1.6 milestone Jun 24, 2020

simeonschaub force-pushed the fix_36272_2 branch from 93e818c to 99adf59 Compare June 28, 2020 16:18

simeonschaub commented Jun 28, 2020

View reviewed changes

KristofferC reviewed Jun 29, 2020

View reviewed changes

base/methodshow.jl Outdated Show resolved Hide resolved

simeonschaub force-pushed the fix_36272_2 branch from 317d75f to 1b6055f Compare July 2, 2020 08:51

simeonschaub added 9 commits July 3, 2020 01:17

alternative fix for 36340

36eb2b2

add tests

f431089

refactor method hint mechanism

5cb0c8e

make LAST_SHOWN_LINE_INFOS lowercase

517d254

fix stupid mistakes

55641b3

fix tests

caa066e

implement style suggestion from @KristofferC

7630332

fix use of LAST_SHOWN_LINE_INFOS on latest master

ba9c3b6

fix bad rebase

b2f9f74

simeonschaub force-pushed the fix_36272_2 branch from 858d234 to b2f9f74 Compare July 2, 2020 23:18

tkf approved these changes Jul 3, 2020

View reviewed changes

tkf merged commit 114e581 into JuliaLang:master Jul 8, 2020

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

Fix Ctrl+Q for stack traces #36344

Fix Ctrl+Q for stack traces #36344

simeonschaub commented Jun 18, 2020 •

edited by tkf

Loading

tkf commented Jun 18, 2020

simeonschaub commented Jun 18, 2020

tkf commented Jun 18, 2020

simeonschaub commented Jun 18, 2020

tkf commented Jun 18, 2020

simeonschaub commented Jun 19, 2020

tkf commented Jun 19, 2020

tkf left a comment

KristofferC commented Jun 26, 2020

simeonschaub left a comment

simeonschaub Jun 28, 2020

simeonschaub Jun 28, 2020

KristofferC Jun 29, 2020

simeonschaub Jun 29, 2020

KristofferC Jun 29, 2020

simeonschaub Jun 29, 2020

staticfloat commented Jun 30, 2020

simeonschaub commented Jul 1, 2020

tkf commented Jul 1, 2020

staticfloat commented Jul 1, 2020

simeonschaub commented Jul 1, 2020

staticfloat commented Jul 1, 2020

simeonschaub commented Jul 2, 2020

simeonschaub commented Jul 2, 2020

simeonschaub commented Jul 3, 2020

tkf left a comment

tkf Jul 3, 2020

tkf commented Jul 3, 2020

simeonschaub commented Jul 7, 2020

tkf commented Jul 8, 2020

Fix Ctrl+Q for stack traces #36344

Fix Ctrl+Q for stack traces #36344

Conversation

simeonschaub commented Jun 18, 2020 • edited by tkf Loading

tkf commented Jun 18, 2020

simeonschaub commented Jun 18, 2020

tkf commented Jun 18, 2020

simeonschaub commented Jun 18, 2020

tkf commented Jun 18, 2020

simeonschaub commented Jun 19, 2020

tkf commented Jun 19, 2020

tkf left a comment

Choose a reason for hiding this comment

KristofferC commented Jun 26, 2020

simeonschaub left a comment

Choose a reason for hiding this comment

simeonschaub Jun 28, 2020

Choose a reason for hiding this comment

simeonschaub Jun 28, 2020

Choose a reason for hiding this comment

KristofferC Jun 29, 2020

Choose a reason for hiding this comment

simeonschaub Jun 29, 2020

Choose a reason for hiding this comment

KristofferC Jun 29, 2020

Choose a reason for hiding this comment

simeonschaub Jun 29, 2020

Choose a reason for hiding this comment

staticfloat commented Jun 30, 2020

simeonschaub commented Jul 1, 2020

tkf commented Jul 1, 2020

staticfloat commented Jul 1, 2020

simeonschaub commented Jul 1, 2020

staticfloat commented Jul 1, 2020

simeonschaub commented Jul 2, 2020

simeonschaub commented Jul 2, 2020

simeonschaub commented Jul 3, 2020

tkf left a comment

Choose a reason for hiding this comment

tkf Jul 3, 2020

Choose a reason for hiding this comment

tkf commented Jul 3, 2020

simeonschaub commented Jul 7, 2020

tkf commented Jul 8, 2020

simeonschaub commented Jun 18, 2020 •

edited by tkf

Loading