[CLI-339] Help formatter extension by Claudenw · Pull Request #314 · apache/commons-cli

Claudenw · 2024-09-28T13:05:19Z

Implementation of a commons client help production system.

Creates a new package containing the classes used by commons-cli to produce the help output.
In general there are 4 classes that users/developers may be interested in.

HelpFormatter - the class used to produce the help output for most users.
HelpPrinter - Writes the output in a specific output serialization format (e.g. text, XHTML, Markdown, etc.)
OptionFormatter - Determines how to format the various data elements in an Option
TableDef - Defines a table to be displayed. Useful for developers who want to build custom option displays or use the help system to produce additional information in the help system
TextStyle - Defines the style for text in tables and the default style for the TextHelpPrinter.

Claudenw · 2024-10-02T07:47:02Z

Java 24-ea fails due to issues with spotbugs plugin under java-24

src/main/java/org/apache/commons/cli/Util.java

garydgregory

Hi @Claudenw

Great start ! :-)

I think that HelpWriter is really an Appendable since you already have 2 out of 3 methods of Appendable's implemented in kind with writeDirect(char|String) where Appendable has append(ch|CharSequence). I made the following change locally and carried it through with a local green build:

/*
  Licensed to the Apache Software Foundation (ASF) under one or more
  contributor license agreements.  See the NOTICE file distributed with
  this work for additional information regarding copyright ownership.
  The ASF licenses this file to You under the Apache License, Version 2.0
  (the "License"); you may not use this file except in compliance with
  the License.  You may obtain a copy of the License at

      http://www.apache.org/licenses/LICENSE-2.0

  Unless required by applicable law or agreed to in writing, software
  distributed under the License is distributed on an "AS IS" BASIS,
  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  See the License for the specific language governing permissions and
  limitations under the License.
 */
package org.apache.commons.cli.help;

import java.io.IOException;
import java.util.Collection;

/**
 * The definition of a semantic scribe.  The semantic scribe write string to output based on the semantic meaning
 * of the type of string.  e.g. a Paragraph -vs- a Heading.
 * <p>
 *     the representation of the semantics is dependant upon the format being output.  For example the plain text
 *     output for a paragraph may print the text followed by two line breaks, while an XHTML output would print the
 *     text surrounded by &lt;p&gt; and &lt;/p&gt;
 * </p>
 */
public interface HelpWriter extends Appendable {

    /**
     * Writes a title.
     *
     * @param title the title to write.
     * @throws IOException on write failure
     */
    void writeTitle(String title) throws IOException;

    /**
     * Writes a paragraph.
     *
     * @param paragraph the paragraph to write.
     * @throws IOException on write failure
     */
    void writePara(String paragraph) throws IOException;

    /**
     * Writes a header.
     *
     * @param level  the level of the header
     * @param text   the text for the header
     * @throws IOException on write failure
     */
    void writeHeader(int level, String text) throws IOException;

    /**
     * Writes a list.
     *
     * @param ordered {@code true} if the list should be ordered.
     * @param list   the list to write.
     * @throws IOException on write failure
     */
    void writeList(boolean ordered, Collection<String> list) throws IOException;

    /**
     * Writes a table.
     *
     * @param table   the table definition to write.
     * @throws IOException on write failure
     */
    void writeTable(TableDef table) throws IOException;
}

and:

public abstract class AbstractHelpWriter implements HelpWriter {
    /**
     * The Appendable instance to write to.
     */
    protected final Appendable output;

    /**
     * Constructs an instance using the provided Appendable instance.
     * @param output the Appendable instance to write to.
     */
    protected AbstractHelpWriter(final Appendable output) {
        this.output = output;
    }

    @Override
    public Appendable append(final CharSequence text) throws IOException {
        output.append(text);
        return this;
    }

    @Override
    public Appendable append(final char ch) throws IOException {
        output.append(ch);
        return this;
    }

