Add support for relative parameter indexes (was: Mixins and parameter indexes)

[krisleonard-mcafee]

The indexes of Parameters inherited from Mixin's remain their original value. If for instance you have the following from the command line:

$ testCommand COMMON-PARAM-TWO TEST-COMMAND-PARAM COMMON-PARAM-ONE

With the following code:

public class CommonMixinOne {
    @CommandLine.Parameters(index = "0", paramLabel = "COMMON-PARAM-ONE")
    private String commonMixinOneParam;
}

public class CommonMixinTwo {
    @CommandLine.Parameters(index = "0", paramLabel = "COMMON-PARAM-TWO")
    private String commonMixinTwoParam;
}

@CommandLine.Command(name = "testCommand")
public class testCommand {
    @CommandLine.Mixin
    private CommonMixinTwo myCommonMixinTwo;

    @CommandLine.Parameters(index = "0", paramLabel = "TEST-COMMAND-PARAM")
    private String testCommandParam;

    @CommandLine.Mixin
    private CommonMixinOne myCommonMixinOne ;
}

After parsing input args all of the Parameters will be at index zero and in this example the parameters TEST-COMMAND-PARAM and COMMON-PARAM-ONE from the command line will be unmatched. Also assigning any other index in the code will result in a:

ParameterIndexGapException: Command definition should have a positional parameter with index=0. Nearest positional parameter 'COMMON-PARAM-ONE' has index=1

Am I missing something and there is a way around this without having to drop the common Mixin code in to the main command? I realize this might be a strange use case but it seems like perhaps positional parameters should have their indexes incremented when Mixins are used.

[remkop]

Yes, currently the application author needs to explicitly set the index of positional parameters and picocli will not modify this index.

I don't think it is possible to implement exactly what you are asking for, but I may be able to offer a reasonable alternative.

First, why can picocli not automatically increment the index for positional parameters?
The reason is that picocli uses reflection to discover the fields and methods annotated with @Parameters and this reflection does not always return them in the same order.
See #508 for details, but to me this means that automatically incrementing the index is probably not a good idea.

The alternative I have in mind is to allow applications to use variables for the index in the annotations.
See #526 for background.

Variables will allow you to do something like this:

public class CommonMixinOne {
    @Parameters(index = "${sys:commonParam1}" paramLabel = "COMMON-PARAM-ONE")
	private String commonMixinOneParam;
}

public class CommonMixinTwo {
    @Parameters(index = "${sys:commonParam2}" paramLabel = "COMMON-PARAM-TWO")
	private String commonMixinTwoParam;
}

@Command(name = "testCommand")
public class TestCommand {

	@Mixin
	private CommonMixinTwo myCommonMixinTwo;
	
	@Parameters(index = "1", paramLabel = "TEST-COMMAND-PARAM")
	private String testCommandParam;
	
	@Mixin
	private CommonMixinOne myCommonMixinOne;
	
	public static void main(String... args) {
		// in this particular application,
		// the indices of the mixin parameters are set as below:
		System.setProperty("commonParam2", "0");
		System.setProperty("commonParam1", "2");
		
		ParseResult result = CommandLine.parseArgs(new TestCommand(), args);
		// ...
	}
}

[krisleonard-mcafee]

Thanks @remkop for the response! It is helpful and I like your alternative to use variables.

[remkop]

This is now available in master. See draft release notes for details.

I’ll try to add a test for your use case before the release if I get a chance.

[remkop]

I added a test but this still fails with a validation exception; when creating the CommonMixinOne instance with index = "2", picocli cannot see that this mixin will be combined with other positional parameters that have indices 0 and 1, so it complains. I need to think about this further.

[remkop]

Some ideas for addressing this:

• introduce a @Command(fragment = true) annotation attribute to signify that some validation should be omitted
• support relative index values, perhaps something like index = "${relative:XYX+1}", where XYZ is the paramLabel` of another positional parameter.

We probably need something to postpone validation anyway: picocli will create a CommandLine instance for the mixin object, but it won’t be able to validate until after the mixin is added to main command.

The relative index idea looks interesting but by itself won’t solve this problem. It may still be a useful feature to add later.

[remkop]

This is related to #370: relative indexing.

If an index can be defined as a relative index, like this:

// add parameter to whatever was previously defined
@Parameters(index = "+") String a;

Also, if the Range class can preserve both the original value, and a “resolved” value, then at construction time the validation can be postponed (or omitted) for parameters with relative indices.

Perhaps index = "+" should even be the default index for positional parameters?

[remkop]

picocli 4.3 will offer better support for positional parameters in reusable components: parameters can have a relative index. An index of "+" means this parameter should be added to the end of the list, with a realized index of zero if it is the first positional parameter, or otherwise a realized index that is the maxIndex of the previous positional parameter, plus one.

A relative index can also be "anchored" to an absolute index. For example, an index of "1+" means that this parameter will be added after the positional parameter with max index "1".

For example:

static class CommonMixinOne {
    @Parameters(index = "0+", paramLabel = "COMMON-PARAM-ONE")
    private String commonMixinOneParam;
}

static class CommonMixinTwo {
    @Parameters(index = "2+", paramLabel = "COMMON-PARAM-TWO")
    private String commonMixinTwoParam;
}

@Test
// test for https://github.com/remkop/picocli/issues/564
public void testMixinsWithVariableIndex() {
    @Command(name = "testCommand", description = "Example for issue 564")
    class TestCommand {

        @Mixin
        private CommonMixinOne myCommonMixinOne;

        @Parameters(index = "1", paramLabel = "TEST-COMMAND-PARAM")
        private String testCommandParam;

        @Mixin
        private CommonMixinTwo myCommonMixinTwo;
    }

    CommandLine cmd = new CommandLine(new TestCommand());
    //ParseResult result = cmd.parseArgs(args);
    // ...

    String expected = String.format("" +
            "Usage: testCommand COMMON-PARAM-ONE TEST-COMMAND-PARAM COMMON-PARAM-TWO%n" +
            "Example for issue 564%n" +
            "      COMMON-PARAM-ONE%n" +
            "      TEST-COMMAND-PARAM%n" +
            "      COMMON-PARAM-TWO%n");
    String actual = cmd.getUsageMessage();
    assertEquals(expected, actual);
}