Skip to content

Add support for YAML configuration files#189

Merged
fbricon merged 1 commit intoredhat-developer:masterfrom
angelozerr:yaml-support
Jan 20, 2020
Merged

Add support for YAML configuration files#189
fbricon merged 1 commit intoredhat-developer:masterfrom
angelozerr:yaml-support

Conversation

@angelozerr
Copy link
Copy Markdown
Contributor

Add support for YAML configuration files

See redhat-developer/quarkus-ls#112

Signed-off-by: azerr azerr@redhat.com

Comment thread package.json Outdated
Comment thread src/yaml/YamlSchema.ts Outdated
Comment thread src/yaml/YamlSchema.ts
return undefined;
}

if (ext.packageJSON.version && !semver.gte(ext.packageJSON.version, '0.0.15')) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@tsmaeder if Che doesn't have the proper vscode-yaml version, will users be able to install a more recent version?

@angelozerr
Copy link
Copy Markdown
Contributor Author

This PR requires the MicroProfile LS PR redhat-developer/quarkus-ls#191 (this PR provides a new LSP service which convert the MicroProfileProjectInfo in a JSON Schema used by the YAML support).

Please note this MicroProfile LS PR must be improved to support value type (string, number, array, etc). I will continue to work on it.

This PR uses the vscode-yaml, so you need to install vscode-yaml to play with it. The check of vscode-yaml installation is done with the same thing than https://github.com/Azure/vscode-kubernetes-tools but I can change anything if you wish.

As it uses the vscode-yaml, if you have Spring Boot vscod extension, please change the type of the file to YAML otherwise the yaml ls will never called.

The YAML support have some limitations:

  • when you open the application.yaml at first, it will not work because vscode-yaml starts, checks if there is a registerContributor (to register a custom JSON Schema that we use in our case), the vscode-quarkus is started after and it cannot register the registerContributor. In other words you need to type something in the YAML editor, wait for compute of MP properties and after that it should work. If you close and reopen the application.yaml you will not have this problem.

  • the classpath changed has been managed. In other words if you modify the classpath (or modify a MP rest client) you should see the new property (please note that value doesn't work at this moment). But you need to change something in the editor because vscode-yaml cannot detect the change of JSON Schema)

@angelozerr angelozerr force-pushed the yaml-support branch 2 times, most recently from 0cfc0cf to 2bd8d50 Compare January 14, 2020 15:33
Comment thread package.json Outdated
Comment thread src/extension.ts Outdated
Comment thread src/extension.ts Outdated
Comment thread src/yaml/YamlSchema.ts Outdated
Comment thread src/yaml/YamlSchema.ts Outdated
@xorye
Copy link
Copy Markdown
Contributor

xorye commented Jan 14, 2020

For this popup:
image

I think it would be nice if there was a button that allowed you to install vscode-yaml without going to the extensions pane. It would be very convenient for the user.

The Java extension pack has similar functionality where you can click a link to automatically install a missing extension:
https://github.com/microsoft/vscode-java-pack/blob/30689fe903d15677ffe0cd45de9d9f15cc604dc6/src/commands/handler.ts#L43-L50

@fbricon @angelozerr Should I create a separate issue for this?

@angelozerr
Copy link
Copy Markdown
Contributor Author

@fbricon @angelozerr Should I create a separate issue for this?

Yes please!

@xorye
Copy link
Copy Markdown
Contributor

xorye commented Jan 14, 2020

Please delete bindNotification(),

function bindNotification(notification: string) {
context.subscriptions.push(commands.registerCommand(notification, (event: any) => {
languageClient.sendNotification(notification, event);
}));
}

since bindNotification() is not being used anymore

Comment thread src/yaml/YamlSchema.ts Outdated
Comment thread src/yaml/YamlSchema.ts Outdated
Comment thread src/extension.ts Outdated
Comment thread src/extension.ts Outdated
@angelozerr
Copy link
Copy Markdown
Contributor Author

Please delete bindNotification(),

@xorye done

@fbricon
Copy link
Copy Markdown
Collaborator

fbricon commented Jan 16, 2020

It asks me to install the YAML extension on startup, even before I started to open an application.yaml

@angelozerr
Copy link
Copy Markdown
Contributor Author

Indeed but is it something that @xorye could improve it once the pr is merged? Or perhaps you want that i remove the dialog?

Comment thread src/yaml/YamlSchema.ts Outdated
Comment thread src/yaml/YamlSchema.ts
Comment thread src/yaml/YamlSchema.ts Outdated
@fbricon
Copy link
Copy Markdown
Collaborator

fbricon commented Jan 17, 2020

When adding a new quarkus extension, after opening application.yaml, there's an error in the log:

ejected promise not handled within 1 second: TypeError: Method Map.prototype.delete called on incompatible receiver undefined
extensionHostProcess.js:832
stack trace: TypeError: Method Map.prototype.delete called on incompatible receiver undefined
	at delete (<anonymous>)
	at Array.forEach (<anonymous>)
	at YamlSchemaCache.evict (/Users/fbricon/Dev/projects/vscode-quarkus/dist/extension.js:83053:21)
	at /Users/fbricon/Dev/projects/vscode-quarkus/dist/extension.js:80758:23
	at o (/Users/fbricon/.vscode-insiders/extensions/vscjava.vscode-java-test-0.22.0/dist/extension.bundle.js:9:136323)
	at /Users/fbricon/.vscode-insiders/extensions/vscjava.vscode-java-test-0.22.0/dist/extension.bundle.js:9:136820

then completion stops working

@fbricon fbricon merged commit 044e1ba into redhat-developer:master Jan 20, 2020
@fbricon fbricon added this to the 1.3.0 milestone Feb 6, 2020
@fbricon fbricon added the enhancement New feature or request label Feb 6, 2020
@fbricon fbricon mentioned this pull request Feb 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants