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

Argument groups ignored in mixins (was: Heading section ignored when used inside mixins) #776

Closed
teoincontatto opened this issue Jul 29, 2019 · 13 comments
Milestone

Comments

@teoincontatto
Copy link

I am not sure if this is expected behaviour or a bug but here it goes.

When I declare some heading section to show in the help using a mixin those sections does not appear:

  • Test code
import picocli.CommandLine;
import picocli.CommandLine.ArgGroup;
import picocli.CommandLine.Mixin;
import picocli.CommandLine.Option;

@picocli.CommandLine.Command(name = "test",
    description = "Picocli test.",
    abbreviateSynopsis = true,
    sortOptions = false,
    usageHelpAutoWidth = true)
public class Test {

  @Mixin
  TestConfig testConfig = new TestConfig();

  private static class TestConfig {
    @ArgGroup(validate = false, heading = "A1")
    private final A1 a1 = new A1();

    private static class A1 {
      @Option(names = {"-ta1", "--test-a1"})
      private String testA1;
      @Option(names = {"-ta2", "--test-a2"})
      private String testA2;
    }

    @ArgGroup(validate = false, heading = "A2")
    private final A2 a2 = new A2();

    private static class A2 {
      @Option(names = {"-tb1", "--test-b1"})
      private String testB1;
      @Option(names = {"-tb2", "--test-b2"})
      private String testB2;
    }
  }

  /**
   * Picocli test.
   */
  public static void main(String[] args) {
    new CommandLine(new Test()).usage(System.out);
  }

}
  • Output
Usage: test [OPTIONS]
Picocli test.
      -ta1, --test-a1=<testA1>

      -ta2, --test-a2=<testA2>

      -tb1, --test-b1=<testB1>

      -tb2, --test-b2=<testB2>

Removing the mixin and defining the options directly in the command has the expected result:

  • Test code
import picocli.CommandLine;
import picocli.CommandLine.ArgGroup;
import picocli.CommandLine.Option;

@picocli.CommandLine.Command(name = "test",
    description = "Picocli test.",
    abbreviateSynopsis = true,
    sortOptions = false,
    usageHelpAutoWidth = true)
public class Test {

  @ArgGroup(validate = false, heading = "A1")
  private final A1 a1 = new A1();

  private static class A1 {
    @Option(names = {"-ta1", "--test-a1"})
    private String testA1;
    @Option(names = {"-ta2", "--test-a2"})
    private String testA2;
  }

  @ArgGroup(validate = false, heading = "A2")
  private final A2 a2 = new A2();

  private static class A2 {
    @Option(names = {"-tb1", "--test-b1"})
    private String testB1;
    @Option(names = {"-tb2", "--test-b2"})
    private String testB2;
  }

  /**
   * Picocli test.
   */
  public static void main(String[] args) {
    new CommandLine(new Test()).usage(System.out);
  }

}
  • Output:
Usage: test [OPTIONS]
Picocli test.
A1      -ta1, --test-a1=<testA1>

      -ta2, --test-a2=<testA2>

A2      -tb1, --test-b1=<testB1>

      -tb2, --test-b2=<testB2>
@remkop remkop added this to the 4.0.2 milestone Jul 29, 2019
@remkop
Copy link
Owner

remkop commented Jul 29, 2019

Thank you for the bug report and for the code to reproduce the issue.
I'm looking into this now.

remkop added a commit that referenced this issue Jul 29, 2019
@remkop
Copy link
Owner

remkop commented Jul 30, 2019

Update: the problem is bigger than just the headers: it turns out that the logic for importing mixin elements does not take argument groups into account at all. Only the options and positional parameters are mixed in, but the argument group information is ignored.

This is an oversight, not by design. I'm looking now at how to address this.


Side note:

picocli does not add a newline after section headings. You may want to add a %n after the custom option section headings.

@remkop remkop changed the title Heading section ignored when used inside mixins Argument groups ignored in mixins (was: Heading section ignored when used inside mixins) Jul 30, 2019
@remkop
Copy link
Owner

remkop commented Jul 30, 2019

@teoincontatto Thank you again for raising this!
I'm planning to add some more tests but I believe I found a fix that addresses the issue you reported and the bigger problem that @ArgGroups in mixins were ignored altogether.
I pushed a fix to master. Can you verify?

@teoincontatto
Copy link
Author

Hi @remkop, glad to help picocli to get better with my 2 cents. Sorry for not being able to test the master branch with my code base. I'll try it out this monday hopefully.

Regards

@remkop
Copy link
Owner

remkop commented Jul 31, 2019

Great! The easiest way to do this is to check out the project in git, then run gradlew publishToMavenLocal on the command line.

After that you can use the picocli-4.0.2-SNAPSHOT dependency in your project.

@teoincontatto
Copy link
Author

teoincontatto commented Aug 6, 2019

I was able to pass the simple test using the 4.0.2-SNAPSHOT version. Now I am facing a DuplicateOptionAnnotationsException in a more complex use case. The difference here is that the mixin is using a nested group. I managed to generate a simple test case that reproduce the new issue:

  • Test code:
@picocli.CommandLine.Command(name = "test-with-mixin-and-nested-group",
    description = "Picocli section test with mixin and nested group.",
    abbreviateSynopsis = true,
    sortOptions = false)
