geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mark...@apache.org
Subject incubator-geode git commit: GEODE-1598: fix auto-completion problems by telling jopt that all options are optional
Date Tue, 12 Jul 2016 22:26:58 GMT
Repository: incubator-geode
Updated Branches:
  refs/heads/release/1.0.0-incubating.M3 19cb8e13b -> 6b4c637ed


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/release/1.0.0-incubating.M3
Commit: 6b4c637edc4964d6f40a17c9c1cd703f2dc68f31
Parents: 19cb8e1
Author: gmeilen <gracemeilen@gmail.com>
Authored: Mon Jul 11 13:45:39 2016 -0700
Committer: William Markito <wmarkito@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


Mime
View raw message