Skip to content
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

Enable fallback to pyls for goto definition of normal python functions #1

Closed
wants to merge 2 commits into from

Conversation

masahi
Copy link
Collaborator

@masahi masahi commented Dec 28, 2019

The current setting overwrites the language server with the custom one, so goto definition to normal python functions no longer work. This PR enables fallback to pyls when tvm-ffi-navigator couldn't find the definition.

Also added instructions for eglot.

@masahi masahi changed the title Enable fallback to pyls for goto definition for normal python functions Enable fallback to pyls for goto definition of normal python functions Dec 28, 2019
@tqchen
Copy link
Owner

tqchen commented Dec 28, 2019

Thanks @masahi ! I am not sure it it is the right approach though. I believe that the fallback should be handled by the client side(vscode or lsp-mode).

For example, when a getDefintion request comes into the client, the client could query all the relevant lsp servers and combine their results. In this way there will be more composability, i.e. we can use ffi-navigator with any other python language servers

I believe this is the way vscode handles the multiple lsp server cases, so the original goto def in python also works fine in vscode. Would be nice if we can dig deeper to see if there is such option in lsp-mode, or request/contribute such features

@masahi
Copy link
Collaborator Author

masahi commented Dec 28, 2019

yes, lsp-mode does seem to have multi servers support per lang, but I couldn't quickly figure out how to configure correctly and I also wanted to support other lsp client for emacs, eglot, which takes a minimalist approach and doesn't seem to support multi servers at the moment (I have better experience with eglot for python-mode). This PR is the quickest and most obvious way to fix my immediate need and I am quite happy with my new workflow :)

Feel free to reopen if other people want this.

@masahi masahi closed this Dec 28, 2019
@tqchen
Copy link
Owner

tqchen commented Dec 28, 2019

Perhaps we should raise the question in the elgot dev to see if it can be easily supported :)?

@masahi
Copy link
Collaborator Author

masahi commented Dec 28, 2019

There is already discussion at joaotavora/eglot#90
The discussion seems to have stopped one year ago though.

@tqchen
Copy link
Owner

tqchen commented Dec 28, 2019

I confirm that multiple server settings works perfectly for lsp

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

Successfully merging this pull request may close these issues.

2 participants