> On May 31, 2017, 10:18 p.m., Jared Stewart wrote: > > geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java > > Lines 48 (patched) > > <https://reviews.apache.org/r/59686/diff/1/?file=1735898#file1735898line48> > > > > I worry that a user may at some point specify a `--J` option that > > includes `,,`. I think our code could be more robust by using a delimeter > > that a user can't type on a standard keyboard. The ASCII "Unit separator" > > character (decimal code 31, hex 0x1f) seems like a natural candidate. That > > would make this look like: > > > > ``` > > private static final char ASCII_UNIT_SEPARATOR = '\u001F'; > > public static final String JARGUMENT_SPLIREGX = "" + > > ASCII_UNIT_SEPARATOR; > > ``` > > > > > > I also think that `J_ARGUMENT_DELIMITER` might be a more informative > > name.
+1 - Ken ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59686/#review176535 ----------------------------------------------------------- On May 31, 2017, 5:42 p.m., Jinmei Liao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59686/ > ----------------------------------------------------------- > > (Updated May 31, 2017, 5:42 p.m.) > > > Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and > Patrick Rhomberg. > > > Repository: geode > > > Description > ------- > > GEODE-2983: correctly handling --J option value that has "," inside. > > > Diffs > ----- > > > geode-core/src/main/java/org/apache/geode/management/internal/cli/GfshParser.java > 288ea054ae1230c480d141c0159d6ccf9c299a7d > > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/LauncherLifecycleCommands.java > 74acfd6e03613ac4d0c62fcdd4ea859d1c74d2f2 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/GfshParserParsingTest.java > 4467792f60a2a3bf7cc53cf35e997e7462882539 > > > Diff: https://reviews.apache.org/r/59686/diff/1/ > > > Testing > ------- > > precheckin running > > > Thanks, > > Jinmei Liao > >
