Language-server-protocol: MarkupContent in Hover lacks support for "regions"

[Gama11]

(as discussed in https://github.com/Microsoft/vscode-languageserver-node/issues/328 and https://github.com/Microsoft/vscode-languageserver-node/issues/374)

When using MarkedString[] as a Hover's contents, a client can interpret this as different "regions" and format it accordingly, e.g. with a horizontal separator like VSCode does. However, MarkedString is deprecated, and this can't be communicated with MarkupContent. So effectively, one is forced to use deprecated API to achieve this formatting right now.

Perhaps MarkupContent[] could be allowed as a possible type for Hover.contents as well to remedy this.

[LaurentTreguier]

As a workaround for this, I've been using MarkupContent with markdown and using \n\n---\n\n to put horizontal separators.

[Gama11]

The problem is that that looks different (and not nearly as good in my opinion). See also the screenshots in https://github.com/Microsoft/vscode-languageserver-node/issues/374.

[LaurentTreguier]

Yes, I see what you mean. And it definitely would make sense to be able to use MarkupContent[] just like MarkedString[] can be used.

[DanTup]

I'm looking at adding LSP support to Dart and just hit this. In the built-in provider (which uses Dart's proprietary protocol) we return a section at the top that has the type info/deprecated note/etc. above the doc comment.

I'm curious what the other languages that use LSP do here (for ex. in TypeScript it seems to render like the Dart one does... does it use the deprecated types?)

[Gama11]

As far as I know, TypeScript uses the VSCode API directly, rather than the LSP:

https://github.com/Microsoft/vscode/blob/0f4fab159fcd047ffb690a85f5a0ab971024d823/extensions/typescript-language-features/src/features/hover.ts

[KamasamaK]

This is a strange regression. Was this intentional? The native VS Code API still allows for this.

[DanTup]

As far as I know, TypeScript uses the VSCode API directly, rather than the LSP:

Ah, that explains that. From what I can see, the Omnisharp implementation uses the older MarkedStrings which I think allow for arrays. I'll go back to them for now - probably it's more likely a client will support that than the newer types anyway.

[felixfbecker]

I solved this by concatenating items into markdown, separated by horizontal rules (--- or <hr>): https://github.com/sourcegraph/lang-typescript/blob/8456bed0730557aea672ec727cb023979e489496/extension/src/lsp-conversion.ts#L55-L75

I modified the styles on Sourcegraph so that sections separated by horizontal rules look the same as multiple array items: https://github.com/sourcegraph/codeintellify/commit/2ab2bf62289fc0266b3772fe07aee9d82c1786e7

[dbaeumer]

Fixed in the VS Code side. See https://github.com/Microsoft/vscode-languageserver-node/pull/417#issuecomment-447361795

[dbaeumer]

I fixed this by rendering a --- (e.g <hr> tag) as a semantic break in the VS Code UI. I am not a fan of supporting MarkedContent[]. Reason being is that we then needs to specify what it means when a array is received and how that should be rendered on the client side. Having only one element makes this a lot easier.