Customizable usage() formatting

[SR-G]

Hey. Not a big deal but I'm not really fond of the usage output since 1.30 (quite messy to my taste - the previous "nearly one line" was really more clear in my opinon).

I don't think there will be one definitive usage for everyone, so could we either :

• have a usage() and submethods that can easily be overriden by a jcommander sub-class (quite ugly, but would work) (right now a lot of methods are private)
• or have a usage() customizable with more parameters (multiline vs single line, and so on)
• or, better, have an injectable IUsageFormatter, with a default implementation that would have the exact current output - but that would allow to easily write other formatters for people / projects willing to output the usage descriptions differently ...

[cbeust]

This is almost exactly why I pretty much stopped accepting pull requests to change the usage() format: these are guaranteed to make as many people happy as unhappy.

The bottom line is: usage() is whatever I find pleasing to my eyes and people who want to change it are welcome to implement their own by accessing the ParameterDescriptions directly :)

[lazee]

@cbeust I understand your view on this, but there are some problems when trying to just use the parameterdescriptions directly. First of all we shouldn't forget about the commands. People want to print them as well in their usage implementation. And luckily we have access to them as well. But there it stops. We cannot get access to programName information, the createDescription() method and several other properties and methods within JCommander.

If the current implementation was good enough then it should be possible to produce the same usage output from an outside implementation, but its not. Primarily because of limited access to programName. But there is also a problem with the signatures of several methods that simplifies the model by returning Strings instead of the implementations that are used internally. By splitting the production of the usage output into a separate class would "force" JCommander to fix these issues.

Here's my attempt to mimic todays usage() output from an outside class (see comments):

package test;

import com.beust.jcommander.JCommander;
import com.beust.jcommander.ParameterDescription;
import com.beust.jcommander.ParameterException;
import com.beust.jcommander.Parameters;
import com.beust.jcommander.Strings;
import com.beust.jcommander.WrappedParameter;
import com.beust.jcommander.internal.Lists;

import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.Map;

public class JCommanderUsage {

    private final JCommander jc;

    private int m_columnSize = 79;

    private Comparator<? super ParameterDescription> m_parameterDescriptionComparator
            = new Comparator<ParameterDescription>() {
        @Override
        public int compare(ParameterDescription p0, ParameterDescription p1) {
            return p0.getLongestName().compareTo(p1.getLongestName());
        }
    };

    public JCommanderUsage(JCommander jc) {
        this.jc = jc;
    }

    /**
     * Display the usage for this command.
     */
    public void usage(String commandName) {
        StringBuilder sb = new StringBuilder();
        usage(commandName, sb);
        JCommander.getConsole().println(sb.toString());
    }

    /**
     * Store the help for the command in the passed string builder.
     */
    public void usage(String commandName, StringBuilder out) {
        usage(commandName, out, "");
    }

    /**
     * Store the help for the command in the passed string builder, indenting
     * every line with "indent".
     */
    public void usage(String commandName, StringBuilder out, String indent) {
        String description = null;
        try {
            description = jc.getCommandDescription(commandName);
        } catch (ParameterException e) {
            // Simplest way to handle problem with fetching descriptions for the main command.
            // In real implementations we would have done this another way.
        }

        if (description != null) {
            out.append(indent).append(description);
            out.append("\n");
        }
        // PROBLEM : JCommander jcc = jc.findCommandByAlias(commandName); // Wops, not public!
        JCommander jcc = findCommandByAlias(commandName);
        if (jcc != null) {
            jcc.usage(out, indent);
        }
    }

    private JCommander findCommandByAlias(String commandOrAlias) {
        // Wops, not public and return ProgramName class that neither is public.
        // No way to get around this.
        // PROBLEM : JCommander.ProgramName progName = jc.findProgramName(commandOrAlias);

        // So, then it turns out we cannot mimic the functionality implemented in usage for
        // printing command usage :(
        /*if(progName == null) {
            return null;
        } else {
            JCommander jc = this.findCommand(progName);
            if(jc == null) {
                throw new IllegalStateException("There appears to be inconsistency in the internal command database.  This is likely a bug. Please report.");
            } else {
                return jc;
            }
        }*/
        // Lets go for the solution which is available to us and ignore the logic implemented in
        // JCommander for this lookup.
        return jc.getCommands().get(commandOrAlias);
    }

