cider icon indicating copy to clipboard operation
cider copied to clipboard

Use ClojureScript-specific REPL helpers for ClojureScript REPLs

Open bbatsov opened this issue 6 years ago • 4 comments

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.

bbatsov avatar Jun 15 '19 03:06 bbatsov

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:

  • Remove cider-repl-init-code
  • Have separate defcustoms one for clj and one for cljs
  • Keep cider-repl-init-code but extract the repl util loading part. Allowing for additional init code to be customized with cider-repl-init-code

rpkarlsson avatar Jul 10 '19 08:07 rpkarlsson

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.

bbatsov avatar Jul 10 '19 09:07 bbatsov

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?

rpkarlsson avatar Oct 14 '19 19:10 rpkarlsson

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.

bbatsov avatar Oct 15 '19 05:10 bbatsov