tweaks to new REPL

[richardsheridan]

A follow up to #2972. These would have been review comments earlier if I knew the code was going to be (a) this simple and (b) in my wheelhouse!

I think runcode can just be:

    def runcode(self, code: types.CodeType) -> None:
        try:
            func = types.FunctionType(code, self.locals)
            if inspect.iscoroutinefunction(func):
                trio.from_thread.run(func)
            else:
                trio.from_thread.run_sync(func)
        except SystemExit:
            # ...snip...
            raise
        except BaseException:
            self.showtraceback()

It's less code, less layers of onion unwrapping, and the more common from_thread.run_sync is more efficient (not that anyone would notice latency from two extra checkpoints at the REPL). That's the first commit.

I also noticed one fragility, which is that KI works thanks to the thread task reentry feature combined with the fact that we inject KeyboardInterrupt into the main task. If we were to go the route of #733 (comment) and just cancel everything and transmute to KeyboardInterrupt when the run finishes, the run would be cancelled. I put a test for this in the second commit.

Finally, we can hide the threading code frames from the traceback, although I'm not sure about editing the global exception state like that. that's the last commit.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.63%. Comparing base (3350c11) to head (b4af483).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3002   +/-   ##
=======================================
  Coverage   99.63%   99.63%           
=======================================
  Files         120      120           
  Lines       17783    17865   +82     
  Branches     3197     3212   +15     
=======================================
+ Hits        17718    17800   +82     
  Misses         46       46           
  Partials       19       19           

Files	Coverage Δ
src/trio/_repl.py	100.00% <100.00%> (ø)
src/trio/_tests/test_repl.py	100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

[richardsheridan]

Looks like that traceback cleanup trick only works on 3.11 later. Either we could find a different trick or only assert it on recent python versions!

[A5rocks]

Cc @clint-lawrence cause you made the original PR

[CoolCat467]

Out of date branch so updated

[richardsheridan]

The difference in editing exceptions that showed up in 3.11 is documented so I think we can just rely on it.

[richardsheridan]

And to follow up on people interested in a nursery for background tasks, my test accidentally shows that you can just use the system nursery.

trio/src/trio/_tests/test_repl.py

Lines 106 to 117 in 8a0b573

	"from trio._util import signal_raise",
	"import signal, trio, trio.lowlevel",
	"async def f():",
	" trio.lowlevel.spawn_system_task("
	" trio.to_thread.run_sync,"
	" signal_raise,signal.SIGINT,"
	" )", # just awaiting this kills the test runner?!
	" await trio.sleep_forever()",
	" print('should not see this')",
	"",
	"await f()",
	"print('AFTER KeyboardInterrupt')",

[richardsheridan]

A really clean (IMO) implementation breaks certain features of InteractiveConsole intended to enable customization in subclasses. So I made it final to discourage people from trying and noted in comments which things won't work for future contributors to this class.

[clint-lawrence]

This looks good to me.

[CoolCat467]

Looks good, only thing I can think of is that comment after isinstance(result.error, SystemExit): is a bit interesting:

So we print the exception and stay in the repl

Not immediately obvious that raising the error in that branch causes error to be only printed and not close repl if I am reading the comment right.

[richardsheridan]

Not immediately obvious that raising the error in that branch causes error to be only printed and not close repl if I am reading the comment right.

Yes, I think I did a poor job of splitting up that comment. I'm going clarify that a bit and then someone can do a nice squash merge for us.