clojure-emacs / cider

The Clojure Interactive Development Environment that Rocks for Emacs
https://cider.mx
GNU General Public License v3.0
3.52k stars 644 forks source link

Use ClojureScript-specific REPL helpers for ClojureScript REPLs #2647

Open bbatsov opened 5 years ago

bbatsov commented 5 years ago

This recent convo with @PEZ reminded that we don't check the REPL type before loading the REPL utils and they are basically the Clojure ones all the time. That should be very easy to fix.

rpkarlsson commented 5 years ago

Been having a look at this one and getting cider-repl-require-repl-utils to load the cljs utils is pretty straightforward.

However the repl utils are also used by cider-repl-init-code, a defcustom.

https://github.com/clojure-emacs/cider/blob/master/cider-repl.el#L174

(defcustom cider-repl-init-code (list cider-repl-require-repl-utils-code)
  "Clojure code to evaluate when starting a REPL.
Will be evaluated with bindings for set!-able vars in place."
  :type '(list string)
  :group 'cider-repl
  :package-version '(cider . "0.21.0"))

Unsure how to handle cider-repl-init-code correctly as it should be override-able by a user and also dynamic depending on the repl type. Some ideas:

bbatsov commented 5 years ago

I think it'd be best to make cider-repl-require-repl-utils an alist or a plist and have there keys for clj, cljs (and hopefully down the road for clr as well. Then the existing defcustom will simply pull the clj key in, as that's meant to be run in the initial Clojure REPL anyways (before it's upgraded to ClojureScript).

Unsure how to handle cider-repl-init-code correctly as it should be override-able by a user and also dynamic depending on the repl type.

Maybe we should rename this to cider-repl-init-code-clj and add some matching cljs counterpart (or make it an alist/plist) as well, so there's some init code that we'd run in ClojureScript REPLs after they are upgraded as well.

rpkarlsson commented 4 years ago

Been revisiting this after a while and I'm a bit confused as to what needs to happen. AFAICT https://github.com/nrepl/piggieback/blob/master/src/cider/piggieback_impl.clj#L157 already loads cljs specific repl utils once a session has been upgraded? So if anything more is needed to be done perhaps it's for piggieback to require the same utils as the cljs field of cider-repl-require-repl-utils-code?

bbatsov commented 4 years ago

Great catch! But I think there's one more thing to consider - the code in piggieback gets loaded only for the initial ns there, so if you switch the ns the REPL utilities wouldn't be around anymore.