You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ma...@apache.org on 2016/07/26 04:57:06 UTC
[2/9] incubator-geode git commit: GEODE-1598: fix auto-completion
problems by telling jopt that all options are optional
GEODE-1598: fix auto-completion problems by telling jopt that all options are optional
Spring shell, jopt-simple and Geode GFSH code all duplicated the concept of required options. jopt-simple can be blind to this, which prevents OptionParser.parse from throwing an Exception when a required option is missing at time of hitting tab for auto-complete. This allows OptionParser to return an OptionSet containing all detected options which is necessary for auto-completion to behave correctly.
This closes #195
Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/6b4c637e
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/6b4c637e
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/6b4c637e
Branch: refs/heads/develop
Commit: 6b4c637edc4964d6f40a17c9c1cd703f2dc68f31
Parents: 19cb8e1
Author: gmeilen <gr...@gmail.com>
Authored: Mon Jul 11 13:45:39 2016 -0700
Committer: William Markito <wm...@pivotal.io>
Committed: Tue Jul 12 15:26:22 2016 -0700
----------------------------------------------------------------------
.../internal/cli/parser/jopt/JoptOptionParser.java | 7 ++++---
.../management/internal/cli/JoptOptionParserTest.java | 8 ++++++--
2 files changed, 10 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/6b4c637e/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/parser/jopt/JoptOptionParser.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/parser/jopt/JoptOptionParser.java b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/parser/jopt/JoptOptionParser.java
index 18ea581..82760c8 100644
--- a/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/parser/jopt/JoptOptionParser.java
+++ b/geode-core/src/main/java/com/gemstone/gemfire/management/internal/cli/parser/jopt/JoptOptionParser.java
@@ -103,9 +103,10 @@ public class JoptOptionParser implements GfshOptionParser {
argumentSpecs = optionBuilder.withOptionalArg();
}
- if (option.isRequired()) {
- argumentSpecs.required();
- }
+ // TODO: temporarily commented out as workaround for GEODE-1598
+ // if (option.isRequired()) {
+ // argumentSpecs.required();
+ // }
if (option.getValueSeparator() != null) {
argumentSpecs.withValuesSeparatedBy(option.getValueSeparator());
}
http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/6b4c637e/geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/JoptOptionParserTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/JoptOptionParserTest.java b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/JoptOptionParserTest.java
index 587cfc2..c0aeb1c 100644
--- a/geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/JoptOptionParserTest.java
+++ b/geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/JoptOptionParserTest.java
@@ -239,8 +239,12 @@ public class JoptOptionParserTest {
}
@Test
- public void parseInputWithUndefinedOptionShouldThrow() throws Exception {
- assertThatThrownBy(() -> this.simpleOptionParser.parse("command1 argument1_value argument2_value --undefinedOption")).isExactlyInstanceOf(CliCommandOptionException.class);
+ public void parseInputShouldIgnoreUndefinedOption() throws Exception {
+ // one fix for GEODE-1598 has a side effect of preventing our detection of undefined options
+ OptionSet optionSet = this.simpleOptionParser.parse("command1 argument1_value argument2_value --undefinedOption");
+ assertThat(optionSet.areOptionsPresent()).isFalse();
+ assertThat(optionSet.hasOption(this.requiredOption)).isFalse();
+ assertThat(optionSet.hasOption(this.optionalOption)).isFalse();
}
@Test