[cp-patches] [PATCH] Implement Pattern.quote() API method

Pekka Enberg penberg at kernel.org
Wed Jan 26 20:29:07 UTC 2011


On Wed, Jan 26, 2011 at 10:25 PM, Dr Andrew John Hughes
<ahughes at 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... :-)



More information about the Classpath-patches mailing list