Duplicate help output for ArgGroup from a Mixin

The following example uses a @Mixin in order to import an @ArgGroup:

import picocli.CommandLine;
import picocli.CommandLine.ArgGroup;
import picocli.CommandLine.Command;
import picocli.CommandLine.Mixin;
import picocli.CommandLine.Option;

@Command
class MyMixin
{
    @ArgGroup(exclusive = true, multiplicity = "1")
    Exclusive exclusive;

    static class Exclusive {
        @Option(names = "-a", required = true, description = "Use A.") int a;
        @Option(names = "-b", required = true, description = "Use B.") int b;
        @Option(names = "-c", required = true, description = "Use C.") int c;
    }
}

@Command(mixinStandardHelpOptions = true, name = "exclusivedemo")
public class MutuallyExclusiveOptionsDemo {
    @Mixin
    MyMixin mixin;

    public static void main(String... args) {
        int exitCode = new CommandLine(new MutuallyExclusiveOptionsDemo()).execute(args);
        System.exit(exitCode);
    }
}

The help output generated using picocli 4.7.6 lists the options -a, -b, and -c twice:

Usage: exclusivedemo [-hV] (-a=<a> | -b=<b> | -c=<c>)
  -a=<a>          Use A.
  -a=<a>          Use A.
  -b=<b>          Use B.
  -b=<b>          Use B.
  -c=<c>          Use C.
  -c=<c>          Use C.
  -h, --help      Show this help message and exit.
  -V, --version   Print version information and exit.

In contrast, picocli 4.7.5 lists them only once:

Usage: exclusivedemo [-hV] (-a=<a> | -b=<b> | -c=<c>)
  -a=<a>          Use A.
  -b=<b>          Use B.
  -c=<c>          Use C.
  -h, --help      Show this help message and exit.
  -V, --version   Print version information and exit.

That certainly looks like a bug. Thank you for raising this!

Any updates on this one?

Bump. This one hit me. For existing projects it's prob not a huge deal but while developing a new project I burned countless hours trying to figure out what I was doing wrong in my complex argument relationships...

Thanks for this awesome library!

fix: prevent duplicate printing of options

@simschla

Many thanks to @simschla for the PR #2377 for this issue!
I simplified the test a little:

@Test
public void testIssue2309OptionsForArgGroupInMixinsAreNotDuplicatedInHelp() {
    String output = new CommandLine(new Issue2309MutuallyExclusiveOptionsDemo()).getUsageMessage(Help.Ansi.OFF);
    String expected = String.format(
        "Usage: exclusivedemo [-hV] (-a=<a> | -b=<b> | -c=<c>)%n" +
        "  -a=<a>          Use A.%n" +
        "  -b=<b>          Use B.%n" +
        "  -c=<c>          Use C.%n" +
        "  -h, --help      Show this help message and exit.%n" +
        "  -V, --version   Print version information and exit.%n");

    assertEquals(expected, output);
}

@Command
static class Issue2309MyMixin {
    @ArgGroup(exclusive = true, multiplicity = "1") Exclusive exclusive;

    class Exclusive {
        @Option(names = "-a", required = true, description = "Use A.") int a;
        @Option(names = "-b", required = true, description = "Use B.") int b;
        @Option(names = "-c", required = true, description = "Use C.") int c;
    }
}

@Command(mixinStandardHelpOptions = true, name = "exclusivedemo")
class Issue2309MutuallyExclusiveOptionsDemo {
    @CommandLine.Mixin Issue2309MyMixin mixin;
}

I have taken a different approach than in the PR.

With the current implementation of ArgSpec::equals, adding an option may cause OptionSpec hashCode to change.
I should revise the implementation of ArgSpec::equals, but meanwhile, using Lists instead of Sets solves this particular issue.

NOTE TO SELF:

