#185 Missing option exception text should not use field names but be …

…more descriptive and consistent with usage help. Thanks to [AlexFalappa](https://github.com/AlexFalappa). Closes #185
remkop · Oct 1, 2017 · 39f1944 · 39f1944
1 parent d4accd1
commit 39f1944
Show file tree

Hide file tree

Showing 5 changed files with 267 additions and 77 deletions.
diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md
@@ -55,6 +55,7 @@ class Args {
 - #193 Splitting an argument should not cause max arity to be exceeded
 - #191 Arity should not limit the total number of values put in an array or collection
 - #195 Usage help should show Map types if paramLabel not specified
+- #185 Missing option exception text should not use field names but be more descriptive and consistent with usage help. Thanks to [AlexFalappa](https://github.com/AlexFalappa). 
 - #186 Confusing usage message for collection options
 - #181 Fixed bug where incorrect help message is displayed for short options with paramLabel when arity > 1
 - #184 Improved CommandLine.setSeparator javadoc to clarify that this affects parsing only and link to the `@Command` `separator` annotation attribute.

diff --git a/src/main/java/picocli/CommandLine.java b/src/main/java/picocli/CommandLine.java
@@ -1528,7 +1528,7 @@ private void parse(List<CommandLine> parsedCommands, Stack<String> argumentStack
             }
             if (!isAnyHelpRequested() && !required.isEmpty()) {
                 if (required.get(0).isAnnotationPresent(Option.class)) {
-                    throw MissingParameterException.create(required);
+                    throw MissingParameterException.create(required, separator);
                 } else {
                     try {
                         processPositionalParameters0(required, true, new Stack<String>());
@@ -1567,7 +1567,7 @@ private void processArguments(List<CommandLine> parsedCommands,
                 // if we find another command, we are done with the current command
                 if (commands.containsKey(arg)) {
                     if (!isHelpRequested && !required.isEmpty()) { // ensure current command portion is valid
-                        throw MissingParameterException.create(required);
+                        throw MissingParameterException.create(required, separator);
                     }
                     if (tracer.isDebug()) {tracer.debug("Found subcommand '%s' (%s)%n", arg, commands.get(arg).interpreter.command.getClass().getName());}
                     commands.get(arg).interpreter.parse(parsedCommands, args, originalArgs);
@@ -2908,14 +2908,7 @@ public static IParameterRenderer createMinimalParameterRenderer() {
         public static IParamLabelRenderer createMinimalParamLabelRenderer() {
             return new IParamLabelRenderer() {
                 public Text renderParameterLabel(Field field, Ansi ansi, List<IStyle> styles) {
-                    String paramLabel = null;
-                    Parameters parameters = field.getAnnotation(Parameters.class);
-                    if (parameters != null) {
-                        paramLabel = parameters.paramLabel();
-                    } else {
-                        paramLabel = field.isAnnotationPresent(Option.class) ? field.getAnnotation(Option.class).paramLabel() : null;
-                    }
-                    String text = paramLabel == null || paramLabel.length() == 0 ? field.getName() : paramLabel;
+                    String text = DefaultParamLabelRenderer.renderParameterName(field);
                     return ansi.apply(text, styles);
                 }
                 public String separator() { return ""; }
@@ -3158,7 +3151,7 @@ public Text renderParameterLabel(Field field, Ansi ansi, List<IStyle> styles) {
                 }
                 return result;
             }
-            private String renderParameterName(Field field) {
+            private static String renderParameterName(Field field) {
                 String result = null;
                 if (field.isAnnotationPresent(Option.class)) {
                     result = field.getAnnotation(Option.class).paramLabel();
@@ -3172,7 +3165,7 @@ private String renderParameterName(Field field) {
                 if (Map.class.isAssignableFrom(field.getType())) { // #195 better param labels for map fields
                     Class<?>[] paramTypes = getTypeAttribute(field);
                     if (paramTypes.length < 2 || paramTypes[0] == null || paramTypes[1] == null) {
-                        name = "Object=Object";
+                        name = "String=String";
                     } else { name = paramTypes[0].getSimpleName() + "=" + paramTypes[1].getSimpleName(); }
                 }
                 return "<" + name + ">";
@@ -4094,17 +4087,23 @@ public MissingParameterException(String msg) {
             super(msg);
         }
 
-        private static MissingParameterException create(Collection<Field> missing) {
+        private static MissingParameterException create(Collection<Field> missing, String separator) {
             if (missing.size() == 1) {
                 return new MissingParameterException("Missing required option '"
-                        + missing.iterator().next().getName() + "'");
+                        + describe(missing.iterator().next(), separator) + "'");
             }
             List<String> names = new ArrayList<String>(missing.size());
             for (Field field : missing) {
-                names.add(field.getName());
+                names.add(describe(field, separator));
             }
             return new MissingParameterException("Missing required options " + names.toString());
         }
+        private static String describe(Field field, String separator) {
+            String prefix = (field.isAnnotationPresent(Option.class))
+                ? field.getAnnotation(Option.class).names()[0] + separator
+                : "params[" + field.getAnnotation(Parameters.class).index() + "]" + separator;
+            return prefix + Help.DefaultParamLabelRenderer.renderParameterName(field);
+        }
     }
 
     /**

diff --git a/src/test/java/picocli/AutoCompleteTest.java b/src/test/java/picocli/AutoCompleteTest.java
@@ -150,7 +150,7 @@ private static String toString(Object obj) {
             "Usage: picocli.AutoComplete [-fhw] [-n=<commandName>] [-o=<autoCompleteScript>]%n" +
             "                            <commandLineFQCN>%n" +
             "Generates a bash completion script for the specified command class.%n" +
-            "      commandLineFQCN         Fully qualified class name of the annotated%n" +
+            "      <commandLineFQCN>       Fully qualified class name of the annotated%n" +
             "                                @Command class to generate a completion script%n" +
             "                                for.%n" +
             "  -n, --name=<commandName>    Name of the command to create a completion script%n" +