Fix issue with groups not being reported in help output. (#411)

* Fix issue with groups not being reported in help output.

AbstractHelpFormatter:

 * Swapped printHelp using Iterable<Option> with printHelp using Options.  Effectively switching to using Options class to print the help.

 * Removed public String toSyntaxOptions(final Iterable<Option> options) as it was added in this version and should have only been used internally.

HelpFormatterTest:

 * removed toSyntaxOptions using Iterable<Option> test.

 * added a printHelpTest with Iterable<Option> as previously we depended on printHelpTest with Option to test the code branch.

 * added verifyOptionGroupingOutput as a test to show that reported issue is corrected.

* Don't break binary compatibility

* Restore test

Add test for toSyntaxOptions with iterable of options

* fixed issues arising from reverting incompatible binary changes

---------

Co-authored-by: Gary Gregory <[email protected]>

Author: Claudenw

src/main/java/org/apache/commons/cli/help/AbstractHelpFormatter.java

@@ -293,7 +293,7 @@ public final String getSyntaxPrefix() {
      * @param autoUsage     whether to print an automatically generated usage statement.
      * @throws IOException If the output could not be written to the {@link HelpAppendable}.
      */
-    public void printHelp(final String cmdLineSyntax, final String header, final Iterable<Option> options, final String footer, final boolean autoUsage)
+    public void printHelp(final String cmdLineSyntax, final String header, final Options options, final String footer, final boolean autoUsage)
             throws IOException {
         if (Util.isEmpty(cmdLineSyntax)) {
             throw new IllegalArgumentException("cmdLineSyntax not provided");
@@ -306,7 +306,7 @@ public void printHelp(final String cmdLineSyntax, final String header, final Ite
         if (!Util.isEmpty(header)) {
             helpAppendable.appendParagraph(header);
         }
-        helpAppendable.appendTable(getTableDefinition(options));
+        helpAppendable.appendTable(getTableDefinition(options.getOptions()));
         if (!Util.isEmpty(footer)) {
             helpAppendable.appendParagraph(footer);
         }
@@ -317,14 +317,16 @@ public void printHelp(final String cmdLineSyntax, final String header, final Ite
      *
      * @param cmdLineSyntax the syntax for this application.
      * @param header        the banner to display at the beginning of the help.
-     * @param options       the {@link Options} to print.
+     * @param options       the collection of {@link Option} objects to print.
      * @param footer        the banner to display at the end of the help.
      * @param autoUsage     whether to print an automatically generated usage statement.
      * @throws IOException If the output could not be written to the {@link HelpAppendable}.
      */
-    public final void printHelp(final String cmdLineSyntax, final String header, final Options options, final String footer, final boolean autoUsage)
+    public void printHelp(final String cmdLineSyntax, final String header, final Iterable<Option> options, final String footer, final boolean autoUsage)
             throws IOException {
-        printHelp(cmdLineSyntax, header, options.getOptions(), footer, autoUsage);
+        Options optionsObject = new Options();
+        options.forEach(optionsObject::addOption);
+        printHelp(cmdLineSyntax, header, optionsObject, footer, autoUsage);
     }
 
     /**
@@ -403,7 +405,11 @@ public final String toArgName(final String argName) {
 
     /**
      * Return the string representation of the options as used in the syntax display.
-     *
+     * <p>
+     *     This is probably not the method you want.  This method does not track the presence
+     *     of option groups.  To display the option grouping use {@link #toSyntaxOptions(Options)} or
+     *     {@link #toSyntaxOptions(OptionGroup)} for individual groups.
+     * </p>
      * @param options The collection of {@link Option} instances to create the string representation for.
      * @return the string representation of the options as used in the syntax display.
      */

src/test/java/org/apache/commons/cli/help/HelpFormatterTest.java

@@ -147,6 +147,85 @@ void testPrintHelp() throws IOException {
         assertEquals(0, sb.length(), "Should not write to output");
     }
 
+    @Test
+    public void testPrintHelpWithIterableOptions() throws IOException {
+        final StringBuilder sb = new StringBuilder();
+        final TextHelpAppendable serializer = new TextHelpAppendable(sb);
+        HelpFormatter formatter = HelpFormatter.builder().setHelpAppendable(serializer).get();
+
+        final List<Option> options = new ArrayList<>();
+        options.add(Option.builder("a").since("1853").hasArg().desc("aaaa aaaa aaaa aaaa aaaa").build());
+
+        List<String> expected = new ArrayList<>();
+        expected.add(" usage:  commandSyntax [-a <arg>]");
+        expected.add("");
+        expected.add(" header");
+        expected.add("");
+        expected.add(" Options      Since           Description       ");
+        expected.add(" -a <arg>     1853      aaaa aaaa aaaa aaaa aaaa");
+        expected.add("");
+        expected.add(" footer");
+        expected.add("");
+
+        formatter.printHelp("commandSyntax", "header", options, "footer", true);
+        List<String> actual = IOUtils.readLines(new StringReader(sb.toString()));
+        assertEquals(expected, actual);
+
+        formatter = HelpFormatter.builder().setShowSince(false).setHelpAppendable(serializer).get();
+        expected = new ArrayList<>();
+        expected.add(" usage:  commandSyntax [-a <arg>]");
+        expected.add("");
+        expected.add(" header");
+        expected.add("");
+        expected.add(" Options            Description       ");
+        expected.add(" -a <arg>     aaaa aaaa aaaa aaaa aaaa");
+        expected.add("");
+        expected.add(" footer");
+        expected.add("");
+
+        sb.setLength(0);
+        formatter.printHelp("commandSyntax", "header", options, "footer", true);
+        actual = IOUtils.readLines(new StringReader(sb.toString()));
+        assertEquals(expected, actual);
+
+        sb.setLength(0);
+        formatter.printHelp("commandSyntax", "header", options, "footer", false);
+        expected.set(0, " usage:  commandSyntax");
+        actual = IOUtils.readLines(new StringReader(sb.toString()));
+        assertEquals(expected, actual);
+
+        sb.setLength(0);
+        formatter.printHelp("commandSyntax", "", options, "footer", false);
+        expected.remove(3);
+        expected.remove(2);
+        actual = IOUtils.readLines(new StringReader(sb.toString()));
+        assertEquals(expected, actual);
+
+        sb.setLength(0);
+        formatter.printHelp("commandSyntax", null, options, "footer", false);
+        actual = IOUtils.readLines(new StringReader(sb.toString()));
+        assertEquals(expected, actual);
+
+        sb.setLength(0);
+        formatter.printHelp("commandSyntax", null, options, "", false);
+        expected.remove(6);
+        expected.remove(5);
+        actual = IOUtils.readLines(new StringReader(sb.toString()));
+        assertEquals(expected, actual);
+
+        sb.setLength(0);
+        formatter.printHelp("commandSyntax", null, options, null, false);
+        actual = IOUtils.readLines(new StringReader(sb.toString()));
+        assertEquals(expected, actual);
+
+        sb.setLength(0);
+        final HelpFormatter fHelp = formatter;
+        assertThrows(IllegalArgumentException.class, () -> fHelp.printHelp("", "header", options, "footer", true));
+        assertEquals(0, sb.length(), "Should not write to output");
+        assertThrows(IllegalArgumentException.class, () -> fHelp.printHelp(null, "header", options, "footer", true));
+        assertEquals(0, sb.length(), "Should not write to output");
+    }
+
     /**
      * Tests example from the mailing list that caused an infinite loop.
      *
@@ -415,4 +494,28 @@ void testToSyntaxOptionOptionsTest() {
                 "options with required group failed");
     }
 
+    @Test
+    void verifyOptionGroupingOutput() throws IOException {
+         // create options and groups
+         final Option o1 = new Option("o1", "Descr");
+         final Option o2 = new Option("o2", "Descr");
+
+         final Options options = new Options();
+         options.addOption(o1);
+         options.addOption(o2);
+
+         final OptionGroup group = new OptionGroup();
+         group.addOption(o1);
+         group.addOption(o2);
+         options.addOptionGroup(group);
+
+         final StringBuilder output = new StringBuilder();
+         //format options with new formatter
+         final org.apache.commons.cli.help.HelpFormatter newFormatter =
+                 org.apache.commons.cli.help.HelpFormatter.builder().setShowSince(false)
+        .setHelpAppendable(new TextHelpAppendable(output)).get();
+         newFormatter.printHelp("Command", null, options, null, true);
+         assertTrue(output.toString().contains("[-o1 | -o2]"));
+     }
+
 }