Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert "hover" action to return MarkedContent instead of MarkedString #77

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

apexskier
Copy link

@apexskier apexskier commented Oct 1, 2020

MarkedString is deprecated by the LSP, and an editor I'm working in isn't escaping it properly. This updates the hover action to return MarkedContent instead of MarkedString[]. As part of this, it can now return plain text using MarkedKind.PlainText, which avoids some markdown escaping. I've maintained this package's custom extension to Schemas to support markdown content documentation though.

Additionally, I've added some basic tests for hover contributions (resolves #53) and replaced the deprecated assert.deepEqual method in these tests.


export interface JSONWorkerContribution {
getInfoContribution(uri: string, location: JSONPath): Thenable<MarkedString[]>;
getInfoContribution(uri: string, location: JSONPath): Thenable<Hover["contents"]>;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a wider type, so is fully backwards compatible.

@apexskier apexskier marked this pull request as ready for review October 1, 2020 07:31
@aeschli
Copy link
Contributor

aeschli commented Oct 14, 2020

Thanks a lot!

We have this client capability that is passed when service is created:

clientCapabilities?: ClientCapabilities;

It lets the API user control whether the hover can use Markdown. Now with the MarkedContent we should probably observe that capability.

Base automatically changed from master to main February 26, 2021 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tests for hover contributions
2 participants