You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2021/01/05 08:56:28 UTC

[GitHub] [netbeans] sdedic opened a new pull request #2641: WIP: LSP support for ProgressHandles

sdedic opened a new pull request #2641:
URL: https://github.com/apache/netbeans/pull/2641


   Still WIP, opened for comments to polish the implementation. Cancel support is still missing.


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] sdedic commented on pull request #2641: LSP support for ProgressHandles

Posted by GitBox <gi...@apache.org>.
sdedic commented on pull request #2641:
URL: https://github.com/apache/netbeans/pull/2641#issuecomment-755217635


   Possibly ready to review. Licenses and formatting will be corrected in next commit. 
   
   Please focus on NB API changes - I moved some functionality from `org.netbeans.api.progress.nb` to `org.netbeans.api.progress`: the reason is that some features that are of interest of LSP clients use default action that "only" uses some cookies or API, and is not strictly visual. There's a chance that such action can be (in the future) reflected back to the LSP protocol and client. Short term, this allowed to capture progress from Maven, Gradle and others.
   
   Short overview:
   - Default (view) action support APIs moved to `org.netbeans.api.progress`
   - The message wrapper in LSP `Server` interprets `workDoneProgressToken` in request messages, so potential ProgressHandles connect back to a token that is issued by the LSP client
   - The wrapper handles centrally `workDoneCancel` message by cancelling ProgressHandle on behalf of the client.
   - `OperationContext` is present in the default lookup (and is propagated through spawned `RequestProcessor.Task`s), ProgressHandle creation works with that to connect back to the client and issue tokens.
   - LSP `InternalHandle` implementation sends notification on start, progress and finish events
   
   I adapted certain NB usages of Progress API to use just the basic API, and removed dependencies on Swing-specific implementation, when possible.
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] sdedic commented on a change in pull request #2641: LSP support for ProgressHandles

Posted by GitBox <gi...@apache.org>.
sdedic commented on a change in pull request #2641:
URL: https://github.com/apache/netbeans/pull/2641#discussion_r552550046



##########
File path: java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/debugging/Debugger.java
##########
@@ -48,4 +60,41 @@ public static void startDebugger(InputStream in, OutputStream out) {
             Exceptions.printStackTrace(ex);
         }
     }
+    
+    private static class ConsumeWithLookup implements MessageConsumer {
+        private final MessageConsumer delegate;
+        private OperationContext topContext;
+
+        public ConsumeWithLookup(MessageConsumer delegate) {
+            this.delegate = delegate;
+        }
+        
+        @Override
+        public void consume(Message message) throws MessageIssueException, JsonRpcException {
+            InstanceContent ic = new InstanceContent();
+            ProxyLookup ll = new ProxyLookup(new AbstractLookup(ic), Lookup.getDefault());
+            // HACK: piggyback on LSP's client.
+            if (topContext == null) {

Review comment:
       Tracked as [NETBEANS-5218](https://issues.apache.org/jira/browse/NETBEANS-5218)




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] sdedic commented on a change in pull request #2641: LSP support for ProgressHandles

Posted by GitBox <gi...@apache.org>.
sdedic commented on a change in pull request #2641:
URL: https://github.com/apache/netbeans/pull/2641#discussion_r552552292



##########
File path: platform/api.progress/src/org/netbeans/api/progress/aggregate/BasicAggregateProgressFactory.java
##########
@@ -53,6 +54,42 @@ public static ProgressContributor createProgressContributor(String trackingId) {
         return new ProgressContributor(trackingId);
     }
     
+    /**
+     * Create an aggregating progress ui handle for a long lasting task.
+     * @param contributors the initial set of progress indication contributors that are aggregated in the UI.
+     * @param allowToCancel either null, if the task cannot be cancelled or 
+     *          an instance of {@link org.openide.util.Cancellable} that will be called when user 
+     *          triggers cancel of the task.
+     * @param linkOutput an <code>Action</code> instance that links the running task in the progress bar
+     *                   to an output of the task. The action is assumed to open the apropriate component with the task's output.
+     * @param displayName to be shown in the progress UI
+     * @return an instance of <code>ProgressHandle</code>, initialized but not started.
+     * @since 1.59
+     */
+    public static AggregateProgressHandle createSystemHandle(String displayName, ProgressContributor[] contributors, 

Review comment:
       I could migrate them (add new ones, deprecated old ones), too - now when the API is being touched. Do you think it is beneficial ?
   




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] sdedic commented on pull request #2641: LSP support for ProgressHandles

Posted by GitBox <gi...@apache.org>.
sdedic commented on pull request #2641:
URL: https://github.com/apache/netbeans/pull/2641#issuecomment-755949479


   Consolidated before merging.


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] sdedic commented on pull request #2641: LSP support for ProgressHandles

Posted by GitBox <gi...@apache.org>.
sdedic commented on pull request #2641:
URL: https://github.com/apache/netbeans/pull/2641#issuecomment-756579048


   oops :) thanks for correction.


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] sdedic commented on a change in pull request #2641: LSP support for ProgressHandles

