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

Allow connecting to nbb via connect-clj (#3061) #3272

Merged
merged 4 commits into from
Dec 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
- [#3251](https://github.com/clojure-emacs/cider/pull/3251): Disable undo in `*cider-stacktrace*` buffers.
- Consecutive overlays will not be spuriously deleted.
- [#3260](https://github.com/clojure-emacs/cider/pull/3260): Scroll REPL buffer in other frame.
- [#3061](https://github.com/clojure-emacs/cider/issues/3061): Allow
connect-clj for plain cljs repls (nbb etc).

## 1.5.0 (2022-08-24)

Expand Down
34 changes: 33 additions & 1 deletion cider-connection.el
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,19 @@ buffer."
(when cider-auto-mode
(cider-enable-on-existing-clojure-buffers))

(setf cider-connection-capabilities
(append
(pcase (cider-runtime)
('clojure '(clojure jvm-compilation-errors))
('babashka '(babashka jvm-compilation-errors))
(_ '()))
(when
;; see `cider-sync-tooling-eval', but it is defined on a higher layer
(nrepl-dict-get
(nrepl-sync-request:eval "cljs.core/demunge" (current-buffer) nil 'tooling)
"value")
'(cljs))))

(run-hooks 'cider-connected-hook)))))

(defun cider--disconnected-handler ()
Expand Down Expand Up @@ -437,6 +450,19 @@ about this buffer (like variable `cider-repl-type')."
(plist-get nrepl-endpoint :host)
(plist-get nrepl-endpoint :port))))))

(defvar-local cider-connection-capabilities '()
"A list of some of the capabilites of this connection buffer.
Aka what assumptions we make about the runtime.
This is more general than
`cider-nrepl-op-supported-p' and `cider-library-present-p'.
But does not need to replace them.")

(defun cider-connection-has-capability-p (capability :optional connection-buffer)
"Return non nil when the cider connection has CAPABILITY.
By default it assumes the connection buffer is current."
(with-current-buffer (or connection-buffer (current-buffer))
(member capability cider-connection-capabilities)))


;;; Connection Management Commands

Expand Down Expand Up @@ -885,7 +911,13 @@ no linked session or there is no REPL of TYPE within the current session."
(cond ((null buffer-repl-type) nil)
((or (null type) (eq type 'multi) (eq type 'any)) t)
((listp type) (member buffer-repl-type type))
(t (string= type buffer-repl-type)))))
(t
(or (string= type buffer-repl-type)
(let ((capabilities
(buffer-local-value 'cider-connection-capabilities buffer)))
(cond ((listp type)
(cl-some (lambda (it) (member it capabilities)) type))
(t (member type capabilities)))))))))

(defun cider--get-host-from-session (session)
"Returns the host associated with SESSION."
Expand Down
19 changes: 15 additions & 4 deletions cider-eval.el
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,13 @@ Uses the value of the `out' slot in RESPONSE."
(cider-nrepl-sync-request:eval
"(clojure.stacktrace/print-cause-trace *e)")))

(defun cider-default-err-eval-print-handler ()
"Display the last exception without middleware support.
When clojure.stracktrace is not present."
(cider--handle-err-eval-response
(cider-nrepl-sync-request:eval
"(println (ex-data *e))")))

(defun cider--render-stacktrace-causes (causes &optional error-types)
"If CAUSES is non-nil, render its contents into a new error buffer.
Optional argument ERROR-TYPES contains a list which should determine the
Expand Down Expand Up @@ -498,9 +505,10 @@ into a new error buffer."
(defun cider-default-err-handler ()
"This function determines how the error buffer is shown.
It delegates the actual error content to the eval or op handler."
(if (cider-nrepl-op-supported-p "stacktrace")
(cider-default-err-op-handler)
(cider-default-err-eval-handler)))
(cond ((cider-nrepl-op-supported-p "stacktrace") (cider-default-err-op-handler))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to me this change won't result in anything different.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, actually the similar names confused me. Now I see the last branch is different.

((cider-library-present-p "clojure.stacktrace") (cider-default-err-eval-handler))
(t (cider-default-err-eval-print-handler))))


;; The format of the error messages emitted by Clojure's compiler changed in
;; Clojure 1.10. That's why we're trying to match error messages to both the
Expand Down Expand Up @@ -739,7 +747,10 @@ when `cider-auto-inspect-after-eval' is non-nil."
(cider-emit-interactive-eval-output out))
(lambda (_buffer err)
(cider-emit-interactive-eval-err-output err)
(unless cider-show-error-buffer

(when (or (not cider-show-error-buffer)
(not (cider-connection-has-capability-p 'jvm-compilation-errors)))

;; Display errors as temporary overlays
(let ((cider-result-use-clojure-font-lock nil))
(cider--display-interactive-eval-result
Expand Down
1 change: 1 addition & 0 deletions cider-overlays.el
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ focused."
(let* ((font-value (if cider-result-use-clojure-font-lock
(cider-font-lock-as-clojure value)
value))
(font-value (string-trim-right font-value))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite understand the purpose of this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nbb errors always had a trailing newline and the I think the visual of the overlays is better without the newline.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. This name font-value confused me, as that's just value (which is potentially using font-locking). I was wondering why someone would be trimming the font-value. :D

(used-overlay (when (and point cider-use-overlays)
(cider--make-result-overlay font-value
:where point
Expand Down
3 changes: 2 additions & 1 deletion cider-repl.el
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ you'd like to use the default Emacs behavior use
(make-obsolete-variable 'cider-repl-print-level 'cider-print-options "0.21")

(defvar cider-repl-require-repl-utils-code
'((clj . "(clojure.core/apply clojure.core/require clojure.main/repl-requires)")
'((clj . "(when-let [requires (resolve 'clojure.main/repl-requires)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll have to explain me the need for this change.

Copy link
Contributor Author

@benjamin-asdf benjamin-asdf Dec 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you currently connect to joyride it throws "cannot resolve symbol repl-requires". It was the same for an earlier version of nbb I basiclly want to make the clj connection as un-assuming as possible.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be good to mention such things in comments, so those things are clearer someone doesn't optimize this away. As @ikappaki mentioned in #3274 we don't really have integration tests, so it's not hard to imagine someone doing a change that would break this again for other runtimes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also document somewhere that such REPL automatic util requires are limited to Clojure and the hosted ClojureScript.

(clojure.core/apply clojure.core/require @requires))")
(cljs . "(require '[cljs.repl :refer [apropos dir doc find-doc print-doc pst source]])")))

(defcustom cider-repl-init-code (list (cdr (assoc 'clj cider-repl-require-repl-utils-code)))
Expand Down
6 changes: 5 additions & 1 deletion cider.el
Original file line number Diff line number Diff line change
Expand Up @@ -778,9 +778,13 @@ Generally you should not disable this unless you run into some faulty check."
:safe #'booleanp
:package-version '(cider . "0.17.0"))

(defun cider-clojurescript-present-p ()
"Return non nil when ClojureScript is present."
(nrepl-dict-get (cider-sync-tooling-eval "cljs.core/demunge") "value"))
Copy link
Member

@bbatsov bbatsov Dec 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the advantage of this over the previous check?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#3255 Basically (require 'cljs.core) did not work for nbb.


(defun cider-verify-clojurescript-is-present ()
"Check whether ClojureScript is present."
(unless (cider-library-present-p "cljs.core")
(unless (cider-clojurescript-present-p)
(user-error "ClojureScript is not available. See https://docs.cider.mx/cider/basics/clojurescript for details")))

(defun cider-verify-piggieback-is-present ()
Expand Down
1 change: 1 addition & 0 deletions doc/modules/ROOT/nav.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
* Alternative Platforms
** xref:platforms/overview.adoc[Overview]
** xref:platforms/babashka.adoc[Babashka]
** xref:platforms/scittle_and_friends.adoc[Scittle and Friends]
* Using CIDER
** xref:usage/interactive_programming.adoc[Interactive Programming]
** xref:usage/cider_mode.adoc[Using cider-mode]
Expand Down
29 changes: 29 additions & 0 deletions doc/modules/ROOT/pages/platforms/scittle_and_friends.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
= Scittle, Nbb, Joyride, and future plain repls

The default cider (clj-repl) should be capable of connecting to any
nrepl server that provides minimal functionality.

As such, all of these work:
https://github.com/babashka/nbb[nbb],
https://github.com/babashka/scittle[scittle], https://github.com/BetterThanTomorrow/joyride[joyride]

First start an nrepl server (the project's Readme usually has a section
on starting a nrepl server).

You can use

kbd:[M-x `cider-connect-clj` <RET>]

to connect to any plain Clojure(Script) nrepl server.

Features:

* Eval, load file etc.
* Errors as overlays. (The default cider error buffer is not implemented currently).
* Other nrepl features the server provides; This might be rather minimal.

Nbb, Scittle and Joyride all have quite cool completions already.

== Note

For nbb you can alternatively connect via cljs, see xref:platform/nbb.adoc[Nbb]
20 changes: 20 additions & 0 deletions test/cider-connection-tests.el
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,26 @@
(expect (cider-repls) :to-equal (list a b))
(kill-buffer b)
(expect (cider-repls) :to-equal (list a))
(sesman-unregister 'CIDER session))))))

(describe "cljs capability"
(it "Upgraded clj repl counts as cljs"
(let ((default-directory (expand-file-name "/tmp/some-dir")))
(cider-test-with-buffers
(a b)
(let ((session (list "some-session" a b)))
(with-current-buffer a
(setq cider-repl-type 'clj))
(with-current-buffer b
(setq cider-repl-type 'cljs))
(sesman-register 'CIDER session)
(expect (cider-repls 'cljs) :to-equal (list b))

(with-current-buffer a
(setf cider-connection-capabilities
(append cider-connection-capabilities '(cljs))))

(expect (cider-repls) :to-equal (list a b))
(sesman-unregister 'CIDER session)))))))

(describe "cider--connection-info"
Expand Down