dumb-jump icon indicating copy to clipboard operation
dumb-jump copied to clipboard

[Question] How to combine with lsp-mode?

Open condy0919 opened this issue 3 years ago • 24 comments

lsp-mode overwrites xref-backend-functions using the following code which makes xref-backend-functions become buffer-local:

(add-hook 'xref-backend-functions #'lsp--xref-backend nil t)

How to combine dumb-jump with lsp-mode so that when lsp-mode finds nothing M-. (xref-find-definitions) can be fallback to dumb-jump?

condy0919 avatar Jul 18 '20 15:07 condy0919

This doesn't seem that doable, as of now. Xref checks all functions in xref-backend-functions until the first one returns nil, but since LSP doesn't do any checking, it would always run.

The real problem is that xref doesn't (currently) have an option to say "if the first method fails, try the next function in xref-backend-functions".

But assuming that could be fixed (and it is doable, I think I remember this being mentioned on the emacs-devel mailing list), you'd probably just have to add a hook to lsp-mode, to add dumb-jump-xref-activate to xref-backend-functions, unless LSP does some additional magic.

phikal avatar Jul 19 '20 10:07 phikal

I was annoyed with all the "obsolete" compiler warnings I get and I am trying to understand why this deprecation. I also saw this thread, so I have a question:

Why deprecating dumb-jumps interface? Dumb-jump could just use xref as another "source" under the hood so to say. So methods like dumb-jumb-go could actually call xref-whatever which really is call into lsp behind the scene, and if it returns nada, it could do it's own internal magic and search gymnastics? We don't need lsp to call dumb-jumb as it's source. We will anyway bind this functionality to a key, so what function is bound to a key, like dumb-jumb-go or lsp-find-references or whatever it might be called is irrelevant. Or do I missunderstand how it works :-)?

amno1 avatar Jul 20 '20 11:07 amno1

@amno1 The rationale was explained in #349 (and related/linked threads), so it's probably better to discuss the issue over there. Basically it boils down to not having to maintain multiple interfaces.

But to answer your immediate question, if you want silent Emacs' "obsolete" warnings, you can just unmark it as so

(put 'dumb-jump-go 'byte-obsolete-info nil)

Dumb-jump could just use xref as another "source" under the hood so to say.

This seems to capture the issue on it's head. The goal with using xref is to integrate dumb-jump into the standard interface, so that the same keybindings can be used, regardless of what system (etags, dumb-jump, LSP, etc.) is being used. With xref, dumb-jump becomes a method for finding definitions, amongst others, instead of another user-inferface you and I have to learn, with it's own practices and quirks.

I hope this could help you make sense of the change, and I'm of course sorry for any annoyances this might have caused.

phikal avatar Jul 20 '20 11:07 phikal

Sorry I didn't know there was other thread. I usually don't look at packages untill I get some problem.

This seems to capture the issue on it's head.

Yeah, I know :-). When I saw the problem, I was just like, reverse it and problem solved.

With xref, dumb-jump becomes a method for finding definitions, amongst others, instead of another user-inferface you and I have to learn, with it's own practices and quirks.

I know, I am aware of the philosophy behind xref; but then you have LSP hijacking it as described. I don't see it as much of a problem, since I won't be learning much of either xref or dumb-jump. We just bind this to keys so we can jump to declaration/implementation and forgett what things are called (at least I function like that). I haven't used xref before anyway, so I don't really care about it. But I understand if other folks like it.

amno1 avatar Jul 20 '20 14:07 amno1

Arthur Miller [email protected] writes:

This seems to capture the issue on it's head.

Yeah, I know :-). When I saw the problem, I was just like, reverse it and problem solved.

I don't mean to be rude, but I don't see the problem? No workflow should have been interrupted by the switch to xref, @jacktasia was very adamant about this, and using xref internals would be unreliable and hack'ish.

I know, I am aware of the philosophy behind xref; but then you have LSP hijacking it as described. I don't see it as much of a problem, since I won't be learning much of either xref or dumb-jump.

It's an implementation detail of xref. I think it is a mistake, and will be working on it personally, but from what I've seen, it's not an essential issue.

We just bind this to keys so we can jump to declaration/implementation and forgett what things are called (at least I function like that). I haven't used xref before anyway, so I don't really care about it. But I understand if other folks like it.

The reason I implemented xref and pushed for it to become the standard UI, is that I think integrating into Emacs default subsystems is to be preferred over building parallel systems. I have to admit that part of my motivation is also a "consciousness campaign" for this "realisation".

As already mentioned, old users can go on using the legacy interface (though they are politely nudged to reconsider). New users on the other hand will not have to learn anything new, and get to keep their existing muscle memory.

-- Philip K.

phikal avatar Jul 20 '20 16:07 phikal

I don't mean to be rude, but I don't see the problem?

I was referring to this:

This doesn't seem that doable, as of now. Xref checks all functions in xref-backend-functions until the first one returns nil, but since LSP doesn't do any checking, it would always run.

