----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58589/#review173100 -----------------------------------------------------------
Ship it! Ship It! - Kirk Lund On April 26, 2017, 7:48 p.m., Jinmei Liao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58589/ > ----------------------------------------------------------- > > (Updated April 26, 2017, 7:48 p.m.) > > > Review request for geode, Jared Stewart, Ken Howe, Kirk Lund, and Patrick > Rhomberg. > > > 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 > ----- > > > 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/ > > > Testing > ------- > > precheckin successful with the latest changes. > > > Thanks, > > Jinmei Liao > >
