Allow fallback to xref backends if cider-nrepl is unloaded #3145
Conversation
This commit addresses the following use-case: Connecting to a running REPL which does not have `cider-nrepl` middleware loaded. This can happen when connecting to remote REPLs, and even locally (admittedly minor use-case) if you don't want to turn the middleware on. If `cider--xref-backend` returns nil, `xref` will move on to the next backend. This allows using something like `dumb-jump` to move around in the code-base, while also getting the benefits of a REPL window.
| 'cider) | ||
| ;; Check if `cider-nrepl` middleware is loaded. Allows fallback to other xref | ||
| ;; backends, if cider-nrepl is not loaded. | ||
| (when (cider-nrepl-op-supported-p "ns-path") |
There was a problem hiding this comment.
Won't it be better to check about this in the backend itself? Or if we go with your proposal I'd remove the connected checks in the backend implementation.
There was a problem hiding this comment.
I'm not sure I understand. I made the change in this particular function because of this line:
(when cider-use-xref
(add-hook 'xref-backend-functions #'cider--xref-backend cider-xref-fn-depth 'local))
The documentation for xref-backend-functions says that the provided function -- cider--xref-backend in this case -- should return either nil (not applicable) or an xref-backend (value for function dispatch). Once a value is returned, that backend is the only one which is used.
In the case of cider, once the backend function is registered, it will always return a truthy value of 'cider. Registration is done when cider-mode is invoked, and undone in cleanup.
Hence, I think, a basic check should be done in cider--xref-backend and better, more detailed checks should be done in the actual functions (as are implemented today).
There was a problem hiding this comment.
But will this work properly if someone sideloads the middleware later? The way you describe it seems that that if the backend evaluates to nil at registration time it will just be discarded until cider-mode gets reloaded.
|
It will work properly if cider-nrepl is side loaded.
This function (and every other function registered with
`xref-backend-functions`) will be run on every `M-.` command.
Basically, `xref-backend-functions` is a list of functions to try and get a
xref backend. The first of these functions to return a truthy value is used
to try and find an xref for thing at point. The function
`cider--xref-backend` is added to this list and removed from this list only
by `cider-mode`.
This commit only changes whether `cider--xref-backend` returns a truthy
value or not. Previously, the function _always_ returned a truthy value,
whether cider actually supported the relevant functionality or not. Now, it
returns true only when `cider-nrepl` middleware is loaded, allowing other
xref backends to work alongside cider-mode when `cider-nrepl` is not loaded.
Hope this answers your question.
…On Thu, Feb 3, 2022, 7:36 PM Bozhidar Batsov ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In cider-find.el
<#3145 (comment)>:
> @@ -232,7 +232,10 @@ thing at point."
;; CIDER's xref backend was added in CIDER 1.2.
(defun cider--xref-backend ()
"Used for xref integration."
- 'cider)
+ ;; Check if `cider-nrepl` middleware is loaded. Allows fallback to other xref
+ ;; backends, if cider-nrepl is not loaded.
+ (when (cider-nrepl-op-supported-p "ns-path")
But will this work properly if someone sideloads the middleware later? The
way you describe it seems that that if the backend evaluates to nil at
registration time it will just be discarded until cider-mode gets reloaded.
—
Reply to this email directly, view it on GitHub
<#3145 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAUUHRFKGBUZXVJ3PQIWP3UZKDWXANCNFSM5NLSU3LQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Got it. I just wasn't sure when exactly is this function invoked. |
This commit addresses the following use-case: Connecting to a running
REPL which does not have
cider-nreplmiddleware loaded.This can happen when connecting to remote REPLs, and even
locally (admittedly minor use-case) if you don't want to turn the
middleware on.
If
cider--xref-backendreturns nil,xrefwill move on to the nextbackend. This allows using something like
dumb-jumpto move aroundin the code-base, while also getting the benefits of a REPL window.
Before submitting the PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):
eldev test): Did not check this, I hope they are.eldev lint) which is based onelisp-lintand includes: Did not check linter, but there are no errors fromM-x checkdoccheckdoc, check-declare, packaging metadata, indentation, and trailing whitespace checks.Thanks!