Then re-write relevant lsp functions and install them when you detect lsp-mode so they check for other backends. Should work too, no? It is elisp afterall; we can always rewrite and re-evalute any function. Or maybe advising would work too in this case, with :after or :around keywords?

The reason I implemented xref and pushed for it to become the standard UI, is that I think integrating into Emacs default subsystems is to be preferred over building parallel systems.

Sure I agree that we should have less parallel solutions, definitely. I just want to jump do darn include file when I press a key, how it is implemented, I don't really care :-). Sorry if I sound uninterested. I am not an xref user, so I really don't care about it, at least currently. If you can get things to work with it, go for it, I have nothing against it. I am not saying xref should not be used, by all means. What I am saying is I don't wanna be "bothered" with it If I don't have to. Kind-of :-).

I don't wanna sound rude either, I am trying to express idea that things should "just work". I have no problems if I am using xref "under the hood", but I don't wont to re-confgure, re-flow, etc my config.

Also I think we can have two different interfaces, old dumb-jumb and xref, I don't see why the old one should be deprecated if xref is present. It is equally possible to use both together too. That is just persnal opinion. Idea of Emacs is to let people tweak everything the way they want, not to force them into anything.

amno1 avatar Jul 21 '20 10:07 amno1

Then re-write relevant lsp functions and install them when you detect lsp-mode so they check for other backends. Should work too, no? It is elisp afterall; we can always rewrite and re-evalute any function. Or maybe advising would work too in this case, with :after or :around keywords?

It's not customary for packages to install advice, others say "it's the last resort before modifying the source code". Then you have the issue that some don't use LSP, but they use Eglot (I use neither, but if I had to I'd use the latter). So that should be supported too. Then there are those who use irony for C++/C, those who use X for Y, etc. So then dumb-jump would have to be aware of all of these jumping mechanisms, which stands in conflict with a separation of concerns.

What you might be interested in is smart-jump?

What I am saying is I don't wanna be "bothered" with it If I don't have to.

I don't wanna sound rude either, I am trying to express idea that things should "just work". I have no problems if I am using xref "under the hood", but I don't wont to re-confgure, re-flow, etc my config.

I guess what I find confusing is your resistance to xref? Even the term "xref user" seems like a exaggeration, it's just a different key (by default), that provides a generic interface to solve the problem of source-jumping. The workflow stays the same, the reconfiguration needed is minimal -- and all of it is optional.

Also I think we can have two different interfaces, old dumb-jumb and xref, I don't see why the old one should be deprecated if xref is present.

A reason is that I'm working on #334 that would fundamentally rework and clean up the internals of dumb-jump's implementation, making it more maintainable, extensible and standard-compliant (dropping libraries such as dash, s and popup). Porting the legacy interface is possible, but not pretty.

Ultimately, what I think that the deprecation should signal is that the legacy interface should only receive bug-fixes, not feature improvements.

phikal avatar Jul 21 '20 16:07 phikal

I guess what I find confusing is your resistance to xref? Even the term

"xref user" seems like a exaggeration, it's just a different key (by default), that provides a generic interface to solve the problem of source-jumping. The workflow stays the same, the reconfiguration needed is minimal -- and all of it is optional.

I am just not familiar with it, so no idea how it works or what not :-). I am sure you gonna make it fine. Thanks for dumb-jump, I like it. I will check smart-jump too one day, thanks.

amno1 avatar Jul 21 '20 16:07 amno1

I guess what I find confusing is your resistance to xref? Even the term

"x" stands for cross, and "ref" for reference. It's the "Cross-referencing" framework. Serious names were never Emacs' strength.

I am just not familiar with it, so no idea how it works or what not :-)

For you (or anyone else reading this thread), everything you need to know is documented in the Manual.

Thanks for dumb-jump, I like it.

I feel obliged to point out that I'm not the maintainer, I just recently implemented a few patches and am trying to help @jacktasia :)

phikal avatar Jul 21 '20 18:07 phikal

For you (or anyone else reading this thread), everything you need to know is documented in the Manual.

Yes, dude, we all know there is a manual. Thank you for stating the obvious. If I don't care about your package, than I don't; to me it appears a bit offensive if you are pushing things this hard. It is not your business what package I care about or not (FYI, if you didn't got it and I have to spell it out, I am not even reluctant to xref, I already told you I just don't care to actively get into it).

I feel obliged to point out that I'm not the maintainer, I just recently implemented a few patches and am trying to help @jacktasia :)

If wasn't to you personally, it was to creator and everyone else who helped making dumb-jumb a great package. It was also a polite way of saying I am not interested to continue this discussion with you.

amno1 avatar Jul 22 '20 08:07 amno1

I also want to know how to enable dumb-jump with lsp-mode if dumb-jump-mode is obsolete.

seagle0128 avatar Aug 09 '20 06:08 seagle0128

dumb-jump-mode is only transitively obsolete, but it was never necessary to use dumb jump. The actual change was deprecating dumb-jump-go & co. Activating dumb jump via the new interface is described here.

phikal avatar Aug 09 '20 09:08 phikal

dumb-jump-mode is only transitively obsolete, but it was never necessary to use dumb jump. The actual change was deprecating dumb-jump-go & co. Activating dumb jump via the new interface is described here.

