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

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

Open
angelozerr opened this issue Nov 30, 2020 · 12 comments

Comments

@angelozerr
Copy link

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
Copy link
Collaborator

@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
Copy link
Author

angelozerr commented Nov 30, 2020

@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:

  • avoid double parse of yaml content (do by yaml language server and do by a custom need like my selectProperty feature).
  • avoid developing the same feature that yaml language server because yaml language server could not provide some features.

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:

  • in JDT LS, you can access to the Java AST (ICompilationUnit) which is the result of parse of the Java editor content. The AST manages fault tolerant. After that you can visit the AST and returns any information that you need to manage your custom need.
  • in XML Language Server, we did the same thing, we provide the capability to register custom command and provide the capability to access to the DOM document which is the result of the parse of the XML editor content. Even if XML content is broken, you can access to the DOM document since LemMinX can parse XML content which is broken (fault tolerant).

Custom command provides the capability to:

  • avoid double parsing of Java, XML content.
  • benefit with advanced features of the ICompilationUnit, DOM Document (like fault tolerant, like providing range location, etc)
  • manage your custom need.

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
Copy link
Collaborator

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
Copy link
Contributor

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

@angelozerr
Copy link
Author

angelozerr commented Nov 30, 2020

@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:

  • on yaml language server:

    • you implement the standard LSP request workspace/executeCommand which execute a given custom command which is registered by external extension.
    • the question is how to register the custom command in yaml language server? I think it's the hard part (perhaps it's easy but never done that). In JDT LS, custom command are registered with plugin.xml (see IDelegateCommandHandler). In XML Language Server LemMinx, we use Java SPI to register custom command. When the XML Language server is started, we loop for each external JARs to register custom commands.
  • on vscode-yaml client side:

@evidolob
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Contributor

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

@angelozerr
Copy link
Author

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:

  • in Java you need to retrieve in your command a ICompilationUnit from an uri. JDT manages a static context kind to retrieve a ICompilationUnit from an URI.
  • in XML context you need to retrieve a DOM document from an uri. LemMinX provides this kind of feature by using the opened documents.

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

  • YamlDocument getYamlDocument(string uri)
  • YamlDocument parseYaml(string content)

I think it can be a good start.

@evidolob
Copy link
Collaborator

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

@JPinkney
Copy link
Contributor

JPinkney commented Nov 30, 2020

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: #403
The server-side changes: redhat-developer/yaml-language-server#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
Copy link
Collaborator

gorkem commented Dec 1, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants