trptcolin / reply

REPL-y: A fitter, happier, more productive REPL for Clojure.
Eclipse Public License 1.0
337 stars 44 forks source link

Bump the nREPL dep #185

Closed bbatsov closed 6 years ago

trptcolin commented 5 years ago

Hey, sorry, I needed to revert back to nrepl 0.4.2, because of some breaking changes in 0.4.3 (in particular the structure of nrepl.server.Server).

I made a couple new issues for the missing integration test coverage, and the bump to the latest nrepl.

bbatsov commented 5 years ago

@trptcolin What breaking changes? The removal of the legacy code from nREPL 0.1 what treated the Server as a ref?

bbatsov commented 5 years ago

Seems so. :-) This was deprecated so many years ago that I assumed no one was using this at point. Anyways, at least the fix for this is simple - it's now just a regular record.

trptcolin commented 5 years ago

Yeah, I think that and also the field access needs to change from :ss to :server-socket. And now that I look closer, it looks like the place that threw could be changed to (:port server) (instead of drilling through deref, :ss, .getLocalPort (at least as of 2012; based on https://github.com/nrepl/nREPL/commit/8c52a260765103e76c8b8d9f2877fc7f323ada7b)

bbatsov commented 5 years ago

Yep, you can access the port directly now.

bbatsov commented 5 years ago

Btw, this 0 check is no longer needed as well:

https://github.com/trptcolin/reply/blob/3fb81561c077e40e33b8caaa1432e0daca1ad521/src/clj/reply/eval_modes/nrepl.clj#L159

If there's no port nREPL assumes 0 anyways.

bbatsov commented 5 years ago

And sorry for the oversight on my part! I thought I checked explicitly for this, but I guess I didn't.