branch: elpa/cider
commit 3dfb1c858279f0b02472a57d8ebc9b2c62da3ae6
Author: Bozhidar Batsov <[email protected]>
Commit: Bozhidar Batsov <[email protected]>
Route stdin and interrupts to the session that requested input
The need-input handler answered via `cider-current-repl' and, on cancel,
interrupted the whole connection. With several connections (clj+cljs,
sibling REPLs, a tooling eval) that could send the input to the wrong
session - leaving the blocked read hung - or interrupt more than the one
evaluation waiting for input.
Thread the need-input response through to the handler so it can use the
response's `session' to pick the exact connection and its `id' to
interrupt just the blocked evaluation. The three dispatch sites now pass
the response; `cider-need-input' takes it as an optional argument and
falls back to the old behavior when absent.
---
CHANGELOG.md | 1 +
lisp/cider-client.el | 33 ++++++++++++++++++++++++++++-----
lisp/cider-eval.el | 2 +-
lisp/nrepl-client.el | 13 ++++++++-----
test/cider-client-tests.el | 44 ++++++++++++++++++++++++++++++++++++++++++++
test/cider-eval-tests.el | 5 +++--
6 files changed, 85 insertions(+), 13 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8ba728b0d3..a7bd86acf2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -47,6 +47,7 @@
### Bugs fixed
+- [#4107](https://github.com/clojure-emacs/cider/pull/4107): Route stdin input
(and a cancel's interrupt) to the exact connection that requested it, so reads
no longer misroute to the wrong session when several connections are active.
- [#4106](https://github.com/clojure-emacs/cider/pull/4106): Cancelling a
stdin prompt (when evaluated code reads input) now interrupts the evaluation
instead of sending a malformed `stdin` op that left the read hung. The prompt
also names the REPL when the session has more than one connection.
- [#4093](https://github.com/clojure-emacs/cider/pull/4093): Fix a crash
(`wrong-type-argument stringp nil`) when a REPL result is truncated by the
print middleware.
- [#4089](https://github.com/clojure-emacs/cider/issues/4089): Fix
`cider-macroexpand-undo` failing with a read-only error in the macroexpansion
buffer.
diff --git a/lisp/cider-client.el b/lisp/cider-client.el
index ebc6a652d0..e8e1c05e3f 100644
--- a/lisp/cider-client.el
+++ b/lisp/cider-client.el
@@ -1198,15 +1198,36 @@ evaluation is waiting for input."
(format "Stdin for %s (C-c C-c to cancel): " (buffer-name repl))
"Stdin (C-c C-c to cancel): "))
-(defun cider-need-input (buffer)
+(defun cider--connection-for-session (session)
+ "Return the connection buffer whose nREPL SESSION matches, or nil.
+Matches the main or the tooling session, so stdin and interrupts can be
+routed to the exact connection that requested input rather than whichever
+one `cider-current-repl' happens to resolve to."
+ (when session
+ (seq-find (lambda (buf)
+ (and (buffer-live-p buf)
+ (or (equal session (buffer-local-value 'nrepl-session
buf))
+ (equal session (buffer-local-value
'nrepl-tooling-session buf)))))
+ (buffer-list))))
+
+(defun cider-need-input (buffer &optional response)
"Handle a need-input request from BUFFER.
Read a line at the minibuffer and send it to the blocked evaluation.
Cancelling the prompt interrupts that evaluation instead: the previous
behavior sent nil, which bencodes to an empty list rather than a string, so
-the `stdin' op was malformed and the read never unblocked cleanly."
+the `stdin' op was malformed and the read never unblocked cleanly.
+
+RESPONSE is the nREPL need-input response (the message whose status is
+\"need-input\"). When given, its `session' selects the exact connection to
+answer (important with several connections) and its `id' lets a cancel
+interrupt just the blocked evaluation. Without it, both fall back to
+`cider-current-repl' and a connection-wide interrupt."
(with-current-buffer buffer
- (let ((repl (cider-current-repl))
- (map (make-sparse-keymap)))
+ (let* ((session (and response (nrepl-dict-get response "session")))
+ (id (and response (nrepl-dict-get response "id")))
+ (repl (or (cider--connection-for-session session)
+ (cider-current-repl)))
+ (map (make-sparse-keymap)))
(set-keymap-parent map minibuffer-local-map)
(define-key map (kbd "C-c C-c") #'abort-recursive-edit)
(condition-case nil
@@ -1216,7 +1237,9 @@ the `stdin' op was malformed and the read never unblocked
cleanly."
repl))
(quit
(when (buffer-live-p repl)
- (cider-interrupt-repl repl)))))))
+ (if id
+ (nrepl-request:interrupt id (cider-interrupt-handler repl) repl)
+ (cider-interrupt-repl repl))))))))
(provide 'cider-client)
diff --git a/lisp/cider-eval.el b/lisp/cider-eval.el
index 2e398a3ffe..e6009b2ed5 100644
--- a/lisp/cider-eval.el
+++ b/lisp/cider-eval.el
@@ -268,7 +268,7 @@ have the same semantics as in `nrepl-make-eval-handler'."
(nrepl-dbind-response response (ns)
(message "Namespace `%s' not found." ns)))
(when (member "need-input" status)
- (cider-need-input buffer)))
+ (cider-need-input buffer response)))
:on-eval-error (or on-eval-error
(lambda () (cider-default-err-handler buffer)))))
diff --git a/lisp/nrepl-client.el b/lisp/nrepl-client.el
index b2a2828bb6..3fe466f452 100644
--- a/lisp/nrepl-client.el
+++ b/lisp/nrepl-client.el
@@ -731,7 +731,9 @@ Called with one argument: the REPL buffer.")
(defvar nrepl-need-input-handler-function nil
"Function to call when the server requests stdin input.
-Called with one argument: the REPL buffer.
+Called with two arguments: the REPL buffer and the nREPL need-input response
+\(the message whose status is \"need-input\"; its `session' and `id' identify
+the blocked evaluation).
When nil, need-input requests are ignored.")
(defvar nrepl-namespace-handler-function nil
@@ -888,7 +890,7 @@ results in the corresponding response branch being a no-op."
(message "Namespace `%s' not found." ns)))
(when (and (member "need-input" status)
nrepl-need-input-handler-function)
- (funcall nrepl-need-input-handler-function buffer)))
+ (funcall nrepl-need-input-handler-function buffer response)))
:on-eval-error (cond (eval-error-handler
(lambda () (funcall eval-error-handler buffer)))
(nrepl-err-handler-function
@@ -972,11 +974,12 @@ positional shim retained for backward compatibility."
(not (and abort-on-input
(input-pending-p))))
(setq status (nrepl-dict-get response "status"))
- ;; If we get a need-input message then the repl probably isn't
going
- ;; anywhere, and we'll just timeout. So we forward it to the user.
+ ;; On a need-input response the eval is blocked reading stdin and
+ ;; won't finish on its own, so forward it to the user (who provides
+ ;; input or cancels) instead of waiting for the timeout.
(if (and (member "need-input" status)
nrepl-need-input-handler-function)
- (progn (funcall nrepl-need-input-handler-function
(current-buffer))
+ (progn (funcall nrepl-need-input-handler-function
(current-buffer) response)
;; If the user took a few seconds to respond, we might
;; unnecessarily timeout, so let's reset the timer.
(setq time0 (current-time)))
diff --git a/test/cider-client-tests.el b/test/cider-client-tests.el
index b0d5747f48..65fdea275d 100644
--- a/test/cider-client-tests.el
+++ b/test/cider-client-tests.el
@@ -601,3 +601,47 @@
(cider-need-input (current-buffer)))
(expect 'cider-interrupt-repl :to-have-been-called-with repl)
(expect 'nrepl-request:stdin :not :to-have-been-called)))
+
+(describe "cider--connection-for-session"
+ (it "finds the connection buffer whose session matches"
+ (let ((a (generate-new-buffer " *repl-a*"))
+ (b (generate-new-buffer " *repl-b*")))
+ (unwind-protect
+ (progn
+ (with-current-buffer a (setq-local nrepl-session "sess-a"))
+ (with-current-buffer b (setq-local nrepl-tooling-session "tool-b"))
+ (expect (cider--connection-for-session "sess-a") :to-be a)
+ (expect (cider--connection-for-session "tool-b") :to-be b)
+ (expect (cider--connection-for-session "nope") :to-be nil)
+ (expect (cider--connection-for-session nil) :to-be nil))
+ (kill-buffer a)
+ (kill-buffer b)))))
+
+(describe "cider-need-input session routing"
+ :var (conn)
+ (before-each
+ (setq conn (generate-new-buffer " *repl-s1*"))
+ (with-current-buffer conn (setq-local nrepl-session "s1"))
+ (spy-on 'cider-repls :and-return-value (list conn))
+ (spy-on 'nrepl-request:stdin)
+ (spy-on 'nrepl-request:interrupt)
+ (spy-on 'cider-interrupt-repl)
+ (spy-on 'cider-interrupt-handler :and-return-value #'ignore))
+ (after-each
+ (when (buffer-live-p conn) (kill-buffer conn)))
+
+ (it "sends stdin to the connection named by the response session"
+ (spy-on 'read-from-minibuffer :and-return-value "hi")
+ (with-temp-buffer
+ (cider-need-input (current-buffer) '(dict "session" "s1" "id" "42")))
+ (expect 'nrepl-request:stdin :to-have-been-called)
+ ;; third arg is the connection - the one matching the session, not
+ ;; whatever `cider-current-repl' would resolve to
+ (expect (nth 2 (spy-calls-args-for 'nrepl-request:stdin 0)) :to-be conn))
+
+ (it "interrupts only the blocked evaluation, by id, on cancel"
+ (spy-on 'read-from-minibuffer :and-call-fake (lambda (&rest _) (signal
'quit nil)))
+ (with-temp-buffer
+ (cider-need-input (current-buffer) '(dict "session" "s1" "id" "42")))
+ (expect 'nrepl-request:interrupt :to-have-been-called-with "42" #'ignore
conn)
+ (expect 'cider-interrupt-repl :not :to-have-been-called)))
diff --git a/test/cider-eval-tests.el b/test/cider-eval-tests.el
index 0d230c119d..19bb2af817 100644
--- a/test/cider-eval-tests.el
+++ b/test/cider-eval-tests.el
@@ -428,11 +428,12 @@
(expect custom-called :to-be t)
(expect 'cider-default-err-handler :not :to-have-been-called)))
- (it "prompts via cider-need-input on need-input status"
+ (it "prompts via cider-need-input on need-input status, passing the response"
(spy-on 'cider-need-input)
(let ((handler (cider-make-eval-handler :buffer 'sentinel-buf)))
(funcall handler '(dict "id" "1" "status" ("need-input"))))
- (expect 'cider-need-input :to-have-been-called-with 'sentinel-buf))
+ (expect 'cider-need-input :to-have-been-called-with
+ 'sentinel-buf '(dict "id" "1" "status" ("need-input"))))
(it "prints a message on namespace-not-found status, with the ns name"
(spy-on 'message)