    public void usage(StringBuilder out, String indent) {

        // Why is this done on this stage of the process?
        // Looks like something that should have been done earlier on?
        // Anyway the createDescriptions() method is private and not possible to
        // trigger from the outside.
        // I haven't spend time on considering the consequences of not executing the method.
        /* PROBLEM : if (m_descriptions == null) createDescriptions(); */

        boolean hasCommands = !jc.getCommands().isEmpty();

        //
        // First line of the usage
        //

        // The JCommander does not provide a getProgramName() method and therefore
        // makes it impossible for other usage implementations to use it.
        // My first idea was to use the reflection api to change the access level of
        // the m_programName attribute, but then I saw that the ProgramName class is
        // private as well :(
        // Of course it is possible to set an alternative program name in the usage
        // implementation, but that is kind of a second best solution.

        /* PROBLEM : String programName = m_programName != null ? m_programName.getDisplayName() : "<main class>"; */
        String programName = "<who knows>";

        out.append(indent).append("Usage: ").append(programName).append(" [options]");
        if (hasCommands) {
            out.append(indent).append(" [command] [command options]");
        }
        if (jc.getMainParameterDescription() != null) {
            out.append(" ").append(jc.getMainParameterDescription());
        }
        out.append("\n");

        //
        // Align the descriptions at the "longestName" column
        //
        int longestName = 0;
        List<ParameterDescription> sorted = Lists.newArrayList();
        for (ParameterDescription pd : jc.getParameters()) {
            if (!pd.getParameter().hidden()) {
                sorted.add(pd);
                // + to have an extra space between the name and the description
                int length = pd.getNames().length() + 2;
                if (length > longestName) {
                    longestName = length;
                }
            }
        }

        //
        // Sort the options
        //
        Collections.sort(sorted, getParameterDescriptionComparator());

        //
        // Display all the names and descriptions
        //
        int descriptionIndent = 6;
        if (sorted.size() > 0) {
            out.append(indent).append("  Options:\n");
        }
        for (ParameterDescription pd : sorted) {
            WrappedParameter parameter = pd.getParameter();
            out.append(indent)
                    .append("  ")
                    .append(parameter.required() ? "* " : "  ")
                    .append(pd.getNames())
                    .append("\n")
                    .append(indent)
                    .append(s(descriptionIndent));
            int indentCount = indent.length() + descriptionIndent;
            wrapDescription(out, indentCount, pd.getDescription());
            Object def = pd.getDefault();
            if (pd.isDynamicParameter()) {
                out.append("\n")
                        .append(s(indentCount + 1))
                        .append("Syntax: ")
                        .append(parameter.names()[0])
                        .append("key").append(parameter.getAssignment())
                        .append("value");
            }
            if (def != null) {
                String displayedDef = Strings.isStringEmpty(def.toString())
                        ? "<empty string>"
                        : def.toString();
                out.append("\n")
                        .append(s(indentCount + 1))
                        .append("Default: ")
                        .append(parameter.password() ? "********" : displayedDef);
            }
            out.append("\n");
        }

        //
        // If commands were specified, show them as well
        //
        if (hasCommands) {
            out.append("  Commands:\n");
            // The magic value 3 is the number of spaces between the name of the option
            // and its description
            for (Map.Entry<String, JCommander> commands : jc.getCommands().entrySet()) {
                Object arg = commands.getValue().getObjects().get(0);
                Parameters p = arg.getClass().getAnnotation(Parameters.class);
                // I'm not sure why, but this simply doesn't work in my test project.
                // But this is not important in this POC
                //if (!p.hidden()) {
                    String dispName = commands.getKey();
                    out.append(indent).append("    " +
                                                      dispName); // + s(spaceCount) + getCommandDescription(progName.name) + "\n");

                    // Options for this command
                    usage(dispName, out, "      ");
                    out.append("\n");
                //}
            }
        }
    }

    private void wrapDescription(StringBuilder out, int indent, String description) {
        int max = getColumnSize();
        String[] words = description.split(" ");
        int current = indent;
        int i = 0;
        while (i < words.length) {
            String word = words[i];
            if (word.length() > max || current + word.length() <= max) {
                out.append(" ").append(word);
                current += word.length() + 1;
            } else {
                out.append("\n").append(s(indent + 1)).append(word);
                current = indent;
            }
            i++;
        }
    }

    private Comparator<? super ParameterDescription> getParameterDescriptionComparator() {
        return m_parameterDescriptionComparator;
    }

    public void setParameterDescriptionComparator(Comparator<? super ParameterDescription> c) {
        m_parameterDescriptionComparator = c;
    }

    public void seColumnSize(int m_columnSize) {
        this.m_columnSize = m_columnSize;
    }

    public int getColumnSize() {
        return m_columnSize;
    }

    /**
     * @return n spaces
     */
    private String s(int count) {
        StringBuilder result = new StringBuilder();
        for (int i = 0; i < count; i++) {
            result.append(" ");
        }

        return result.toString();
    }
}

[lazee]

And BTW: You might wouldn't have to reject pull requests for fixing the usage output if a Usage formatter interface was introduced like @SR-G suggested ;) I just don't see why this would make people unhappy if the current usage() methods are kept as-is.

[cbeust]

Thanks for the detailed answer, @lazee.

I think the take away is that I'd rather focus my energy on improving ParameterDescription and surrounding API's so that everyone can write their own usage() implementation rather than modifying the default one...

[lazee]

Your call :)

[lazee]

BTW: Let me know if you would like a suggestion in a PR. I see you get a lot of pull requests and I don't want to spend time on it if you want to do this yourself.

[adzubla]

+1 for a usage formatter interface!

[martinjmares]

+1 for a usage formatter interface!

[cbeust]

