Support for positional args in CLI help (#91)

- CLI command help includes positional arguments in its output - Additional tests for using @PositionalArgument without @arguments present and associated bug fix
rvesse · Apr 29, 2019 · 7f16266 · 7f16266
1 parent 45858e2
commit 7f16266
Show file tree

Hide file tree

Showing 8 changed files with 226 additions and 33 deletions.
diff --git a/airline-core/src/main/java/com/github/rvesse/airline/help/cli/CliCommandUsageGenerator.java b/airline-core/src/main/java/com/github/rvesse/airline/help/cli/CliCommandUsageGenerator.java
@@ -30,6 +30,7 @@
 import com.github.rvesse.airline.model.MetadataLoader;
 import com.github.rvesse.airline.model.OptionMetadata;
 import com.github.rvesse.airline.model.ParserMetadata;
+import com.github.rvesse.airline.model.PositionalArgumentMetadata;
 
 public class CliCommandUsageGenerator extends AbstractPrintedCommandUsageGenerator {
 
@@ -83,12 +84,13 @@ public <T> void usage(String programName, String[] groupNames, String commandNam
         }
 
         // Synopsis
-        List<OptionMetadata> options = outputSynopsis(out, programName, groupNames, commandName, command);
+        List<OptionMetadata> options = outputSynopsis(out, programName, groupNames, commandName, command, parserConfig);
 
         // Options
         ArgumentsMetadata arguments = command.getArguments();
-        if (options.size() > 0 || arguments != null) {
-            outputOptionsAndArguments(out, command, options, arguments, parserConfig);
+        if (options.size() > 0 || (arguments != null
+                || (command.getPositionalArguments() != null && command.getPositionalArguments().size() > 0))) {
+            outputOptionsAndArguments(out, command, options, command.getPositionalArguments(), arguments, parserConfig);
         }
 
         // Output post help sections
@@ -106,6 +108,8 @@ public <T> void usage(String programName, String[] groupNames, String commandNam
      *            Command meta-data
      * @param options
      *            Options meta-data
+     * @param positionalArgs
+     *            Positional arguments meta-data
      * @param arguments
      *            Arguments meta-data
      * @param parserConfig
@@ -116,10 +120,10 @@ public <T> void usage(String programName, String[] groupNames, String commandNam
      *             Thrown if there is a problem generating usage output
      */
     protected <T> void outputOptionsAndArguments(UsagePrinter out, CommandMetadata command,
-            List<OptionMetadata> options, ArgumentsMetadata arguments, ParserMetadata<T> parserConfig)
-            throws IOException {
+            List<OptionMetadata> options, List<PositionalArgumentMetadata> positionalArgs, ArgumentsMetadata arguments,
+            ParserMetadata<T> parserConfig) throws IOException {
         helper.outputOptions(out, options);
-        helper.outputArguments(out, arguments, parserConfig);
+        helper.outputArguments(out, positionalArgs, arguments, parserConfig);
     }
 
     /**
@@ -139,8 +143,8 @@ protected <T> void outputOptionsAndArguments(UsagePrinter out, CommandMetadata c
      * @throws IOException
      *             Thrown if there is a problem generating usage output
      */
-    protected List<OptionMetadata> outputSynopsis(UsagePrinter out, String programName, String[] groupNames,
-            String commandName, CommandMetadata command) throws IOException {
+    protected <T> List<OptionMetadata> outputSynopsis(UsagePrinter out, String programName, String[] groupNames,
+            String commandName, CommandMetadata command, ParserMetadata<T> parserConfig) throws IOException {
         out.append("SYNOPSIS").newline();
         UsagePrinter synopsis = out.newIndentedPrinter(8).newPrinterWithHangingIndent(8);
         List<OptionMetadata> options = new ArrayList<>();
@@ -155,10 +159,19 @@ protected List<OptionMetadata> outputSynopsis(UsagePrinter out, String programNa
         }
         synopsis.append(commandName).appendWords(toSynopsisUsage(sortOptions(command.getCommandOptions())));
         options.addAll(command.getCommandOptions());
+
+        boolean needsArgumentsSeparator = command.hasAnyArguments();
+        if (needsArgumentsSeparator) {
+            synopsis.append("[").append(parserConfig.getArgumentsSeparator()).append("]");
+        }
+
+        if (command.hasPositionalArguments()) {
+            synopsis.append(toUsage(command.getPositionalArguments()));
+        }
 
         // command arguments (optional)
-        if (command.getArguments() != null) {
-            synopsis.append("[--]").append(toUsage(command.getArguments()));
+        if (command.hasNonPositionalArguments()) {
+            synopsis.append(toUsage(command.getArguments()));
         }
         synopsis.newline();
         synopsis.newline();
@@ -178,7 +191,8 @@ protected List<OptionMetadata> outputSynopsis(UsagePrinter out, String programNa
      *            Command name
      * @param command
      *            Command meta-data
-     * @throws IOException Thrown if there is a problem generating usage output
+     * @throws IOException
+     *             Thrown if there is a problem generating usage output
      */
     protected void outputDescription(UsagePrinter out, String programName, String[] groupNames, String commandName,
             CommandMetadata command) throws IOException {

diff --git a/airline-core/src/main/java/com/github/rvesse/airline/help/cli/CliUsageHelper.java b/airline-core/src/main/java/com/github/rvesse/airline/help/cli/CliUsageHelper.java
@@ -31,6 +31,7 @@
 import com.github.rvesse.airline.model.ArgumentsMetadata;
 import com.github.rvesse.airline.model.OptionMetadata;
 import com.github.rvesse.airline.model.ParserMetadata;
+import com.github.rvesse.airline.model.PositionalArgumentMetadata;
 import com.github.rvesse.airline.restrictions.ArgumentsRestriction;
 import com.github.rvesse.airline.restrictions.OptionRestriction;
 
@@ -187,21 +188,50 @@ public static int calculateMaxRows(HelpHint hint) {
         return maxRows;
     }
 
-    public <T> void outputArguments(UsagePrinter out, ArgumentsMetadata arguments, ParserMetadata<T> parserConfig)
-            throws IOException {
-        if (arguments != null) {
+    public <T> void outputArguments(UsagePrinter out, List<PositionalArgumentMetadata> positionalArgs,
+            ArgumentsMetadata arguments, ParserMetadata<T> parserConfig) throws IOException {
+        UsagePrinter optionPrinter = out.newIndentedPrinter(8);
+        UsagePrinter descriptionPrinter;
+        boolean needsArgsSeparator = ((positionalArgs != null && positionalArgs.size() > 0) || arguments != null);
+        if (needsArgsSeparator) {
             // Arguments separator option
-            UsagePrinter optionPrinter = out.newIndentedPrinter(8);
             optionPrinter.append(parserConfig.getArgumentsSeparator()).newline();
             optionPrinter.flush();
 
             // Description
-            UsagePrinter descriptionPrinter = optionPrinter.newIndentedPrinter(4);
-            descriptionPrinter
-                    .append("This option can be used to separate command-line options from the list of arguments (useful when arguments might be mistaken for command-line options)")
+            descriptionPrinter = optionPrinter.newIndentedPrinter(4);
+            descriptionPrinter.append(
+                    "This option can be used to separate command-line options from the list of arguments (useful when arguments might be mistaken for command-line options)")
                     .newline();
             descriptionPrinter.newline();
+            descriptionPrinter.flush();
+        } else {
+            // No positional or non-positional arguments so just return
+            return;
+        }
+
+        if (positionalArgs != null && positionalArgs.size() > 0) {
+            for (PositionalArgumentMetadata posArg : positionalArgs) {
+                // Argument name
+                optionPrinter.append(toDescription(posArg)).newline();
+
+                // Description
+                descriptionPrinter = optionPrinter.newIndentedPrinter(4);
+                descriptionPrinter.append(posArg.getDescription()).newline();
 
+                List<HelpHint> hints = sortArgumentsRestrictions(arguments.getRestrictions());
+                for (HelpHint hint : hints) {
+                    // Safe to cast back to ArgumentsRestriction as must have
+                    // come from an ArgumentsRestriction to start with
+                    outputArgumentsRestriction(descriptionPrinter, posArg, (ArgumentsRestriction) hint, hint);
+                }
+
+                descriptionPrinter.newline();
+                descriptionPrinter.flush();
+            }
+        }
+
+        if (arguments != null) {
             // Arguments name(s)
             optionPrinter.append(toDescription(arguments)).newline();
 
@@ -219,6 +249,27 @@ public <T> void outputArguments(UsagePrinter out, ArgumentsMetadata arguments, P
             descriptionPrinter.newline();
             descriptionPrinter.flush();
         }
+
+        optionPrinter.flush();
+    }
+
+    /**
+     * Outputs documentation about a restriction on an option
+     * 
+     * @param descriptionPrinter
+     *            Description printer
+     * @param arguments
+     *            Arguments meta-data
+     * @param restriction
+     *            Restriction
+     * @param hint
+     *            Help hint
+     * @throws IOException
+     */
+    protected void outputArgumentsRestriction(UsagePrinter descriptionPrinter, PositionalArgumentMetadata arguments,
+            ArgumentsRestriction restriction, HelpHint hint) throws IOException {
+        descriptionPrinter.newline();
+        outputHint(descriptionPrinter, hint, false);
     }
 
     /**

diff --git a/airline-core/src/main/java/com/github/rvesse/airline/help/common/AbstractUsageGenerator.java b/airline-core/src/main/java/com/github/rvesse/airline/help/common/AbstractUsageGenerator.java
@@ -29,6 +29,7 @@
 import com.github.rvesse.airline.model.ArgumentsMetadata;
 import com.github.rvesse.airline.model.CommandMetadata;
 import com.github.rvesse.airline.model.OptionMetadata;
+import com.github.rvesse.airline.model.PositionalArgumentMetadata;
 import com.github.rvesse.airline.restrictions.ArgumentsRestriction;
 import com.github.rvesse.airline.restrictions.OptionRestriction;
 
@@ -91,20 +92,20 @@ protected List<HelpHint> sortOptionRestrictions(List<OptionRestriction> restrict
         List<HelpHint> hints = new ArrayList<>();
         for (OptionRestriction restriction : restrictions) {
             if (restriction instanceof HelpHint) {
-                hints.add((HelpHint)restriction);
+                hints.add((HelpHint) restriction);
             }
         }
         if (hintComparator != null) {
             Collections.sort(hints, hintComparator);
         }
         return hints;
     }
-    
+
     protected List<HelpHint> sortArgumentsRestrictions(List<ArgumentsRestriction> restrictions) {
         List<HelpHint> hints = new ArrayList<>();
         for (ArgumentsRestriction restriction : restrictions) {
             if (restriction instanceof HelpHint) {
-                hints.add((HelpHint)restriction);
+                hints.add((HelpHint) restriction);
             }
         }
         if (hintComparator != null) {
@@ -184,9 +185,13 @@ protected List<String> toSynopsisUsage(List<OptionMetadata> options) {
     protected String toUsage(ArgumentsMetadata arguments) {
         boolean required = arguments.isRequired();
         StringBuilder stringBuilder = new StringBuilder();
+
+        // NB Any additional arguments are either considered all required or
+        // optional whether that is actually the case or not. If users want fine
+        // grained control over whether each argument is required or not they
+        // need to use positional arguments instead
+
         if (!required) {
-            // TODO: be able to handle required arguments individually, like
-            // arity for the options
             stringBuilder.append("[ ");
         }
 
@@ -202,6 +207,30 @@ protected String toUsage(ArgumentsMetadata arguments) {
         return stringBuilder.toString();
     }
 
+    protected String toUsage(List<PositionalArgumentMetadata> posArgs) {
+        StringBuilder builder = new StringBuilder();
+
+        boolean first = true;
+        for (PositionalArgumentMetadata posArg : posArgs) {
+            if (first) {
+                first = false;
+            } else {
+                builder.append(' ');
+            }
+
+            boolean required = posArg.isRequired();
+            if (!required) {
+                builder.append("[ ");
+            }
+            builder.append(toDescription(posArg));
+            if (!required) {
+                builder.append(" ]");
+            }
+        }
+
+        return builder.toString();
+    }
+
     protected String toUsage(OptionMetadata option) {
         Set<String> options = option.getOptions();
         boolean required = option.isRequired();
@@ -264,6 +293,10 @@ protected String toDescription(ArgumentsMetadata arguments) {
         return stringBuilder.toString();
     }
 
+    protected String toDescription(PositionalArgumentMetadata posArg) {
+        return String.format("<%s>", posArg.getTitle());
+    }
+
     protected String toDescription(OptionMetadata option) {
         Set<String> options = option.getOptions();
         StringBuilder stringBuilder = new StringBuilder();

diff --git a/airline-core/src/main/java/com/github/rvesse/airline/model/CommandMetadata.java b/airline-core/src/main/java/com/github/rvesse/airline/model/CommandMetadata.java
@@ -89,15 +89,14 @@ public CommandMetadata(String name,
                 }
             }
             if (this.positionalArgs.size() > 0 && !posArgsRequired) {
-                if (this.arguments.isRequired()) {
+                if (this.arguments != null && this.arguments.isRequired()) {
                     throw new IllegalArgumentException(
                             "Non-positional arguments are declared as required but one/more preceding positional arguments are optional");
                 }
             }
         }
 
-        if (this.defaultOption != null
-                && (this.arguments != null || (this.positionalArgs != null && this.positionalArgs.size() > 0))) {
+        if (this.defaultOption != null && hasAnyArguments()) {
             throw new IllegalArgumentException(
                     "Command cannot declare both @Arguments/@PositionalArgument and use @DefaultOption");
         }
@@ -110,6 +109,34 @@ public CommandMetadata(String name,
         this.sections = AirlineUtils.unmodifiableListCopy(sections);
     }
 
+    /**
+     * Gets whether this command has any positional and/or non-positional
+     * arguments
+     * 
+     * @return True if any arguments are defined, false otherwise
+     */
+    public boolean hasAnyArguments() {
+        return hasNonPositionalArguments() || hasPositionalArguments();
+    }
+
+    /**
+     * Gets whether this command has any positional arguments
+     * 
+     * @return True if positional arguments are defined, false otherwise
+     */
+    public boolean hasPositionalArguments() {
+        return this.positionalArgs != null && this.positionalArgs.size() > 0;
+    }
+
+    /**
+     * Gets whether this command has any non-positional arguments
+     * 
+     * @return True if non-positional arguments are defined, false otherwise
+     */
+    public boolean hasNonPositionalArguments() {
+        return this.arguments != null;
+    }
+
     public String getName() {
         return name;
     }

diff --git a/...-core/src/main/java/com/github/rvesse/airline/restrictions/common/PartialRestriction.java b/...-core/src/main/java/com/github/rvesse/airline/restrictions/common/PartialRestriction.java
@@ -20,7 +20,6 @@
 import java.util.Set;
 import java.util.TreeSet;
 
-import org.apache.commons.collections4.CollectionUtils;
 import org.apache.commons.collections4.IterableUtils;
 
 import com.github.rvesse.airline.help.sections.HelpFormat;

diff --git a/airline-core/src/test/java/com/github/rvesse/airline/args/positional/ArgsPositional.java b/airline-core/src/test/java/com/github/rvesse/airline/args/positional/ArgsPositional.java
@@ -26,13 +26,13 @@
 @Command(name = "ArgsPositional", description = "ArgsPositional description")
 public class ArgsPositional
 {
-    @PositionalArgument(position = PositionalArgument.FIRST, title = "File")
+    @PositionalArgument(position = PositionalArgument.FIRST, title = "File", description = "File to operate on")
     @Required
     public String file;
 
-    @PositionalArgument(position = PositionalArgument.SECOND, title = "Mode")
+    @PositionalArgument(position = PositionalArgument.SECOND, title = "Mode", description = "Mode to set on the file")
     public Integer mode;
 
-    @Arguments
+    @Arguments(title = { "ExtraArg" }, description = "Additional argument(s)")
     public List<String> parameters = new ArrayList<>();
 }
diff --git a/...-core/src/test/java/com/github/rvesse/airline/args/positional/ArgsPositionalNoExtras.java b/...-core/src/test/java/com/github/rvesse/airline/args/positional/ArgsPositionalNoExtras.java
@@ -0,0 +1,31 @@
+/**
+ * Copyright (C) 2010-16 the original author or authors.
+ *
+ * Licensed 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 com.github.rvesse.airline.args.positional;
+
+import com.github.rvesse.airline.annotations.Command;
+import com.github.rvesse.airline.annotations.PositionalArgument;
+import com.github.rvesse.airline.annotations.restrictions.Required;
+
+@Command(name = "ArgsPositional", description = "ArgsPositional description")
+public class ArgsPositionalNoExtras
+{
+    @PositionalArgument(position = PositionalArgument.FIRST, title = "File", description = "File to operate on")
+    @Required
+    public String file;
+
+    @PositionalArgument(position = PositionalArgument.SECOND, title = "Mode", description = "Mode to set on the file")
+    public Integer mode;
+}