    @Override
    public Appendable append(final CharSequence csq, final int start, final int end) throws IOException {
        output.append(csq, start, end);
        return this;
    }
}

WDYT?

src/main/java/org/apache/commons/cli/help/package-info.java

src/test/java/org/apache/commons/cli/UtilTest.java

src/test/java/org/example/AptHelpWriter.java

garydgregory · 2024-10-02T15:25:47Z

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

+     * @param options the collection of {@link Option} instances to create the table from.
+     * @return A {@link TableDef} to display the options.
+     */
+    public TableDef defaultTableBuilder(final Iterable<Option> options) {


This is a bit odd. If it's the default behavior AND it's overridable from the builder, then it should be the default value. This would also help validate that the class is configurable enough if the default behavior is coded the same way a user would override it. IOW, can I call the Build with the behavior in this method?

This is a chicken and egg problem.

The builder needs a Function<Iterable, TableDef>.

The HelpFormatter instance class can not pass a reference to a non-static method to the builder.

If the method from the HelpFormatter instance needs to use instance variables it can not be passed.

I need to spend a bit of time to figure out how to untangle this. Perhaps the clarity on the showSince flag will help resolve this.

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

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

src/test/java/org/example/XhtmlHelpWriter.java

Claudenw · 2024-10-05T08:27:20Z

As far as making HelpWriter extends Appendable I think that is a good idea. However I feel that it moves the class further from the "Writer" concept as delineated by the Javadoc for Writer. I want to rename HelpWriter to Scribe and the implementing calsses to TextScribe, AptScribe, etc.

garydgregory · 2024-10-05T12:05:29Z

As far as making HelpWriter extends Appendable I think that is a good idea. However I feel that it moves the class further from the "Writer" concept as delineated by the Javadoc for Writer. I want to rename HelpWriter to Scribe and the implementing calsses to TextScribe, AptScribe, etc.

😄 Since Java's Write implements Appendable, HelpWriter extending Appendable would move the class closer to the "Writer" concept IMO, not further. But, this could be the issue with having named the class "*Writer" in the first place 😄 ❓ Either way, we agree: extend Appendable, whatever the class name is.

garydgregory

Hi @Claudenw
Nice tests! :-)
Please see my assorted comments.
TY.

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

pom.xml

src/main/java/org/apache/commons/cli/Util.java

garydgregory · 2024-10-06T11:30:58Z

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