Please see my comment at https://github.com/cbeust/jcommander/issues/217#issuecomment-76851212

I don't think it's worth it to make usage() customizable for the reason above.

[martinjmares]

OK. Thanks. I appreciate your strong opinion. But do you plan at least to provide Internationalisation for usage() and (what is more important) for Exceptions? Or will you, at least, accept pull requests in this area? BTW: There is also opened issue from 2013 #169 and very old pull request #114 from 2012. (Yes, I understand that so old pull request is unusable now, but somebody can provide new one.)

[cbeust]

@martinjmares Sure, happy to accept internationalization pull requests (note that JCommander already lets you internationalize your parameters ).

And yes, that PR would need to be rebased to be useful (or I'd have to merge it manually).

[alexkli]

The current (v 1.58) usage format is pretty unusual compared to standard *nix command line tools:

JCommander:

  Options:
    -h, --help
       show help
    -r
       runtime in seconds
       Default: 10
    -t
       number of concurrent request threads
       Default: 10

*nix tool standard:

  Options:
    -h, --help show help
    -r         runtime in seconds (default: 10)
    -t         number of concurrent request threads (default: 10)

It is more readable if you can have each option on one line and the output is smaller in terms of lines (important with many options). Quick grepping only works well if each option is on one line: <tool> -h | grep something. Last but not least the "Default: x" on a separate line seems off (given it is capitalized).

A custom usage formatter would be great, but having an option for standard *nix single-line output ootb would be useful. There is a reason this is used by most tools and in terms of good defaults I would even argue this should be the default one that usage() outputs.

@cbeust

The bottom line is: usage() is whatever I find pleasing to my eyes and people who want to change it are welcome to implement their own by accessing the ParameterDescriptions directly :)

One main benefit of a command line parsing utility is that you don't have to write your own usage/help printer 😄

[Hubbitus]

There also private com.beust.jcommander.JCommander.Options#columnSize which I also want autodetect to make format wider.

Now I use some sort of hack (on Groovy):

@InheritConstructors
public class JCommanderAutoWidth extends JCommander{
    @Override
    public int getColumnSize() {
        try{
            return ["bash", "-c", "tput cols 2> /dev/tty"].execute().text.toInteger()
        }
        catch(IOException ignore){
            /**
             * Already hardcoded default, but Options private.
             * @see com.beust.jcommander.JCommander.Options#columnSize
             */
            return 79
        }
    }
}

Using that class instead of original make usage output instead of:

Usage: <main class> [options]
  Options:
    --clean
      List of rules to clean! F.e.: --clean='[regex=/egaisapp/, keep-top=10, 
      keep-period=7d, always-keep=/^(tag|release)_\\d\\.\\d$/]'
    --help

    -v, --verbose
      More verbosity
      Default: false

wider and more readable:

Usage: <main class> [options]
  Options:
    --clean
      List of rules to clean! F.e.: --clean='[regex=/egaisapp/, keep-top=10, keep-period=7d, always-keep=/^(tag|release)_\\d\\.\\d$/]'
    --help

    -v, --verbose
      More verbosity
      Default: false

[cbeust]

Sounds like columnsSize should be configurable with the builder, without requiring to override the JCommander class. Would you like to send a pull request?

[Hubbitus]

Yes, I would. Actually it already there in com.beust.jcommander.JCommander.Builder#columnSize. But I would also see width auto-detection. In that case it also bad idea always copy/paste that logic outside and use builder.

[dozmus]

@cbeust let me know if you would consider a PR for this.

I want to create a IUsageFormatter which defines how the usage for a given JCommander is formatted (like the third suggestion in the original post). Then we have a DefaultUsageFormatter which would format it as it currently is, and be used if no IUsageFormatter is passed to the usage method.

The benefits of this are as follows, in addition to the aforementioned:

• Extensibility - currently the usage formatting code is hardcoded (correct me if im wrong). Trying to address this by hardcoding strings in our own applications for custom usage formats is inflexible (requires manual updates). Trying to address this by using reflection in our own applications defeats the purpose of using this library. Playing with ParameterDescription is apparently inflexible, and personally seems unclear.
• We could eventually subclass it to introduce more standard defaults, ie the UnixStyleUsageFormatter mentioned earlier. And possibly a IUsageFormatter which formats based on a user input string, ie {names}: {description}
• Separation of concerns and easier testing.
• A very clear way to define the usage format.
• Future flexibility.
• We keep the default style as you wanted.

The main concern I have is that the current implementation of usage formatting is very confusing (which we hope to address), and implementing this solution may require extensive refactoring (extracting classes, changing visibility for modifiers, etc.).

[cbeust]

I'd be open to that. The current implementation (which is pretty ugly, I agree) could simply be hidden behind the DefaultUsage instance.

ParameterDescription is the standard way to obtain parameter information, though, I'm not sure how you'd get around that, but if you want to send a few PR's (even early ones just to discuss the general outline of what you have in mind), I'm happy to discuss.

[dozmus]

@cbeust I have opened this pull-request for us to discuss and work on this. Feel free to let me know of any concerns or needs you have.