eclipse-jdtls / eclipse.jdt.ls

Java language server
1.75k stars 389 forks source link

Gives the capability to use JDT LS progress monitor for a IDelegeteCommandHandler command #1349

Open angelozerr opened 4 years ago

angelozerr commented 4 years ago

Today IDelegeteCommandHandler receives a CancellableProgressMonitor as IProgressMonitor which means it's impossible to use progress capability for a given command.

It should be fantastic if we could declare a command with progress attribute like this:

<extension point="org.eclipse.jdt.ls.core.delegateCommandHandler">
      <delegateCommandHandler class="com.redhat.microprofile.jdt.internal.core.ls.MicroProfileDelegateCommandHandler">
            <command id="microprofile/projectInfo" progress="true"/>
            <command id="microprofile/propertyDefinition"/>
       </delegateCommandHandler>
   </extension>

In this sample:

akaroml commented 4 years ago

@yaohaizh what do you think?

akaroml commented 4 years ago

@angelozerr here to learn more about the reason why you want to use ProgressReporter. Is it because you want to report progress to the client (vscode-java)?

ProgressReporter extends CancellableProgressMonitor to report back to the client. In your example, I don't see microprofile/projectInfo can do more with ProgressReporter. Could you further clarify the use case?

angelozerr commented 4 years ago

Is it because you want to report progress to the client (vscode-java)?

Yes exactly. I would like to avoid implementing my own progress report (on server and client side) and use the same progress reporter than JDT LS (which I think should implement LSP progress but it's an another topic).

Today the only solution we found is to in the IDelegateCommandHandler, create a Job and call join.. See https://github.com/redhat-developer/quarkus-ls/blob/0ff91b4d0fe4670584a3ad23fe645c8141df7f3d/microprofile.jdt/com.redhat.microprofile.jdt.core/src/main/java/com/redhat/microprofile/jdt/internal/core/ls/MicroProfileDelegateCommandHandler.java#L128

Creating a job inside a CompletableFture just to manage progress reporter is not very clean, so if JDT LS could simplify that it should be very nice.

akaroml commented 4 years ago

First of all, I like this idea because I like transparency.

A while ago, we started an effort to show the internal job status of LS. And you can see it by clicking the status icon of Java LS in the status bar.

It is very valuable to show the progress of time-consuming operations. But for very quick ones, it would be quite noisy at the same time. AFAIK, most of the delegate commands are short living and quick to finish. So the concern here is how we could show something useful to the users without bringing much noise. It would be very helpful if you could share some use cases.

angelozerr commented 4 years ago

First of all, I like this idea because I like transparency.

Great :)

But for very quick ones, it would be quite noisy at the same time. AFAIK, most of the delegate commands are short living and quick to finish.

In Quarkus usecase, the collection of properties can take some times according the classpath (around 9 seconds). That's why we have managed progress monitor by given information about progression.

The collection of properties is done when user open an application.properties (validation is processed and call microprofile/projectInfo delegate command handler). The other features like completion, hover, consumes it too the microprofile/projectInfo.

If you want a little demo, I can do it.

akaroml commented 4 years ago

Clear enough. Thanks @angelozerr

@jdneo do you also have similar cases in Test Runner or Dependency Viewer parts?

jdneo commented 4 years ago

Test Runner's searching all test cases is a time consuming operation.

Thank you @angelozerr for this. May I ask where can I find this API's usage in the Quarkus extension?