OhMyREPL.jl
OhMyREPL.jl copied to clipboard
Run OhMyREPL keybindings in a fixed world age
Hopefully this should avoid most invalidations caused by loading other packages as discussed in https://github.com/JuliaLang/JuliaSyntax.jl/issues/252.
Also refactor/replace the NEW_KEYBINDINGS global as it's hard to follow where this is added to, and surface a bunch of other uses of global variables like Base.active_repl up to the top level init.
I'm not sure whether there's some additional wrapping which is needed around the pirating in refresh_lines.jl, etc?
Seems to fix #316 ?
@jakobnissen You've used OhMyREPL a lot more than me, perhaps you could give this a test drive? I'm not entirely sure of all the ways I should be trying to test this.
This PR indeed removes all (noticable) latency even when running with JuliaSyntax 0.3.4 (before Kristoffer's recent type-stability improvements to JuliaSyntax) and importing ProgressLogging.
When using the latest JuliaSyntax, loading ProgressLogging still leads to 277 invalidations before your PR, and 253 with your PR. In either case, latency is not noticable even when the invalidated code is in the REPL - maybe I just can't figure out how to trigger the invalidated code to recompile.
Still, from a latency viewport this is a minor win. If there a no/few downsides, it looks good. If there are any downsides to running in a fixed world age, perhaps a better solution might be to make REPL.jl more type stable.
loading ProgressLogging still leads to 277 invalidations before your PR, and 253 with your PR
Right. This doesn't prevent invalidations in the latest world. But running the key bindings in an older world avoids the compiler having to do work to deal with them. So latency should be much improved.
Codecov Report
Patch coverage: 1.69% and project coverage change: -5.22 :warning:
Comparison is base (
0d60955) 56.88% compared to head (b360ca4) 51.67%.
:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.
Additional details and impacted files
@@ Coverage Diff @@
## master #321 +/- ##
==========================================
- Coverage 56.88% 51.67% -5.22%
==========================================
Files 15 13 -2
Lines 1111 1134 +23
==========================================
- Hits 632 586 -46
- Misses 479 548 +69
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/BracketInserter.jl | 1.09% <ø> (-20.02%) |
:arrow_down: |
| src/hooks.jl | 0.00% <0.00%> (ø) |
|
| src/repl.jl | 0.00% <0.00%> (-13.13%) |
:arrow_down: |
| src/OhMyREPL.jl | 19.35% <5.26%> (-1.40%) |
:arrow_down: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
Good to go?
I've updated this to also fix the world age of the pirated LineEdit.refresh_line, and to precompile the implementations of all pirated methods. This should reduce latency a little.
Oh I didn't see your comment. Yes, I think it's good to go now :-)
Ugh this has gotten stale/conflicted.
I've fixed the conflicts and pushed again.
It would be nice to merge this? it contains improvements to precompilation, along with the invoke_in_world stuff.
Windows nightly test failures are probably due to https://github.com/JuliaDebug/TerminalRegressionTests.jl/pull/14 ?