+ * The class for help formatters provides the framework to link the {@link HelpWriter} with the {@link OptionFormatter}
+ * and a default {@link TableDef} so to produce a standard format help page.
+ */
+public abstract class AbstractHelpFormatter {


Let's not bake in an odd design for users because its uncomfortable for us developers to put in place a good one ;-) all for one special attribute.

Extensible builders are normal and we do it in several places in Commons (Configuration, Exec, IO), the pattern is [abstract or not] class Builder<T extends Builder<T>> implements Supplier<T> and SubBuilder extends Builder<SubBuilder>.

A simple example for Commons Exec:

The parent builder https://github.com/apache/commons-exec/blob/e99527c1c7c42940bfda9ea8787db8c43573c4c3/src/main/java/org/apache/commons/exec/DefaultExecutor.java#L57

The child builder: https://github.com/apache/commons-exec/blob/e99527c1c7c42940bfda9ea8787db8c43573c4c3/src/main/java/org/apache/commons/exec/DaemonExecutor.java#L34

Commons IO and Configuration have more advanced and complex examples.

src/main/java/org/apache/commons/cli/HelpFormatter.java

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

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

src/test/java/org/apache/commons/cli/UtilTest.java

src/test/java/org/apache/commons/example/cli/XhtmlHelpWriter.java

garydgregory · 2024-10-06T12:17:48Z

The Javadoc is still off (See Java 23 build output):

[ERROR] C:\Users\ggregory\git\a\commons-cli\src\main\java\org\apache\commons\cli\HelpFormatter.java:597: error: reference not found
[ERROR]      * @deprecated use {@link OptionFormatter#getArgName()} or {@link OptionFormatter.Builder#getArgName()}
[ERROR]                                                                       ^

garydgregory

Hi @Claudenw

Thank you for your updates. I've added to a couple of conversations, resolved a few others, and added some new comments.

TY!

src/main/java/org/apache/commons/cli/Util.java

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

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

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

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

src/main/java/org/apache/commons/cli/help/package-info.java

src/test/java/org/apache/commons/cli/UtilTest.java

HelpWriter naming chagnes Changed scaling to boolean scalable fixed weird license comment formatting.

@SInCE

- Update changes.xml - Sort members - Only use Javadoc @SInCE tags for public and protected elements - Javadoc: Break before tags - Javadoc: Write complete sentences. - Javadoc: Use @code - Javadoc: Sentences start with a capital letter - Add missing @OverRide annotation - Use final - Rename asArgName to toArgName (it calls another toArgName) - Whitespace - Use longer lines - Normalize test method names - Name test methods after the methods they test

### What changes were proposed in this pull request? This PR aims to upgrade `commons-cli` to 1.10.0 which is the first release tested with **Java 25-ea**. ### Why are the changes needed? To bring the latest improvements and bug fixes. - https://commons.apache.org/proper/commons-cli/changes.html#a1.10.0 (2025-07-30) - [Run Java 24 and 25-ea](apache/commons-cli@e6a611f) - [[CLI-333] org.apache.commons.cli.Option.Builder implements](apache/commons-cli@c9698e6) - apache/commons-cli#314 - apache/commons-cli#334 - [[CLI-341] HelpFormatter infinite loop with 0 width input](apache/commons-cli@bce0f6a) - [[CLI-343] OptionFormatter.getBothOpt() lacks validation for Options](apache/commons-cli@859d5e5) - [[CLI-344] Option.processValue() throws NullPointerException when passed](apache/commons-cli@4e0cdd0) - [[CLI-347] Options.addOptionGroup(OptionGroup) does not remove required](apache/commons-cli@42be792) - [[CLI-349] DefaultParser.parse() throws NullPointerException when options](apache/commons-cli@ff678b0) ### Does this PR introduce _any_ user-facing change? No behavior change. ### How was this patch tested? Pass the CIs. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #51816 from dongjoon-hyun/SPARK-53102. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

### What changes were proposed in this pull request? This PR aims to upgrade `commons-cli` to 1.10.0 which is the first release tested with Java 25-ea. ### Why are the changes needed? To bring the latest improvements and bug fixes. - https://commons.apache.org/proper/commons-cli/changes.html#a1.10.0 (2025-07-30) - [Run Java 24 and 25-ea](apache/commons-cli@e6a611f) - [[CLI-333] org.apache.commons.cli.Option.Builder implements](apache/commons-cli@c9698e6) - apache/commons-cli#314 - apache/commons-cli#334 - [[CLI-341] HelpFormatter infinite loop with 0 width input](apache/commons-cli@bce0f6a) - [[CLI-343] OptionFormatter.getBothOpt() lacks validation for Options](apache/commons-cli@859d5e5) - [[CLI-344] Option.processValue() throws NullPointerException when passed](apache/commons-cli@4e0cdd0) - [[CLI-347] Options.addOptionGroup(OptionGroup) does not remove required](apache/commons-cli@42be792) - [[CLI-349] DefaultParser.parse() throws NullPointerException when options](apache/commons-cli@ff678b0) ### How was this patch tested? Pass the CIs. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #2349 from dongjoon-hyun/ORC-1968. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

### What changes were proposed in this pull request? This PR aims to upgrade `commons-cli` to 1.10.0 which is the first release tested with Java 25-ea. ### Why are the changes needed? To bring the latest improvements and bug fixes. - https://commons.apache.org/proper/commons-cli/changes.html#a1.10.0 (2025-07-30) - [Run Java 24 and 25-ea](apache/commons-cli@e6a611f) - [[CLI-333] org.apache.commons.cli.Option.Builder implements](apache/commons-cli@c9698e6) - apache/commons-cli#314 - apache/commons-cli#334 - [[CLI-341] HelpFormatter infinite loop with 0 width input](apache/commons-cli@bce0f6a) - [[CLI-343] OptionFormatter.getBothOpt() lacks validation for Options](apache/commons-cli@859d5e5) - [[CLI-344] Option.processValue() throws NullPointerException when passed](apache/commons-cli@4e0cdd0) - [[CLI-347] Options.addOptionGroup(OptionGroup) does not remove required](apache/commons-cli@42be792) - [[CLI-349] DefaultParser.parse() throws NullPointerException when options](apache/commons-cli@ff678b0) ### How was this patch tested? Pass the CIs. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #2349 from dongjoon-hyun/ORC-1968. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 7a1a34f) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

Claudenw added 4 commits September 25, 2024 11:40

initial checkin

610d670

fixed up for Rat processing

2581d43

fixed checkstyle and some javadoc

06d0efd

fixed javadoc errors

d6eabca

Claudenw self-assigned this Sep 28, 2024

Claudenw added 6 commits September 29, 2024 16:30

added more tests and fixed some errors

804903a

added tests and fixed errors

fc362db

checkstyle and jacoco fixes

b6514cb

fixed javadoc and pmd errors

e502cff

fixed javadoc error

868a296

renamed Serializer to HelpWriter

6a55618

Claudenw marked this pull request as ready for review October 2, 2024 07:45

garydgregory reviewed Oct 2, 2024

View reviewed changes

src/main/java/org/apache/commons/cli/Util.java Outdated Show resolved Hide resolved

garydgregory requested changes Oct 2, 2024

View reviewed changes

garydgregory reviewed Oct 2, 2024

View reviewed changes

src/test/java/org/example/XhtmlHelpWriter.java Outdated Show resolved Hide resolved

Claudenw added 2 commits October 5, 2024 14:19

changes as per review

680acb8

Deprecated HelpFormatter and ensured replacement methods were available

29b6538

garydgregory requested changes Oct 6, 2024

View reviewed changes

Fixed javadoc issues

d5b1484

updated as per review

be85b9c

garydgregory requested changes Oct 6, 2024

View reviewed changes

Claudenw added 4 commits October 12, 2024 17:31

Partial changes as per review

7efdf80

HelpWriter naming chagnes Changed scaling to boolean scalable fixed weird license comment formatting.

Added java bug reference

b3ff12d

changed 'asX' to 'toX'

ffb240b

Split Util into Util and halp/Util and make package private

3e52010

garydgregory merged commit e9a72ea into apache:master Oct 14, 2024

garydgregory changed the title ~~CLI-339: Help formatter extension~~ [CLI-339] Help formatter extension Oct 14, 2024

This was referenced Aug 4, 2025

[SPARK-53102][BUILD] Upgrade commons-cli to 1.10.0 apache/spark#51816

Closed

ORC-1968: Upgrade commons-cli to 1.10.0 apache/orc#2349

Closed

Conversation

Claudenw commented Sep 28, 2024 • edited by garydgregory Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Claudenw commented Oct 2, 2024

Uh oh!

Uh oh!

garydgregory left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

garydgregory Oct 2, 2024

Choose a reason for hiding this comment

Uh oh!

Claudenw Oct 5, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Claudenw commented Oct 5, 2024

Uh oh!

garydgregory commented Oct 5, 2024

Uh oh!

garydgregory left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

garydgregory Oct 6, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

garydgregory commented Oct 6, 2024

Uh oh!

garydgregory left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Claudenw commented Sep 28, 2024 •

edited by garydgregory

Loading