Issue 522 - Allow custom notifications #575
Conversation
vrubezhny
force-pushed
the
ls522
branch
2 times, most recently
from
fa24a11 to
c51f798
Compare
|
|
||
| @RunWith(MockitoJUnitRunner.class) | ||
| public class JavaModelEventProviderTest extends AbstractProjectsManagerBasedTest { | ||
| private DocumentLifeCycleHandler lifeCycleHandler; |
There was a problem hiding this comment.
lifeCycleListener isn't used.
There was a problem hiding this comment.
Sure. I forgot to remove it
| @@ -725,4 +725,7 @@ public JavaClientConnection getClientConnection() { | |||
| return client; | |||
| } | |||
|
|
|||
| public void notify(String method, Object parameter) { | |||
There was a problem hiding this comment.
I have tried to use this notification from VS Code, but lsp4j sends only the method property.
I think you need to create an object containing the method and parameter properties. See ActionableNotification - https://github.com/eclipse/eclipse.jdt.ls/blob/master/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JavaClientConnection.java#L47.
There was a problem hiding this comment.
To make the test working I had to modify https://github.com/eclipse/eclipse.jdt.ls/blob/master/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/managers/AbstractProjectsManagerBasedTest.java#L93 by adding a case for the 2nd parameter. Can it be something similar to this or sending the only first arg is like a 'hardcoded' limitation?
There was a problem hiding this comment.
Try to use the notify method in vscode-java. See https://github.com/redhat-developer/vscode-java/blob/master/src/extension.ts#L108
| * @param method | ||
| * @param event | ||
| */ | ||
| @JsonNotification("language/event") |
There was a problem hiding this comment.
You need to remove this method.
|
|
||
| @Override | ||
| public void elementChanged(ElementChangedEvent event) { | ||
| client.notify("notify", event); |
There was a problem hiding this comment.
Use a different name for the notification message here. For example: "elementChanged"
| @@ -90,6 +90,14 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl | |||
| clientRequests.put(name, params); | |||
| } | |||
| params.add(args[0]); | |||
| } else if (args.length == 2 && "notify".equals(method.getName())) { | |||
There was a problem hiding this comment.
Because you added "notify" as a method to JavaLanguageClient, you're not getting the notification "notify(param)", but notify("notify", param);
Removing that method means you cannot use the reflection-based mechanism for checking invocations. Either you need to build your own or we need to rewrite this mechanism to be based on Endpoints, not proxies
vrubezhny
force-pushed
the
ls522
branch
2 times, most recently
from
8605626 to
114aa91
Compare
The public `void notify(String method, Object parameter)` method is added to `JavaClientConnection` thus allowing sending the notifications from a Server to a Client Fixes eclipse-jdtls#522 Signed-off-by: Victor Rubezhny <[email protected]>
The public
void notify(String method, Object parameter)method is added toJavaClientConnectionthus allowing sending the notifications from a Server to a ClientFixes #522
Signed-off-by: Victor Rubezhny [email protected]