From f9c3e479f086dc30f4308d3c827edcaa5952bc58 Mon Sep 17 00:00:00 2001 From: Tom Hombergs Date: Wed, 9 Oct 2013 21:21:49 +0200 Subject: [PATCH 1/2] fix for issue #22 --- src/main/java/io/airlift/command/Cli.java | 45 ++++++++++--------- src/main/java/io/airlift/command/Command.java | 5 +++ .../io/airlift/command/SingleCommand.java | 44 +++++++++++------- .../command/model/CommandMetadata.java | 9 +++- .../airlift/command/model/MetadataLoader.java | 10 ++--- .../command/PingWithRequiredOption.java | 33 ++++++++++++++ .../command/PingWithRequiredOptionTest.java | 21 +++++++++ 7 files changed, 121 insertions(+), 46 deletions(-) create mode 100644 src/test/java/io/airlift/command/PingWithRequiredOption.java create mode 100644 src/test/java/io/airlift/command/PingWithRequiredOptionTest.java diff --git a/src/main/java/io/airlift/command/Cli.java b/src/main/java/io/airlift/command/Cli.java index f83500d22..5c70f206a 100644 --- a/src/main/java/io/airlift/command/Cli.java +++ b/src/main/java/io/airlift/command/Cli.java @@ -23,12 +23,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; -import io.airlift.command.model.ArgumentsMetadata; -import io.airlift.command.model.CommandGroupMetadata; -import io.airlift.command.model.CommandMetadata; -import io.airlift.command.model.GlobalMetadata; -import io.airlift.command.model.MetadataLoader; -import io.airlift.command.model.OptionMetadata; +import io.airlift.command.model.*; import java.util.List; import java.util.Map; @@ -129,6 +124,7 @@ public C parse(Iterable args) private void validate(ParseState state) { CommandMetadata command = state.getCommand(); + if (command == null) { List unparsedInput = state.getUnparsedInput(); if (unparsedInput.isEmpty()) { @@ -139,23 +135,32 @@ private void validate(ParseState state) } } - ArgumentsMetadata arguments = command.getArguments(); - if (state.getParsedArguments().isEmpty() && arguments != null && arguments.isRequired()) { - throw new ParseArgumentsMissingException(arguments.getTitle()); - } - - if (!state.getUnparsedInput().isEmpty()) { - throw new ParseArgumentsUnexpectedException(state.getUnparsedInput()); - } + try{ + ArgumentsMetadata arguments = command.getArguments(); + if (state.getParsedArguments().isEmpty() && arguments != null && arguments.isRequired()) { + throw new ParseArgumentsMissingException(arguments.getTitle()); + } - if (state.getLocation() == Context.OPTION) { - throw new ParseOptionMissingValueException(state.getCurrentOption().getTitle()); - } + if (!state.getUnparsedInput().isEmpty()) { + throw new ParseArgumentsUnexpectedException(state.getUnparsedInput()); + } - for (OptionMetadata option : command.getAllOptions()) { - if (option.isRequired() && !state.getParsedOptions().containsKey(option)) { - throw new ParseOptionMissingException(option.getOptions().iterator().next()); + if (state.getLocation() == Context.OPTION) { + throw new ParseOptionMissingValueException(state.getCurrentOption().getTitle()); + } + + for (OptionMetadata option : command.getAllOptions()) { + if (option.isRequired() && !state.getParsedOptions().containsKey(option)) { + throw new ParseOptionMissingException(option.getOptions().iterator().next()); + } + } + }catch(ParseException e){ + if(command.isShowHelpOnError()){ + System.out.println(e.getMessage()); + System.out.println("Usage:"); + Help.help(command); } + throw e; } } diff --git a/src/main/java/io/airlift/command/Command.java b/src/main/java/io/airlift/command/Command.java index da0734f86..d86ac5ebf 100644 --- a/src/main/java/io/airlift/command/Command.java +++ b/src/main/java/io/airlift/command/Command.java @@ -47,4 +47,9 @@ * If true, this command won't appear in the usage(). */ boolean hidden() default false; + + /** + * If true, the help will be shown + */ + boolean showHelpOnError() default true; } diff --git a/src/main/java/io/airlift/command/SingleCommand.java b/src/main/java/io/airlift/command/SingleCommand.java index 1d732d18d..ca1c2af65 100644 --- a/src/main/java/io/airlift/command/SingleCommand.java +++ b/src/main/java/io/airlift/command/SingleCommand.java @@ -55,11 +55,11 @@ public C parse(String... args) { return parse(ImmutableList.copyOf(args)); } - + public C parse(Iterable args) { checkNotNull(args, "args is null"); - + Parser parser = new Parser(); ParseState state = parser.parseCommand(commandMetadata, args); validate(state); @@ -74,10 +74,11 @@ public C parse(Iterable args) command.getMetadataInjections(), ImmutableMap., Object>of(CommandMetadata.class, commandMetadata)); } - + private void validate(ParseState state) { CommandMetadata command = state.getCommand(); + if (command == null) { List unparsedInput = state.getUnparsedInput(); if (unparsedInput.isEmpty()) { @@ -88,23 +89,32 @@ private void validate(ParseState state) } } - ArgumentsMetadata arguments = command.getArguments(); - if (state.getParsedArguments().isEmpty() && arguments != null && arguments.isRequired()) { - throw new ParseArgumentsMissingException(arguments.getTitle()); - } - - if (!state.getUnparsedInput().isEmpty()) { - throw new ParseArgumentsUnexpectedException(state.getUnparsedInput()); - } + try{ + ArgumentsMetadata arguments = command.getArguments(); + if (state.getParsedArguments().isEmpty() && arguments != null && arguments.isRequired()) { + throw new ParseArgumentsMissingException(arguments.getTitle()); + } - if (state.getLocation() == Context.OPTION) { - throw new ParseOptionMissingValueException(state.getCurrentOption().getTitle()); - } + if (!state.getUnparsedInput().isEmpty()) { + throw new ParseArgumentsUnexpectedException(state.getUnparsedInput()); + } - for (OptionMetadata option : command.getAllOptions()) { - if (option.isRequired() && !state.getParsedOptions().containsKey(option)) { - throw new ParseOptionMissingException(option.getOptions().iterator().next()); + if (state.getLocation() == Context.OPTION) { + throw new ParseOptionMissingValueException(state.getCurrentOption().getTitle()); + } + + for (OptionMetadata option : command.getAllOptions()) { + if (option.isRequired() && !state.getParsedOptions().containsKey(option)) { + throw new ParseOptionMissingException(option.getOptions().iterator().next()); + } + } + }catch(ParseException e){ + if(command.isShowHelpOnError()){ + System.out.println(e.getMessage()); + System.out.println("Usage:"); + Help.help(command); } + throw e; } } } diff --git a/src/main/java/io/airlift/command/model/CommandMetadata.java b/src/main/java/io/airlift/command/model/CommandMetadata.java index e9f4f647b..59a298826 100644 --- a/src/main/java/io/airlift/command/model/CommandMetadata.java +++ b/src/main/java/io/airlift/command/model/CommandMetadata.java @@ -4,7 +4,6 @@ import com.google.common.collect.ImmutableList; import io.airlift.command.Accessor; -import javax.annotation.Nullable; import java.util.List; public class CommandMetadata @@ -18,6 +17,7 @@ public class CommandMetadata private final ArgumentsMetadata arguments; private final List metadataInjections; private final Class type; + private final boolean showHelpOnError; public CommandMetadata(String name, String description, @@ -26,7 +26,7 @@ public CommandMetadata(String name, Iterable commandOptions, ArgumentsMetadata arguments, Iterable metadataInjections, - Class type) + Class type, boolean showHelpOnError) { this.name = name; this.description = description; @@ -37,6 +37,7 @@ public CommandMetadata(String name, this.arguments = arguments; this.metadataInjections = ImmutableList.copyOf(metadataInjections); this.type = type; + this.showHelpOnError = showHelpOnError; } public String getName() @@ -117,4 +118,8 @@ public String apply(CommandMetadata input) } }; } + + public boolean isShowHelpOnError() { + return showHelpOnError; + } } diff --git a/src/main/java/io/airlift/command/model/MetadataLoader.java b/src/main/java/io/airlift/command/model/MetadataLoader.java index 0279108c9..ea5e012ed 100644 --- a/src/main/java/io/airlift/command/model/MetadataLoader.java +++ b/src/main/java/io/airlift/command/model/MetadataLoader.java @@ -6,12 +6,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.common.collect.ListMultimap; -import io.airlift.command.Accessor; -import io.airlift.command.Arguments; -import io.airlift.command.Command; -import io.airlift.command.Option; -import io.airlift.command.OptionType; -import io.airlift.command.Suggester; +import io.airlift.command.*; import javax.annotation.Nullable; import javax.inject.Inject; @@ -82,6 +77,7 @@ public static CommandMetadata loadCommand(Class commandType) String name = command.name(); String description = command.description().isEmpty() ? null : command.description(); boolean hidden = command.hidden(); + boolean showHelpOnError = command.showHelpOnError(); InjectionMetadata injectionMetadata = loadInjectionMetadata(commandType); @@ -93,7 +89,7 @@ public static CommandMetadata loadCommand(Class commandType) injectionMetadata.commandOptions, Iterables.getFirst(injectionMetadata.arguments, null), injectionMetadata.metadataInjections, - commandType); + commandType, showHelpOnError); return commandMetadata; diff --git a/src/test/java/io/airlift/command/PingWithRequiredOption.java b/src/test/java/io/airlift/command/PingWithRequiredOption.java new file mode 100644 index 000000000..19986643b --- /dev/null +++ b/src/test/java/io/airlift/command/PingWithRequiredOption.java @@ -0,0 +1,33 @@ +package io.airlift.command; + +import javax.inject.Inject; + +@Command(name = "ping", description = "network test utility") +public class PingWithRequiredOption +{ + @Inject + public HelpOption helpOption; + + @Option(name = {"-c", "--count"}, required=true, description = "Send count packets") + public int count = 1; + + public static void main(String... args) + { + try{ + PingWithRequiredOption ping = SingleCommand.singleCommand(PingWithRequiredOption.class).parse(args); + if (ping.helpOption.showHelpIfRequested()) { + return; + } + + ping.run(); + }catch(ParseException e){ + System.out.println(e.getMessage()); + return; + } + } + + public void run() + { + System.out.println("Ping count: " + count); + } +} diff --git a/src/test/java/io/airlift/command/PingWithRequiredOptionTest.java b/src/test/java/io/airlift/command/PingWithRequiredOptionTest.java new file mode 100644 index 000000000..23f0177f6 --- /dev/null +++ b/src/test/java/io/airlift/command/PingWithRequiredOptionTest.java @@ -0,0 +1,21 @@ +package io.airlift.command; + +import com.google.common.base.Joiner; +import org.testng.annotations.Test; + +public class PingWithRequiredOptionTest { + + @Test + public void test() + { + // missing parameter => show help + ping(); + } + + private void ping(String... args) + { + System.out.println("$ ping " + Joiner.on(' ').join(args)); + PingWithRequiredOption.main(args); + System.out.println(); + } +} From c5d13649218ae3e43c113e13fa82b4af52c1d461 Mon Sep 17 00:00:00 2001 From: Tom Hombergs Date: Wed, 9 Oct 2013 21:25:45 +0200 Subject: [PATCH 2/2] fix for issue #22 --- src/main/java/io/airlift/command/Command.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/airlift/command/Command.java b/src/main/java/io/airlift/command/Command.java index d86ac5ebf..17fa7233b 100644 --- a/src/main/java/io/airlift/command/Command.java +++ b/src/main/java/io/airlift/command/Command.java @@ -49,7 +49,7 @@ boolean hidden() default false; /** - * If true, the help will be shown + * If true, the help will be shown if the command line arguments cannot be parsed correctly. */ boolean showHelpOnError() default true; }