Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

defaultValue given on top of user-given arguments for list-type #2058

Closed
StaffanArvidsson opened this issue Jul 4, 2023 · 5 comments
Closed
Milestone

Comments

@StaffanArvidsson
Copy link

I have an @option argument that is of type List<MyType> and I have defined a defaultValue that I thought would be used unless the user given an explicit argument to this option. But instead it seems that the option will have both whatever the user give as well as the default value. To me this is not the intuitive behavior, or do you have any reason behind this? Running the latest 4.7.4 version of picocli.

@remkop
Copy link
Owner

remkop commented Jul 4, 2023

Thank you for raising this!

Can you provide a small program that demonstrates the issue?

@StaffanArvidsson
Copy link
Author

Sorry for the late reply, this email got lost somehow in my inbox.

When writing a simplified test-case I found out that it works as expected when using a ITypeConverter but fails when using a IParameterConsumer (i.e. then the value of defaultValue is passed as an additional argument in the stack). See the following test-class:

import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.Stack;
import java.util.concurrent.Callable;

import org.junit.Assert;
import org.junit.Test;

import picocli.CommandLine;
import picocli.CommandLine.IParameterConsumer;
import picocli.CommandLine.ITypeConverter;
import picocli.CommandLine.Model.ArgSpec;
import picocli.CommandLine.Model.CommandSpec;
import picocli.CommandLine.Option;

public class PicocliDefaultValueBug {
    
    // Toy custom class 
    public static class MyCustomCls {
        private final String lowerCase;

        public MyCustomCls(String value){
            lowerCase = value.toLowerCase(Locale.ENGLISH);
        }

        // Check equals in the test-case
        public String toString(){
            return lowerCase;
        }
    }

    // Custom converter version
    public static class MyClsConverter implements ITypeConverter<MyCustomCls> {
        @Override
        public MyCustomCls convert(String value) throws Exception {
            return new MyCustomCls(value);
        }
    }

    public static class TestCmdConverter implements Callable<Integer>{

        @Option(names =  {"-f"}, 
            defaultValue = "defaultArg", 
            converter = MyClsConverter.class,
            split = "\\s")
        public List<MyCustomCls> arguments = new ArrayList<>();
        @Override
        public Integer call() throws Exception {
            throw new UnsupportedOperationException("Unimplemented method 'call'");
        }

    }

    // Consumer version 
    public static class MyClsConsumer implements IParameterConsumer {

        @Override
        public void consumeParameters(Stack<String> args, ArgSpec argSpec, CommandSpec commandSpec) {
            List<MyCustomCls> list = argSpec.getValue();

            // Simple example, all remaining input are of this type
            while (!args.empty()){
                list.add(new MyCustomCls(args.pop()));
            }
        }

    }

    public static class TestCmdConsumer implements Callable<Integer>{

        @Option(names =  {"-f"}, 
            defaultValue = "defaultArg", 
            parameterConsumer = MyClsConsumer.class,
            split = "\\s")
        public List<MyCustomCls> arguments = new ArrayList<>();
        @Override
        public Integer call() throws Exception {
            throw new UnsupportedOperationException("Unimplemented method 'call'");
        }

    }

    @Test
    public void testShowDefaultAlwaysGiven() throws Exception {

        // Converter implementation - no argument given 
        TestCmdConverter program = new TestCmdConverter();
        new CommandLine(program).parseArgs();
        Assert.assertEquals(1, program.arguments.size());
        Assert.assertEquals("defaultarg",program.arguments.get(0).toString());

        // Converter implementation - when giving an expecit argument (works as well)
        program = new TestCmdConverter();
        new CommandLine(program).parseArgs("-f", "explicitArg");
        Assert.assertEquals(1, program.arguments.size());
        Assert.assertEquals("explicitarg",program.arguments.get(0).toString());

        // Test consumer - no argument
        TestCmdConsumer cmd = new TestCmdConsumer();
        new CommandLine(cmd).parseArgs();
        Assert.assertEquals(1, cmd.arguments.size());
        Assert.assertEquals("defaultarg",cmd.arguments.get(0).toString());
        

        // Consumer with explicit arg
        cmd = new TestCmdConsumer();
        new CommandLine(cmd).parseArgs("-f", "explicitArgument");
        Assert.assertEquals(1, cmd.arguments.size()); // fails, cmd.arguments.size == 2 (explicit argument and "defaultarg")
        Assert.assertEquals("explicitargument", cmd.arguments.get(0).toString());
    }
    
}

@remkop
Copy link
Owner

remkop commented Jul 7, 2023

Thank you for the test case!

I'll be traveling for the next 2 weeks and I'll look at this when I get back.

Thank you for raising this!

@remkop remkop added this to the 4.7.6 milestone Aug 27, 2023
@remkop remkop closed this as completed in d3d82b6 Aug 27, 2023
@remkop
Copy link
Owner

remkop commented Aug 27, 2023

@StaffanArvidsson Apologies! I had not marked this ticket as part of the 4.7.5 milestone, and I overlooked it when doing a release earlier today! Darn! 😓

Thank you for raising this!
This was a bug: there is some bookkeeping that was not done for options with a custom IParameterConsumer, and this lack of bookkeeping resulted in the default value being applied even when a user-specified value existed.
Unfortunately I cannot think of a workaround other than not using custom IParameterConsumers for now...

@StaffanArvidsson
Copy link
Author

No worries, glad that it might have helped others apart from myself 👍 thanks for the work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants