On Wed, Jan 26, 2011 at 10:25 PM, Dr Andrew John Hughes
<ahug...@redhat.com> wrote:
> On 22:15 Wed 26 Jan , Pekka Enberg wrote:
>> On Wed, 2011-01-26 at 20:10 +0000, Dr Andrew John Hughes wrote:
>> > On 22:41 Wed 26 Jan , Ivan Maidanski wrote:
>> > > Hi,
>> > >
>> > > It's ok but:
>> > >
>> > > 1. I'd better rewrote check for null (IMHO looks better):
>> > >
>> > > try {
>> > > Pattern.quote(null);
>> > > harness.check(true);
>> > > } catch (NullPointerException e) {
>> > > harness.check(false);
>> > > }
>> > >
>> >
>> > Yeah I like this version better too. The current one reads rather oddly.
>> > However, you seem to have inverted the logic; harness.check(true) should
>> > be called when the NPE is given.
>> >
>> > If an NPE should be thrown for null values, that should be documented in
>> > the Classpath patch too.
>>
>> Here's a new version.
>
> Is \\Q\\Q\\E really the right behaviour? Presumably they don't nest and \E
> closes all open \Qs?
It should be the right behavior. I run the tests always with OpenJDK
and with JamVM and Jato with GNU Classpath CVS head and make sure they
pass with all three of them.
>> I'll update the Javadoc in the patch. Andrew, if
>> your OK with the Mauve test case, feel free to commit it to CVS. I'm
>> still waiting for my Mauve CVS account.
>>
>
> Oh, thought you had access. I'll glad commit the test for you and also see
> if I can find who sorts out access to this.
I did fill out some form as suggested by Mark and now I'm just waiting... :-)
