Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use ClojureScript-specific REPL helpers for ClojureScript REPLs #2647

Open
bbatsov opened this issue Jun 15, 2019 · 4 comments
Open

Use ClojureScript-specific REPL helpers for ClojureScript REPLs #2647

bbatsov opened this issue Jun 15, 2019 · 4 comments
Labels
enhancement good first issue A simple tasks suitable for first-time contributors

Comments

@bbatsov
Copy link
Member

bbatsov commented Jun 15, 2019

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 bbatsov added enhancement good first issue A simple tasks suitable for first-time contributors labels Jun 15, 2019
@rpkarlsson
Copy link
Contributor

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

@bbatsov
Copy link
Member Author

bbatsov commented Jul 10, 2019

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
Copy link
Contributor

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
Copy link
Member Author

bbatsov commented Oct 15, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement good first issue A simple tasks suitable for first-time contributors
Projects
None yet
Development

No branches or pull requests

2 participants