But xref-backend-functions is overrided with lsp-mode as mentioned above.

(add-hook 'xref-backend-functions #'lsp--xref-backend nil t)

seagle0128 avatar Aug 09 '20 16:08 seagle0128

Then you have to use lsp-mode's hooks. But if your intention is to have LSP fallback on dumb-jump, then that's (currently) not possible, as mentioned before. dumb-jump could fall back on another Xref backend though, but I'd guess that that isn't your intention?

phikal avatar Aug 09 '20 17:08 phikal

So, I also don't understand why deprecated the old interfaces since the new method is not able to provide backwards compatibilities. It's annoying.

seagle0128 avatar Aug 10 '20 01:08 seagle0128

Deprecation is rarely pretty or enjoyable, but it's done precisely to maintain backwards compatibility while changing something. If you insist, I've mentioned above how to silence the warnings.

I'd still like to know what specific functionality you're missing in the new interface? When comparing the current functionality with that of the legacy interface, I see:

  • dumb-jump-go-> xref-find-definitions with the dumb-jump activator installed
  • dumb-jump-back -> xref-pop-marker-stack
  • dumb-jump-quick-look -> xref-posframe
  • dumb-jump-go-other-window -> xref-find-definitions-other-window (bound to C-x 4 ., also works with ...-other-frame (C-x 5 .))
  • dumb-jump-go-prompt -> xref-find-definitions with a prefix argument (usually C-u M-.)

The only commands without direct analogues in xref are the dumb-jump-go-prefer-external and dumb-jump-go-prefer-external-other-window pair, although that could be hacked too, if there's a demand for that.

phikal avatar Aug 10 '20 07:08 phikal

AFAIK, the compatibility issues are

  1. Not working well with lsp-mode. How to fallback if lsp-find-definition fails.
  2. When execute command via hydra following by README, the warnings show up.

seagle0128 avatar Aug 10 '20 15:08 seagle0128

Well the first issue will be fixed in xref, or that's what I heard. In the meantime, someone should tell the LSP people to only append their xref object to xref-backend-functions, instead of reserving it for themselves.

And I noticed the hydra issue yesterday, and that should probably be either removed or updated. As mentioned in the other thread I don't know enough about hydra to say more about that.

phikal avatar Aug 10 '20 16:08 phikal

For non-posframe based quicklook there is also https://github.com/iqbalansari/emacs-source-peek which works really well (uses https://github.com/cpitclaudel/quick-peek)

terlar avatar Aug 10 '20 20:08 terlar

Just want to add usecase when dumb-jump-mode is still superior to xref solution. I'm mostly writing python using lsp-mode, and lsp's jump to definition works well enough most of the time, except for writing pytest test. In pytest you may have input parameter in test function named 'somefunc', and function named 'somefunc' somewhere else, and pytest will launch function somefunc before test starts. Ofcourse if I'll try to jump to definition with lsp, it'll find nothing (from python point of view, it's just name), but it works amazing with dumb-jump. So, for me it'll be optimal to have an ability to disable or enable dumb-jump mode per-project or even per-file (for example, via dir-locals).

jumper047 avatar Oct 08 '22 21:10 jumper047

@jumper047 Does the Xref interface behave the way you want to if you disable lsp-mode?

So, for me it'll be optimal to have an ability to disable or enable dumb-jump mode per-project or even per-file (for example, via dir-locals).

You should be able to do this by adding a

(mode . dumb-jump)

to your .dir-locals.el, as described in (emacs) Directory Variables.

phikal avatar Oct 09 '22 11:10 phikal

You should be able to do this ...

Yep, and it's working fine. Just wanted to add another case where old mechanics are useful.

Does the Xref interface behave the way you want to if you disable lsp-mode?

Honestly, didn't test this - even if it's works it's not the solution, because I need all lsp-mode features except jumping to definition.

jumper047 avatar Oct 09 '22 11:10 jumper047

Does the Xref interface behave the way you want to if you disable lsp-mode?

Honestly, didn't test this - even if it's works it's not the solution, because I need all lsp-mode features except jumping to definition.

Of course, but I am not proposing this as a solution, but to find out if adding a small wrapper function like

    (defun i-really-want-to-dumb-jump ()
      "Call `xref-find-definitions' but force the usage of Dumb Jump"
      (interactive)
      (let ((xref-backend-functions '(dumb-jump-xref-activate)))
        (funcall-interactively #'xref-find-definitions)))

would be an alternative.

What I am getting at is that the you want behaviour shouldn't have to depend on what interface you use.

phikal avatar Oct 09 '22 15:10 phikal

Just checked and, sadly, with no luck - even after lsp mode disabled lsp--xref-backend remains the only xref function for buffer. Your wrapper function doesn't work (can't figure out why, but funcall-interactively of that function doesn't works as expected), but I like the idea - on my taste xref interface is better then embedded in dumb-jump. But to cover my case (enable dumb jump only for part of the codebase), I still need minor mode to manage keybindings..

jumper047 avatar Oct 09 '22 16:10 jumper047