Skip to content
This repository has been archived by the owner on Apr 14, 2022. It is now read-only.

Completion item documentation property has null kind #305

Closed
johngalambos opened this issue Oct 24, 2018 · 4 comments
Closed

Completion item documentation property has null kind #305

johngalambos opened this issue Oct 24, 2018 · 4 comments
Assignees
Milestone

Comments

@johngalambos
Copy link

Completion items are returning with documentation properties that look like this:

"documentation": { "kind": null, "value": "None" }

The relevant spec is here (Search for MarkupContent). According to the spec, kind should be one of plaintext or markdown.

Null kind values are causing problems with LanguageClient-neovim as described here autozimu/LanguageClient-neovim#633 (comment):

@jakebailey
Copy link
Member

The only place we don't explicitly choose between plaintext or markdown seems to be:

https://github.com/Microsoft/python-language-server/blob/18e25f19ca54e1b0b879038ca2cb387563b84ecd/src/LanguageServer/Impl/Implementation/CompletionAnalysis.cs#L803-L814

The kind being set comes from initializationOptions.displayOptions; see here in the VS Code extension: https://github.com/Microsoft/vscode-python/blob/efa175ca29476469136c07c783f6cbd28a976ff0/src/client/activation/languageServer/languageServer.ts#L280-L286

You should be able to mirror the options set by the VS Code extension in the settings.json mentioned in the neovim issue as a quick fix. There are probably other things being set there that are good to have. (Those Sublime instructions were contributed from someone outside our team and we haven't really tested them, nor in other editors.)

On our end we could add ?? MarkupKind.PlainText to the PLS code I linked above, but I'm honestly surprised that we're not using TextDocumentClientCapabilities .completion.completionItem.documentationFormat instead of a custom option.

@jakebailey jakebailey self-assigned this Oct 24, 2018
@johngalambos
Copy link
Author

Thanks! Adding the initializationOptions.displayOptions you suggested to the settings file works perfectly to get the values for kind. Still having some issues parsing the completion items--those VS Code settings should hopefully put me on the right track though.

@qubitron
Copy link

VS Code handles null, and there is a performance issue here. Can we check if the spec needs to be updated?

@gramster gramster added this to the Nov 2018.1 milestone Oct 24, 2018
@jakebailey
Copy link
Member

It turns out that VS Code does not handle null:

image

If I go through all places we create MarkupContent and force it to be null, everything that uses those objects breaks, including completion and hovering.

I read through the vscode-languageserver-node implementation, and it explicitly checks for one or the other (plaintext or markdown) and does not handle null (producing the above error message). I don't think this is an issue in the spec, but more of a problem with the fact that this one specific piece of code in the language server uses a setting that only VS Code and VS proper send. We've likely never hit this issue because we've always had the correct settings.

I'm going to PR in a fix for this one specific instance to ensure that null doesn't make it out anymore, but I think we should also consider deprecating preferredFormat passed with our custom initializationOptions in favor of reading TextDocumentClientCapabilities, which all clients should populate. For example, setting "python.trace.server": "verbose" to see raw RPC shows that VS Code sends this capability:

"completionItem": {
    "snippetSupport": true,
    "commitCharactersSupport": true,
    "documentationFormat": [
        "markdown",
        "plaintext"
    ],
    "deprecatedSupport": true,
    "preselectSupport": true
}

Which is exactly the info we need, including the ordering (best choice first).

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

No branches or pull requests

4 participants