Allow more control of stacktrace formatting.

[jeffvalk]

Currently stacktraces are printed with clj-stacktrace, if available, or clojure.stacktrace otherwise. To use a different print function, the common approach seems to be to call alter-var-root on one of these in the lein profile -- a bit of a hack.

This lends some flexibility to stacktrace printing by adding io.aviso.repl/pretty-pst as another automatically searched option (like clj-stacktrace.repl/pst+), and defining a variable with which the user can override the built-in defaults.

[bbatsov]

I'll probably be replacing this inlined code with some middleware. There's a problem with just adding other stacktrace libs (like clj-stacktrace) - if they present the stacktrace data in some different format than the default one, the stacktrace parsing code we currently employ will fail. I don't even know if it works with clj-stracktrace, but it's quite likely that it doesn't. At the very least we'll need a correspondence between the stacktrace library used and a parsing function. I'm not sure we want to do this client-side...

[jeffvalk]

The middleware approach does make more sense. I'd considered that too, but figured I'd submit something simple. Anything deliberate seemed better than the alter-var-root abuse being recommended on blogs.

Which current code fails on some stacktrace formats? Your point about coordination between formatter and parser stands to reason; I'm just curious where this interaction lives (or should).

[bbatsov]

See cider-extract-error-info and cider-highlight-compilation-errors. The first function is the problematic one. Generally a stracktrace format and its parsing function should go hand-in-hand, otherwise compilation errors will not be highlighted in the clojure-mode buffers.

[jeffvalk]

I had a closer look at the code in question, and if I'm seeing this correctly, the output parsed for compile errors/warnings is separate from the formatted stacktrace presented in the popup buffer. An eval request will cause both, but not dependently. (The former is read from the eval err response, whereas the latter comes from a separate nREPL request invoked on the status response.) Unless I'm overlooking something, nrepl-default-err-handler could be changed without impacting compiler error/warning highlighting, and vice versa. (The tests I tried support this.)

Like you mentioned, migrating these functions to middleware seems like a good tack. Is this something you'd entertain pull requests for? In particular, I'm thinking of moving stacktraces from a static printed representation to something navigable using the actual stack frame info. I have a preliminary version of the middleware running, but I don't want to wander to far if you already have a roadmap in mind.

[bbatsov]

Hmm, I'm pretty sure I parsed the stacktrace in the popup buffer at some point, but I guess I might have changed that (or someone else did and I forgot about it).

Yeah, I'd certainly take a PR for that. Unfortunately I'm under a mountain of work these days and my plans for cider improvements are mostly on hold, so it's up to users to implement features that want to see sooner rather than later.

[jeffvalk]

Fair enough. I will send you a PR hopefully this week (schedule permitting).

[bbatsov]

@jeffvalk Ping :-)

[jeffvalk]

Yeah, that was longer than advertised. :-) I had this mostly complete last week...then non-coding things started clamoring for attention. I just got to look at it again last night. I'll tidy up and get it to you this evening.

I take it the middleware should be a PR to the cider-nrepl project?

[bbatsov]

Yep.

[jeffvalk]

Thought this was good to go, but while tidying up, I found a possible bug. Will chase it down.

[jeffvalk]

Sources updated. The middleware for this is cider-nrepl pull request #30.

[bbatsov]

Overall the code looks good, but I'll have a closer at it tomorrow. One thing I noticed is that you haven't mentioned the new functionality in the README and the CHANGELOG.

[jeffvalk]

My oversight. I updated the documentation in both projects.

[bbatsov]

Cider uses lexical scoping by default. Replace lexical-let with a plain old let.

[bbatsov]

Address my minor remarks, rebase on top of the current master and squash the commits into one.

[jeffvalk]

The sit-for line that you commented on is still there. (See my note above.) If you'd prefer that come out, I'll happily yank it. If you're content to leave it, everything should be ready to go.

[bbatsov]

Thanks!