-
Notifications
You must be signed in to change notification settings - Fork 45
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
No way to disable follow_imports in goto #284
Comments
Ideally, "Go to Declaration" should not follow imports - https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_declaration "Go to Definition" should- https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_definition |
@r3m0t I suppose that makes sense, although those pages don't really seem to define "declaration" and "definition". What's even more confusing is that there's also "goto implementation" https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_implementation. How is that different from "definition"? Is this a distinction that doesn't make sense for Python? At the end of the day, I don't particularly care. As long as this behavior is exposed, I can easily configure eglot to do make M-. do the one I want. I also vaguely remember from my use with emacs-jedi (which doesn't enable these flags) that there are some other cases aside from imports where multiple levels of goto are required to get to the actual "definition", but I can't quite remember what they are right now so I could be wrong. But if that is the case an actual "goto implementation/definition" should recursively call goto until it reaches a fixed point. |
Recent python-lsp/python-lsp-server#443 sound potentially related |
I'm also very confused what the differences of the language server gotos are. So if someone understands this well, I'm very interested in answers. |
iiuc some of these distinctions make more sense in some languages than in others
I'm not aware of any clear guidance, let alone rules, on what language servers are supposed to do: roughly, whatever everyone else is doing but as best makes sense in your language, I suppose. That leaves plenty of scope for interpretation. Eg I wrote a paragraph suggesting that the closest thing in python to definition-vs-declaration would be going-to-code vs going-to-type-stubs. But then I remembered "goto type definition", which sounds like an even better fit for going to type stubs, so I guess I take it back. TLDR I don't see any real justification for it but if you don't have any other use in mind for "goto declaration" then I don't think it would do much harm or break any laws to have eg goto-definition follow imports, and goto-declaration not follow imports. |
Thanks a lot for explaining this. I get the following tendencies for Python from what you describe:
Please let me know if you disagree.
This is a very interesting idea that I might steal one day. |
yeah, I guess that's roughly what I had in mind. But my main conclusion was really that the language server author seems to be allowed the discretion and creativity to do whatever they think most appropriate for each command. (and since I am no authority, and any suggestion I made was pretty much the first thing I thought of, it's more than likely that improvements exist!) |
Not sure if related (I guess so), but I came here with this issue #303. I also wrote a reddit question with more details -> https://www.reddit.com/r/neovim/comments/1ay2xoe/comment/krs2chw/ |
This specific issue should be resolved by declaration support, released in https://github.com/pappasam/jedi-language-server/releases/tag/v0.41.3 |
The Jedi
goto
call hard-codesfollow_imports=True
andfollow_builtin_imports=True
jedi-language-server/jedi_language_server/server.py
Lines 318 to 319 in a2efe99
I would prefer these to be disabled, which causes the goto to go to the actual line that imports the name in the current file (and subsequent gotos on that will follow the imports automatically).
Could a configuration option be added for this (or the default changed, I don't care)?
The text was updated successfully, but these errors were encountered: