geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ken Howe <kh...@pivotal.io>
Subject Re: Review Request 59686: GEODE-2983: correctly handling --J option value that has ", " inside.
Date Wed, 31 May 2017 22:34:21 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message