Implement xref interface for dumb-jump

[phikal]

Hi,

this pull request would implement the basic xref interface, just offering definitions for now, but could be expanded to also do apropos or even references in the future. As of writing, it's not perfect, as it doesn't process the results using dumb-jump-handle-results, to remove comments, self-references, etc. (hence just a draft for now).

I would recommend following this up with deprecation notices for the old dumb-jump-go and co., so that #334 might be merged one day, and an additional (local?) minor mode that would add the xref implementation to xref-backend-functions.

[phikal]

The tests fail for Versions <=24.5 (though I can't open the results to see why), since xref depends >=25.1. Instead of trying to work around this, I would recommend bumping the minimal version, since even Debian Stable and CentOS has Emacs 26 now. But if wished for, Emacs 24-compatibility could also be maintained.

[jacktasia]

Thanks for opening this! I don't understand why/how CircleCI has changed where the build results are not available publicly for open source repos. I am almost certain this was not always the case. Does anyone reading this know what's going on with CircleCI in this regard?

Anyway, ideally, we maintain backwards compatibility. Can we just add version and/or is-function-defined guards?

[jacktasia]

@phikal Looks like if you login to CircleCi via Github and select your own username as your org then it will work. You don't have to actually give them any permissions to anything in your account as far as I can tell

[phikal]

83b12a6dc1b56c018d38bc4b38fd56fb736af135 should prevent the tests from failing if not found, and thereby maintain backwards compatibility for Emacs 24.

[jacktasia]

Just in case you can't see into CircleCi Symbol's function definition is void: cl-defmethod is the current issue on the 24.x builds. Seems to be because the (when (featurep 'xref) guard was removed.

[phikal]

Seems to be because the (when (featurep 'xref) guard was removed.

Oh, I seem to have removed it by accident. I'll fix the commits and force-push something tomorrow.

[phikal]

First off: I removed the deprecation notices, since they should probably belong to another pull request.

The last commit (eventually) makes the xref interface behave just like dumb-jump-go. The issue until now was that due to missing "context", a lot more suggestions were generated than are interesting. By overriding xref-backend-identifier-at-point and adding a text-property to store the context in, xref-backend-definitions gets the same context-sensitive suggestions as before. It's hacky, but I guess long we're beyond that point now.

That also fixes the last issue I had, so I'm un-drafting the pull request.

[jacktasia]

@phikal Awesome, thanks! I'll take a closer look either later today or tomorrow.

[phikal]

Jack Angers notifications@github.com writes:

 @@ -3,7 +3,7 @@
  ;; Author: jack angers and contributors
  ;; Url: https://github.com/jacktasia/dumb-jump
  ;; Version: 0.5.3
-;; Package-Requires: ((emacs "24.3") (s "1.11.0") (dash "2.9.0") (popup "0.5.3"))
+;; Package-Requires: ((emacs "25.1") (s "1.11.0") (dash "2.9.0") (popup "0.5.3"))

Is this still true?

No, I missed that, but it's fixed now.

-- Philip K.

[jacktasia]

@phikal Thanks a lot for doing this! What, if anything, do you think makes sense to add to the README.md about this?

[phikal]

Jack Angers notifications@github.com writes:

[1:text/plain Hide]

@phikal Thanks a lot for doing this! What, if anything, do you think makes sense to add to the README.md about this?

I think it would make sense, should I open another pull request?

Also, what was your final take on deprecating dumb-jump-go and co. in favour of xref?

-- Philip K.

[jacktasia]

I think it would make sense, should I open another pull request?

Sure, sounds good. Thanks!

Also, what was your final take on deprecating dumb-jump-go and co. in favour of xref?

I think this needs to stay forever for backwards compatibility unless there's something I am missing. There are too many existing projects and configs out there that reference this function directly.

[andyleejordan]

@jacktasia Does this seem a reasonable way of using dumb-jump as xref if and only if there isn't another (decent) xref engine? For instance, in elisp mode, or modes where lsp are in use, I much prefer using those as the xref backend, but want dumb-jump as a fallback.

(use-package dumb-jump
  :hook (prog-mode . dumb-jump-xref-mode)
  :config
  (defun dumb-jump-xref-mode ()
    (when (eq #'etags--xref-backend (car xref-backend-functions))
      (add-to-list 'xref-backend-functions #'dumb-jump-xref-activate))))

But idk what I'm doing wrt xref 😅

[phikal]

@andschwa dumb-jump-xref-activate can regulate if it should be activated or not, or if it should try a different xref backend. If you want to, a user option could be added to automatically disable dumb-jump under certain conditions (eg. if a major or minor mode is active).

And unless I missed something, there shouldn't be a dumb-jump-xref-mode, it's activated by adding the aforementioned function to xref-backend-functions.

I'll document all of this in the README when I get to that.

[phikal]

I think this needs to stay forever for backwards compatibility unless there's something I am missing. There are too many existing projects and configs out there that reference this function directly.

That was my intention in suggesting to deprecate it, as a way to decrease it's usage. Of course killing it just like that would be too hard, but if it runs, just with a warning, then users might switch.

To rephrase the question: Are you intrinsically opposed to deprecating the old interface, for the newer, simpler one, or are you just arguing from a pragmatic position? I'm of course not unbiased, as I would like to see #334 merged (or to be more precise it's xref variant) one day, and this pull request was fundamentally written to make this more viable.

[jacktasia]

@andschwa Honestly, I too know little about xref since it was released after dumb-jump was created and my problem was already solved. That said, this sounds like what @jojojames has with https://github.com/jojojames/smart-jump

[andyleejordan]

And unless I missed something, there shouldn't be a dumb-jump-xref-mode, it's activated by adding the aforementioned function to xref-backend-functions.

That's just what I named my function which checks if it should add dumb-jump-xref-activate to the buffer local variable xref-backend-functions 😅

dumb-jump-xref-activate can regulate if it should be activated or not, or if it should try a different xref backend.

Oh, I must have missed something. I'll dig into the code a bit more.

[phikal]

Andy Schwartzmeyer notifications@github.com writes:

And unless I missed something, there shouldn't be a dumb-jump-xref-mode, it's activated by adding the aforementioned function to xref-backend-functions.

That's just what I named my function which checks if it should add dumb-jump-xref-activate to the buffer local variable xref-backend-functions 😅

Whoop, I didn't read the code properly. Nevermind then.

dumb-jump-xref-activate can regulate if it should be activated or not, or if it should try a different xref backend.

Oh, I must have missed something. I'll dig into the code a bit more.

The docstring of xref-backend-functions explains everything you need to know:

Each function on this hook is called in turn with no arguments, and should return either nil to mean that it is not applicable, or an xref backend, which is a value to be used to dispatch the generic functions.

And with an additional check in dumb-jump-xref-activate, such as a check like

(not (apply #'derived-mode-p dumb-jump-inhibited-modes))

where dumb-jump-inhibited-modes is a list of major modes where dumb-jump should not be activated. Another approach would just be to change the default value of xref-backend-functions:

(setq-default xref-backend-functions '(dumb-jump-xref-activate))

it's not clean (as in I haven't tested it), but it might work if you know that you'll never be using the etags backend.

-- Philip K.