Skip to content

lsp-find-definition does not push marker onto stack #1034

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jimmywongroo opened this issue Sep 20, 2019 · 11 comments
Closed

lsp-find-definition does not push marker onto stack #1034

jimmywongroo opened this issue Sep 20, 2019 · 11 comments

Comments

@jimmywongroo
Copy link

jimmywongroo commented Sep 20, 2019

Describe the bug
In Emacs 27, (lsp-find-locations) does not push mark to stack, so xref's M-, does not work. The reason is xref-show-definitions-function does not push a mark before displaying the definition.

To Reproduce
Steps to reproduce the behavior:

  1. Use any (lsp-find-definition) function when point is on a symbol
  2. Type M-,

Expected behavior
Emacs will switch back to the buffer the (lsp-find-definition) command was issued from.

Which Language Server did you use
lsp-typescript

OS
macOS

@firstrow
Copy link

I'm currently using next commands for navigation and it works as expected.

  xref-find-definitions
  xref-pop-marker-stack

So M-. and M-, are working good without any configuration.

@yyoncho
Copy link
Member

yyoncho commented Oct 10, 2019

Yes, xref-find-* methods work fine and at this point, we should deprecate the lsp-find-* equivalents except for lsp-find-implementations which does not have the xref version.

@JohnC32
Copy link
Contributor

JohnC32 commented Oct 18, 2019

I tried using xref. It doesn't let one go back and forth. I've been using RTags, https://github.com/Andersbakken/rtags navigation capabilities for a while and the ability to go forward and backward is very a major benefit in code navigation. See their rtags-location-stack-back and 'rtags-location-stack-forward. Please consider adding both directions. Thanks

@seagle0128
Copy link
Collaborator

xref works fine for me.

@JohnC32
Copy link
Contributor

JohnC32 commented Oct 18, 2019

xref does work for some items, but not others.

For example, run M-x xref-find-references and from the list of items shown, pick one. You can then run M-x xref-pop-marker-stack to jump back. However, there's no jump forward. If I want to go forward to where I just was, I need to re-run M-x xref-find-references and then pick the desired item, or I can use ^X^B, etc. There's no clean way to move forward.

In addition, xref doesn't work for other key items such as M-x lsp-find-implementation (to find all implementations of virtual C++ methods), select one of them and you can jump back.

The paradigm in RTags is super helpful. It covers all navigation capabilities, definition, references, virtual method implementations, etc. and lets me go back, then forward. This makes it very easy to focus on the code an not having to worry about switching between buffers.

The top navigation items I find for C++ code are: definitions, references, virtual method implementations, and navigation back and forward. The last two may seem not that important, but once you have them they are a major win.

I could write wrappers around the key lsp/xref commands to give back and forward navigation, though it would be much nicer to see these concepts as part of lsp-mode. Thoughts?

@yyoncho
Copy link
Member

yyoncho commented Oct 18, 2019

I could write wrappers around the key lsp/xref commands to give back and forward navigation,

There was an implementation of forward for xref in stackoverflow(?) but I am unable to find it out now.
Maybe @dgutov could help with that.

As for lsp-find-implementation, it does not work indeed. I think that it only needs to push the marker on the stack. I prefer this solution instead of implementing backward/forward on top of xref since it will lead to less code.

@dgutov
Copy link
Contributor

dgutov commented Oct 18, 2019

@yyoncho I haven't seen it, and I'm sure you know that if the author does not intend to assign the copyright, I better not see the implementation (if you want it in Emacs core).

It should be fairly easy to add, though (I just use the history package, myself).

@JohnC32
Copy link
Contributor

JohnC32 commented Oct 18, 2019

Can you give a pointer to the history package? It doesn't seem to come with Emacs25 and googling on emacs history isn't so helpful. Thanks

@seagle0128
Copy link
Collaborator

I'm using C-u C-SPC for markers.

@dgutov
Copy link
Contributor

dgutov commented Oct 18, 2019

@yyoncho
Copy link
Member

yyoncho commented Oct 19, 2019

@JohnC32 if history package or @seagle0128 solution does not work for you, feel free to open another issue.

we should deprecate the lsp-find-* equivalents

I gave up on that due to #1117 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants