tpope / vim-fireplace

fireplace.vim: Clojure REPL support
https://www.vim.org/scripts/script.php?script_id=4978
1.74k stars 139 forks source link

all these stacktrace bugs - off switch? #350

Closed tolgraven closed 5 years ago

tolgraven commented 5 years ago

While I'm sure it's fixable it'd still be nice to have an option to turn the filename parsing off in the meantime. nvim locks up 0.3 to 5 seconds depending on project size anytime I :Require invalid code. Or anytime I :Eval, just not as bad.

Was always sluggish, but feels like it's been getting worse (or my projects are just bigger), and profiling made me wonder why no cache, and thinking how using fireplace without python must be almost impossible. Gave it a test... 10 seconds the first eval in a small project, then instant!

Looking inside fireplace.vim it would appear vim calling python calling vim somehow fails to set s:jar_contents, hence full lookup is done (in this case) 26 664 times, every single throw. That'd do it.

I've never actually used the loclist feature since it appeared to show everything in reverse order, but looking at it now it actually does go the right way, and filters a bit... only it grabs the wrong side of the trace!! With something like (map + [4 6 "8"]) I obviously get the top 12 lines with pst, and 13 from fireplace - the bottom 13...

tpope commented 5 years ago

If it's not setting s:jar_contents we should definitely fix that. Can you crack open the autoload file, add let g:jar_contents = s:jar_contents to the bottom, and confirm that, after causing an error, :echo g:jar_contents really is empty?

tolgraven commented 5 years ago

Sorry about that, turns out I was running pynvim 0.3.1, 0.3.2 appears to have fixed let!

Tho point stands - even without the python detour it can take a while for big classpaths (heavy test case went from 5-7s to 1-2s on throwy :Require) and since the parsing isn't compatible with pst in :repl-options :caught (which I like to have) I'll leave it off, and an option would be simpler than patching. But I see you've already closed a similar issue way back so I get if it's too niche.

tpope commented 5 years ago

I'm not adding an option that makes Fireplace look broken if you don't realize it's set. I have been considering making stacktrace retrieval an explicit command, though.

What's this :repl-options :caught business?