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/05/13 12:07:01 UTC

[GitHub] [netbeans] JaroslavTulach commented on a change in pull request #2948: Support for Micronaut's automatic restarts (mn:run) for Maven

JaroslavTulach commented on a change in pull request #2948:
URL: https://github.com/apache/netbeans/pull/2948#discussion_r631670131



##########
File path: java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/debugging/launch/NbLaunchDelegate.java
##########
@@ -172,15 +174,37 @@ public void progressHandleCreated(ProgressOperationEvent e) {
                         testProgressHandler != null ? Lookups.fixed(toRun, ioContext, progress, testProgressHandler) : Lookups.fixed(toRun, ioContext, progress),
                         Lookup.getDefault()
                 );
-
+                
+                ProjectConfiguration selectConfiguration = null;
+                Object o = launchArguments.get("launchConfiguration");
+                if (o instanceof String) {
+                    Project p = FileOwnerQuery.getOwner(toRun);
+                    if (p != null) {
+                        ProjectConfigurationProvider<ProjectConfiguration> pcp = p.getLookup().lookup(ProjectConfigurationProvider.class);
+                        if (pcp != null) {
+                            String n = (String)o;
+                            selectConfiguration = pcp.getConfigurations().stream().filter(c -> n.equals(c.getDisplayName())).findAny().orElse(null);
+                        }
+                    }
+                }
+                ProjectConfiguration fSelectConfiguration = selectConfiguration;
+                
                 Lookup lookup;
                 if (singleMethod != null) {
                     lookup = Lookups.fixed(toRun, singleMethod, params, ioContext, progress);
                 } else {
                     lookup = Lookups.fixed(toRun, ioContext, params, progress);
                 }
                 Lookups.executeWith(launchCtx, () -> {
-                    providerAndCommand.first().invokeAction(providerAndCommand.second(), lookup);
+                    ActionProvider ap = providerAndCommand.first();
+                    
+                    if ((fSelectConfiguration != null) && (ap instanceof ActionProvider.ConfigurationAware)) {

Review comment:
       This part of the code would have to be rewritten to
   * include `fSelectConfiguration` in the `lookup`
   * remove the `cap` check and use `ap` directly
   
   should the concept of `ConfigurationAware` be avoided.

##########
File path: java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/WorkspaceServiceImpl.java
##########
@@ -179,6 +201,37 @@
         }
         throw new UnsupportedOperationException("Command not supported: " + params.getCommand());
     }
+    
+    private CompletableFuture<Object> findProjectConfigurations(FileObject ownedFile, String requestAction) {
+        return server.asyncOpenFileOwner(ownedFile).thenApply(p -> {
+            if (p == null) {
+                return CompletableFuture.completedFuture(Collections.emptyList());
+            }
+            ProjectConfigurationProvider<ProjectConfiguration> provider = p.getLookup().lookup(ProjectConfigurationProvider.class);
+            ActionProvider ap = p.getLookup().lookup(ActionProvider.class);
+            if (provider == null) {
+                return CompletableFuture.completedFuture(Collections.emptyList());
+            }
+            ActionProvider.ConfigurationAware cap = null;
+            if (requestAction != null) {
+                if (!(ap instanceof ActionProvider.ConfigurationAware)) {
+                    return CompletableFuture.completedFuture(Collections.emptyList());
+                }
+                cap = (ActionProvider.ConfigurationAware)ap;
+            }
+            List<String> configDispNames = new ArrayList<>();
+            for (ProjectConfiguration c : provider.getConfigurations()) {
+                if (cap != null) {
+                    ActionProvider specific = cap.forConfiguration(c);
+                    if (specific == null || !Arrays.asList(specific.getSupportedActions()).contains(requestAction)) {
+                        continue;
+                    }
+                }
+                configDispNames.add(c.getDisplayName());

Review comment:
       Selecting only configurations that support `requestAction` wouldn't be necessary, if actions from all configurations  were provided by Maven project.

##########
File path: java/maven/src/org/netbeans/modules/maven/execute/ActionToGoalUtils.java
##########
@@ -171,12 +182,17 @@ String getPackaging() {
             return packaging;
         }
     }
+    
     public static boolean isActionEnable(String action, NbMavenProjectImpl project, Lookup lookup) {
-       
+        return isActionEnable(action, project, null, lookup);
+    }
+    
+    public static boolean isActionEnable(String action, NbMavenProjectImpl project, ProjectConfiguration c, Lookup lookup) {

Review comment:
       The `ProjectConfiguration` would have to be extracted from `Lookup` should the `ConfigurationAware` interface be avoided.

##########
File path: java/maven/src/org/netbeans/modules/maven/ActionProviderImpl.java
##########
@@ -156,15 +160,30 @@
     public ActionProviderImpl(Project proj) {
         this.proj = proj;
     }
+    
+    protected M2Configuration usedConfiguration(boolean useActive) {
+        if (!useActive) {
+            return null;
+        }
+        ProjectConfigurationProvider<MavenConfiguration> p = proj.getLookup().lookup(ProjectConfigurationProvider.class);
+        return (M2Configuration)p.getActiveConfiguration();
+    }
 
     @Override
     public String[] getSupportedActions() {
         Set<String> supp = new HashSet<String>();
-        supp.addAll( Arrays.asList( supported));
-        for (MavenActionsProvider add : ActionToGoalUtils.actionProviders(proj)) {
-            Set<String> added = add.getSupportedDefaultActions();
-            if (added != null) {
-                supp.addAll( added);
+        supp.addAll( Arrays.asList(supported));
+        
+        MavenConfiguration c = usedConfiguration(false);

Review comment:
       This code would have to iterate over all configurations and merge the available action IDs together.

##########
File path: java/java.lsp.server/src/org/netbeans/modules/java/lsp/server/protocol/WorkspaceServiceImpl.java
##########
@@ -179,6 +201,37 @@
         }
         throw new UnsupportedOperationException("Command not supported: " + params.getCommand());
     }
+    
+    private CompletableFuture<Object> findProjectConfigurations(FileObject ownedFile, String requestAction) {
+        return server.asyncOpenFileOwner(ownedFile).thenApply(p -> {
+            if (p == null) {
+                return CompletableFuture.completedFuture(Collections.emptyList());
+            }
+            ProjectConfigurationProvider<ProjectConfiguration> provider = p.getLookup().lookup(ProjectConfigurationProvider.class);
+            ActionProvider ap = p.getLookup().lookup(ActionProvider.class);
+            if (provider == null) {
+                return CompletableFuture.completedFuture(Collections.emptyList());
+            }
+            ActionProvider.ConfigurationAware cap = null;
+            if (requestAction != null) {
+                if (!(ap instanceof ActionProvider.ConfigurationAware)) {

Review comment:
       This check disables _on the fly_ switching of configurations in the VS Code UI unless the project is known to do so by implementing the newly introduced `ConfigurationAware` interface.

##########
File path: java/maven/src/org/netbeans/modules/maven/configurations/CustomizedActionsProvider.java
##########
@@ -0,0 +1,104 @@
+/*
+ * 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.maven.configurations;
+
+import java.util.Collections;
+import java.util.Set;
+import org.netbeans.api.project.Project;
+import org.netbeans.modules.maven.api.NbMavenProject;
+import org.netbeans.modules.maven.api.execute.RunConfig;
+import org.netbeans.modules.maven.execute.model.NetbeansActionMapping;
+import org.netbeans.modules.maven.spi.actions.MavenActionsProvider;
+import org.netbeans.spi.project.LookupProvider.Registration.ProjectType;
+import org.netbeans.spi.project.ProjectServiceProvider;
+import org.openide.util.Lookup;
+
+/**
+ * Serves actions from customized configurations only. Ignores all defaulted configurations,
+ * to give a chance to other action providers to step in and provide their definitions.
+ * 
+ * @author sdedic
+ */
+@ProjectServiceProvider(
+        service = MavenActionsProvider.class,
+        projectTypes = @ProjectType(
+                id = NbMavenProject.TYPE, 
+                // attempt to precede most of the providers
+                position = -100000
+        )
+)
+public final class CustomizedActionsProvider implements MavenActionsProvider {

Review comment:
       Here I am starting to get lost. I don't know why the change in the UI(?) is necessary...

##########
File path: java/maven/test/unit/src/org/netbeans/modules/maven/NbMavenProjectImplTest.java
##########
@@ -232,4 +234,41 @@ private void writeMavenConfig(String text) throws IOException, InterruptedExcept
         TestFileUtils.touch(TestFileUtils.writeFile(wd, ".mvn/maven.config", text), null);
     }
 
+    public static interface PS {
+        public String m();
+    }
+
+    // BEGIN:ProjectServiceProvider.pluginSpecific
+    @ProjectServiceProvider(service=PS.class, 
+            projectType=NbMavenProject.TYPE + "/org.ntebeans.modules.maven:test.plugin")

Review comment:
       `ntebeans`?

##########
File path: java/maven/src/org/netbeans/modules/maven/configurations/M2Configuration.java
##########
@@ -44,54 +47,73 @@
 import org.netbeans.modules.maven.execute.model.NetbeansActionReader;
 import org.netbeans.modules.maven.execute.model.io.xpp3.NetbeansBuildActionXpp3Writer;
 import org.netbeans.modules.maven.spi.actions.AbstractMavenActionsProvider;
+import org.netbeans.modules.maven.spi.actions.MavenActionsProvider;
 import org.openide.filesystems.FileChangeAdapter;
 import org.openide.filesystems.FileChangeListener;
 import org.openide.filesystems.FileEvent;
 import org.openide.filesystems.FileObject;
 import org.openide.filesystems.FileRenameEvent;
 import org.openide.filesystems.FileUtil;
-import org.openide.util.Exceptions;
 import org.openide.util.NbBundle.Messages;
 
 /**
+ * Represents a configuration, Properties defined by {@link MavenConfiguration} plus list of action descriptions
+ * for that configuration. This class is used to represent:
+ * <ul>
+ * <li>default configurations, provided by Maven project.
+ * <li>profile-based configurations
+ * <li>user-customized configurations, that loads action configs from the project directory.
+ * </ul>
  *
  * @author mkleint
  */
 public class M2Configuration extends AbstractMavenActionsProvider implements MavenConfiguration, Comparable<M2Configuration> {
     private static final Logger LOG = Logger.getLogger(M2Configuration.class.getName());
 
     public static final String DEFAULT = "%%DEFAULT%%"; //NOI18N
+    public static final String FILENAME = "nbactions.xml"; //NOI18N
+    public static final String FILENAME_PREFIX = "nbactions-"; //NOI18N
+    public static final String FILENAME_SUFFIX = ".xml"; //NOI18N
     
     public static M2Configuration createDefault(FileObject projectDirectory) {
         return new M2Configuration(DEFAULT, projectDirectory);
     }
-    
+
+    /**
+     * True, if the M2Configuration comes from a customized project storage.
+     */
+    private boolean customized;
     private @NonNull final String id;
     private List<String> profiles;
-    public static final String FILENAME = "nbactions.xml"; //NOI18N
-    public static final String FILENAME_PREFIX = "nbactions-"; //NOI18N
-    public static final String FILENAME_SUFFIX = ".xml"; //NOI18N
     private final Map<String,String> properties = new HashMap<String,String>();
     private final FileObject projectDirectory;
     
     private final AtomicBoolean resetCache = new AtomicBoolean(false);
     private FileChangeListener listener = null;
+    private String displayName;
     
     public M2Configuration(String id, FileObject projectDirectory) {

Review comment:
       Pass here an instance of project and store it in `this.project` might be better than...

##########
File path: java/maven/arch.xml
##########
@@ -123,6 +123,14 @@
         </p>
         For the details and examples, see description in <a href="@TOP@/org/netbeans/modules/maven/api/execute/PrerequisitesChecker.html">PrerequisitesChecker javadoc</a>.
     </api>
+    <api group="layer" name="PluginLookup" type="export" category="official">
+     <p>
+       Technology-related services can be registered so they activate in a project that has configured a specific Maven plugin. Such services should
+       be placed in <code>Projects/org-netbeans-modules-maven/&lt;plugin-id>/Lookup</code> folder. Maven core module will plug these providers in
+       as soon as the plugin-id appears in the POM model, and will remote them from project's Lookup when the plugin is no longer part of the project's model.

Review comment:
       Typo _remote them_.

##########
File path: java/maven/src/org/netbeans/modules/maven/configurations/M2Configuration.java
##########
@@ -44,54 +47,73 @@
 import org.netbeans.modules.maven.execute.model.NetbeansActionReader;
 import org.netbeans.modules.maven.execute.model.io.xpp3.NetbeansBuildActionXpp3Writer;
 import org.netbeans.modules.maven.spi.actions.AbstractMavenActionsProvider;
+import org.netbeans.modules.maven.spi.actions.MavenActionsProvider;
 import org.openide.filesystems.FileChangeAdapter;
 import org.openide.filesystems.FileChangeListener;
 import org.openide.filesystems.FileEvent;
 import org.openide.filesystems.FileObject;
 import org.openide.filesystems.FileRenameEvent;
 import org.openide.filesystems.FileUtil;
-import org.openide.util.Exceptions;
 import org.openide.util.NbBundle.Messages;
 
 /**
+ * Represents a configuration, Properties defined by {@link MavenConfiguration} plus list of action descriptions
+ * for that configuration. This class is used to represent:
+ * <ul>
+ * <li>default configurations, provided by Maven project.
+ * <li>profile-based configurations
+ * <li>user-customized configurations, that loads action configs from the project directory.
+ * </ul>
  *
  * @author mkleint
  */
 public class M2Configuration extends AbstractMavenActionsProvider implements MavenConfiguration, Comparable<M2Configuration> {
     private static final Logger LOG = Logger.getLogger(M2Configuration.class.getName());
 
     public static final String DEFAULT = "%%DEFAULT%%"; //NOI18N
+    public static final String FILENAME = "nbactions.xml"; //NOI18N
+    public static final String FILENAME_PREFIX = "nbactions-"; //NOI18N
+    public static final String FILENAME_SUFFIX = ".xml"; //NOI18N
     
     public static M2Configuration createDefault(FileObject projectDirectory) {
         return new M2Configuration(DEFAULT, projectDirectory);
     }
-    
+
+    /**
+     * True, if the M2Configuration comes from a customized project storage.
+     */
+    private boolean customized;
     private @NonNull final String id;
     private List<String> profiles;
-    public static final String FILENAME = "nbactions.xml"; //NOI18N
-    public static final String FILENAME_PREFIX = "nbactions-"; //NOI18N
-    public static final String FILENAME_SUFFIX = ".xml"; //NOI18N
     private final Map<String,String> properties = new HashMap<String,String>();
     private final FileObject projectDirectory;
     
     private final AtomicBoolean resetCache = new AtomicBoolean(false);
     private FileChangeListener listener = null;
+    private String displayName;
     
     public M2Configuration(String id, FileObject projectDirectory) {
         assert id != null;
         this.id = id;
         this.projectDirectory = projectDirectory;
         profiles = Collections.<String>emptyList();
     }
-
+    
+    protected final Project getProject() {
+        return FileOwnerQuery.getOwner(projectDirectory);

Review comment:
       ...computing a project based on the `projectDirectory`.




-- 
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