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

Make it possible to map XXX-Providers to Plugins #6151

Closed
tsmaeder opened this issue Sep 10, 2019 · 14 comments
Closed

Make it possible to map XXX-Providers to Plugins #6151

tsmaeder opened this issue Sep 10, 2019 · 14 comments
Assignees
Labels
metrics issues related to metrics and logging plug-in system issues related to the plug-in system

Comments

@tsmaeder
Copy link
Contributor

In order to provide metrics related to language plugins, but also to implement tests for particular plugins, it would be helpful if registered providers could be mapped to a theia plugin. For example, we may want to test what VS Code-Java returns when we invoke code-assist at a particular place in a file. We could rely on VS Code-Java being the only extension installed in the system (aka we set up the test case that way), but being able to target a plugin specifically would be more stable.
Also in order to attribute failures to a particular extensions (metrics), we would need to know which extension a particular provider corresponds to.

What I would propose is to extend the main/ext interface with a parameter for the plugin id. For example in languages-main.ts, we could extend

$registerCompletionSupport(handle: number, selector: SerializedDocumentFilter[],...

by adding the plugin id:

$registerCompletionSupport(handle: number, plugindId: string, selector: SerializedDocumentFilter[],...

To address a particular plugin, we could then look up the handle by plugin id. I believe the registrations should be unique per plugin.

@tsmaeder tsmaeder added the plug-in system issues related to the plug-in system label Sep 10, 2019
@tsmaeder
Copy link
Contributor Author

@JPinkney @akosyakov would be interested in hearing your opinions on this.

@akosyakov
Copy link
Member

@tsmaeder I will be fine with such changes.

@benoitf
Copy link
Contributor

benoitf commented Sep 10, 2019

could we use an interface so we won't break callers every time a new parameter is added (if we need to provide more than id/string)

@JPinkney
Copy link
Contributor

Yeah, I like this idea. Assuming microsoft/vscode-languageserver-node#517 gets resolved, we would be able to create metrics that show exactly which part of the language server is having the issue and then also display which extension the issue was coming from.

Without this, we would at best be able to show a metric for only the current configured language (e.g. Java, YAML, XML, etc). The configured language doesn't really help though because if you are running vscode-apache-camel and vscode-java there isn't really a way to find where which one a request is failing in (without manually looking), only that a failing request happened with a file that had its configured language set to java.

@tsmaeder
Copy link
Contributor Author

could we use an interface so we won't break callers every time a new parameter is added (if we need to provide more than id/string)

I'm not sure if it's not better to "fail early" in this case: generally, the ext and main interfaces need to match exactly: since we don't have a protocol version, we cannot detect whether both sides are using a compatible version.

@benoitf
Copy link
Contributor

benoitf commented Sep 10, 2019

well it can still fail, but we'll change less lines of code.

@tsmaeder
Copy link
Contributor Author

But we won't notice that we have to update callers at compile time.

@benoitf
Copy link
Contributor

benoitf commented Sep 10, 2019

even with strictNullChecks ?

@tsmaeder
Copy link
Contributor Author

ok, I see what you want to do. I guess that might work.

@akosyakov
Copy link
Member

akosyakov commented Sep 10, 2019

We can introduce new things as optional, so another side has to check first whether they are present.

@JPinkney
Copy link
Contributor

To address a particular plugin, we could then look up the handle by plugin id. I believe the registrations should be unique per plugin.

How would we go about associating a handle with a plugin id? From what I understand handles are created here but when you are actually registering the feature like here it doesn't know that it's registering the feature for vscode-java for example

@akosyakov
Copy link
Member

@JPinkney
Copy link
Contributor

Ha thanks! Yes it is working, for some reason I didn't see that line of code

@akosyakov akosyakov added emmet issues related to emmet metrics issues related to metrics and logging and removed emmet issues related to emmet labels Sep 19, 2019
@JPinkney
Copy link
Contributor

JPinkney commented Oct 1, 2019

Closed via: #6214

@JPinkney JPinkney closed this as completed Oct 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
metrics issues related to metrics and logging plug-in system issues related to the plug-in system
Projects
None yet
Development

No branches or pull requests

4 participants