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.

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 final 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);
     }
 
     /**
@@ -401,16 +403,6 @@ public final String toArgName(final String argName) {
         return optionFormatBuilder.toArgName(argName);
     }
 
-    /**
-     * Return the string representation of the options as used in the syntax display.
-     *
-     * @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.
-     */
-    public String toSyntaxOptions(final Iterable<Option> options) {
-        return toSyntaxOptions(options, o -> null);
-    }
-
     /**
      * 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 @@ public 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");
+    }
+
     @Test
     public void testPrintHelpXML() throws IOException {
         final StringBuilder sb = new StringBuilder();
@@ -334,21 +413,6 @@ public void testToSyntaxOptionGroupTest() {
         assertEquals("", underTest.toSyntaxOptions(new OptionGroup()), "empty group should return empty string");
     }
 
-    @Test
-    public void testToSyntaxOptionIterableTest() {
-        final HelpFormatter underTest = HelpFormatter.builder().get();
-        final List<Option> options = new ArrayList<>();
-
-        options.add(Option.builder().option("o").longOpt("one").hasArg().build());
-        options.add(Option.builder().option("t").longOpt("two").hasArg().required().argName("other").build());
-        options.add(Option.builder().option("th").longOpt("three").required().argName("other").build());
-        options.add(Option.builder().option("f").argName("other").build());
-        options.add(Option.builder().longOpt("five").hasArg().argName("other").build());
-        options.add(Option.builder().longOpt("six").required().hasArg().argName("other").build());
-        options.add(Option.builder().option("s").longOpt("sevem").hasArg().build());
-        assertEquals("[-f] [--five <other>] [-o <arg>] [-s <arg>] --six <other> -t <other> -th", underTest.toSyntaxOptions(options));
-
-    }
 
     @Test
     public void testToSyntaxOptionOptionsTest() {
@@ -402,4 +466,28 @@ public 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]"));
+     }
+
 }