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

add redhat vscode-yaml lsp as dependency #5

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

emilkje
Copy link
Collaborator

@emilkje emilkje commented Mar 14, 2023

No description provided.

@emilkje
Copy link
Collaborator Author

emilkje commented Mar 14, 2023

@h3rmanj @JonasKs @daniwk This PR fixes #4 by adding redhat language server as a hard dependency. It would make the installation easier for the end user, but it would also take away the flexibility of choosing your own yaml lsp implementation. One could argue that redhat.vscode-yaml is the de-facto standard language server and would work perfectly fine >90% of the time, but in theory this could make intility.vscode-backstage unusable for some people.

I tried to find some mechanism to promote redhat.vscode-yaml as a recommended extension, but could only find extensionDependencies as an integration hook. I'm inclined to not merge this, but would appreciate some input in this regard.

@h3rmanj
Copy link
Member

h3rmanj commented Mar 14, 2023

We should probably avoid adding it as a dependency unless we actually interact with it. One way we could do that, is set the correct scheme after a snippet has been used, so the user don't have to do it manually. The YAML extension will only set the correct scheme automatically if the file your editing is named catalog-info.yaml, but not for other files. Not sure if this is possible to achieve though.

For now I'm leaning towards just mentioning it in the readme, even though it would be a bit hidden.

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