redhat-developer / vscode-yaml

YAML support for VS Code with built-in kubernetes syntax support
MIT License
665 stars 223 forks source link

Provide a custom request to open yaml file and select a given property key #402

Open angelozerr opened 3 years ago

angelozerr commented 3 years ago

Is your enhancement related to a problem? Please describe.

vscode-microprofile depends vscode-yaml to manage completion, validation, hover, etc for application.yaml. To do that we generate a JSON Schema according to the Java project classpath.

In Quarkus/MicroProfile, we can declare a property foo.bar in Java file:

public class Test {

 @ConfigProperty(name = "foo.bar")
  private Integer test;
}

and we can configure this property with the application.yaml like this:

foo:
  bar: 10

I'm implementing go to the definition from the Java file to the properties file and yaml file. In other words, when cursor is inside the name of the annotation (see cursor at |);

@ConfigProperty(name = "foo|.bar")

I would like to open the application.yaml and select the bar property key even if application.yaml is not opened.

Describe the solution you would like

It should be nice if we could consume a custom service like selectPropertyKey("foo.bar") to open the application.yaml and select the property key.

Describe alternatives you have considered

I will try for the moment to open the application.yaml with standard LSP definition request by setting 0 as location.

Thanks for your help!

evidolob commented 3 years ago

@angelozerr I'm assuming that you implement go to the definition with vscode.DefinitionProvider in such case you provide vscode.Location object, which contains file URI and Range where some symbol defined. VSCode use that uri to open file and Range to place cursor.

I'm not sure why do you need to open YAML manually or using API from yaml extension. I can assume that you may need some sort of API to calculate where some property defined in yaml file, to build Range object.

Another good question, is how to pass path to yaml node, I think we can use yaml path or jsonpath

angelozerr commented 3 years ago

@angelozerr I'm assuming that you implement go to the definition with vscode.DefinitionProvider in such case you provide vscode.Location object, which contains file URI and Range where some symbol defined. VSCode use that uri to open file and Range to place cursor.

I'm supporting go to the definition from Java to properties files and from java to yaml file with a language server written in Java. I don't use vscode API since vscode LSP client manages that. In the case of properties file, it's easy for my usecase because my language server takes care of parse of properties files and I can retrieve proper range location for a given property. But for yaml file, I delegate all work on yaml language server by using your custom extension to bind a given yaml file (in my case application.yaml file) with a JSON Schema that I compute dynamically with JDT project classpath.

I'm not sure why do you need to open YAML manually or using API from yaml extension.

My goal is to delegate all yaml stuff to the yaml language server like completion, diagnostics, hover based on JSON Schema. And I would like to continue my other features to the yaml language server. We did that with vscode-yaml in vscode context and Eclipse IDE Wild Web Developer in Eclipse IDE by consuming in the both cases the yaml language server.

The main idea that I follow is to delegate all yaml stuff to the yaml language server to:

In other words I would like to avoid parse yaml file in Java or TypeScript just to retrieve the proper location. IMHO I think a X language server must be extensible and must provide features that other extensions would require. Your yaml language server can parse yaml content and provides a lot of relevant information, so why don't expose those information (like select a property key like I need).

Perhaps my need is too specific and you don't want to provide this kind of feature, that's why I think the best idea is to provide a command mechanism to implement your custom service. It's the feature that it was implemented in vscode-java with JDT LS and that we have implemented too in vscode-xml with XML Language Server LemMinX. The main is that you can register a custom command and returns the information that you need.

For example:

Custom command provides the capability to:

IMHO I think yaml language server should provide this kind of custom command which will open the door to manage advanced support.

Another good question, is how to pass path to yaml node, I think we can use yaml path or jsonpath

Yes it could be a good idea.

evidolob commented 3 years ago

Hm, I see your point, and I like an idea to expose yaml AST for other vscode extensions. I also have a need to access yaml AST in vscode-tekton extension, so I'm +1 on this.

Perhaps my need is too specific and you don't want to provide this kind of feature, that's why I think the best idea is to provide a command mechanism to implement your custom service. It's the feature that it was implemented in vscode-java with JDT LS and that we have implemented too in vscode-xml with XML Language Server LemMinX. The main is that you can register a custom command and returns the information that you need.

I need to look in to vscode-java and JDT-LS to see how them implement this command service.

