Locate multiple ports for a single project #3234
Conversation
| (nrepl--port-from-file (expand-file-name ".nrepl-port" dir)) | ||
| (nrepl--port-from-file (expand-file-name "target/repl-port" dir)) | ||
| (nrepl--port-from-file (expand-file-name ".shadow-cljs/nrepl.port" dir)))) | ||
| (defun nrepl-extract-ports (dir) |
There was a problem hiding this comment.
That's a public function, so renaming it would be a breaking change and we want to avoid those. You can just introduce a few function instead and perhaps mark the old function as obsolete.
nrepl-client.el
Outdated
| (nrepl--port-from-file (expand-file-name ".shadow-cljs/nrepl.port" dir)))) | ||
| (defun nrepl-extract-ports (dir) | ||
| "Read ports from applicable repl-port files in directory DIR." | ||
| (list (nrepl--port-from-file (expand-file-name "repl-port" dir)) |
There was a problem hiding this comment.
I'd probably filter out the nil values here.
cider.el
Outdated
| (mapcar (lambda (d) | ||
| (when-let* ((ports (and d (nrepl-extract-ports (cider--file-path d))))) | ||
| (mapcar (lambda (p) | ||
| (when-let* ((p p)) |
There was a problem hiding this comment.
If you apply the filter to the returned ports this when-let will become redundant and the code would read better.
|
Looks good to me overall. This need a changelog entry and perhaps we should explain somewhere in the user docs how CIDER finds nREPL ports automatically in a couple of sentences. |
|
@danmidwood Ping :-) |
I agree with all of the feedback, I will schedule some time this week to make the changes to the PR. |
danmidwood
force-pushed
the
autocomplete-for-multiple-project-ports-on-nrepl-connect
branch
2 times, most recently
from
eae81ad
to
c1211bd
Compare
Searching for a nrepl port to connect to would return a maximum of one port per project, ignoring any others that you might want to connect to. This creates a case where when connecting to a shadow-cljs nrepl server the port is not presented to you as a completion when you already have another nrepl server running. This commit changes nrepl-extract-port to nrepl-extract-ports and the return type from a single port to a list of ports (including nils where a specific project type nrepl port file doesn't exist) to provide the full view of nrepl servers that are available in the project, and then the cider-locate-running-nrepl-ports fn is changed to accommodate that. This fixes clojure-emacs#3140.
danmidwood
force-pushed
the
autocomplete-for-multiple-project-ports-on-nrepl-connect
branch
from
c1211bd
to
afc1296
Compare
|
@bbatsov I have made some changes as well as rebase on master. I marked I didn't make any doc changes. I think the up_and_running.adoc is probably the place to do it but there's nothing there rendered out of date and I feel like adding more may make it more cumbersome. I'm going to be away for a few weeks after this comment so if there's anything else to do then I will catch up with this when I'm back. |
|
Thanks! |
Searching for a nrepl port to connect to would return a maximum of one
port per project, ignoring any others that you might want to connect
to. This creates a case where when connecting to a shadow-cljs nrepl
server the port is not presented to you as a completion when you
already have another nrepl server running.
This commit changes nrepl-extract-port to nrepl-extract-ports and the
return type from a single port to a list of ports (including nils
where a specific project type nrepl port file doesn't exist) to
provide the full view of nrepl servers that are available in the
project, and then the cider-locate-running-nrepl-ports fn is changed
to accommodate that.
This fixes #3140.
Replace this placeholder text with a summary of the changes in your PR.
The more detailed you are, the better.
Before submitting the PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):
eldev test)eldev lint) which is based onelisp-lintand includescheckdoc, check-declare, packaging metadata, indentation, and trailing whitespace checks.Thanks!
If you're just starting out to hack on CIDER you might find this section of its
manual extremely useful.