python-lsp / python-lsp-server

Fork of the python-language-server project, maintained by the Spyder IDE team and the community
MIT License
1.76k stars 186 forks source link

Improve hover results #452

Open HedgehogCode opened 9 months ago

HedgehogCode commented 9 months ago

This PR solves #444.

I tried to describe the changes pretty well in the comments in the code. Note that there are two TODO(Review) comments that I would like feedback on.

Here are some examples of how the hover results change with this code change. The new results are highlighted with a green border.

hover_preview_1 hover_preview_2 hover_preview_3

PeterCardenas commented 6 months ago

@tkrabel-db @HedgehogCode any way we can get this merged? looking forward to this change

tkrabel-db commented 6 months ago

@PeterCardenas I'm afraid I have no rights to do so. @ccordoba12 can you TAL?

tbung commented 5 months ago

Thanks for your work on this! I am really looking forward for this to get merged.

I think there is still some room for improvement here, though. For variables with None in their union type or Optional type, the variable gets simply shown as NoneType. jedi-language-server is slightly better with showing the complete definition line in that regard.

Here is what python-lsp-server with this PR shows (without this PR python-lsp-server shows nothing at all in this situation): Screenshot from 2024-01-14 03-09-21

And here is what jedi-language-server shows: Screenshot from 2024-01-14 03-10-40

The real solution would be to correctly show a union type here. Since both lsps don't do that I assume this is an issue with jedi, I haven't check though.

HedgehogCode commented 5 months ago

Sorry for the late reply.

The example above showed NoneType() because jedi returns a signature for this definition. However, signatures are only interesting for "function" and "class" (BaseName#get_signatures). I changed the code to only show signatures for "function" and "class". The new code shows Union[NoneType, int] on hover (this was already implemented - we just did not get there because of the signature).

I also changed the code to show the type in cases where we have a signature but this is not the only option. Example:

def b():
    return 1

if input() == "yes":
    b = 10

b

On hover over "b":

b()
Union[b, int]