setTrimQuotes does not trim quotes

[juddgaddie]

        final AgentConfig config = new AgentConfig();
        final CommandLine commandLine = new CommandLine(config);
        commandLine.setTrimQuotes(true);
        commandLine.setSplitQuotedStrings(true);
        commandLine.setCaseInsensitiveEnumValuesAllowed(true);
        try
        {
            commandLine.parse(args);
        }
        catch (CommandLine.ParameterException e)
        {
            System.err.println(e.getMessage());
            commandLine.usage(System.err);
            return;
        }

[picocli DEBUG] Creating CommandSpec for object of class com.package.AgentConfig with factory picocli.CommandLine$DefaultFactory
[picocli INFO] Picocli version: 4.0.0-beta-1b, JVM: 1.8.0_202 (Oracle Corporation Java HotSpot(TM) 64-Bit Server VM 25.202-b08), OS: Linux 4.15.0-51-generic amd64
[picocli INFO] Parsing 8 command line args ["--env", "ACCEPTANCE_MULTIBOX", "--command", "DEPLOY", "--release-revision", "0.28.0-ci", "--servers", "10.24.2.24-LARGE,10.24.2.183-LARGE,10.24.2.13-LARGE,10.24.2.166-LARGE,10.24.2.53-LARGE,10.24.2.100-LARGE"]
[picocli DEBUG] Parser configuration: posixClusteredShortOptionsAllowed=true, stopAtPositional=false, stopAtUnmatched=false, separator=null, overwrittenOptionsAllowed=false, unmatchedArgumentsAllowed=false, expandAtFiles=true, atFileCommentChar=#, useSimplifiedAtFiles=false, endOfOptionsDelimiter=--, limitSplit=false, aritySatisfiedByAttachedOptionParam=false, toggleBooleanFlags=false, unmatchedOptionsArePositionalParams=false, collectErrors=false,caseInsensitiveEnumValuesAllowed=true, trimQuotes=true, splitQuotedStrings=true
[picocli DEBUG] (ANSI is disabled by default: isatty=false, XTERM=null, OSTYPE=null, isWindows=false, JansiConsoleInstalled=false, ANSICON=null, ConEmuANSI=null, NO_COLOR=null, CLICOLOR=null, CLICOLOR_FORCE=null)
[picocli DEBUG] Set initial value for field com.package.environment.Environment com.package.AgentConfig.environment of type class com.package.environment.Environment to null.
[picocli DEBUG] Set initial value for field com.package.command.Command com.package.AgentConfig.command of type class com.package.command.Command to null.
[picocli DEBUG] Set initial value for field String com.package.AgentConfig.releaseRevision of type class java.lang.String to SNAPSHOT.
[picocli DEBUG] Set initial value for field String com.package.AgentConfig.backupId of type class java.lang.String to null.
[picocli DEBUG] Set initial value for field java.util.List<String> com.package.AgentConfig.serviceNames of type interface java.util.List to [].
[picocli DEBUG] Set initial value for field java.util.List<String> com.package.AgentConfig.hosts of type interface java.util.List to [].
[picocli DEBUG] Set initial value for field boolean com.package.AgentConfig.force of type boolean to false.
[picocli DEBUG] Set initial value for field java.util.List<com.package.environment.HostInfo> com.package.AgentConfig.serversForDynamicTopologies of type interface java.util.List to [].
[picocli DEBUG] Set initial value for field java.util.List<java.util.Map$Entry<String, String>> com.package.AgentConfig.configProperties of type interface java.util.List to [].
[picocli DEBUG] Initializing com.package.AgentConfig: 9 options, 0 positional parameters, 2 required, 0 groups, 0 subcommands.
[picocli DEBUG] Processing argument '"--env"'. Remainder=["ACCEPTANCE_MULTIBOX", "--command", "DEPLOY", "--release-revision", "0.28.0-ci", "--servers", "10.24.2.24-LARGE,10.24.2.183-LARGE,10.24.2.13-LARGE,10.24.2.166-LARGE,10.24.2.53-LARGE,10.24.2.100-LARGE"]
[picocli DEBUG] '"--env"' cannot be separated into <option>=<option-parameter>
[picocli DEBUG] Could not find option '"--env"', deciding whether to treat as unmatched option or positional parameter...
...

```java
 @CommandLine.Option(names = {"--env", "-e"}, description = "Environment to execute on", required = true)
    private Environment environment;

    @CommandLine.Option(names = {"--command", "-c"}, description = "Command to execute", required = true)
    private Command command;
...

[remkop]

That looks like a bug, I will take a look as soon as I can.
Thanks for the detailed report!

[remkop]

The cause is that picocli currently only strips quotes from option parameters and positional parameters, not from option names.

I will fix that so that quotes are stripped from all arguments as they are processed.

Just out of interest, can I ask why the option names are quoted in your use case?

[juddgaddie]

I was migrating from JCommander and noticed the break. There is no reason that I'm aware of to quote the options so I've removed it to solve my problem, however the behaviour was very unexpected.

[remkop]

Agreed that it is inconsistent to not strip quotes from named options, 😁 I will fix this before the 4.0 release. Thanks for raising this! 🙏

I'm actually quite interested in use cases involving quotes. Usually the shell removes quotes before passing parameters to the application, so I was wondering how these quotes came to be there. 🤔 Any background is useful to know.

[remkop]

I am also interested in general feedback on the experience migrating from JCommander. (If you have time)

[juddgaddie]

We are executing the process from Runtime.getRuntime().exec() and were quoting all the arguments, there may have been a historical reason for this but it works now in our CI environment.

Generally migrating was simple, it was a drop in replacement, the only issue was the quoting.

[juddgaddie]

Another difference with JCommander is Lists are automatically created from a comma separated list of args where picocli requires splitter ","

[remkop]

I have finally made some progress with this issue. The parser will now be able to correctly handle cases like this:

• "-x" "abc" -> recognises value abc for option -x
• "a,b","x,y" -> correctly splits into 2 values (not 4): a,b and x,y
• "-x" a,b,"c,d,e",f -> correctly finds 4 values for option -x: first is a, second is b, third is c,d,e and last is f

I also updated the user manual section on how the parser handles quotes.

However, values with nested quotes like this still give it trouble:

• "-x=a,b,\"c,d,e\",f" -> unmatched argument error
• "-x" "a,b,\"c,d,e\",f" -> finds 3 values for option -x: first is a,b,c, second is d, and last is e,f

I need to think further about how to deal with this. Ideas welcome. :-)

[remkop]

@juddgaddie Your example uses setSplitQuotedStrings. That made me realize that the documentation for this backwards-compatibility switch was inadequate and perhaps misleading. I have updated the user manual and javadocs, but I am considering deprecating this method. Can you take a look at #760 and comment there?

[juddgaddie]

The cases I would expect the parser to handle for this are:
As above:
"-x" "abc" -> recognises value abc for option -x
"a,b","x,y" -> correctly splits into 2 values (not 4): a,b and x,y

For
"-x" "a,b,c" -> to convert to 3 values for option -x?

For the other cases I would not expect the behaviour to be defined, if it is great..

[remkop]

Thanks for the update. I was able to find a solution that handles nested quotes if they are escaped correctly. Now writing docs.