FIx issues with multiple positional args

Signed-off-by: Ben Sherman <[email protected]>

Author: bentsherman

modules/nextflow/src/main/groovy/nextflow/cli/v2/PluginCmd.groovy

@@ -41,10 +41,10 @@ class PluginCmd extends AbstractCmd {
     @ParentCommand
     private Launcher launcher
 
-    @Parameters
+    @Parameters(index = '0')
     String command
 
-    @Parameters
+    @Parameters(index = '1..*')
     List<String> args
 
     @Override

modules/nextflow/src/main/groovy/nextflow/cli/v2/RunCmd.groovy

@@ -52,10 +52,10 @@ class RunCmd extends AbstractCmd implements RunImpl.Options, HubOptions {
     @ParentCommand
     private Launcher launcher
 
-    @Parameters(description = 'Project name or repository url')
+    @Parameters(index = '0', description = 'Project name or repository url')
     String pipeline
 
-    @Parameters(description = 'Pipeline script args')
+    @Parameters(index = '1..*', description = 'Pipeline script args')
     List<String> args
 
     @Option(names = ['--ansi-log'], arity = '1', description = 'Use ANSI logging')
@@ -224,54 +224,88 @@ class RunCmd extends AbstractCmd implements RunImpl.Options, HubOptions {
     @Option(names = ['--with-weblog'], arity = '0..1', fallbackValue = '-', description = 'Send workflow status messages via HTTP to target URL')
     String withWebLog
 
-    @Parameters(description = 'Pipeline parameters')
-    List<String> params
+    private List<String> pipelineArgs = null
 
-    private Map<String,String> paramsMap = null
+    private Map<String,String> pipelineParams = null
 
     /**
-     * Get the pipeline params as a map.
+     * Parse the pipeline args and params from the positional
+     * args parsed by picocli. This method assumes that the first
+     * positional arg that starts with '--' is the first param,
+     * and parses the remaining args as params.
      *
-     * The double-dash ('--') notation is normally used to separate
-     * positional parameters from options. As a result, params will also
-     * contain the positional parameters of the `run` command (i.e. args),
-     * so they must be skipped when constructing the params map.
-     *
-     * This method assumes that params are specified as option-value pairs
-     * separated by a space. The equals-sign ('=') separator is not supported.
+     * NOTE: While the double-dash ('--') notation can be used to
+     * distinguish pipeline params from CLI options, it cannot be
+     * used to distinguish pipeline params from pipeline args.
      */
-    @Override
-    Map<String,String> getParams() {
-        if( paramsMap == null ) {
-            paramsMap = [:]
-
-            int i = args.size()
-            while( i < params.size() ) {
-                String current = params[i++]
-
-                String key
-                String value
-                if( current.contains('=') ) {
-                    int split = current.indexOf('=')
-                    key = current.substring(0, split)
-                    value = current.substring(split+1)
-                }
-                else if( i < params.size() && !params[i].startsWith('--') ) {
-                    key = current
-                    value = params[i++]
-                }
-                else {
-                    key = current
-                    value = 'true'
-                }
-
-                paramsMap.put(key, value)
+    private void parseArgs() {
+        // parse pipeline args
+        int i = args.findIndexOf { it.startsWith('--') }
+        pipelineArgs = args[0..<i]
+
+        // parse pipeline params
+        pipelineParams = [:]
+
+        if( i == -1 )
+            return
+
+        while( i < args.size() ) {
+            String current = args[i++]
+            if( !current.startsWith('--') ) {
+                throw new IllegalArgumentException("Invalid argument '${current}' -- unable to parse it as a pipeline arg, pipeline param, or CLI option")
+            }
+
+            String key
+            String value
+
+            // parse '--param=value'
+            if( current.contains('=') ) {
+                int split = current.indexOf('=')
+                key = current.substring(2, split)
+                value = current.substring(split+1)
+            }
+
+            // parse '--param value'
+            else if( i < args.size() && !args[i].startsWith('--') ) {
+                key = current.substring(2)
+                value = args[i++]
             }
 
-            log.trace "Parsing params from CLI: $paramsMap"
+            // parse '--param1 --param2 ...' as '--param1 true --param2 ...'
+            else {
+                key = current.substring(2)
+                value = 'true'
+            }
+
+            pipelineParams.put(key, value)
+        }
+
+        log.trace "Parsing pipeline args from CLI: $pipelineArgs"
+        log.trace "Parsing pipeline params from CLI: $pipelineParams"
+    }
+
+    /**
+     * Get the list of pipeline args.
+     */
+    @Override
+    List<String> getArgs() {
+        if( pipelineArgs == null ) {
+            parseArgs()
+        }
+
+        return pipelineArgs
+    }
+
+    /**
+     * Get the map of pipeline params.
+     */
+    @Override
+    Map<String,String> getParams() {
+        if( pipelineParams == null ) {
+            parseArgs()
         }
 
-        return paramsMap
+        return pipelineParams
     }
 
     @Override

modules/nextflow/src/test/groovy/nextflow/cli/v1/LauncherTest.groovy

@@ -45,7 +45,6 @@ class LauncherTest extends Specification {
         then:
         assert launcher.options.fullVersion
 
-
     }
 
     def 'should return `help` command' () {
@@ -135,9 +134,11 @@ class LauncherTest extends Specification {
         launcher.command.hubProvider == 'github'
 
         when:
-        launcher = new Launcher().parseMainArgs('run', 'script.nf', '--alpha', '0', '--omega', '9')
+        launcher = new Launcher().parseMainArgs('run', 'script.nf', 'arg1', 'arg2', '--alpha', '0', '--omega', '9')
         then:
         launcher.command instanceof RunCmd
+        launcher.command.pipeline == 'script.nf'
+        launcher.command.args == ['arg1', 'arg2']
         launcher.command.params.'alpha' == '0'
         launcher.command.params.'omega' == '9'
 
@@ -357,4 +358,5 @@ class LauncherTest extends Specification {
         String opt4
 
     }
+
 }

modules/nextflow/src/test/groovy/nextflow/cli/v2/LauncherTest.groovy

@@ -0,0 +1,162 @@
+/*
+ * Copyright 2023, Seqera Labs
+ *
+ * Licensed 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 nextflow.cli.v2
+
+import java.nio.file.Files
+
+import picocli.CommandLine
+import spock.lang.Specification
+
+/**
+ *
+ * @author Ben Sherman <[email protected]>
+ */
+class LauncherTest extends Specification {
+
+    def parseArgs(String... args) {
+        def launcher = new Launcher()
+        def cmd = new CommandLine(launcher)
+        cmd.parseArgs(args)
+
+        return launcher
+    }
+
+    def getCommand(Launcher launcher) {
+        launcher.spec.commandLine().getParseResult().subcommand().commandSpec().commandLine().getCommand()
+    }
+
+    def getArgs(Launcher launcher) {
+        launcher.spec.commandLine().getParseResult().originalArgs()
+    }
+
+
+    def 'should return `version` option' () {
+
+        when:
+        def launcher = parseArgs('-v')
+        then:
+        assert launcher.options.version
+
+        when:
+        launcher = parseArgs('--version')
+        then:
+        assert launcher.options.fullVersion
+
+    }
+
+    def 'should return `info` command'() {
+
+        when:
+        def launcher = parseArgs('info')
+        def command = getCommand(launcher)
+        then:
+        command instanceof InfoCmd
+        command.pipeline == null
+
+        when:
+        launcher = parseArgs('info', 'xxx')
+        command = getCommand(launcher)
+        then:
+        command instanceof InfoCmd
+        command.pipeline == 'xxx'
+
+    }
+
+    def 'should return `pull` command'() {
+
+        when:
+        def launcher = parseArgs('pull', 'alpha')
+        def command = getCommand(launcher)
+        then:
+        command instanceof PullCmd
+        command.pipeline == 'alpha'
+
+        when:
+        launcher = parseArgs('pull', 'xxx', '--hub', 'bitbucket', '--user', 'xx:11')
+        command = getCommand(launcher)
+        then:
+        command instanceof PullCmd
+        command.pipeline == 'xxx'
+        command.hubProvider == 'bitbucket'
+        command.hubUser == 'xx'
+        command.hubPassword == '11'
+
+    }
+
+    def 'should return `clone` command'() {
+        when:
+        def launcher = parseArgs('clone', 'xxx', 'yyy')
+        def command = getCommand(launcher)
+        then:
+        command instanceof CloneCmd
+        command.pipeline == 'xxx'
+        command.targetName == 'yyy'
+
+        when:
+        launcher = parseArgs('clone', 'xxx', '--hub', 'bitbucket', '--user', 'xx:yy')
+        command = getCommand(launcher)
+        then:
+        command instanceof CloneCmd
+        command.pipeline == 'xxx'
+        command.targetName == null
+        command.hubProvider == 'bitbucket'
+        command.hubUser == 'xx'
+        command.hubPassword == 'yy'
+    }
+
+    def 'should return `run` command'() {
+        when:
+        def launcher = parseArgs('run', 'xxx', '--hub', 'bitbucket', '--user', 'xx:yy')
+        def command = getCommand(launcher)
+        then:
+        command instanceof RunCmd
+        command.pipeline == 'xxx'
+        command.hubProvider == 'bitbucket'
+        command.hubUser == 'xx'
+        command.hubPassword == 'yy'
+
+        when:
+        launcher = parseArgs('run', 'alpha', '--hub', 'github')
+        command = getCommand(launcher)
+        then:
+        command instanceof RunCmd
+        command.pipeline == 'alpha'
+        command.hubProvider == 'github'
+
+        when:
+        launcher = parseArgs('run', 'script.nf', 'arg1', 'arg2', '--', '--alpha', '0', '--omega', '9')
+        command = getCommand(launcher)
+        then:
+        command instanceof RunCmd
+        command.pipeline == 'script.nf'
+        command.args == ['arg1', 'arg2']
+        command.params.'alpha' == '0'
+        command.params.'omega' == '9'
+
+    }
+
+    def 'should make cli' () {
+        given:
+        def launcher = new Launcher()
+        expect:
+        launcher.makeCli('nextflow', 'run', 'foo.nf') == 'nextflow run foo.nf'
+        launcher.makeCli('nextflow', 'run', 'foo.nf', '*.txt') == "nextflow run foo.nf '*.txt'"
+        launcher.makeCli('/this/that/nextflow run foo.nf', 'run', 'foo.nf', 'a{1,2}.z') == "nextflow run foo.nf 'a{1,2}.z'"
+        launcher.makeCli('/this/that/launch run bar.nf', 'run', 'bar.nf') == '/this/that/launch run bar.nf'
+    }
+
+}