Posted by GitBox <gi...@apache.org>.
sdedic commented on a change in pull request #2641:
URL: https://github.com/apache/netbeans/pull/2641#discussion_r552560001



##########
File path: platform/api.progress/src/org/netbeans/api/progress/ProgressHandle.java
##########
@@ -77,6 +86,48 @@ public static ProgressHandle createHandle(String displayName, Cancellable allowT
     public static ProgressHandle createSystemHandle(String displayName, Cancellable allowToCancel) {
         return DefaultHandleFactory.get().createHandle(displayName, allowToCancel, false);
     }
+    
+    /**
+     * Creates a potentially cancellable handle, which can deliver an event if the user 'triggers' the handle. 
+     * How the handle will be presented, and what the "trigger" gesture is depends on the {@link Controller} 
+     * implementation. The user may make an action on the handle (other than cancel), which will
+     * be reflected back. Typically used to focus or display some part of the UI. 
+     * If {@code triggerCallback} is provided, it should handle at least {@link #ACTION_VIEW} action command
+     * {@link ActionEvent#getActionCommand}.
+     * 
+     * @param viewAction callback on user's trigger.
+     * @param displayName to be shown in the progress UI
+     * @param allowToCancel either null, if the task cannot be cancelled or 
+     *          an instance of {@link org.openide.util.Cancellable} that will be called when user 
+     *          triggers cancel of the task.
+     * @return an instance of {@link org.netbeans.api.progress.ProgressHandle}, initialized but not started.
+     * @since 1.59
+     */
+    public static ProgressHandle createHandle(String displayName, Cancellable allowToCancel, Action viewAction) {
+        ProgressHandle h = DefaultHandleFactory.get().createHandle(displayName, allowToCancel, true);
+        if (viewAction != null) {
+            h.addDefaultAction(viewAction);
+        }
+        return h;
+    }
+
+    /**
+     * Creates a possibly cancellable handle, <b>not initiated</b> by the user, e.g. for indexing, 
+     * repository or other system-launched tasks. See documentation for {@link #createHandle(java.lang.String, org.openide.util.Cancellable, java.awt.event.ActionListener)}

Review comment:
       My bad - I prototyped some variants, one of them accepted just `ActionListener`; but it turned out that some Action features (display name, icon, ...) could be also needed in displayed implementation. So I've ended up with `Action` and forgot to update the javadoc.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] sdedic commented on a change in pull request #2641: LSP support for ProgressHandles

Posted by GitBox <gi...@apache.org>.
sdedic commented on a change in pull request #2641:
URL: https://github.com/apache/netbeans/pull/2641#discussion_r552546750



##########
File path: extide/gradle/src/org/netbeans/modules/gradle/api/execute/GradleDistributionManager.java
##########
@@ -500,7 +499,7 @@ public String toString() {
         })
         public DownloadTask(GradleDistribution dist) {
             this.dist = dist;
-            handle = ProgressHandleFactory.createSystemHandle(Bundle.TIT_Download_Gradle(dist.getVersion()));
+            handle = ProgressHandle.createSystemHandle(Bundle.TIT_Download_Gradle(dist.getVersion()), null);

Review comment:
       Tracked as [NETBEANS-5217](https://issues.apache.org/jira/browse/NETBEANS-5217)




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] JaroslavTulach commented on a change in pull request #2641: LSP support for ProgressHandles

Posted by GitBox <gi...@apache.org>.
JaroslavTulach commented on a change in pull request #2641:
URL: https://github.com/apache/netbeans/pull/2641#discussion_r552515876



##########
File path: extide/gradle/nbproject/project.xml
##########
@@ -57,15 +57,7 @@
                     <compile-dependency/>
                     <run-dependency>
                         <release-version>1</release-version>
-                        <specification-version>1.47.1</specification-version>
-                    </run-dependency>
-                </dependency>
-                <dependency>
-                    <code-name-base>org.netbeans.api.progress.nb</code-name-base>
-                    <build-prerequisite/>
-                    <compile-dependency/>
-                    <run-dependency>
-                        <specification-version>1.47.1</specification-version>

Review comment:
       Removing dependency on `org.netbeans.api.progress.nb` is great!

##########
File path: java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/debugging/Debugger.java
##########
@@ -48,4 +60,41 @@ public static void startDebugger(InputStream in, OutputStream out) {
             Exceptions.printStackTrace(ex);
         }
     }
+    
+    private static class ConsumeWithLookup implements MessageConsumer {
+        private final MessageConsumer delegate;
+        private OperationContext topContext;
+
+        public ConsumeWithLookup(MessageConsumer delegate) {
+            this.delegate = delegate;
+        }
+        
+        @Override
+        public void consume(Message message) throws MessageIssueException, JsonRpcException {
+            InstanceContent ic = new InstanceContent();
+            ProxyLookup ll = new ProxyLookup(new AbstractLookup(ic), Lookup.getDefault());
+            // HACK: piggyback on LSP's client.
+            if (topContext == null) {

Review comment:
       OK, probably enough for 12.2.1. Would be good to have this recorded in an issue tracker.

##########
File path: extide/gradle/src/org/netbeans/modules/gradle/api/execute/GradleDistributionManager.java
##########
@@ -500,7 +499,7 @@ public String toString() {
         })
         public DownloadTask(GradleDistribution dist) {
             this.dist = dist;
-            handle = ProgressHandleFactory.createSystemHandle(Bundle.TIT_Download_Gradle(dist.getVersion()));
+            handle = ProgressHandle.createSystemHandle(Bundle.TIT_Download_Gradle(dist.getVersion()), null);

Review comment:
       I suggest to create a `META-INF/upgrade/*.hint` [Jackpot hint](https://github.com/oracle/graal/blob/master/truffle/src/com.oracle.truffle.api/src/META-INF/upgrade/sourcebuilder.hint) that would recommend this change. It should be straightforward following [this example](https://github.com/oracle/graal/blob/master/truffle/src/com.oracle.truffle.api/src/META-INF/upgrade/sourcebuilder.hint) moreover I believe @jlahoda can help if there are any issues.

##########
File path: platform/api.progress.nb/src/org/netbeans/api/progress/ProgressHandleFactory.java
##########
@@ -65,41 +73,62 @@ public static ProgressHandle createHandle(String displayName, Cancellable allowT
     }
 
     /**
-     * Create a progress ui handle for a long lasting task.
+     * Create a progress ui handle for a long lasting task. Since {@code 1.59}, this method was
+     * migrated to {@link ProgressHandle basic Progress API}.
      * @param linkOutput an <code>Action</code> instance that links the running task in the progress bar
      *                   to an output of the task. The action is assumed to open the apropriate component with the task's output.
      * @param displayName to be shown in the progress UI
      * @return an instance of {@link org.netbeans.api.progress.ProgressHandle}, initialized but not started.
+     * @deprecated Please use {@link ProgressHandle#createHandle(java.lang.String, org.openide.util.Cancellable, javax.swing.Action)}
      */
+    @Deprecated
     public static ProgressHandle createHandle(String displayName, Action linkOutput) {

Review comment:
       Please prepare a Jackpot hint that would rewrite to `ProgressHandle.createHandle`.

##########
File path: platform/api.progress.nb/src/org/netbeans/api/progress/aggregate/AggregateProgressFactory.java
##########
@@ -47,12 +48,12 @@ private AggregateProgressFactory() {
      *                   to an output of the task. The action is assumed to open the apropriate component with the task's output.
      * @param displayName to be shown in the progress UI
      * @return an instance of <code>ProgressHandle</code>, initialized but not started.
-     *
+     * @deprecated use {@link BasicAggregateProgressFactory#createHandle(java.lang.String, org.netbeans.api.progress.aggregate.ProgressContributor[], org.openide.util.Cancellable, javax.swing.Action)}.
      */
+    @Deprecated

Review comment:
       Another Jackpot hint, please.

##########
File path: platform/api.progress.nb/src/org/netbeans/modules/progress/spi/ProgressUIWorkerProvider.java
##########
@@ -29,4 +29,13 @@
     public ProgressUIWorkerWithModel getDefaultWorker();
     
     public ExtractedProgressUIWorker getExtractedComponentWorker();
+
+    /**
+     * Provides an extracted worker instance for the given internal handle.
+     * @param handle internal handle for the worker.
+     * @return the worker instance, possibly {@code null}.
+     */
+    public default ExtractedProgressUIWorker extractProgressWorker(InternalHandle handle) {

Review comment:
       Missing versioning of this API change.

##########
File path: platform/api.progress.nb/src/org/netbeans/api/progress/ProgressHandleFactory.java
##########
@@ -119,44 +148,171 @@ public static JLabel createMainLabelComponent(ProgressHandle handle) {
      * @since org.netbeans.api.progress 1.8
      */
     public static JLabel createDetailLabelComponent(ProgressHandle handle) {
-        return ih(handle).extractDetailLabel();
+        return ihextract(handle).getDetailLabelComponent();
     }
     
     /**
      * Create a handle for a long lasting task that is not triggered by explicit user action.
      * Such tasks have lower priority in the UI.
+     * Since {@code 1.59}, the functionality moves to basic {@link ProgressHandle Progress API}; this method is retained for smooth transition of older API clients.
      * @param displayName to be shown in the progress UI
      * @return an instance of {@link org.netbeans.api.progress.ProgressHandle}, initialized but not started.
+     * @deprecated Use {@link ProgressHandle#createSystemHandle(java.lang.String, org.openide.util.Cancellable)}.
      */
+    @Deprecated
     public static ProgressHandle createSystemHandle(String displayName) {
         return createSystemHandle(displayName, null, null);
     }
 
     /**
      * Create a cancelable handle for a task that is not triggered by explicit user action.
      * Such tasks have lower priority in the UI.
+     * Since {@code 1.59}, the functionality moves to basic {@link ProgressHandle Progress API}; this method is retained for smooth transition of older API clients.
      * @param displayName to be shown in the progress UI
      * @param allowToCancel either null, if the task cannot be cancelled or 
      *          an instance of {@link org.openide.util.Cancellable} that will be called when user 
      *          triggers cancel of the task.
      * @return an instance of {@link org.netbeans.api.progress.ProgressHandle}, initialized but not started.
+     * @deprecated Use {@link ProgressHandle#createSystemHandle(java.lang.String, org.openide.util.Cancellable)}.
      */
     public static ProgressHandle createSystemHandle(String displayName, Cancellable allowToCancel) {
         return createSystemHandle(displayName, allowToCancel, null);
     }
     
     /**
-     * Create a progress ui handle for a task that is not triggered by explicit user action.
+     * Create a progress UI handle for a task that is not triggered by explicit user action.
+     * Starting from {@code 1.59}, this method is fully superseded by {@link ProgressHandle#createSystemHandle(java.lang.String, org.openide.util.Cancellable, javax.swing.Action)}.
+     * Since {@code 1.59}, the functionality moves to basic {@link ProgressHandle Progress API}; this method is retained for smooth transition of older API clients.
      * @param allowToCancel either null, if the task cannot be cancelled or 
      *          an instance of {@link org.openide.util.Cancellable} that will be called when user 
      *          triggers cancel of the task.
      * @param linkOutput an <code>Action</code> instance that links the running task in the progress bar
      *                   to an output of the task. The action is assumed to open the apropriate component with the task's output.
      * @param displayName to be shown in the progress UI
      * @return an instance of {@link org.netbeans.api.progress.ProgressHandle}, initialized but not started.
-     *
+     * @deprecated Please use {@link ProgressHandle#createSystemHandle(java.lang.String, org.openide.util.Cancellable, javax.swing.Action)}
+     * @Deprecated
      */
     public static ProgressHandle createSystemHandle(String displayName, Cancellable allowToCancel, Action linkOutput) {

Review comment:
       Please prepare a Jackpot hint that would rewrite to `ProgressHandle.createSystemHandle`.

##########
File path: java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/ui/AbstractProgressEnvironment.java
##########
@@ -0,0 +1,135 @@
+/*
+ * 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.ui;
+
+import java.util.regex.Pattern;
+import org.netbeans.api.progress.ProgressHandle;
+import org.netbeans.modules.java.lsp.server.progress.LspInternalHandle;
+import org.netbeans.modules.java.lsp.server.progress.LspProgressUIWorker;
+import org.netbeans.modules.java.lsp.server.progress.OperationContext;
+import org.netbeans.modules.java.lsp.server.protocol.NbCodeLanguageClient;
+import org.netbeans.modules.progress.spi.Controller;
+import org.netbeans.modules.progress.spi.InternalHandle;
+import org.netbeans.modules.progress.spi.ProgressEnvironment;
+import org.openide.util.Cancellable;
+import org.openide.util.Lookup;
+import org.openide.util.lookup.Lookups;
+
+/**
+ *
+ * @author sdedic
+ */
+public class AbstractProgressEnvironment implements ProgressEnvironment {

Review comment:
       Make `abstract`?

##########
File path: java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/ui/AbstractProgressEnvironment.java
##########
@@ -0,0 +1,135 @@
+/*
+ * 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.ui;
+
+import java.util.regex.Pattern;
+import org.netbeans.api.progress.ProgressHandle;
+import org.netbeans.modules.java.lsp.server.progress.LspInternalHandle;
+import org.netbeans.modules.java.lsp.server.progress.LspProgressUIWorker;
+import org.netbeans.modules.java.lsp.server.progress.OperationContext;
+import org.netbeans.modules.java.lsp.server.protocol.NbCodeLanguageClient;
+import org.netbeans.modules.progress.spi.Controller;
+import org.netbeans.modules.progress.spi.InternalHandle;
+import org.netbeans.modules.progress.spi.ProgressEnvironment;
+import org.openide.util.Cancellable;
+import org.openide.util.Lookup;
+import org.openide.util.lookup.Lookups;
+
+/**
+ *
+ * @author sdedic
+ */
+public class AbstractProgressEnvironment implements ProgressEnvironment {
+    /**
+     * Some cancellables should not be accepted: they actually do weird things like
+     * RepositoryUpdater's one: it will do further UI operations when cancelled. Add
+     * impl class patterns here to make such progresses not cancellable in LSP clients.
+     */
+    private static final String[] MASK_CANCELLABLES = {
+        "org.netbeans.modules.parsing.impl.indexing.RepositoryUpdater$"
+    };
+    
+    private static final Controller NO_CONTEXT_CONTROLLER = new Controller(null);
+    private final Lookup env;
+    
+    private Pattern patternMaskingCancellables;
+    
+    public AbstractProgressEnvironment() {

Review comment:
       Make `protected`.

##########
File path: java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/ui/AbstractProgressEnvironment.java
##########
@@ -0,0 +1,135 @@
+/*
+ * 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.ui;
+
+import java.util.regex.Pattern;
+import org.netbeans.api.progress.ProgressHandle;
+import org.netbeans.modules.java.lsp.server.progress.LspInternalHandle;
+import org.netbeans.modules.java.lsp.server.progress.LspProgressUIWorker;
+import org.netbeans.modules.java.lsp.server.progress.OperationContext;
+import org.netbeans.modules.java.lsp.server.protocol.NbCodeLanguageClient;
+import org.netbeans.modules.progress.spi.Controller;
+import org.netbeans.modules.progress.spi.InternalHandle;
+import org.netbeans.modules.progress.spi.ProgressEnvironment;
+import org.openide.util.Cancellable;
+import org.openide.util.Lookup;
+import org.openide.util.lookup.Lookups;
+
+/**
+ *
+ * @author sdedic
+ */
+public class AbstractProgressEnvironment implements ProgressEnvironment {
+    /**
+     * Some cancellables should not be accepted: they actually do weird things like
+     * RepositoryUpdater's one: it will do further UI operations when cancelled. Add
+     * impl class patterns here to make such progresses not cancellable in LSP clients.
+     */
+    private static final String[] MASK_CANCELLABLES = {
+        "org.netbeans.modules.parsing.impl.indexing.RepositoryUpdater$"
+    };
+    
+    private static final Controller NO_CONTEXT_CONTROLLER = new Controller(null);
+    private final Lookup env;
+    
+    private Pattern patternMaskingCancellables;
+    
+    public AbstractProgressEnvironment() {
+        this(Lookups.proxy(() -> Lookup.getDefault()));
+    }
+
+    public AbstractProgressEnvironment(Lookup env) {

Review comment:
       Make `protected`.

##########
File path: platform/api.progress.nb/src/org/netbeans/api/progress/ProgressHandleFactory.java
##########
@@ -119,44 +148,171 @@ public static JLabel createMainLabelComponent(ProgressHandle handle) {
      * @since org.netbeans.api.progress 1.8
      */
     public static JLabel createDetailLabelComponent(ProgressHandle handle) {
-        return ih(handle).extractDetailLabel();
+        return ihextract(handle).getDetailLabelComponent();
     }
     
     /**
      * Create a handle for a long lasting task that is not triggered by explicit user action.
      * Such tasks have lower priority in the UI.
+     * Since {@code 1.59}, the functionality moves to basic {@link ProgressHandle Progress API}; this method is retained for smooth transition of older API clients.
      * @param displayName to be shown in the progress UI
      * @return an instance of {@link org.netbeans.api.progress.ProgressHandle}, initialized but not started.
+     * @deprecated Use {@link ProgressHandle#createSystemHandle(java.lang.String, org.openide.util.Cancellable)}.
      */
+    @Deprecated
     public static ProgressHandle createSystemHandle(String displayName) {
         return createSystemHandle(displayName, null, null);
     }
 
     /**
      * Create a cancelable handle for a task that is not triggered by explicit user action.
      * Such tasks have lower priority in the UI.
+     * Since {@code 1.59}, the functionality moves to basic {@link ProgressHandle Progress API}; this method is retained for smooth transition of older API clients.
      * @param displayName to be shown in the progress UI
      * @param allowToCancel either null, if the task cannot be cancelled or 
      *          an instance of {@link org.openide.util.Cancellable} that will be called when user 
      *          triggers cancel of the task.
      * @return an instance of {@link org.netbeans.api.progress.ProgressHandle}, initialized but not started.
+     * @deprecated Use {@link ProgressHandle#createSystemHandle(java.lang.String, org.openide.util.Cancellable)}.
      */
     public static ProgressHandle createSystemHandle(String displayName, Cancellable allowToCancel) {
         return createSystemHandle(displayName, allowToCancel, null);
     }
     
     /**
-     * Create a progress ui handle for a task that is not triggered by explicit user action.
+     * Create a progress UI handle for a task that is not triggered by explicit user action.
+     * Starting from {@code 1.59}, this method is fully superseded by {@link ProgressHandle#createSystemHandle(java.lang.String, org.openide.util.Cancellable, javax.swing.Action)}.
+     * Since {@code 1.59}, the functionality moves to basic {@link ProgressHandle Progress API}; this method is retained for smooth transition of older API clients.
      * @param allowToCancel either null, if the task cannot be cancelled or 
      *          an instance of {@link org.openide.util.Cancellable} that will be called when user 
      *          triggers cancel of the task.
      * @param linkOutput an <code>Action</code> instance that links the running task in the progress bar
      *                   to an output of the task. The action is assumed to open the apropriate component with the task's output.
      * @param displayName to be shown in the progress UI
      * @return an instance of {@link org.netbeans.api.progress.ProgressHandle}, initialized but not started.
-     *
+     * @deprecated Please use {@link ProgressHandle#createSystemHandle(java.lang.String, org.openide.util.Cancellable, javax.swing.Action)}
+     * @Deprecated
      */
     public static ProgressHandle createSystemHandle(String displayName, Cancellable allowToCancel, Action linkOutput) {
+        return ProgressHandle.createSystemHandle(displayName, allowToCancel, linkOutput);
+    }    
+
+    /**
+     * Create a progress UI handle for a task that is not triggered by explicit user action.
+     * This call creates a Swing-based implementation. Use only when the Handle is directly used in
+     * Swing UIs. These handles will never work in other presenters.
+     * @param allowToCancel either null, if the task cannot be cancelled or 
+     *          an instance of {@link org.openide.util.Cancellable} that will be called when user 
+     *          triggers cancel of the task.
+     * @param linkOutput an <code>Action</code> instance that links the running task in the progress bar
+     *                   to an output of the task. The action is assumed to open the apropriate component with the task's output.
+     * @param displayName to be shown in the progress UI
+     * @return an instance of {@link org.netbeans.api.progress.ProgressHandle}, initialized but not started.
+     * @since 1.59
+     */
+    public static ProgressHandle createSystemUIHandle(String displayName, Cancellable allowToCancel, Action linkOutput) {
         return new UIInternalHandle(displayName, allowToCancel, false, linkOutput).createProgressHandle();
     }    
+
+    private static ProgressUIWorkerProvider TRIVIAL_PROVIDER = new TrivialProgressUIWorkerProvider();
+    
+    private static ExtractedProgressUIWorker ihextract(ProgressHandle h) {
+        InternalHandle ih = h.getInternalHandle();
+        if (ih instanceof UIInternalHandle) {
+            return new UIHandleExtractor((UIInternalHandle)ih);
+        } else {
+            // fallback for non-UIInternalHandles. Let the environment to create
+            // a suitable component representation.
+            ProgressUIWorkerProvider prov = Lookup.getDefault().lookup(ProgressUIWorkerProvider.class);
+            if (prov == null) {
+                prov = TRIVIAL_PROVIDER;
+            }
+            ExtractedProgressUIWorker worker = prov.extractProgressWorker(ih);
+            if (worker != null) {
+                return new StrangeExtractor(ih, worker);
+            } else {
+                return null;
+            }
+        }
+    }
+    
+    private static class StrangeExtractor implements ExtractedProgressUIWorker {

Review comment:
       `StrangeExtractor` is really a strange name! NonUI, Foreign, Generic, Delegating?

##########
File path: platform/api.progress.nb/test/unit/src/org/netbeans/api/progress/ProgressHandleFactoryTest.java
##########
@@ -179,5 +183,60 @@ private void waitForTimerFinish() {
 
     }
      
+
+    /**
+     * Checks that handles produced by other env than org.netbeans.modules.progress.ui module
+     * can still get some +- suitable Progress components.
+     */
+    public void testUnexpectedInternalHandleExtraction() throws Exception {
+        MockLookup.setLayersAndInstances(new StrangeEnvironment());
+        
+        ProgressHandle handle = ProgressHandle.createHandle("task 1");
+        assertFalse(handle.getInternalHandle() instanceof UIInternalHandle);
+        
+        InternalHandle ih = handle.getInternalHandle();
+        assertFalse(ih.isCustomPlaced());
+        
+        // now attempt to extract a component from it:
+        JLabel l = ProgressHandleFactory.createMainLabelComponent(handle);
+        assertNotNull(l);
+        // the handle changed its placement:
+        assertTrue(ih.isCustomPlaced());
+    }
+
+    /**
+     * Checks semantics of the InternalHandle + extraction after the progress starts.
+     */
+    public void testUnexpectedInternalHandleExtractFailsAfterStart() throws Exception {
+        MockLookup.setLayersAndInstances(new StrangeEnvironment());
+        ProgressHandle handle = ProgressHandle.createHandle("task 1");
+        handle.start(100);
+        
+        // should throw an exception:
+        try {
+            JLabel l = ProgressHandleFactory.createMainLabelComponent(handle);
+            fail("Exppected ISE.");
+        } catch (IllegalStateException ex) {
+            // OK
+        }
+    }
+    
+    public class StrangeHandle extends InternalHandle {

Review comment:
       It is OK to use `StrangeXYZ` in tests, I guess.

##########
File path: platform/api.progress/src/org/netbeans/api/progress/ProgressHandle.java
##########
@@ -77,6 +86,48 @@ public static ProgressHandle createHandle(String displayName, Cancellable allowT
     public static ProgressHandle createSystemHandle(String displayName, Cancellable allowToCancel) {
         return DefaultHandleFactory.get().createHandle(displayName, allowToCancel, false);
     }
+    
+    /**
+     * Creates a potentially cancellable handle, which can deliver an event if the user 'triggers' the handle. 
+     * How the handle will be presented, and what the "trigger" gesture is depends on the {@link Controller} 
+     * implementation. The user may make an action on the handle (other than cancel), which will
+     * be reflected back. Typically used to focus or display some part of the UI. 
+     * If {@code triggerCallback} is provided, it should handle at least {@link #ACTION_VIEW} action command
+     * {@link ActionEvent#getActionCommand}.
+     * 
+     * @param viewAction callback on user's trigger.
+     * @param displayName to be shown in the progress UI
+     * @param allowToCancel either null, if the task cannot be cancelled or 
+     *          an instance of {@link org.openide.util.Cancellable} that will be called when user 
+     *          triggers cancel of the task.
+     * @return an instance of {@link org.netbeans.api.progress.ProgressHandle}, initialized but not started.
+     * @since 1.59
+     */
+    public static ProgressHandle createHandle(String displayName, Cancellable allowToCancel, Action viewAction) {
+        ProgressHandle h = DefaultHandleFactory.get().createHandle(displayName, allowToCancel, true);
+        if (viewAction != null) {
+            h.addDefaultAction(viewAction);
+        }
+        return h;
+    }
+
+    /**
+     * Creates a possibly cancellable handle, <b>not initiated</b> by the user, e.g. for indexing, 
+     * repository or other system-launched tasks. See documentation for {@link #createHandle(java.lang.String, org.openide.util.Cancellable, java.awt.event.ActionListener)}

Review comment:
       Is the `{@link ...}` correct? I am surprised to see `java.awt.event` package...

##########
File path: platform/api.progress/src/org/netbeans/api/progress/ProgressHandle.java
##########
@@ -240,6 +291,15 @@ public final void setDisplayName(String newDisplayName) {
         internal.requestDisplayNameChange(newDisplayName);
     }
     
+    /**
+     * Adds an action to the progress indicator. 
+     * @param action describes the action
+     * @return true, if actions are supported by the progress implementation.

Review comment:
       Missing versioning.

##########
File path: platform/api.progress/src/org/netbeans/api/progress/aggregate/BasicAggregateProgressFactory.java
##########
@@ -53,6 +54,42 @@ public static ProgressContributor createProgressContributor(String trackingId) {
         return new ProgressContributor(trackingId);
     }
     
+    /**
+     * Create an aggregating progress ui handle for a long lasting task.
+     * @param contributors the initial set of progress indication contributors that are aggregated in the UI.
+     * @param allowToCancel either null, if the task cannot be cancelled or 
+     *          an instance of {@link org.openide.util.Cancellable} that will be called when user 
+     *          triggers cancel of the task.
+     * @param linkOutput an <code>Action</code> instance that links the running task in the progress bar
+     *                   to an output of the task. The action is assumed to open the apropriate component with the task's output.
+     * @param displayName to be shown in the progress UI
+     * @return an instance of <code>ProgressHandle</code>, initialized but not started.
+     * @since 1.59
+     */
+    public static AggregateProgressHandle createSystemHandle(String displayName, ProgressContributor[] contributors, 

Review comment:
       Probably a bit late to change, but: why the factory methods are in `ProgressHandle`, but not in `AggregateProgressHandle`?

##########
File path: platform/api.progress/src/org/netbeans/api/progress/ProgressHandle.java
##########
@@ -77,6 +86,48 @@ public static ProgressHandle createHandle(String displayName, Cancellable allowT
     public static ProgressHandle createSystemHandle(String displayName, Cancellable allowToCancel) {
         return DefaultHandleFactory.get().createHandle(displayName, allowToCancel, false);
     }
+    
+    /**
+     * Creates a potentially cancellable handle, which can deliver an event if the user 'triggers' the handle. 
+     * How the handle will be presented, and what the "trigger" gesture is depends on the {@link Controller} 
+     * implementation. The user may make an action on the handle (other than cancel), which will
+     * be reflected back. Typically used to focus or display some part of the UI. 
+     * If {@code triggerCallback} is provided, it should handle at least {@link #ACTION_VIEW} action command
+     * {@link ActionEvent#getActionCommand}.
+     * 
+     * @param viewAction callback on user's trigger.
+     * @param displayName to be shown in the progress UI
+     * @param allowToCancel either null, if the task cannot be cancelled or 
+     *          an instance of {@link org.openide.util.Cancellable} that will be called when user 
+     *          triggers cancel of the task.
+     * @return an instance of {@link org.netbeans.api.progress.ProgressHandle}, initialized but not started.
+     * @since 1.59
+     */
+    public static ProgressHandle createHandle(String displayName, Cancellable allowToCancel, Action viewAction) {
+        ProgressHandle h = DefaultHandleFactory.get().createHandle(displayName, allowToCancel, true);
+        if (viewAction != null) {
+            h.addDefaultAction(viewAction);
+        }
+        return h;
+    }
+
+    /**
+     * Creates a possibly cancellable handle, <b>not initiated</b> by the user, e.g. for indexing, 
+     * repository or other system-launched tasks. See documentation for {@link #createHandle(java.lang.String, org.openide.util.Cancellable, java.awt.event.ActionListener)}
+     * for more details.
+     * @param displayName to be shown in the progress UI
+     * @param allowToCancel either {@code null}, if the task cannot be cancelled or an {@link Cancellable} instance
+     * that will be called when the user requests cancel.
+     * @param viewAction listener that will be triggered when the progress is 'activated'.
+     * @return 

Review comment:
       Missing `return` description.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] JaroslavTulach commented on pull request #2641: LSP support for ProgressHandles

Posted by GitBox <gi...@apache.org>.
JaroslavTulach commented on pull request #2641:
URL: https://github.com/apache/netbeans/pull/2641#issuecomment-756540992


   Certainly this fix shall not be backported into `12.0-u2`. Right now it has been integrated, so it will become part of `12.3`. However we want to have it available as part of `12.2-u1` unless I am mistaken. A new PR to backport these changes into `release122` branch has to be created.


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] sdedic merged pull request #2641: LSP support for ProgressHandles

Posted by GitBox <gi...@apache.org>.
sdedic merged pull request #2641:
URL: https://github.com/apache/netbeans/pull/2641


   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists