pappasam / jedi-language-server

A Python language server exclusively for Jedi. If Jedi supports it well, this language server should too.
MIT License
623 stars 45 forks source link

No way to disable follow_imports in goto #284

Closed asmeurer closed 9 months ago

asmeurer commented 1 year ago

The Jedi goto call hard-codes follow_imports=True and follow_builtin_imports=True https://github.com/pappasam/jedi-language-server/blob/a2efe99ec0fe1fb8d2589f04ee43ca900ee94ebd/jedi_language_server/server.py#L318-L319

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)?

r3m0t commented 1 year ago

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

asmeurer commented 1 year ago

@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.

krassowski commented 1 year ago

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"

Recent https://github.com/python-lsp/python-lsp-server/pull/443 sound potentially related

davidhalter commented 1 year ago

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.

dimbleby commented 1 year ago

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 clangd seems to use goto-definition to alternate between definitions and declarations. That's surely not following the letter of any law, but it is kinda convenient.

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.

davidhalter commented 1 year ago

Thanks a lot for explaining this. I get the following tendencies for Python from what you describe:

Please let me know if you disagree.

That leaves plenty of scope for interpretation. Eg clangd seems to use goto-definition to alternate between definitions and declarations. That's surely not following the letter of any law, but it is kinda convenient.

This is a very interesting idea that I might steal one day.

dimbleby commented 1 year ago

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!)

antoniogamiz commented 9 months ago

Not sure if related (I guess so), but I came here with this issue https://github.com/pappasam/jedi-language-server/issues/303. I also wrote a reddit question with more details -> https://www.reddit.com/r/neovim/comments/1ay2xoe/comment/krs2chw/

pappasam commented 9 months ago

This specific issue should be resolved by declaration support, released in https://github.com/pappasam/jedi-language-server/releases/tag/v0.41.3