JaroslavTulach commented on a change in pull request #2478:
URL: https://github.com/apache/netbeans/pull/2478#discussion_r509000593
##########
File path: java/java.lsp.server/vscode/src/extension.ts
##########
@@ -197,6 +199,9 @@ export function activate(context: ExtensionContext) {
client.start();
client.onReady().then((value) => {
commands.executeCommand('setContext', 'nbJavaLSReady', true);
+ client.onNotification(StatusMessageRequest.type, (mp) => {
Review comment:
`client.onNotification(StatusMessageRequest.type, showStatusBarMessage)`
would be shorter. Shall this registration really be done in `onReady` callback,
shall not it be done sooner without any callback?
##########
File path: java/java.lsp.server/vscode/src/extension.ts
##########
@@ -240,6 +245,29 @@ export function activate(context: ExtensionContext) {
}
+function showStatusBarMessage(params : ShowStatusMessageParams) {
+ let decorated : string = params.message;
+ let timeout : number = 10000;
+
+ if (params.type) {
Review comment:
Maybe you don't need the `if` check. `switch (undefined)` goes to
default.
##########
File path: java/java.lsp.server/vscode/src/extension.ts
##########
@@ -240,6 +245,29 @@ export function activate(context: ExtensionContext) {
}
+function showStatusBarMessage(params : ShowStatusMessageParams) {
+ let decorated : string = params.message;
+ let timeout : number = 10000;
Review comment:
Shoudn't the timeout be read from `params.timeout`, if present?
##########
File path:
java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/NbCodeLanguageClient.java
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.lsp.server.protocol;
+
+import org.eclipse.lsp4j.MessageParams;
+import org.eclipse.lsp4j.jsonrpc.services.JsonNotification;
+import org.eclipse.lsp4j.jsonrpc.validation.NonNull;
+import org.eclipse.lsp4j.services.LanguageClient;
+
+/**
+ * An extension to the standard LanguageClient that adds several messages
missing
+ * from the official LSP protocol.s
+ * @author sdedic
+ */
+public interface NbCodeLanguageClient extends LanguageClient {
+
+ /**
+ * Shows a message in the status bar. Log- and Info-type messages are
shown "as is".
+ * The other message types can be decorated by an icon according to {@link
MessageParams#getType}.
+ * The message will be hidden after specified number of milliseconds; 0
means the client
+ * controls when the message is hidden.
+ *
+ * @param params message type and text.
+ */
+ @JsonNotification("window/showStatusBarMessage")
+ public void showStatusBarMessage(@NonNull ShowStatusMessageParams params);
Review comment:
Not urgent, but shouldn't there be some client/server capabilities
handshake to find out if this message is supported on both sides? If this
message isn't supported, then send some standard message like `logMessage` or
`showMessage`? @jlahoda tends to repeat that VSCode isn't the only client for
this server.
##########
File path:
java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/Server.java
##########
@@ -77,14 +85,56 @@ public static void launchServer(InputStream in,
OutputStream out) {
Exceptions.printStackTrace(ex);
}
}
+
+ private static Launcher<NbCodeLanguageClient>
createLauncher(LanguageServerImpl server, InputStream in, OutputStream out) {
+ return new LSPLauncher.Builder<NbCodeLanguageClient>()
+ .setLocalService(server)
+ .setRemoteInterface(NbCodeLanguageClient.class)
+ .setInput(in)
+ .setOutput(out)
+ .wrapMessages(new
ConsumeWithLookup(server.getSessionLookup())::attachLookup)
Review comment:
`wrapMessages`! That's the trick. Cute.
##########
File path:
java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/ShowStatusMessageParams.java
##########
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.java.lsp.server.protocol;
+
+import org.eclipse.lsp4j.MessageParams;
+import org.eclipse.lsp4j.MessageType;
+import org.eclipse.xtext.xbase.lib.Pure;
+
+/**
+ *
+ * @author sdedic
+ */
+public class ShowStatusMessageParams extends MessageParams {
+ private int timeout = 0;
+
+ public ShowStatusMessageParams(MessageType type, String message) {
+ super(type, message);
+ }
+
+ public ShowStatusMessageParams(MessageType type, String message, int
timeout) {
+ super(type, message);
+ this.timeout = timeout;
+ }
+
+ @Pure
+ public int getTimeout() {
Review comment:
The TypeScript definition says `timeout` is optional. Shouldn't this
method return `Integer` and `null` if the timeout isn't set?
##########
File path:
java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/Server.java
##########
@@ -185,6 +236,15 @@ public WorkspaceService getWorkspaceService() {
@Override
public void connect(LanguageClient client) {
this.client = client;
Review comment:
Wouldn't it be simpler if the type of `client` field was changed to
`NbCodeLanguageClient`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
