Re: RFR (JAXP) 8169450: StAX parse error if there is a newline in xml declaration

2017-02-14 Thread huizhe wang
Thanks Lance, Daniel! -Joe On 2/14/2017 10:53 AM, Lance Andersen wrote: Looks good… ship it. On Feb 14, 2017, at 1:41 PM, Daniel Fuchs > wrote: On 14/02/17 17:21, huizhe wang wrote: Thanks! Here's an updated webrev:

Re: RFR (JAXP) 8169450: StAX parse error if there is a newline in xml declaration

2017-02-14 Thread Lance Andersen
Looks good… ship it. > On Feb 14, 2017, at 1:41 PM, Daniel Fuchs wrote: > > On 14/02/17 17:21, huizhe wang wrote: >> Thanks! >> >> Here's an updated webrev: >> http://cr.openjdk.java.net/~joehw/jdk9/8169450/webrev/ > > +1 > > -- daniel > >> >> -Joe >> >> On

Re: RFR (JAXP) 8169450: StAX parse error if there is a newline in xml declaration

2017-02-14 Thread Daniel Fuchs
On 14/02/17 17:21, huizhe wang wrote: Thanks! Here's an updated webrev: http://cr.openjdk.java.net/~joehw/jdk9/8169450/webrev/ +1 -- daniel -Joe On 2/14/2017 4:07 AM, Lance Andersen wrote: Looks good overall Joe. I would agree that I would clean up the minor comment alignment issues.

Re: RFR (JAXP) 8169450: StAX parse error if there is a newline in xml declaration

2017-02-14 Thread huizhe wang
Thanks! Here's an updated webrev: http://cr.openjdk.java.net/~joehw/jdk9/8169450/webrev/ -Joe On 2/14/2017 4:07 AM, Lance Andersen wrote: Looks good overall Joe. I would agree that I would clean up the minor comment alignment issues. Best Lance On Feb 13, 2017, at 9:27 PM, huizhe wang

Re: RFR (JAXP) 8169450: StAX parse error if there is a newline in xml declaration

2017-02-14 Thread huizhe wang
On 2/14/2017 3:16 AM, Daniel Fuchs wrote: Looks good Joe. I wonder about this though (which may be an issue for another time): 102 [25] Eq ::= S? '=' S? Do we support space (new line?) before and after the '=' sign? Yes, we do. I've added a couple of test cases to cover

Re: RFR (JAXP) 8169450: StAX parse error if there is a newline in xml declaration

2017-02-14 Thread huizhe wang
Thanks Christoph. Will fix those comments. -Joe On 2/13/2017 11:56 PM, Langer, Christoph wrote: Hi Joe, this looks right to me, quite straightforward. I just have some formatting nits for the test: 1. Line 107 (XMLInputFactory xif=): space missing before '=' 2. Method comments on test

Re: RFR (JAXP) 8169450: StAX parse error if there is a newline in xml declaration

2017-02-14 Thread Lance Andersen
Looks good overall Joe. I would agree that I would clean up the minor comment alignment issues. Best Lance > On Feb 13, 2017, at 9:27 PM, huizhe wang wrote: > > A quick fix for the error parsing xml declaration. This is one of the three > outstanding issues in the xml

Re: RFR (JAXP) 8169450: StAX parse error if there is a newline in xml declaration

2017-02-14 Thread Daniel Fuchs
Looks good Joe. I wonder about this though (which may be an issue for another time): 102 [25] Eq ::= S? '=' S? Do we support space (new line?) before and after the '=' sign? best regards, -- daniel On 14/02/17 02:27, huizhe wang wrote: A quick fix for the error parsing xml

RE: RFR (JAXP) 8169450: StAX parse error if there is a newline in xml declaration

2017-02-13 Thread Langer, Christoph
Hi Joe, this looks right to me, quite straightforward. I just have some formatting nits for the test: 1. Line 107 (XMLInputFactory xif=): space missing before '=' 2. Method comments on test methods would look nicer if formatted like: /* * line 1 * line 2 * ... */ ... I want to say that the '*'