Hi,
        I have incorporated the changes as per the feedback and here is the 
updated webrev .
http://cr.openjdk.java.net/~rraghavan/8164781/webrev.02/ .
Bug: https://bugs.openjdk.java.net/browse/JDK-8164781 

Here is the related csr https://bugs.openjdk.java.net/browse/JDK-8200603 

I will try to address Uwe's point with a fix separately.

Regards
Vivek
-----Original Message-----
From: Stuart Marks 
Sent: Wednesday, April 04, 2018 6:13 AM
To: Vivek Theeyarath <vivek.theeyar...@oracle.com>
Cc: Paul Sandoz <paul.san...@oracle.com>; Core-Libs-Dev 
<core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8164781: Pattern.asPredicate specification is incomplete

Hi Vivek,

Thanks for taking on this task.

In case it wasn't clear from Paul's mail, what I think you should do is 
continue with this fix as a doc-only (and test-only) change, and not modify the 
behavior of this method. Doing that would be an incompatible change.

Uwe's point is a reasonable one, which is that you can't tell from the method 
name "asPredicate" whether it uses find() or match() semantics. Oh well, I 
think we just have to live with this, and document it clearly.

Adding a method to create a Predicate that has match() semantics would be a 
fine task to consider separately.

Also, in RegExTest.java,

4686         if (p.test("word1234")) {
4687             failCount++;
4688         }

I think the logic should be negated, as the predicate should properly find the 
pattern in this string.

Thanks,

s'marks

On 4/2/18 10:56 AM, Paul Sandoz wrote:
> Hi,
> 
> Looks good, expect for:
> 
> 5823      * @return  The predicate which can be used for finding on a string
> 
> “finding on a… ” is a little awkward to parse . I recommend to either change 
> it back, since the first sentence of the method doc says what it means by 
> matches, or being a little more verbose:
> 
>    The predicate which can be used for finding a match on a 
> subsequence of a string
> 
> You will need a CSR to document the clarification in specification behavior.
> 
> —
> 
> To Uwe’s point, we could have chosen a more descriptive method name, e.g. 
> asFinding/Predicate, leaving logical space for say any future 
> asMatching/Predicate if we chose to add it.
> 
> Paul.
> 
> 
>> On Apr 1, 2018, at 1:11 AM, Vivek Theeyarath <vivek.theeyar...@oracle.com> 
>> wrote:
>>
>> Hi all,
>>
>>                Please review.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8164781
>>
>> Webrev: http://cr.openjdk.java.net/~rraghavan/8164781/webrev.01/
>>
>>
>>
>> Regards
>>
>> Vivek
> 

Reply via email to