----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58589/#review172962 -----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandManager.java Line 127 (original) <https://reviews.apache.org/r/58589/#comment245992> Collapse the duplicate `ClassNotFoundException | IOException` catch blocks. geode-core/src/main/java/org/apache/geode/management/internal/cli/CommandManager.java Line 182 (original) <https://reviews.apache.org/r/58589/#comment245993> Collapse duplicate catch blocks. geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java Lines 334 (patched) <https://reviews.apache.org/r/58589/#comment245994> Delete dead code. geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/Gfsh.java Lines 348 (patched) <https://reviews.apache.org/r/58589/#comment245995> Delete dead code. geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java Line 97 (original), 97 (patched) <https://reviews.apache.org/r/58589/#comment245996> Huzzah for correcting typos. - Patrick Rhomberg On April 25, 2017, 1:08 a.m., Jinmei Liao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58589/ > ----------------------------------------------------------- > > (Updated April 25, 2017, 1:08 a.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/cli/CliMetaData.java > e69d78a2b42b4a7f21066712b9f763dc1cdb92c8 > geode-core/src/main/java/org/apache/geode/management/cli/ConverterHint.java > f45abc432dce8af67b71ca29fdfb85e0e9f6f87a > > 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/CommandResponseBuilder.java > bda030de50cca303e2aa6d3d82b17a9111fd25ef > > geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParseResult.java > d879e2d6c253eb3306fb7558fa0a2b7fbe7d1e40 > > 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/Launcher.java > fc0427e20144a9092d97d05fc3ccbd0e21d5b35e > > geode-core/src/main/java/org/apache/geode/management/internal/cli/annotation/CliArgument.java > e20e73112c3b6545771c62e58795fdc53fc279c5 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ClientCommands.java > 39d0442f2cb4ec4276652169a0cfa962a78af2c9 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ConfigCommands.java > 50c9caaee88426f9a8c49d8b1abca9c2fbc66163 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateAlterDestroyRegionCommands.java > 28ce0921e0cba412063d3a59566165544dfda3dc > > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DataCommands.java > 6324b5cfc70c0f02233f73ceab6dc56cb253cbb2 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommands.java > 144408854daf2cd5b33dcdcf995efef77b3e942e > > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommands.java > 9ad206043a284144293175baa7144bb835652a46 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/FunctionCommands.java > 752ca2a612eb59c137de568f80223d58339e7231 > > 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/LauncherLifecycleCommands.java > 313d1bdfb4d1e610f2353db2a4496449453a0fbe > > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/MiscellaneousCommands.java > 22981e72f4971f3bb297a53e9ac4d6f8ac7e149e > > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/PDXCommands.java > 4327decc98e3d0fe637d73857c9eed83f7bc88b9 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/QueueCommands.java > 8c568336aa62c6b6cbf4e1a29584d514e84f1c96 > > 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/commands/WanCommands.java > 239db48417cdae0d415830ac411cb611b8433919 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/BooleanConverter.java > 5e9cdb9a642e9cda4934e7934c2fc022e7d9eabe > > geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/DirConverter.java > c97057bb281e64c96983e38172bcf7953f492c83 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/EnumConverter.java > 354cef9708623d9141caea08fe497deaa75f606f > > geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/HelpConverter.java > e670274f89ce4423a77715dcee07f119c1568929 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/HintTopicConverter.java > b6f9f81af10a7773df27f77ca5707238e7ef3a3b > > geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/LogLevelConverter.java > 3a262405da0d9821a1c9729ca0aad21ccf626a02 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/StringArrayConverter.java > eacf1810ba53fc0c389cff0d5ae30ba5fea6b0c3 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/converters/StringListConverter.java > eab096b9a5c93fd71f9a0b757d80a2d726690ad1 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/exceptions/CliCommandException.java > 7e5cba05d93a88fa722d98babfdabf89e4e07be0 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/exceptions/CliCommandInvalidException.java > a14005923ee3343d2e2e8e1818b8fbac16410ca6 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/exceptions/CliCommandMultiModeOptionException.java > acbc49618504bec9f8eb6f04124e02a47e0a92c3 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/exceptions/CliCommandNotAvailableException.java > c471df298f10211a812ae0e644c88f81c68b8132 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/exceptions/CliCommandOptionException.java > a7e56be98afe94690cdc1b61ea5476a72fbd9140 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/exceptions/CliCommandOptionHasMultipleValuesException.java > 4b365e3f9d611e74f345f50f87c682f272d20ba4 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/exceptions/CliCommandOptionInvalidException.java > 1db8906024934a024febf447f8d5facc5f1b2ee8 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/exceptions/CliCommandOptionMissingException.java > f263dce0be966a361b8b4e84ed8360cc1a659e3b > > geode-core/src/main/java/org/apache/geode/management/internal/cli/exceptions/CliCommandOptionNotApplicableException.java > 98147787eff00d74ed5e32d2c684867fd1bf605a > > geode-core/src/main/java/org/apache/geode/management/internal/cli/exceptions/CliCommandOptionValueConversionException.java > 7dbf869eab0c8331c217aed6eb82b57ea7696be6 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/exceptions/CliCommandOptionValueException.java > ee02df8423ef48f02ed482ad3e40260799b2e7d7 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/exceptions/CliCommandOptionValueMissingException.java > 023a8789fe4a0ca2fc11bce7482d0e9ed4aa1b2a > > geode-core/src/main/java/org/apache/geode/management/internal/cli/exceptions/ExceptionGenerator.java > fda31357a39830b7414b4751966dba2e57526b13 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/exceptions/ExceptionHandler.java > 95afbafe55746bb0159e5067ae4d5aa9c0701b56 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/help/CliTopic.java > 791cdca5ce954153ed5013f3108d738d970b32dd > > geode-core/src/main/java/org/apache/geode/management/internal/cli/help/HelpBlock.java > PRE-CREATION > > 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/help/format/Block.java > dde1a20ef0bfa98a7b5dcf57c1775f1769ab3c97 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/help/format/DataNode.java > 8f5e570cc9761ddedcb7fb16b0a9615f2da87209 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/help/format/Help.java > 68c10bb211c319bed679e39718660d1a466967a9 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/help/format/NewHelp.java > 90f9eda49326fa008cc5c1abb5c02f9dcfd94fd6 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/help/format/Row.java > ac1ca218668aff03d3a774ae2e1ebaebe8215b3c > > geode-core/src/main/java/org/apache/geode/management/internal/cli/help/utils/FormatOutput.java > 44998a03bed5203200115ce1b2023a436a52fd84 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/help/utils/HelpUtils.java > 11765c54df0df755b7f9b12b9d50986223d6b775 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java > 126eb47e90c2390953a91136e8cfb36687baf2fd > > geode-core/src/main/java/org/apache/geode/management/internal/cli/multistep/CLIMultiStepHelper.java > 0d215433c548db82b3c37b3e53be42cb0e1765fd > > geode-core/src/main/java/org/apache/geode/management/internal/cli/parser/Argument.java > 9acbc2a82c9cf4d4ccdb629798bdf70e1e48a305 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/parser/AvailabilityTarget.java > eff5fd2c2f67280ed9037216c1b15ab8b4a462d1 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/parser/CommandTarget.java > 3dfc01ab26d0ad6ee1867ab4014028f5cc0a3649 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/parser/GfshMethodTarget.java > 6f28830c314593004dfbb5eda113540d951fb343 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/parser/GfshOptionParser.java > a64933c2e9c5e3122a856d292b2ad70949e5017c > > geode-core/src/main/java/org/apache/geode/management/internal/cli/parser/MethodParameter.java > 599fb00d511de87d5a0b439371a9fec62377c027 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/parser/Option.java > 4eec1128c6e4646cde7b89e04f567fee2e419ee9 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/parser/OptionSet.java > 42270e53d047de566943a59c53969bdae8b32f54 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/parser/Parameter.java > dc371b3ba58b2ede0663e3167835926d17a3a418 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/parser/ParserUtils.java > 80f12867b7c0144ef9cb5aa5d5e7cc4f5c36b9f1 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/parser/SyntaxConstants.java > 52d92128b96e119f3157e5431d579e90218cbaad > > geode-core/src/main/java/org/apache/geode/management/internal/cli/parser/jopt/JoptOptionParser.java > 52224d40c9c47dd730432859033701e9b8faac4d > > geode-core/src/main/java/org/apache/geode/management/internal/cli/parser/preprocessor/EnclosingCharacters.java > a35e6269bd5ba2644fd2fbeb90c6fd8a92a8b971 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/parser/preprocessor/Preprocessor.java > 0dd875ad95ae30a6f2ca962da683e9e9ad0b9559 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/parser/preprocessor/PreprocessorUtils.java > a1872c92568a65c11f52603575a7814f2eb4c3f9 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/parser/preprocessor/Stack.java > ae47723f1ef2ff9af2e92c9991d02205fa044bac > > geode-core/src/main/java/org/apache/geode/management/internal/cli/parser/preprocessor/TrimmedInput.java > 8740f003f5a90a6ac296317ae4efee799e713baf > > 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/main/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategy.java > d74f5d6b4fce9a44d9eeebdfc7dcf716e9b1652b > > geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/MultiCommandHelper.java > 89f93d5a4f491addc955eeaaa11f0318e00f35de > > geode-core/src/main/java/org/apache/geode/management/internal/cli/shell/jline/GfshHistory.java > dc4a42fc014c73df16f057155229a51bd309efa3 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/util/CommandStringBuilder.java > 16efda5f772e50b3dea4899733760b0a25513a0e > > geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/DataCommandsController.java > cfe60899503193d4079cff4798422cd4da00cd74 > > geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java > 527e059d1dfbe03778141ec0470d7340343f9e56 > > 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/JoptOptionParserTest.java > 9815a9cbadae77ea94d49288922f204acf308d61 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/annotations/CliArgumentJUnitTest.java > 161f7c65e62f11aa7b25fb31cd94ad256ea497a6 > > 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/commands/CreateAlterDestroyRegionCommandsDUnitTest.java > 5f885e136e09541a6d095516fc7cb7db88f659aa > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandsDUnitTest.java > 9ed5bed8faa6bfceb0849347cd94b7c8effd3191 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DiskStoreCommandsDUnitTest.java > 5b07f282b83d1539df6a7ea7bf19888436738723 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/GemfireDataCommandsDUnitTest.java > e99a7fbc859243579b75a76742e23fec9b738ef5 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/HelpCommandsIntegrationTest.java > b91a1f35efaa00f29a0e858a90e55c1c39c163f9 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/QueueCommandsDUnitTest.java > 42a0624546742491e0c8c4d8c406ae8e567c1f85 > > 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/parser/ParserUtilsJUnitTest.java > 9b22e64d263d6cb8c4453e2eea42fa85e7f1f922 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/parser/preprocessor/PreprocessorJUnitTest.java > 97325cb16b42a112680a793b6581a1fea0e8c621 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/parser/preprocessor/PreprocessorUtilsJUnitTest.java > b56cff2cfb7027999a420ae91f1f983243f865f0 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshExecutionStrategyJUnitTest.java > 54c7cf7074435b48232b61916627eed69a201f09 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshHistoryJUnitTest.java > 58453b7c70b552af30ca4e16cd624475a89e3426 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshJunitTest.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigDistributionDUnitTest.java > abbc5c0c7e83d259cd13bae61c419f8cf07db1f0 > > geode-core/src/test/java/org/apache/geode/management/internal/security/TestCommand.java > ffe6a28163acf20d13b62347c40d5cceea22c629 > > geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommands.java > 4bfa86842fd81873c132e3bbd7b529ff3e64dbc7 > > geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneCommandsSecurityDUnitTest.java > ad734e8436822112c1eb8f5c6c0be676884dd009 > > geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommandsDUnitTest.java > 7f203cee7899340e19d589a4618be4ea7632faac > > geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java > 5c1832575ec5c5cccdce670bd9b3477708f79148 > > > Diff: https://reviews.apache.org/r/58589/diff/3/ > > > Testing > ------- > > precheckin passing except one unittest that always fails in concourse but > succeed in local. > > > Thanks, > > Jinmei Liao > >
