-
-
Notifications
You must be signed in to change notification settings - Fork 645
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
nbb nrepl-server: can't send expressions from .cljs file #3061
Comments
Do you have the same problem with |
With |
Got it. |
I finally looked at this and the problem is that when you use I guess we'll have to add some simple detection that we're connected to |
Thanks for looking into this. |
with cider
output from nbb server:
cider-repl contents:
And here is the stacktrace for the "List expression" read from minibuffer call (which feels like a bug in that situation)
|
any updates? |
Had no time for this so far. The way things are going it's unlikely that I'll be able to tackle it before April/May. If someone wants to beat me to it - be my guest. |
(It's not just |
@bbatsov I'm probably going to look into this next Monday/Tuesday. Any pointers that will set me on the right track will be appreciated. As |
Looking at the spec and a trace of an nREPL session between cider and a jack'd in JVM, it appears that cider wants a few non-standard ops and falls back to trying to execute JVM-specific code when they're absent. (For example, cider seems to want a Should we discuss improving the protocol a bit? |
I think this isn't a protocol problem, it's a problem of a client that expects the nREPL server to be of a certain implementation (JVM Clojure) rather than expecting it to just talk nREPL and implement the specification. Or maybe it is this way because the protocol isn't rich or specified/documented enough? |
The protocol may not be perfectly suited to purpose. The official documentation lacks a way to explicitly request a stacktrace, for example. It looks like cider is using |
@jackrusher Yeah, that's more or less the case. In general I've always aimed to have a minimal API for nREPL itself, as it had to be something fairly portable across programming languages.
You receive it upon an evaluation error automatically. There's no way to request it manually.
I like the
Even now CIDER works decently without any additional middleware. Obviously some features don't work this way, but the core functionality is there. The problem is mostly that in this case CIDER falls back to evaluating Clojure code to provide some features and this obviously is implementation specific. In the early days of CIDER it didn't seem like a problem as back then I never thought they'd be multiple Clojure implementations/dialects. |
What we see in practice is that every editor implementing gets some kind of dropdown menu: connect to clj, cljs, nbb (Calva), babashka (Calva) so the editor knows what to probe. Ideally any editor / nREPL client could talk to any nREPL server in a language agnostic way (like LSP is a language agnostic protocol), but that's not the world we live in. So the information I would like to have from @bbatsov: how should we proceed enabling CIDER to connect to an nREPL server that is implemented purely in CLJS and can eval CLJS (no JVM in between). Should we add bespoke
or so, so it will work with the new targets? Beyond nbb there will be at least two or three new ones running in pure JS:
|
@bbatsov Our messages crossed each other, I'm going to read yours now. |
If you grep for "babashka" in CIDER you'll see the kind of changes I had to do to make |
@bbatsov I'll experiment with both next week then, thanks. |
@borkdude funny, I was just digging into it and up with this: (cider-register-cljs-repl-type 'nbb "(+ 42)")
(defun mm/cider-connected-hook ()
(when (eq 'nbb cider-cljs-repl-type)
(cider-set-repl-type 'cljs)))
(add-hook 'cider-connected-hook #'mm/cider-connected-hook) |
@benjamin-asdf Interesting, keep going please :) |
both of these pieces are of course a bit hacky.
for 1)
it is currently enforced to define a init form |
I, for one, would very much prefer that the protocol be both generic and capable enough that (as the nREPL spec claims) it doesn't make any assumptions about what language/implementation is on the other side of the conversation. My main reason for this is that I would very much not like to implement and maintain a connection function for every nREPL-enable editor/execution context pair. |
Well, as The problems that people typically experience with CIDER when trying to use it with something other than Clojure have little to do with nREPL and a lot to do with assumptions that CIDER made early on (that'd it'd be used for Clojure only). 10 years ago I didn't think big and I aimed to focus only my immediate problem - build a Clojure IDE. I got interested in nREPL only later and I've started to maintain it a lot later. Making CIDER more generic would be nice, but as the primary focus remain Clojure and friends putting in the time to make it a "well-behaved" nREPL client never seemed worth it. It's on the wishlist, but I doubt it will happen any time soon. |
now I also have a workaround for the errors (cider-register-cljs-repl-type 'nbb "(+ 1 2 3)")
(defun mm/cider-connected-hook ()
(when (eq 'nbb cider-cljs-repl-type)
(setq-local cider-show-error-buffer nil)
(cider-set-repl-type 'cljs)))
(add-hook 'cider-connected-hook #'mm/cider-connected-hook) I don't know yet why the default stacktrace buffer does not pop up. But this oldschool overlay is actually ok for me. Because the error message doesn't have a stacktrace anyway. |
A very generic protocol isn't a very useful protocol imo. A protocol should be an agreement between to parties so they can talk in an agnostic way. Relying on |
Meanwhile @benjamin-asdf is solving our problems, 👏 👏 |
Seems to work well! Thank you @benjamin-asdf ! |
New issue for that conversation... |
In
I don't know why some fns work and others don't , and I don't care but hopefully this helps someone else. 😅 |
Could you explain what "fns" you mean? nbb fns or cider fns? |
@borkdude Apologies. I meant emacs/spacemacs function calls in the context of the different stages of bootstrapping. |
@dmg46664 probably because spacemacs lazy loads everything so even if you call the function in your user config cider is not loaded yet. Guess your options are the |
With @benjamin-asdf's snippet, all works pretty well I must say. So perhaps I should just document that snippet in nbb's repo and we could close this issue? I documented a similar snippet here: https://github.com/nextjournal/clerk/blob/sci-nrepl/doc/browser-repl.md It's basically the same for all SCI JS based nREPLs I assume. |
@borkdude you could change the cljs-repl type to "plain-cljs" or something. In the snippet I mean, because it is not special to nbb. |
@benjamin-asdf Exactly! Meanwhile @bbatsov is improving CIDER in such a way that a snippet should maybe not be necessary at all. Not sure how far he is with that. |
I'm on a business trip for the next 10 days, but I'll focus more on the necessary CIDER changes when I'm back. |
This way it does not throw when connecting to joyride. Originally was also an issue for nbb (clojure-emacs#3061)
This way it does not throw when connecting to joyride. Originally was also an issue for nbb (clojure-emacs#3061)
This way it does not throw when connecting to joyride. Originally was also an issue for nbb (clojure-emacs#3061)
This way it does not throw when connecting to joyride. Originally was also an issue for nbb (clojure-emacs#3061)
This way it does not throw when connecting to joyride. Originally was also an issue for nbb (clojure-emacs#3061)
This way it does not throw when connecting to joyride. Originally was also an issue for nbb.
This is for nbb, scittle, joyride. Better support for "pain" repls, support "cljs" without setup. we want to 1. connect with a plain client 2. make any assumptions explicit 3. connect cljs buffers with those plain repls 1. Check for cider middleware being present, before using it (already being done) 2. Check for cider-library-present-p, before relying on anything in the runtime 3. Make assumptions about the runtime explicit My suggestion is =cider-connection-capabilities=. Currently, there was an implicit assumption about the compilation error format. =cider-connection-capabilities= Is my suggestion for 2. =cider-repls= Now returns cljs, if the repl capabilities include 'cljs This way we can make a "plain" clj client, upgrade on connect with cljs capability and have it be connected in cljs buffers. This is more a concession / workaround the current repl-type setup. In the future we might get rid of repl-type? The only reason we have it, and creaating issue 3., is because we want to be ergonomic about which buffer to use inside a given source buffer. I found that I am able to juggle multiple clients by swapping to the buffer I want, thereby setting it active. This can be a user command. Can also make a modeline element that swaps between clj/cljs like calva. slack discussion: https://clojurians.slack.com/archives/C04CAKAGADU
This is for nbb, scittle, joyride. Better support for "pain" repls, support "cljs" without setup. we want to 1. connect with a plain client 2. make any assumptions explicit 3. connect cljs buffers with those plain repls 1. Check for cider middleware being present, before using it (already being done) 2. Check for cider-library-present-p, before relying on anything in the runtime 3. Make assumptions about the runtime explicit My suggestion is =cider-connection-capabilities=. Currently, there was an implicit assumption about the compilation error format. =cider-connection-capabilities= Is my suggestion for 2. =cider-repls= Now returns cljs, if the repl capabilities include 'cljs This way we can make a "plain" clj client, upgrade on connect with cljs capability and have it be connected in cljs buffers. This is more a concession / workaround the current repl-type setup. In the future we might get rid of repl-type? The only reason we have it, and creaating issue 3., is because we want to be ergonomic about which buffer to use inside a given source buffer. I found that I am able to juggle multiple clients by swapping to the buffer I want, thereby setting it active. This can be a user command. Can also make a modeline element that swaps between clj/cljs like calva. slack discussion: https://clojurians.slack.com/archives/C04CAKAGADU
This is for nbb, scittle, joyride. Better support for "pain" repls, support "cljs" without setup. we want to 1. connect with a plain client 2. make any assumptions explicit 3. connect cljs buffers with those plain repls 1. Check for cider middleware being present, before using it (already being done) 2. Check for cider-library-present-p, before relying on anything in the runtime 3. Make assumptions about the runtime explicit My suggestion is =cider-connection-capabilities=. Currently, there was an implicit assumption about the compilation error format. =cider-connection-capabilities= Is my suggestion for 2. =cider-repls= Now returns cljs, if the repl capabilities include 'cljs This way we can make a "plain" clj client, upgrade on connect with cljs capability and have it be connected in cljs buffers. This is more a concession / workaround the current repl-type setup. In the future we might get rid of repl-type? The only reason we have it, and creaating issue 3., is because we want to be ergonomic about which buffer to use inside a given source buffer. I found that I am able to juggle multiple clients by swapping to the buffer I want, thereby setting it active. This can be a user command. Can also make a modeline element that swaps between clj/cljs like calva. slack discussion: https://clojurians.slack.com/archives/C04CAKAGADU
This is for nbb, scittle, joyride. Better support for "pain" repls, support "cljs" without setup. we want to 1. connect with a plain client 2. make any assumptions explicit 3. connect cljs buffers with those plain repls 1. Check for cider middleware being present, before using it (already being done) 2. Check for cider-library-present-p, before relying on anything in the runtime 3. Make assumptions about the runtime explicit My suggestion is =cider-connection-capabilities=. Currently, there was an implicit assumption about the compilation error format. =cider-connection-capabilities= Is my suggestion for 2. =cider-repls= Now returns cljs, if the repl capabilities include 'cljs This way we can make a "plain" clj client, upgrade on connect with cljs capability and have it be connected in cljs buffers. This is more a concession / workaround the current repl-type setup. In the future we might get rid of repl-type? The only reason we have it, and creaating issue 3., is because we want to be ergonomic about which buffer to use inside a given source buffer. I found that I am able to juggle multiple clients by swapping to the buffer I want, thereby setting it active. This can be a user command. Can also make a modeline element that swaps between clj/cljs like calva. slack discussion: https://clojurians.slack.com/archives/C04CAKAGADU Format docstring Fix cljs check
This is for nbb, scittle, joyride. Better support for "pain" repls, support "cljs" without setup. we want to 1. connect with a plain client 2. make any assumptions explicit 3. connect cljs buffers with those plain repls 1. Check for cider middleware being present, before using it (already being done) 2. Check for cider-library-present-p, before relying on anything in the runtime 3. Make assumptions about the runtime explicit My suggestion is =cider-connection-capabilities=. Currently, there was an implicit assumption about the compilation error format. =cider-connection-capabilities= Is my suggestion for 2. =cider-repls= Now returns cljs, if the repl capabilities include 'cljs This way we can make a "plain" clj client, upgrade on connect with cljs capability and have it be connected in cljs buffers. This is more a concession / workaround the current repl-type setup. In the future we might get rid of repl-type? The only reason we have it, and creaating issue 3., is because we want to be ergonomic about which buffer to use inside a given source buffer. I found that I am able to juggle multiple clients by swapping to the buffer I want, thereby setting it active. This can be a user command. Can also make a modeline element that swaps between clj/cljs like calva. slack discussion: https://clojurians.slack.com/archives/C04CAKAGADU Format docstring Fix cljs check
This is for nbb, scittle, joyride. Better support for "pain" repls, support "cljs" without setup. We want to: 1. connect with a plain nREPL client 2. make any assumptions explicit about the nREPL server/runtime 3. connect cljs buffers with those plain repls 1. Check for cider middleware being present, before using it (already being done) 2. Check for cider-library-present-p, before relying on anything in the runtime. 3. Make assumptions about the runtime explicit My suggestion is to solve these problems is `cider-connection-capabilities`. Currently, there was an implicit assumption about the compilation error format. Changes to `cider-repls`: Now returns cljs, if the repl capabilities include 'cljs. This way we can make a "plain" clj client, upgrade on connect with cljs capability and have it be connected in cljs buffers. This is more a concession / workaround the current repl-type setup. Here's the slack discussion that lead to this PR: https://clojurians.slack.com/archives/C04CAKAGADU
Is this issue still valid? I see commits at the bottom of the timeline so I'd guess not cc/ @benjamin-asdf |
@vemv No I wouild say this is fixed. Cider nbb support is quite complete as far as I am aware. |
Nice! |
nice work all, big achievement here |
Confirmed I needed to remove the snippet. Nice one. |
Expected behavior
I expect to be able to send expressions from a
.cljs
file inclojurescript-mode
to an nbb nrepl server.Actual behavior
Nothing happens until I activate
clojure-mode
in a.cljs
file.Steps to reproduce the problem
Then
cider-connect
from emacs to that server and try to evaluate some expressions from a.cljs
file:foo.cljs:
Environment & Version information
CIDER version information
Include here the version string displayed when
CIDER's REPL is launched. Here's an example:
Emacs version
27.2
Operating system
macOS
The text was updated successfully, but these errors were encountered: