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

Gives the capability to execute command with client progress support #1186

Open
angelozerr opened this issue Sep 20, 2019 · 1 comment · May be fixed by #2821
Open

Gives the capability to execute command with client progress support #1186

angelozerr opened this issue Sep 20, 2019 · 1 comment · May be fixed by #2821

Comments

@angelozerr
Copy link

angelozerr commented Sep 20, 2019

JDT LS provides a client progress support which is used only internally.

It should be very fantastic if we could benefit of this progress support with custom delegate command handler.

It will avoid for external JDL LS extension like STS to implement their own progress support. @martinlippert what do you think about this idea?

Please note that LSP progress support is planned :

Manage of progress support for delegate command handler is to create an instance of ProgressReporter instead of CancellableProgressMonitor like today.

Here the current code of JDTLanguageServer:

public CompletableFuture<Object> executeCommand(ExecuteCommandParams params) {
	logInfo(">> workspace/executeCommand " + (params == null ? null : params.getCommand()));
	return computeAsync((monitor) -> {
		return commandHandler.executeCommand(params, monitor);
	});
}

A very simple fix is to replace computeAsync with computeAsyncWithClientProgress

public CompletableFuture<Object> executeCommand(ExecuteCommandParams params) {
	logInfo(">> workspace/executeCommand " + (params == null ? null : params.getCommand()));
	return computeAsyncWithClientProgress((monitor) -> {
		return commandHandler.executeCommand(params, monitor);
	});
}

But it will throw some progression information although command handler doesn't want to manage progress.

A clean solution should check if command to execute has a progress capability or not.

To do that we could manage this capability in plugin.xml with command/@progress:

<extension point="org.eclipse.jdt.ls.core.delegateCommandHandler">
      <delegateCommandHandler class="com.redhat.quarkus.jdt.internal.core.ls.QuarkusDelegateCommandHandler">
            <command id="quarkus.java.projectInfo"
                                progress="true"/>
       </delegateCommandHandler>
   </extension>

It means that monitor must be created after the collect of DelegateCommandHandlerDescriptor:

Collection<DelegateCommandHandlerDescriptor> candidates = handlers.stream().filter(desc -> desc.getAllCommands().contains(params.getCommand())).collect(Collectors.toSet()); //no cancellation here but it's super fast so it's ok.

What about if some candidates have a progress capability and other have not progress capability?

@angelozerr
Copy link
Author

For LSP progress support, I created an issue for LSP4J eclipse-lsp4j/lsp4j#370

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

Successfully merging a pull request may close this issue.

2 participants