Completion Candidates in Groovy Scripts? #1258

aadrian · 2020-11-11T22:06:03Z

Hi,

is it possible to have completion candidates https://picocli.info/#_completion_candidates_variable in Groovy Scripts ?

( I could not find any working example :( )

Thanks in advance.

remkop · 2020-11-11T23:21:59Z

Hi, good question!
One way I can think of is to add the built-in GenerateCompletion subcommand to your script. Something like this:

@Command(name = "myscript", subcommands = GenerateCompletion.class)
@PicocliScript

@Option(names = "-x") 
@Field int x

println "hello $x"

Assuming you can execute this script with a wrapper shell script like myscript.sh or something, users can then generate and install the completion script by executing the following command:

source <(./myscript.sh generate-completion)

aadrian · 2020-11-11T23:33:28Z

@remkop Thank you very much for the help.

What about a pure groovy script, i.e. no classes? i.e. when there's only the myscript.groovy file?

In a Groovy Script, something like the example in documentation:

static class MyAbcCandidates extends ArrayList<String> {
    MyAbcCandidates() { super(Arrays.asList("A", "B", "C")); }
}

would not be possible.

Is there any other alternative to specify the candidates (since the @Option(completionCandidates=xx seems to expect a class only) ?

remkop · 2020-11-11T23:40:01Z

Oops, I misread your question!
Let me play around a bit to see if I can find a solution for your question on completion candidates.

remkop · 2020-11-12T01:52:50Z

The problem seems to be that the class defined in the script cannot be referred to from the script itself.

class MyAbcCandidates extends ArrayList<String> {
    MyAbcCandidates() { super(Arrays.asList("A", "B", "C")); }
}

@Option(names = '-s', completionCandidates = MyAbcCandidates.class)
@Field String s

The above script gives this error:

org.codehaus.groovy.control.MultipleCompilationErrorsException: startup failed:
Script1.groovy: 17: unable to find class 'MyAbcCandidates.class' for annotation attribute constant
 @ line 17, column 46.
    '-s', completionCandidates = MyAbcCandi
                                 ^

1 error

I am not sure yet if this can be avoided by referencing the class in a different way.
@Option(names = '-s', completionCandidates = Script1.MyAbcCandidates.class) did not work either...

A different approach, that may not meet your requirements but does successfully generate completion candidates, is using an enum for the type of the option. For example:

import picocli.CommandLine.Model.CommandSpec
import picocli.CommandLine
import static picocli.CommandLine.*

@Command(name="testCompletionCandidates")
@picocli.groovy.PicocliScript2
import groovy.transform.Field

@picocli.CommandLine.Spec
@Field CommandSpec spec;

enum MyType { A, B, C}

@Option(names = '-x')
@Field MyType x

Iterable<String> iter = spec.findOption("-x").completionCandidates()
println iter

This script successfully compiles and prints [A, B, C].

remkop · 2020-11-12T02:16:39Z

I found a nicer solution, but it requires some changes in picocli.

The idea is to support closures in the picocli annotations. So, for example:

import picocli.CommandLine.Model.CommandSpec
import picocli.CommandLine
import static picocli.CommandLine.*

@Command(name="testCompletionCandidatesWithClosure")
@picocli.groovy.PicocliScript2
import groovy.transform.Field

@picocli.CommandLine.Spec
@Field CommandSpec spec;

@Option(names = '-s', completionCandidates = {["A", "B", "C"]}) // <-- a closure that returns a List (which implements Iterable)
@Field String s

Iterable<String> iter = spec.findOption("-s").completionCandidates()
println iter

That looks very natural, and seems like a nice feature to support.

It does not seem that difficult to support. I did some prototyping and the below patch is sufficient to make the above example work.

The below patch supports Groovy closures in all places where a class is specified in the annotations; with this change, if the specified class extends groovy.lang.Closure, then the closure is called, and the result of executing the closure is used instead of the class instance.

I haven't committed it yet because I have some other work in progress, and I want to add some more tests for other use cases than just completion candidates: things like type converter, default provider, parameter consumer, and version provider.
Also, I want to make sure that this feature does not interfere with subcommands referenced as @Command(subcommands = MySubcommand.class). (Also, would this change make it possible to define subcommands as closures in this annotation? Something to try...)

Also, all this needs to be documented.

Do you feel like providing a pull request for this?

Index: picocli-groovy/src/main/java/picocli/groovy/GroovyFactory.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- picocli-groovy/src/main/java/picocli/groovy/GroovyFactory.java	(date 1605146146814)
+++ picocli-groovy/src/main/java/picocli/groovy/GroovyFactory.java	(date 1605146146814)
@@ -0,0 +1,15 @@
+package picocli.groovy;
+
+import groovy.lang.Closure;
+import picocli.CommandLine;
+
+public class GroovyFactory implements CommandLine.IFactory {
+    private final CommandLine.IFactory defaultFactory = CommandLine.defaultFactory();
+
+    public <K> K create(Class<K> cls) throws Exception {
+        if (Closure.class.isAssignableFrom(cls)) {
+            return cls.getConstructor(Object.class, Object.class).newInstance(null, null);
+        }
+        return defaultFactory.create(cls);
+    }
+}
Index: picocli-groovy/src/main/java/picocli/groovy/PicocliBaseScript2.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- picocli-groovy/src/main/java/picocli/groovy/PicocliBaseScript2.java	(revision 755ef4841dfae94a0a451774a59099693c4a73ee)
+++ picocli-groovy/src/main/java/picocli/groovy/PicocliBaseScript2.java	(date 1605144531835)
@@ -292,7 +292,7 @@
      * @return A CommandLine instance.
      */
     public CommandLine createCommandLine() {
-        CommandLine commandLine = new CommandLine(this);
+        CommandLine commandLine = new CommandLine(this, new GroovyFactory());
         if (CommandSpec.DEFAULT_COMMAND_NAME.equals(commandLine.getCommandName())) { // only if user did not specify @Command(name) attribute
             commandLine.setCommandName(this.getClass().getSimpleName());
         }
Index: src/main/java/picocli/CommandLine.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/main/java/picocli/CommandLine.java	(revision 755ef4841dfae94a0a451774a59099693c4a73ee)
+++ src/main/java/picocli/CommandLine.java	(date 1605146146641)
@@ -5245,12 +5245,23 @@
         static IParameterConsumer createParameterConsumer(IFactory factory, Class<? extends IParameterConsumer> cls) {
             return create(factory, cls);
         }
+        static Class<?> GROOVY_CLOSURE_CLASS = loadClosureClass();
+        private static Class<?> loadClosureClass() {
+            try { return Class.forName("groovy.lang.Closure"); }
+            catch (Exception ignored) { return null;}
+        }
         static <T> T create(IFactory factory, Class<T> cls) {
-            try { return factory.create(cls); }
+            T result = null;
+            try { result = factory.create(cls); }
             catch (NoSuchMethodException ex) { throw new InitializationException("Cannot instantiate " +
                     cls.getName() + ": the class has no constructor", ex); }
             catch (InitializationException ex) { throw ex; }
             catch (Exception ex) { throw new InitializationException("Could not instantiate " + cls + ": " + ex, ex); }
+            if (GROOVY_CLOSURE_CLASS != null && GROOVY_CLOSURE_CLASS.isAssignableFrom(cls)) {
+                try { result = (T) ((Callable) result).call(); }
+                catch (Exception ex) { throw new InitializationException("Error in Groovy closure: " + ex); }
+            }
+            return result;
         }
     }
     /** Describes the number of parameters required and accepted by an option or a positional parameter.

aadrian · 2020-11-12T07:44:12Z

I found a nicer solution ...
The idea is to support closures ...

@remkop Wow, this looks fantastic !

The main usecase for "standalone" Groovy Scripts is as a replacement for fragile and hard to maintain Bash scripts.

The Groovy Scripts can be distributed as simple script source file ( so that security and everybody is happy ), without any manual dependencies. ( so just by using @Grab , and #!/usr/bin/env groovy as a first line) .

Picocli is what makes them really usable :) .

remkop · 2020-11-12T07:53:26Z

@aadrian Glad you like the idea!

Are you interested in providing a pull request that takes the prototype above and adds some test cases and documentation?

aadrian · 2020-11-12T08:43:02Z

Are you interested in providing a pull request that takes the prototype above and adds some test cases and documentation?

@remkop yes. I'll try to add some tests and update the documentation for this scenario.

remkop · 2020-11-12T08:43:44Z

@aadrian Great! Thank you!

remkop · 2020-11-14T06:42:25Z

One idea to improve on this further: we can pull the logic from GroovyFactory into the default factory. That would allow closures to work in picocli annotations in any Groovy code, not just in Groovy scripts.

The GroovyFactory class and the change to PicocliBaseScript2 would no longer be necessary.

remkop · 2020-11-15T01:32:16Z

I investigated a bit further; pulling the logic into DefaultFactory looks good, results as expected.
One thing I noticed is that this cannot be used for type converters... For some reason this does not work:

@Option(names = ['-a', '--algorithm'],
    converter = [{ str -> MessageDigest.getInstance(str) }]) //closure is not passed to picocli from Groovy...
MessageDigest algorithm;

I will raise this on the Groovy mailing list, to see if this is a Groovy issue.

Please find the latest version of the patch below.
I added some tests. Are you still interesting on working on this?

Index: src/main/java/picocli/CommandLine.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/main/java/picocli/CommandLine.java	(revision 28f265ae71d3e80045ae1b1a96da3e81e878dd9d)
+++ src/main/java/picocli/CommandLine.java	(date 1605401317601)
@@ -5199,7 +5199,18 @@
      * @since 4.0 */
     public static IFactory defaultFactory() { return new DefaultFactory(); }
     private static class DefaultFactory implements IFactory {
+        static Class<?> GROOVY_CLOSURE_CLASS = loadClosureClass();
+        private static Class<?> loadClosureClass() {
+            try { return Class.forName("groovy.lang.Closure"); }
+            catch (Exception ignored) { return null;}
+        }
         public <T> T create(Class<T> cls) throws Exception {
+            if (GROOVY_CLOSURE_CLASS != null && GROOVY_CLOSURE_CLASS.isAssignableFrom(cls)) {
+                Callable<?> callable = (Callable<?>) cls.getConstructor(Object.class, Object.class).newInstance(null, null);
+                try { return (T) callable.call(); }
+                catch (Exception ex) { throw new InitializationException("Error in Groovy closure: " + ex); }
+
+            }
             if (cls.isInterface() && Collection.class.isAssignableFrom(cls)) {
                 if (List.class.isAssignableFrom(cls)) {
                     return cls.cast(new ArrayList<Object>());
Index: picocli-groovy/src/test/groovy/picocli/groovy/ClosureInAnnotationsTest.groovy
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- picocli-groovy/src/test/groovy/picocli/groovy/ClosureInAnnotationsTest.groovy	(date 1605403265844)
+++ picocli-groovy/src/test/groovy/picocli/groovy/ClosureInAnnotationsTest.groovy	(date 1605403265844)
@@ -0,0 +1,56 @@
+package picocli.groovy
+
+import org.junit.Ignore
+import org.junit.Test
+import picocli.CommandLine
+
+import java.security.MessageDigest
+import java.util.concurrent.Callable
+
+import picocli.CommandLine.Option;
+import picocli.CommandLine.Parameters;
+
+class ClosureInAnnotationsTest {
+
+    //static class ConverterDemoCompilerFailure  {
+    //    @Option(names = ['-a', '--algorithm'],
+    //            converter = { str -> MessageDigest.getInstance(str) }, // compiler error
+    //            description = ['MD2, MD5, SHA-1, SHA-256, SHA-384, SHA-512,',
+    //            '  or any other MessageDigest algorithm.'])
+    //    MessageDigest algorithm;
+    //}
+
+    static class ConverterDemoFailure  {
+        @Option(names = ['-a', '--algorithm'],
+                converter = [{ str -> MessageDigest.getInstance(str) } ],
+                description = ['MD2, MD5, SHA-1, SHA-256, SHA-384, SHA-512,',
+                        '  or any other MessageDigest algorithm.'])
+        MessageDigest algorithm;
+    }
+
+    @Ignore
+    @Test
+    void testTypeConverter() {
+        ConverterDemoFailure demo = CommandLine.populateCommand(new ConverterDemoFailure(), "-a=SHA-256")
+        assert demo.algorithm == MessageDigest.getInstance("SHA-256")
+
+        // gives
+        // picocli.CommandLine$MissingTypeConverterException:
+        // No TypeConverter registered for java.security.MessageDigest of
+        // field java.security.MessageDigest picocli.groovy.ClosureInAnnotationsTest$ConverterDemoFailure.algorithm
+    }
+
+    static class CompletionCandidatesDemo  {
+        @Option(names = ['-a', '--algorithm'],
+                completionCandidates = {["A", "B", "C"]},
+                description = ['MD2, MD5, SHA-1, SHA-256, SHA-384, SHA-512,',
+                        '  or any other MessageDigest algorithm.'])
+        String algorithm;
+    }
+    @Test
+    void testCompletionCandidates() {
+        CommandLine cmd = new CommandLine(new CompletionCandidatesDemo())
+        def actual = cmd.getCommandSpec().findOption("-a").completionCandidates()
+        assert ["A", "B", "C"] == actual
+    }
+}

remkop · 2020-11-17T00:58:24Z

The below code demonstrates the issue when trying to pass a Groovy closure to the @Option(converter = ...) attribute:

class ClosureTest {
    static class Demo {
        @picocli.CommandLine.Option(names = "-x",
                completionCandidates = {["A", "B", "C"]},
                converter = [{ str -> java.security.MessageDigest.getInstance(str) }])
        java.security.MessageDigest digest
    }

    static void main(String[] args) {
        def annotation = Demo.class.getDeclaredField("digest").getAnnotation(picocli.CommandLine.Option)
        Class ok = annotation.completionCandidates()
        assert ok != null
        assert Closure.class.isAssignableFrom(ok)
        assert ["A", "B", "C"] == ((Closure) ok.getConstructor(Object, Object).newInstance(null, null)).call()

        Class[] bad = annotation.converter()
        assert bad != null
        assert bad.length == 1 // this assert fails:
        //Exception in thread "main" Assertion failed:
        //
        //assert bad.length == 1
        //       |   |      |
        //       []  0      false
        //
        //	at org.codehaus.groovy.runtime.InvokerHelper.assertFailed(InvokerHelper.java:434)
        //	at org.codehaus.groovy.runtime.ScriptBytecodeAdapter.assertFailed(ScriptBytecodeAdapter.java:670)
        //	at closure.ClosureTest.main(ClosureTest.groovy:18)
    }
}

I will pass this example on to the Groovy community.

remkop · 2020-11-17T08:12:46Z

Support for the array values in the annotations will require a change in Groovy.
This is tracked here: https://issues.apache.org/jira/browse/GROOVY-9817
@paulk-asert created this PR with a fix: apache/groovy#1420

paulk-asert · 2020-11-19T02:59:51Z

My initial inclination is to add support in master and the GROOVY_3_0_X branch. Does this work for you?

remkop · 2020-11-19T03:30:20Z

@paulk-asert First, thanks for the amazingly quick turnaround!

I can document in the picocli manual that Groovy 3.0.7 or higher is required to be able to use closures for type converters in the annotations. So master and GROOVY_3_0_X branch is fine.

I think it is a nice feature, and I would not mind having this in Groovy 2.5.14+ as well, but it is not problem for me.

paulk-asert · 2020-11-19T04:31:49Z

@remkop merged. Thanks for the suggested enhancement. I suspect others will also like this capability. For now, I will stick with 3.0.7+ just to give IDE folks (IDEA/Netbeans/Eclipse) a more well-defined boundary for adding support. But if there is sufficient interest over time, we can merge/back-port to the 2_5_X branch.

remkop · 2020-11-19T04:34:04Z

Makes sense. Thank you again!

remkop · 2020-11-21T10:40:39Z

Pushed a fix to master that implements support for closures in the new default factory, tests and documentation.
This will be included in the next picocli release.

…arameter Type Converters section

…ngle Parameter Type Converters section

… from Single Parameter Type Converters section" This reverts commit ccd97b0.

remkop added theme: integration An issue or change related to integration with other frameworks, shells or operating systems type: question ❔ theme: shell An issue or change related to interactive (JLine) applications labels Nov 11, 2020

remkop added the type: doc 📘 label Nov 11, 2020

remkop added a commit that referenced this issue Nov 12, 2020

[#1258] added tests

42b8692

remkop added a commit that referenced this issue Nov 12, 2020

[#1258] simplify closure test

565a8c8

remkop added type: enhancement ✨ and removed type: question ❔ labels Nov 12, 2020

remkop added this to the 4.6 milestone Nov 12, 2020

MarkoMackic pushed a commit to MarkoMackic/picocli that referenced this issue Nov 13, 2020

[remkop#1258] added tests

074834e

MarkoMackic pushed a commit to MarkoMackic/picocli that referenced this issue Nov 13, 2020

[remkop#1258] simplify closure test

5c3c80d

remkop added a commit that referenced this issue Nov 19, 2020

[#1258] WIP DOC for closures in annotations

85791bf

remkop added the status: in-progress 👷‍♂️ label Nov 21, 2020

remkop closed this as completed in 5aa084f Nov 21, 2020

remkop removed the status: in-progress 👷‍♂️ label Nov 21, 2020

remkop added a commit that referenced this issue Jan 13, 2021

[#1258] DOC add link to Closures in Annotations section from Single P…

034c9c4

…arameter Type Converters section

MarkoMackic pushed a commit to MarkoMackic/picocli that referenced this issue Oct 17, 2021

[remkop#1258] DOC add link to Closures in Annotations section from Si…

ccd97b0

…ngle Parameter Type Converters section

MarkoMackic added a commit to MarkoMackic/picocli that referenced this issue Oct 17, 2021

Revert "[remkop#1258] DOC add link to Closures in Annotations section…

60692e2

… from Single Parameter Type Converters section" This reverts commit ccd97b0.

MarkoMackic added a commit to MarkoMackic/picocli that referenced this issue Oct 17, 2021

Revert "[remkop#1258] DOC add link to Closures in Annotations section…

02c38a0

… from Single Parameter Type Converters section" This reverts commit ccd97b0.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Completion Candidates in Groovy Scripts? #1258

Completion Candidates in Groovy Scripts? #1258

aadrian commented Nov 11, 2020

remkop commented Nov 11, 2020 •

edited

aadrian commented Nov 11, 2020

remkop commented Nov 11, 2020

remkop commented Nov 12, 2020

remkop commented Nov 12, 2020

aadrian commented Nov 12, 2020

remkop commented Nov 12, 2020

aadrian commented Nov 12, 2020

remkop commented Nov 12, 2020

remkop commented Nov 14, 2020 •

edited

remkop commented Nov 15, 2020

remkop commented Nov 17, 2020

remkop commented Nov 17, 2020 •

edited

paulk-asert commented Nov 19, 2020

remkop commented Nov 19, 2020

paulk-asert commented Nov 19, 2020

remkop commented Nov 19, 2020

remkop commented Nov 21, 2020

Completion Candidates in Groovy Scripts? #1258

Completion Candidates in Groovy Scripts? #1258

Comments

aadrian commented Nov 11, 2020

remkop commented Nov 11, 2020 • edited

aadrian commented Nov 11, 2020

remkop commented Nov 11, 2020

remkop commented Nov 12, 2020

remkop commented Nov 12, 2020

aadrian commented Nov 12, 2020

remkop commented Nov 12, 2020

aadrian commented Nov 12, 2020

remkop commented Nov 12, 2020

remkop commented Nov 14, 2020 • edited

remkop commented Nov 15, 2020

remkop commented Nov 17, 2020

remkop commented Nov 17, 2020 • edited

paulk-asert commented Nov 19, 2020

remkop commented Nov 19, 2020

paulk-asert commented Nov 19, 2020

remkop commented Nov 19, 2020

remkop commented Nov 21, 2020

remkop commented Nov 11, 2020 •

edited

remkop commented Nov 14, 2020 •

edited

remkop commented Nov 17, 2020 •

edited