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

fix: missing client capabilities and markdown descriptions #248

Conversation

sumimakito
Copy link
Contributor

It seems monaco-yaml does not set clientCapabilities while calling getLanguageService, which is exported by yaml-language-server. This causes the doesSupportMarkdown in yaml-language-server always return undefined and makes the markdownDescription not being correctly rendered in the completion item.

However, I can see clientCapabilities is being set by the test helper in yaml-language-server with a constant imported from vscode-json-languageservice.

Similiar usage in jsonWorker in monaco-editor can be found here: https://github.com/microsoft/monaco-editor/blob/f6dc0eb8fce67e57f6036f4769d92c1666cdf546/src/language/json/jsonWorker.ts#L38

This pull request copied the clientCapabilities constant from vscode-json-languageservice to avoid introducing new dependencies.

Copy link

netlify bot commented Sep 21, 2024

Deploy Preview for monaco-yaml ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 467b134
🔍 Latest deploy log https://app.netlify.com/sites/monaco-yaml/deploys/66eeac16b0b2b500085798cf
😎 Deploy Preview https://deploy-preview-248--monaco-yaml.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@sumimakito sumimakito changed the title fix: missing client capabilities fix: missing client capabilities and markdown descriptions Sep 21, 2024
@sumimakito
Copy link
Contributor Author

Replaced by #250

@sumimakito sumimakito closed this Sep 22, 2024
@remcohaszing
Copy link
Owner

I merged this commit manually.

@sumimakito
Copy link
Contributor Author

@remcohaszing Cool, thanks. I could not decide which of the two PRs better fits this repo, but your choice helps.

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.

2 participants