Connection fixes #2458
Connection fixes #2458
Conversation
…entinel And rename :no-server-kill to :keep-server process property
…uffer from the docs
Needed when cider-repl-pop-to-buffer-on-connect is 'display-only
cider-connection.el
Outdated
| (defun cider--get-reusable-repl-buffer (params) | ||
| "Find connection-less REPL buffer. | ||
| PARAMS is a plist as received by `cider-repl-create'. REPL with similar | ||
| connection PARAMS is proffered. Return nil if none such buffers exist." |
cider-connection.el
Outdated
| (host (plist-get params :host)) | ||
| (port (plist-get params :port)) | ||
| (cljsp (string-match-p "cljs" (plist-get params :repl-type))) | ||
| (build-pred (lambda (&optional no-port no-host no-proj-dir) |
There was a problem hiding this comment.
I'm a bit puzzled here. Shouldn't we always try to reuse a repl matching the host and the project dir? I'm guessing you wrote the code like this to account for REPL without a project dir, right?
There was a problem hiding this comment.
I wrote it to account for possibility to to re-use the REPL even from a different project or host. First try to match all parameters, then all except port, then all except port host etc.
I have no specific preferences over how this should work and I don't think it's important. This one felt like the most flexible approach.
There was a problem hiding this comment.
That's fine by me, but that should probably be documented somewhere in the manual as well, so people know what to expect.
cider-connection.el
Outdated
| (or no-proj-dir (equal proj-dir (plist-get bparams :project-dir))) | ||
| (or no-host (equal host (plist-get bparams :host))) | ||
| (or no-port (equal port (plist-get bparams :port)))))))) | ||
| (repl (car (or |
There was a problem hiding this comment.
I think we should show the user a completing-read with all dead REPLs if there's more than one, instead of assuming they'll go with the best match in case of ambiguity.
There was a problem hiding this comment.
Yes. This could be. But then one would need to sort the repls in the order of similarity.
There was a problem hiding this comment.
I guess that'd be trivial - you just have to concatenate the lists you got from the consecutive filter invocations.
There was a problem hiding this comment.
All fixed. I went for explicit scoring - highest score for project, lower for host, lowest for port. Should do the trick.
cider-connection.el
Outdated
| (or no-port (equal port (plist-get bparams :port)))))))) | ||
| (repl (car (or | ||
| (seq-filter (funcall build-pred) repls) | ||
| (seq-filter (funcall build-pred t) repls) |
There was a problem hiding this comment.
This code would be more readable with 6 t params in it. :-)
There was a problem hiding this comment.
Yeh. I love those too ;)
cider-connection.el
Outdated
| (seq-filter (funcall build-pred t t) repls) | ||
| (seq-filter (funcall build-pred t t t) repls))))) | ||
| (and repl | ||
| (y-or-n-p (format "A dead REPL exists %s, reuse? " repl)) |
There was a problem hiding this comment.
I think %s should go before the word exists.
| @@ -887,6 +887,7 @@ nil." | |||
|
|
|||
| ;;; User Level Connectors | |||
|
|
|||
| ;;;###autoload (autoload 'cider-start-map "cider" "CIDER jack-in and connect keymap." t 'keymap) | |||
There was a problem hiding this comment.
I didn't even know this was possible. :-)
vspinu
force-pushed
the
connection-fixes
branch
from
b0fc713
to
1a66eda
Compare
|
Great work! |
Fix most of the connection related issues except of #2447 for which the details are not yet clear, and #2446 which would re-quire a bit more of the design thinking.