Improve label detail support in completions

[ryuukk]

Before:

After:

[rchl]

Will need to be manually tested with various servers to ensure that it doesn't result in worse behavior in some cases.

I will do my share of testing, hopefully others will do theirs. :)

[rchl]

One datapoint from Pyright.

Before:

After:

I would say that it makes things better in this case too.

[rchl]

@jwortmann can you have a look since you were involved with related changes?

[jwortmann]

So if we prefer lsp_label_description (if set) for the annotation field here, we should probably do the same also in the else block below, right? That would be more consistent.

I think it depends on the server whether it is better or worse than using the lsp_detail. For pyright I would even find the "Auto-import" annotation more important, because those completions automatically add another import line at the top of the file, and this way I could avoid them more easily ;) (but I use "python.analysis.autoImportCompletions": false setting anyway).
So in general this will be a matter of preference too. But from my side it should be okay to merge it (after the two other points are resolved).

[rchl]

For pyright I would even find the "Auto-import" annotation more important, because those completions automatically add another import line at the top of the file, and this way I could avoid them more easily ;)

For this specific case I think it's much more preferred to see the name of the file/package in the completion list rather than just "auto-import" because, if you know the server a little bit, you know that seeing the name equals auto-import and so you still know that the auto-import will happen and at the same time you have much more information up-front.

[rchl]

So if we prefer lsp_label_description (if set) for the annotation field here, we should probably do the same also in the else block below, right? That would be more consistent.

Applied same logic to the else branch and also consolidated those two blocks. It's probably not easy to review.

[rchl]

Those test are poor-mans version of the new ones so it would possibly be OK to remove those and convert to the new ones, if those test anything extra.

I say "poor-mans" version because those ignore "annotation" for example.

[jwortmann]

For this specific case I think it's much more preferred to see the name of the file/package in the completion list rather than just "auto-import" because, if you know the server a little bit, you know that seeing the name equals auto-import and so you still know that the auto-import will happen and at the same time you have much more information up-front.

Yeah okay, I kind of agree, but this is only a valid assumption if this LSP server is the only source of completions which use the annotation field.

[rchl]

Yeah okay, I kind of agree, but this is only a valid assumption if this LSP server is the only source of completions which use the annotation field.

Yes, but that's intentional since we can't implement things the way spec intended due to ST limitations so we have to use heuristics or optimize for specific cases.