JPinkney commented 3 years ago

This all makes sense to have. I'm +1 on this as well

angelozerr commented 3 years ago

@JPinkney @evidolob I'm so happy that you like this idea! It will open the door for a great yaml support extensibility that we require (and I hope other extension will require).

The main idea is:

evidolob commented 3 years ago

OK, I done quick look on vscode-java, it expose API object which has method to query DocumentSymbols for some file. We can do the same for yaml. @angelozerr I think it would be enough for your need.

As for extensibility, it is more hard question. As yaml-ls is not designed to be extensible, basically it is monolithic Node.js/JS app. It would be very hard to add such API, especially if we use json-language-server inside to delegate most of LSP services implementation. At least I do not see the easy way to provide such extensibility without rewriting yaml-ls from scratch.

@JPinkney maybe you have other opinion, as you know code better?

evidolob commented 3 years ago

In other way, the more I think about having yaml-ls extensible the more I like it. We have a lot of projects which more that plain yaml files, for example tekton yaml has a lot of connections between different yaml files. And it will be very good if we can plugin in to yaml-ls support of such files.

Technically it not a problem to load custom plugin code in to yaml-ls. We may follow the vscode-java/JDT-LS way, where extension package.json contains:

"yaml-language-server-plugin": "path/to/plugin.js"

vscode-yaml on start may scan all installed extensions, look on "yaml-language-server-plugin" value, collect those paths and pass them to yaml-ls. Yaml-ls may load such files, but the main problem starts there, as we do not have API with such plugins may use.

JPinkney commented 3 years ago

Yeah, that was the approach I was thinking ^. I think we'd have to do some experimentation to see what's possible

angelozerr commented 3 years ago

vscode-yaml on start may scan all installed extensions, look on "yaml-language-server-plugin" value, collect those paths and pass them to yaml-ls. Yaml-ls may load such files, but the main problem starts there.

I love that :)

as we do not have API with such plugins may use.

I think you should follow the same API that we use for JDT LS and XML Language Server with IDelegateCommandHandler: https://github.com/eclipse/lemminx/blob/11f6f6df3ac2a7c574b399d6ef40a128b2cfd6a7/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/extensions/commands/IXMLCommandService.java#L33

The question is about context:

To avoid breaking the command API, I suggest this idea:

interface IDelegateCommandHandler {

        /**
         * Executes a command
         * @param params command execution parameters
         * @param cancelChecker check if cancel has been requested
         * @return the result of the command
         * @throws Exception the unhandled exception will be wrapped in
         *                   <code>org.eclipse.lsp4j.jsonrpc.ResponseErrorException</code>
         *                   and be wired back to the JSON-RPC protocol caller
         */
        Object executeCommand(ExecuteCommandParams params, IYamlContext context, CancelChecker cancelChecker) throws Exception;     
}

and YamlContext could provide methods like

I think it can be a good start.

evidolob commented 3 years ago

@angelozerr That make more sense and looks more easy to implement. @JPinkney Right, we need to experiment with this.

JPinkney commented 3 years ago

I've done a P.O.C of what this might look like. It's still not complete and I will still need to do things like bundle unloading/refreshing, passing a YAMLContext, etc, etc but it's an interesting start.

The client-side changes: https://github.com/redhat-developer/vscode-yaml/pull/403 The server-side changes: https://github.com/redhat-developer/yaml-language-server/pull/369 The test extension I used: https://github.com/JPinkney/yaml-extension-bundle

I made a video to better describe all the changes since I figured it might be easier: https://www.youtube.com/watch?v=QcXbfdKr7BE

The basic idea is that the yaml extension that wants to contribute a bundle adds a "yamlExtensions" object into their "contributes" inside of their extensions package.json that points to a javascript file. This javascript file must export a name, version number, and commands with actions. Then when VSCode-YAML starts it looks through every extension and tries to find ones with "yamlExtensions" in their package.json. Then it passes those extension locations to the server-side. The server side then registers those commands and actions and then executes them when you call yaml.execute.workspaceCommand from the client side.

gorkem commented 3 years ago

I do not like the idea of loading code from another and executing it without any checks. There are security and performance implications on such a solution. We did it on JDT LS because it is the extension model of the Eclipse IDE which LSP is based on.