Enhance Picocli to support reset instance value while reusing command object

[kaushalkumar]

Picocli reuse command object when used in interactive mode, but doesnt not provide support to optionally reset (or not reset) instance value which are not annotated with @Options, @Parameter etc. This is required in applications where command objects require instance variable (beyond @Option, @Parameter). Although this can be handled by application by resetting the value, but this will be a workaround. An approach could be to reset all the instance fields by default and introduce a new annotation (@NonPicocliInstance - with reset as true/false) which when set on instance fields gets cleared

Example

package example;

import org.jline.console.SystemRegistry;
import org.jline.console.impl.Builtins;
import org.jline.console.impl.SystemRegistryImpl;
import org.jline.reader.LineReader;
import org.jline.reader.LineReaderBuilder;
import org.jline.reader.MaskingCallback;
import org.jline.reader.Parser;
import org.jline.reader.impl.DefaultParser;
import org.jline.terminal.Terminal;
import org.jline.terminal.TerminalBuilder;
import picocli.CommandLine;
import picocli.CommandLine.Command;
import picocli.CommandLine.Option;
import picocli.shell.jline3.PicocliCommands;
import picocli.shell.jline3.PicocliCommands.PicocliCommandsFactory;

import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;
import java.util.function.Supplier;

/**
 * Example.
 */
public class Example {

    /**
     * Top-level command that just prints help.
     */
    @Command(name = "", description = "Example root command.", subcommands = {MyCommand.class})
    static class CliCommands implements Runnable {
        public void run() {
            System.out.println(new CommandLine(this).getUsageMessage());
        }
    }

    /**
     * A command with some options to demonstrate issue.
     */
    @Command(name = "cmd", mixinStandardHelpOptions = true, description = "Example test command.")
    static class MyCommand implements Runnable {

        private List<String> aggregatedModuleNComp;

        private List<String> modules;

        @Option(names = "-c", description = "comps", split = ",")
        private List<String> comps;

        public List<String> getModules() {
            return modules;
        }
        @Option(names = "-m", description = "modules", split = ",")
        public void setModules(final List<String> modules) {
            this.modules = modules;
        }
        public void run() {
            setAggregatedModuleNComp();
            System.out.println("Modules = " + modules);
            System.out.println("Comps = " + comps);
            System.out.println("Modules and Comps  = " + aggregatedModuleNComp);
        }

        private void setAggregatedModuleNComp() {
            if (aggregatedModuleNComp == null) {
                aggregatedModuleNComp = new ArrayList<>();
            }
            if (modules != null) {
                aggregatedModuleNComp.addAll(modules);
            }
            if (comps != null) {
                aggregatedModuleNComp.addAll(comps);
            }
        }
    }

    /**
     * Main method.
     * @param args arguments.
     * @throws Exception exception.
     */
    public static void main(final String[] args) throws Exception {
        Supplier<Path> workDir = () -> Paths.get(System.getProperty("user.dir"));
        // set up JLine built-in commands
        Builtins builtins = new Builtins(workDir, null, null);
        // set up picocli commands
        CliCommands commands = new CliCommands();

        PicocliCommandsFactory factory = new PicocliCommandsFactory();

        CommandLine cmd = new CommandLine(commands, factory);
        PicocliCommands picocliCommands = new PicocliCommands(cmd);

        Parser parser = new DefaultParser();
        Terminal terminal = TerminalBuilder.builder().build();
        SystemRegistry systemRegistry = new SystemRegistryImpl(parser, terminal, workDir, null);
        systemRegistry.setCommandRegistries(builtins, picocliCommands);
        LineReader reader = LineReaderBuilder.builder().terminal(terminal).completer(systemRegistry.completer())
                .parser(parser).variable(LineReader.LIST_MAX, 50).build();
        builtins.setLineReader(reader);
        factory.setTerminal(terminal);

        String prompt = "prompt> ";
        String rightPrompt = null;

        // start the shell and process input until the user quits with Ctrl-D
        String line;
        while (true) {
            systemRegistry.cleanUp();
            line = reader.readLine(prompt, rightPrompt, (MaskingCallback) null, null);
            systemRegistry.execute(line);
        }
    }
}

Execution

prompt> cmd
Modules = null
Comps = null
Modules and Comps  = []
prompt> cmd -c c1 -m m1
Modules = [m1]
Comps = [c1]
Modules and Comps  = [m1, c1]
prompt> cmd -c c2
Modules = [m1]
Comps = [c2]
Modules and Comps  = [m1, c1, m1, c2]
prompt>

[remkop]

Sorry, I am going to give a quick reply (I just skimmed your email and did not look at the code so apologies if I missed the point completely.)

It is possible to do some initialization with a custom ExecutionStrategy. However, resetting values in the application (like you mentioned) may be a better option.

At this point I want to keep picocli's scope limited to just command line options and positional parameters.

[bbodnar]

The point here is resetting the command-object, not the application.

This is a real problem in interactive applications (not exiting after the first command-execution but offering a sort of interactive shell to execute multiple commands), when a command is executed multiple times. The properties populated at previous executions are still set on any later execution, because picocli reuses the same command-instance. A possible workaround (for the application) is, to reset all the properties of the command-object after each execution, making it clean for the next execution.

It would be nice, if picocli would either support resetting commands (eg. optional interface IResettableCommand with a method reset() to be implemented by the command-objects and called by picocli before populating the instance), or optionally not reuse command-object-instances (but instantiate them before each command-execution).

[remkop]

@bbodnar The intention is for picocli to automatically reset all options and positional parameters, just prior to processing the input and (re)populating these options and positional parameters. It should already be possible to reuse a CommandLine instance.

I thought this was implemented correctly, but looking at #1531, this is apparently not working correctly for @Option or @Parameters-annotated setter methods. This is probably a bug, but I have not had time yet to investigate.

It appears to me that this ticket (#1532) is asking for a mechanism for something that picocli is already supposed to do (but which is not working correctly for some reason). Rather than providing such a mechanism, I would rather fix the bug, if possible. 😅

@bbodnar Are you experiencing this issue also only with @Option or @Parameters-annotated setter methods? Or is there some other additional way to reproduce this issue? (Wondering if I need to look for multiple bugs...)

[bbodnar]

@remkop Thank you for the quick reply. You are right, this is really not a generic problem, only argument-groups are affected (in my case). I have created #1536, a demo-application is attached. Members are annotated directly, no setter-methods involved.

[remkop]

@kaushalkumar Issue #1531 was fixed in picocli 4.6.3. Does that address your use case?