----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58589/ -----------------------------------------------------------
(Updated April 25, 2017, 9:19 p.m.) Review request for geode, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg. Changes ------- changes after the reivew: minor fixes, and removing singletons. Precheckin running. Repository: geode Description ------- GEODE-1597: use Spring shell's parser and delete our own parsing code * deleted usage of jopsimple and our own parser code * reworked help/hint The biggest change is in GfshParser, instead of directly implement SpringShell's Parser, now it's just a simple wrapper around SpringShell's SimpleParser so that we can use spring's parsing code. The challenge is that SimpleParser expects the user inupt to be in the format of "command option1 value1 option2 value2", while gfsh expects the format to be like "command option1=value1 option2=value2", so most of the code in GfshParser is to turn the input into the SimpleParser input format and then feed it into SimpleParser to get the validation and autocompletion we needed. So far the difference I noticed with this new implementation are: 1) option validation is what we gain from directly using SpringShell. 2) SpringShell doesn't allow you to specify an option twice, so for multivalued parameters, our old parser can accept command like "change loglevel --member=member1 --member=member2", but now the parser will give you an error, and you should only do "change loglevel --member=member1,member2". The new parser did something speical for --J option though, so for --J, you can specify it multiple times. 3) For value separator, springShell default's to ",", you can only overwrite it with option context "splittingRegex", we do not honor the @CliMetaData(valueSepartor=) anymore. All of our commands uses "," for separators, so this won't break our commands, but if there is any command out there that would define a different @CliMetaData(valueSepartor=) other than ",", SpringShell would not know how to parse it. 4) To specify empty string as parameter value, before you will need to to (put region=A key="''" value="''"), spring shell would think the value you are trying to pass in are two single quotes instead of empty strings, now, you should only do (put region=A key="" value="") Diffs (updated) ----- geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandManager.java 44004454ef1646bfeca8a7af3cffb109fd83e7d7 geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java a1d03e45dd4d6559bd9a0869c7dd95908d1858ca geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/GfshHelpCommands.java 1d1b28e568a1e175690fea5cde1a45a318b6db5d geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ShellCommands.java b37feabe6ed61c081e9653c94128f092ad189d10 geode-core/src/main/java/org/apache/geode/management/internal/cli/help/Helper.java PRE-CREATION geode-core/src/main/java/org/apache/geode/management/internal/cli/remote/CommandProcessor.java c346eaf77087102f51952a567ecd7ec036593a13 geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java bcf1b415e9ee85822c470ae6888920f0a90159fd geode-core/src/test/java/org/apache/geode/management/internal/cli/CommandManagerJUnitTest.java 503ffb2929758d617e8539e6aefb3f7b545de9b6 geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserIntegrationTest.java f3e3bd8754e18d7a75faf4b75e3ba75b778fc9d7 geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserJUnitTest.java 44e99f461b9efc5a439e629751011a6d0edd9213 geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CliCommandTestBase.java 165f66482758dadb75ae95d23443973e9de05891 geode-core/src/test/java/org/apache/geode/management/internal/cli/help/HelpBlockUnitTest.java PRE-CREATION geode-core/src/test/java/org/apache/geode/management/internal/cli/help/HelperUnitTest.java PRE-CREATION geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategyJUnitTest.java 54c7cf7074435b48232b61916627eed69a201f09 Diff: https://reviews.apache.org/r/58589/diff/4/ Changes: https://reviews.apache.org/r/58589/diff/3-4/ Testing ------- precheckin passing except one unittest that always fails in concourse but succeed in local. Thanks, Jinmei Liao
