On 22:29 Wed 26 Jan , Pekka Enberg wrote:
> 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.
>
Ok, cool. I wasn't saying it was wrong (haven't tested this method at all yet)
but just seemed odd from a naive perspective.
> >> 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... :-)
--
Andrew :)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
Fingerprint = F8EF F1EA 401E 2E60 15FA 7927 142C 2591 94EF D9D8
