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.

Sign up for GitHub

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

Fix duplicate client/registerCapability for workspace/executeCommand #938

Merged
merged 1 commit into from
Jan 5, 2021
Merged

Fix duplicate client/registerCapability for workspace/executeCommand #938

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 17 additions & 23 deletions org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/XMLWorkspaceService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@
*/
package org.eclipse.lemminx;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.UUID;
import java.util.concurrent.CancellationException;
import java.util.concurrent.CompletableFuture;

Expand All @@ -32,40 +32,33 @@
import org.eclipse.lsp4j.jsonrpc.messages.ResponseErrorCode;
import org.eclipse.lsp4j.services.WorkspaceService;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
/**
* XML workspace service.
*
*/
public class XMLWorkspaceService implements WorkspaceService, IXMLCommandService {

private static final String WORKSPACE_EXECUTE_COMMAND = "workspace/executeCommand";

private final XMLLanguageServer xmlLanguageServer;

private final Map<String, IDelegateCommandHandler> commands;

private final Map<String, String> commandRegistrations;


public XMLWorkspaceService(XMLLanguageServer xmlLanguageServer) {
this.xmlLanguageServer = xmlLanguageServer;
this.commands = new HashMap<>();
this.commandRegistrations = new HashMap<>();
}

@Override
public CompletableFuture<List<? extends SymbolInformation>> symbol(WorkspaceSymbolParams params) {
return null;
}



@Override
public CompletableFuture<Object> executeCommand(ExecuteCommandParams params) {
synchronized (commands) {
IDelegateCommandHandler handler = commands.get(params.getCommand());
if (handler == null) {
throw new ResponseErrorException(new ResponseError(ResponseErrorCode.InternalError, "No command handler for the command: " + params.getCommand(), null));
throw new ResponseErrorException(new ResponseError(ResponseErrorCode.InternalError,
"No command handler for the command: " + params.getCommand(), null));
}
return CompletableFutures.computeAsync(cancelChecker -> {
try {
Expand All @@ -82,7 +75,7 @@ public CompletableFuture<Object> executeCommand(ExecuteCommandParams params) {
});
}
}

@Override
public void didChangeConfiguration(DidChangeConfigurationParams params) {
xmlLanguageServer.updateSettings(params.getSettings());
Expand All @@ -91,9 +84,10 @@ public void didChangeConfiguration(DidChangeConfigurationParams params) {

@Override
public void didChangeWatchedFiles(DidChangeWatchedFilesParams params) {
XMLTextDocumentService xmlTextDocumentService = (XMLTextDocumentService) xmlLanguageServer.getTextDocumentService();
XMLTextDocumentService xmlTextDocumentService = (XMLTextDocumentService) xmlLanguageServer
.getTextDocumentService();
List<FileEvent> changes = params.getChanges();
for (FileEvent change: changes) {
for (FileEvent change : changes) {
if (!xmlTextDocumentService.documentIsOpen(change.getUri())) {
xmlTextDocumentService.doSave(change.getUri());
}
Expand All @@ -106,9 +100,6 @@ public void registerCommand(String commandId, IDelegateCommandHandler handler) {
if (commands.containsKey(commandId)) {
throw new IllegalArgumentException("Command with id '" + commandId + "' is already registered");
}
String registrationId = UUID.randomUUID().toString();
xmlLanguageServer.getCapabilityManager().registerCapability(registrationId, WORKSPACE_EXECUTE_COMMAND, ImmutableMap.of("commands", ImmutableList.of(commandId)));
commandRegistrations.put(commandId, registrationId);
commands.put(commandId, handler);
}
}
Expand All @@ -117,15 +108,18 @@ public void registerCommand(String commandId, IDelegateCommandHandler handler) {
public void unregisterCommand(String commandId) {
synchronized (commands) {
commands.remove(commandId);
String registrationId = commandRegistrations.remove(commandId);
if (registrationId != null) {
xmlLanguageServer.getCapabilityManager().unregisterCapability(registrationId, WORKSPACE_EXECUTE_COMMAND);
}
}
}

@Override
public CompletableFuture<Object> executeClientCommand(ExecuteCommandParams command) {
return xmlLanguageServer.getLanguageClient().executeClientCommand(command);
}

@Override
public void endCommandsRegistration() {
if (!commands.isEmpty()) {
xmlLanguageServer.getCapabilityManager().registerExecuteCommand(new ArrayList<>(commands.keySet()));
}
}
}
13 changes: 10 additions & 3 deletions ....lemminx/src/main/java/org/eclipse/lemminx/services/extensions/XMLExtensionsRegistry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@ public void initializeParams(InitializeParams params) {
public void doSave(ISaveContext saveContext) {
if (initialized) {
extensions.stream().forEach(extension -> extension.doSave(saveContext));
} else if (this.initialSaveContext == null || (saveContext != null && saveContext.getType() == SaveContextType.SETTINGS)) {
} else if (this.initialSaveContext == null
|| (saveContext != null && saveContext.getType() == SaveContextType.SETTINGS)) {
// capture initial configuration iff:
// 1. the saveContext is for configuration, not document save
// 2. we haven't captured settings before
Expand Down Expand Up @@ -208,11 +209,17 @@ private synchronized void initialize() {
return;
}

if (commandService != null) {
commandService.beginCommandsRegistration();
}
ServiceLoader<IXMLExtension> extensions = ServiceLoader.load(IXMLExtension.class);
extensions.forEach(extension -> {
registerExtension(extension);
});
initialized = true;
if (commandService != null) {
commandService.endCommandsRegistration();
}
}

void registerExtension(IXMLExtension extension) {
Expand Down Expand Up @@ -389,10 +396,10 @@ public IXMLNotificationService getNotificationService() {
public void setNotificationService(IXMLNotificationService notificationService) {
this.notificationService = notificationService;
}

/**
* Returns the XML document validation service
*
*
* @return the validation service
*/
public IXMLValidationService getValidationService() {
Expand Down
34 changes: 22 additions & 12 deletions ...nx/src/main/java/org/eclipse/lemminx/services/extensions/commands/IXMLCommandService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,54 +18,64 @@
import org.eclipse.lsp4j.jsonrpc.CancelChecker;

/**
* Service available to XML LS extensions to add/remove/execute commands via the XML LS
* Service available to XML LS extensions to add/remove/execute commands via the
* XML LS
*
*
* @author Alex Boyko
*
*/
public interface IXMLCommandService {

/**
* Command handler to register with the workspace service
*/
@FunctionalInterface
public interface IDelegateCommandHandler {

/**
* Executes a command
* @param params command execution parameters
*
* @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, CancelChecker cancelChecker) throws Exception;
Object executeCommand(ExecuteCommandParams params, CancelChecker cancelChecker) throws Exception;
}

/**
* Registers a command with the language server
*
* @param commandId unique id of the command
* @param handler command handler function
* @param handler command handler function
*/
void registerCommand(String commandId, IDelegateCommandHandler handler);

/**
* Unregisters the command from the language server
*
* @param commandId unique id of the command to unregister
*/
void unregisterCommand(String commandId);

/**
* Executes a command via the client. The command can be registered by any language server
* Executes a command via the client. The command can be registered by any
* language server
*
* @param command the LSP command
* @param command the LSP command
* @return the result of the command
*/
CompletableFuture<Object> executeClientCommand(ExecuteCommandParams command);


default void beginCommandsRegistration() {

}

default void endCommandsRegistration() {

}

}
2 changes: 1 addition & 1 deletion .../src/main/java/org/eclipse/lemminx/settings/capabilities/ServerCapabilitiesConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ private ServerCapabilitiesConstants() {
public static final String CODE_LENS_ID = UUID.randomUUID().toString();
public static final String SIGNATURE_HELP_ID = UUID.randomUUID().toString();
public static final String RENAME_ID = UUID.randomUUID().toString();
public static final String EXECUTE_COMMAND_ID = UUID.randomUUID().toString();
public static final String WORKSPACE_EXECUTE_COMMAND_ID = UUID.randomUUID().toString();
public static final String WORKSPACE_SYMBOL_ID = UUID.randomUUID().toString();
public static final String DOCUMENT_SYMBOL_ID = UUID.randomUUID().toString();
public static final String CODE_ACTION_ID = UUID.randomUUID().toString();
Expand Down
8 changes: 8 additions & 0 deletions ...lemminx/src/main/java/org/eclipse/lemminx/settings/capabilities/XMLCapabilityManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@
import static org.eclipse.lemminx.settings.capabilities.ServerCapabilitiesConstants.TEXT_DOCUMENT_RENAME;
import static org.eclipse.lemminx.settings.capabilities.ServerCapabilitiesConstants.TEXT_DOCUMENT_TYPEDEFINITION;
import static org.eclipse.lemminx.settings.capabilities.ServerCapabilitiesConstants.TYPEDEFINITION_ID;
import static org.eclipse.lemminx.settings.capabilities.ServerCapabilitiesConstants.WORKSPACE_EXECUTE_COMMAND;
import static org.eclipse.lemminx.settings.capabilities.ServerCapabilitiesConstants.WORKSPACE_EXECUTE_COMMAND_ID;
import static org.eclipse.lemminx.settings.capabilities.ServerCapabilitiesConstants.WORKSPACE_WATCHED_FILES;
import static org.eclipse.lemminx.settings.capabilities.ServerCapabilitiesConstants.WORKSPACE_WATCHED_FILES_ID;

Expand All @@ -56,6 +58,7 @@
import org.eclipse.lemminx.settings.XMLSymbolSettings;
import org.eclipse.lsp4j.ClientCapabilities;
import org.eclipse.lsp4j.DidChangeWatchedFilesRegistrationOptions;
import org.eclipse.lsp4j.ExecuteCommandOptions;
import org.eclipse.lsp4j.FileSystemWatcher;
import org.eclipse.lsp4j.Registration;
import org.eclipse.lsp4j.RegistrationParams;
Expand Down Expand Up @@ -181,6 +184,11 @@ private void registerWatchedFiles() {
registerCapability(WORKSPACE_WATCHED_FILES_ID, WORKSPACE_WATCHED_FILES, options);
}

public void registerExecuteCommand(List<String> commands) {
registerCapability(WORKSPACE_EXECUTE_COMMAND_ID, WORKSPACE_EXECUTE_COMMAND,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't seem to call unregisterCapability anywhere... if commands are registered again does this "registration over registration" works ok in VScode and Eclipse?

new ExecuteCommandOptions(commands));
}

/**
* Registers(indicates the servers ability to support the service) all
* capabilities that have the ability to be turned on/off on the client side
Expand Down