search

trptcolin / reply

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

ClassCastException when passing --port option #198

Closed glts closed 5 years ago

glts commented 5 years ago

Commit 7fe67e4e2eb5580e42da6662fd05cd4c62a6533c introduced a regression where REPL-y will throw an exception when it gets passed the --port command-line argument. The exception is easily reproducible with the command

clojure -Sdeps '{:deps {reply {:mvn/version "0.4.3"}}}' -m reply.main --port 3333

This breaks clients of the REPL-y command-line API such as Leiningen, see technomancy/leiningen#2572.

The proposed change restores the previous behaviour as faithfully as possible. reply.eval-modes.nrepl/main will again accept both string and integer for :port.

trptcolin commented 5 years ago

Thanks! :tada:

bbatsov commented 5 years ago

The proposed change restores the previous behaviour as faithfully as possible. reply.eval-modes.nrepl/main will again accept both string and integer for :port.

I still think that's not the right place to fix this, but I guess that's not a big deal. IMO the conversion should be done when the command-line arguments are read. Pushing this down makes the code slightly more complex than it needs to be.

bbatsov commented 5 years ago

(to be more concrete - I think the command-line params should be normalized before launch is ever called)

glts commented 5 years ago

Belated thank you for the merge …!

Are you planning to cut a release some time soon? Would be cool to pull this into Leiningen, to fix the related issue. No pressure though.

trptcolin commented 5 years ago

Done - thanks for the nudge! 🎉

glts commented 5 years ago

Done and done, thank you very much.