Fix for 8080771

Robert Field robert.field at oracle.com
Wed Jun 3 21:12:42 UTC 2015


[resending, somehow this mail did not get delivered]

On 05/29/15 07:26, andrei.eremeev wrote:
> Hi Robert,
>
> The second version: http://cr.openjdk.java.net/~aeremeev/8080771.01/

Hi Andrei,

I don't understand why the special treatment for dot:    \\.|
This version doesn't handle when quotes start in the middle of the 
string, which is sometimes done in a command:  someeditor editorArg="foo 
bar"
You are using both parsing logic and Patterns, I think that adds 
complexity. -- I would stick with one or the other  I think you could 
use groups to detect unmatched quotes (but maybe that's over the top) 
or, go back to character-by-character parsing.
We are going for user-friendly error messages where we can, maybe change 
"Unexpected EOL while looking for matching quote " to just "Unmatched 
quote in editor command".

Oh, and you are using "+", a string could be empty, so, using "*" would 
be better.

-Robert



>
> Andrei
>
> On 05/28/2015 06:05 PM, Robert Field wrote:
>>
>> Resending....
>>
>> On May 26, 2015 11:49:07 AM Robert Field <robert.field at oracle.com> 
>> wrote:
>>
>>>
>>> On 05/26/15 04:08, Andrei Eremeev wrote:
>>>> Hi all,
>>>>
>>>> Could anybody review the fix?
>>>> Fix:http://cr.openjdk.java.net/~aeremeev/8080771.00/
>>>> Bug:https://bugs.openjdk.java.net/browse/JDK-8080771
>>>>
>>>> Andrei Eremeev
>>>
>>> Sure.
>>>
>>> parseSetEditorCmd --
>>>   * My approach would have been to use regular expressions rather 
>>> than hand rolling.
>>>   * There are many other white-space characters besides space.
>>>   * Multiple initial spaces are handled, but if there are multiple 
>>> non-initial spaces occur empty arguments are added.
>>>   * If the break is is removed from the space case then the ++index 
>>> can be too.
>>>   * What about slash escape?  Escaped quotes?  Do we want to support 
>>> that?
>>>
>>> cmdSetEditor --
>>>   * I'd move the Unexpected EOL message to the cause site in 
>>> parseSetEditorCmd.
>>>
>



More information about the kulla-dev mailing list