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