public class Test {
  @Mixin
  TestConfig testConfig = new TestConfig();

  private static class TestConfig {
    @ArgGroup(validate = false, heading = "A1%n")
    private final A1 a1 = new A1();

    private static class A1 {
      @ArgGroup(exclusive = true)
      private final NestedA1 nestedA1 = new NestedA1();

      private static class NestedA1 {
        @Option(names = {"-ta1", "--test-a1"})
        private String testA1;
        @Option(names = {"-ta2", "--test-a2"})
        private String testA2;
      }
    }

    @ArgGroup(validate = false, heading = "A2%n")
    private final A2 a2 = new A2();

    private static class A2 {
      @Option(names = {"-tb1", "--test-b1"})
      private String testB1;
      @Option(names = {"-tb2", "--test-b2"})
      private String testB2;
    }
  }

  /**
   * Picocli test.
   */
  public static void main(String[] args) {
    new CommandLine(new Test()).usage(System.out);
  }
}
  • Output:
picocli.CommandLine$DuplicateOptionAnnotationsException: Option name '-ta2' is used by both field String com.ongres.egres.TestPicocliHeadingSection$TestWithNestedMixin$TestConfig$A1$NestedA1.testA2 and field String com.ongres.egres.TestPicocliHeadingSection$TestWithNestedMixin$TestConfig$A1$NestedA1.testA2
	at picocli.CommandLine$DuplicateOptionAnnotationsException.create(CommandLine.java:15027)
	at picocli.CommandLine$DuplicateOptionAnnotationsException.access$1700(CommandLine.java:15021)
	at picocli.CommandLine$Model$CommandSpec.addOption(CommandLine.java:5381)
	at picocli.CommandLine$Model$CommandSpec.addMixin(CommandLine.java:5567)
	at picocli.CommandLine$Model$CommandReflection.initFromAnnotatedTypedMembers(CommandLine.java:9641)
	at picocli.CommandLine$Model$CommandReflection.initFromAnnotatedFields(CommandLine.java:9621)
	at picocli.CommandLine$Model$CommandReflection.extractCommandSpec(CommandLine.java:9532)
	at picocli.CommandLine$Model$CommandSpec.forAnnotatedObject(CommandLine.java:5116)
	at picocli.CommandLine.<init>(CommandLine.java:223)
	at picocli.CommandLine.<init>(CommandLine.java:196)
	at Test.main(Test.java:45)

Since this seem to be unrelated to the original issue let me know if you want me to open a new one.

@teoincontatto
Copy link
Author

teoincontatto commented Aug 6, 2019

If I do not use mixin then the result (with version 4.0.1 and 4.0.2-SNAPSHOT) is not trowing exception but it prints the nested options outside of the section in the help:

  • Test code:
@picocli.CommandLine.Command(name = "test-without-mixin-and-nested-group",
    description = "Picocli section test without mixin and nested group.",
    abbreviateSynopsis = true,
    sortOptions = false)
public static class Test {
  @ArgGroup(validate = false, heading = "A1%n")
  private final A1 a1 = new A1();

  private static class A1 {
    @ArgGroup(exclusive = true)
    private final NestedA1 nestedA1 = new NestedA1();

    private static class NestedA1 {
      @Option(names = {"-ta1", "--test-a1"})
      private String testA1;
      @Option(names = {"-ta2", "--test-a2"})
      private String testA2;
    }
  }

  @ArgGroup(validate = false, heading = "A2%n")
  private final A2 a2 = new A2();

  private static class A2 {
    @Option(names = {"-tb1", "--test-b1"})
    private String testB1;
    @Option(names = {"-tb2", "--test-b2"})
    private String testB2;
  }

  /**
   * Picocli test.
   */
  public static void main(String[] args) {
    new CommandLine(new Test()).usage(System.out);
  }
}
  • Output:
Usage: test-without-mixin-and-nested-group [OPTIONS]
Picocli section test without mixin and nested group.
      -ta1, --test-a1=<testA1>

      -ta2, --test-a2=<testA2>

A1
A2
      -tb1, --test-b1=<testB1>

      -tb2, --test-b2=<testB2>

@remkop
Copy link
Owner

remkop commented Aug 6, 2019

@teoincontatto Thanks a lot for finding more issues in this area!
It may be good to open a separate ticket for this, if you don't mind.
I will start investigating.

@remkop
Copy link
Owner

remkop commented Aug 6, 2019

Something strange:
In your last comment, the Output mentions an option -tc1, --test-c1=<testC1>, but I don't see a definition for that option in either the Test class or the TestWithoutMixin class...

@teoincontatto
Copy link
Author

teoincontatto commented Aug 6, 2019

You right, I've mixed code and output with another test I did, I correct the code and output with the correct ones in the previous message.

In any case I'll open separates issues with corrected source and output.

@remkop
Copy link
Owner

remkop commented Aug 6, 2019

@teoincontatto Thank you for raising separate tickets for the new issues. I have fixed (hopefully) #779, and provided some feedback on #778. Let's continue the discussion on the respective tickets.

About the original problem here (Argument groups ignored in mixins), I think that has also been fixed. Are you okay to close this ticket?

@teoincontatto
Copy link
Author

Yes seems resolved.

@remkop
Copy link
Owner

remkop commented Aug 6, 2019

Thanks for the confirmation!

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

No branches or pull requests

2 participants