Index: src/main/java/picocli/CommandLine.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/main/java/picocli/CommandLine.java b/src/main/java/picocli/CommandLine.java
--- a/src/main/java/picocli/CommandLine.java	
+++ b/src/main/java/picocli/CommandLine.java	(date 1744350913664)
@@ -6948,9 +6948,9 @@
              * @throws InitializationException if the specified group or one of its {@linkplain ArgGroupSpec#parentGroup() ancestors} has already been added
              * @since 4.0 */
             public CommandSpec addArgGroup(ArgGroupSpec group) {
-                return addArgGroup(group, new HashSet<OptionSpec>(), new HashSet<PositionalParamSpec>());
+                return addArgGroup(group, new ArrayList<OptionSpec>(), new ArrayList<PositionalParamSpec>());
             }
-            private CommandSpec addArgGroup(ArgGroupSpec group, Set<OptionSpec> groupOptions, Set<PositionalParamSpec> groupPositionals) {
+            private CommandSpec addArgGroup(ArgGroupSpec group, List<OptionSpec> groupOptions, List<PositionalParamSpec> groupPositionals) {
                 Assert.notNull(group, "group");
                 if (group.parentGroup() != null) {
                     throw new InitializationException("Groups that are part of another group should not be added to a command. Add only the top-level group.");
@@ -6961,7 +6961,7 @@
                 return this;
             }
 
-            private void addGroupArgsToCommand(ArgGroupSpec group, Map<String, ArgGroupSpec> added, Set<OptionSpec> groupOptions, Set<PositionalParamSpec> groupPositionals) {
+            private void addGroupArgsToCommand(ArgGroupSpec group, Map<String, ArgGroupSpec> added, List<OptionSpec> groupOptions, List<PositionalParamSpec> groupPositionals) {
                 Map<String, OptionSpec> options = new HashMap<String, OptionSpec>();
                 for (ArgSpec arg : group.args()) {
                     if (arg.isOption()) {
@@ -7028,12 +7028,12 @@
                 for (Map.Entry<String, CommandLine> entry : mixin.subcommands().entrySet()) {
                     addSubcommand(entry.getKey(), entry.getValue());
                 }
-                Set<OptionSpec> options = new LinkedHashSet<OptionSpec>(mixin.options());
-                Set<PositionalParamSpec> positionals = new LinkedHashSet<PositionalParamSpec>(mixin.positionalParameters());
+                List<OptionSpec> options = new ArrayList<OptionSpec>(mixin.options());
+                List<PositionalParamSpec> positionals = new ArrayList<PositionalParamSpec>(mixin.positionalParameters());
                 for (ArgGroupSpec argGroupSpec : mixin.argGroups()) {
-                    Set<OptionSpec> groupOptions = new HashSet<OptionSpec>();
-                    Set<PositionalParamSpec> groupPositionals = new HashSet<PositionalParamSpec>();
-                    addArgGroup(argGroupSpec, groupOptions, groupPositionals);
+                    List<OptionSpec> groupOptions = new ArrayList<OptionSpec>();
+                    List<PositionalParamSpec> groupPositionals = new ArrayList<PositionalParamSpec>();
+                    addArgGroup(argGroupSpec, groupOptions, groupPositionals); // CAUTION: adding to spec may cause OptionSpec hashCode to change
                     options.removeAll(groupOptions);
                     positionals.removeAll(groupPositionals);
                 }

Nice, thanks for picking it up 😄

[remkop#2344][remkop#2309][remkop#2341] Options in ArgGroup from a Mi…

[#2344][#2309][#2341] fix broken test

Navigation Menu

Repository navigation

Duplicate help output for ArgGroup from a Mixin #2309

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Duplicate help output for ArgGroup from a Mixin #2309

Description

Activity

remkop commented on Jun 25, 2024

FreifeldRoyi commented on Jan 29, 2025

jrivard commented on Mar 8, 2025

remkop commented on Apr 11, 2025

simschla commented on Apr 11, 2025

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions