RFR: 8184692: add Pattern.asMatchPredicate

[Vivek Theeyarath]

Hi Roger,

               Please find the updated webrev http://cr.openjdk.java.net/~vtheeyarath/8184692/webrev.04/ . I fixed the indentation with my earlier fix (asPredicate) too. Hope that is fine.

 

Regards

Vivek

From: Roger Riggs 
Sent: Thursday, April 12, 2018 7:43 PM
To: Vivek Theeyarath <vivek.theeyarath at oracle.com>
Cc: Core-Libs-Dev <core-libs-dev at openjdk.java.net>
Subject: Re: RFR: 8184692: add Pattern.asMatchPredicate

 

Hi Vivek,

ok,  

Generally, I like to see an updated webrev so that an old non-final webrev isn't left around to be a source of questions.
Its easy enough to create a simple script to create the webrev and copy it to cr.openjdk...

Continued javadoc @xxx lines should be indented to improve readability of the source.

5845      *           against this pattern.


Thanks, Roger



On 4/12/18 8:49 AM, Vivek Theeyarath wrote:

Hi,
  I have updated the doc as per the suggestion. Have finalized the csr too.
 
Regards
Vivek
-----Original Message-----
From: Paul Sandoz 
Sent: Thursday, April 12, 2018 12:40 AM
To: Vivek Theeyarath HYPERLINK "mailto:vivek.theeyarath at oracle.com"<vivek.theeyarath at oracle.com>
Cc: Roger Riggs HYPERLINK "mailto:roger.riggs at oracle.com"<roger.riggs at oracle.com>; Core-Libs-Dev HYPERLINK "mailto:core-libs-dev at openjdk.java.net"<core-libs-dev at openjdk.java.net>
Subject: Re: RFR: 8184692: add Pattern.asMatchPredicate
 
HI,
 
Hopefully this is the last update :-) 
 
To align with asPredicate and Roger’s prior guidance on that method i think you can just say this:
 
  Creates a predicate that tests if this pattern matches a given input string.
 
which is similar to the language of Pattern.matches.
 
No need for another webrev if this change is acceptable.
 
Paul.
 

On Apr 9, 2018, at 7:53 PM, Vivek Theeyarath HYPERLINK "mailto:vivek.theeyarath at oracle.com"<vivek.theeyarath at oracle.com> wrote:
 
Thanks Paul / Roger for the inputs.
I have updated the javadoc based on the inputs. http://cr.openjdk.java.net/~vtheeyarath/8184692/webrev.03 . Please review.
 
Regards
Vivek
-----Original Message-----
From: Paul Sandoz
Sent: Monday, April 09, 2018 9:25 PM
To: Roger Riggs HYPERLINK "mailto:roger.riggs at oracle.com"<roger.riggs at oracle.com>
Cc: Core-Libs-Dev HYPERLINK "mailto:core-libs-dev at openjdk.java.net"<core-libs-dev at openjdk.java.net>
Subject: Re: RFR: 8184692: add Pattern.asMatchPredicate
 
 
 

On Apr 9, 2018, at 8:41 AM, Roger Riggs HYPERLINK "mailto:Roger.Riggs at Oracle.com"<Roger.Riggs at Oracle.com> wrote:
 
Hi Vivek,
 
As with Pattern.asPredicate the first sentence can be improved.
 
 Creates a predicate that tests if this pattern matches the entire region.
 

 
The region of what? region is clear from the context of a Matcher, but less so from the context of Pattern.
 
Paul.
 

5833: As with the original issue, perhaps adding the word 'whole' or 
'entire' will make it clearer that the pattern must match then entire input string.
 

 

5827:  Split into two sentences, the second one starting  "For example,"
 
5840: add a blank line between methods
 
Regards, Roger
 
 
On 4/9/18 5:05 AM, Vivek Theeyarath wrote:

Hi,
       Please find the updated webrev after incorporating Paul's comments. 
http://cr.openjdk.java.net/~vtheeyarath/8184692/webrev.02/
 
Also, I have created a csr for this issue https://bugs.openjdk.java.net/browse/JDK-8201308 .
 
Regards
Vivek
-----Original Message-----
From: Paul Sandoz
Sent: Friday, April 06, 2018 6:55 AM
To: Vivek Theeyarath HYPERLINK "mailto:vivek.theeyarath at oracle.com"<vivek.theeyarath at oracle.com>
Cc: Core-Libs-Dev HYPERLINK "mailto:core-libs-dev at openjdk.java.net"<core-libs-dev at openjdk.java.net>
Subject: Re: RFR: 8184692: add Pattern.asMatchPredicate
 
 
 

On Apr 4, 2018, at 10:47 AM, Vivek Theeyarath HYPERLINK "mailto:vivek.theeyarath at oracle.com"<vivek.theeyarath at oracle.com> wrote:
 
Hi All,
 
             Please review.
 
Bug: https://bugs.openjdk.java.net/browse/JDK-8184692
 
Webrev : http://cr.openjdk.java.net/~vtheeyarath/8184692/webrev.00/
 

Like with your other patch, alignment to ~80 chars would be good, as that is mostly consistent with other code in the same source file.
 
Let’s not use the word “find" here, so as not to confuse with matcher(s).find().
 
5833      * @return  The predicate which can be used for finding if an input string matches this pattern.
 
I suggest:
 
@return  The predicate which can be used for matching an input 
string against this pattern
 
You could also add a @see Matcher#matches
 
Paul.
 

 
The related jtreg test was run and the test passed .
 
 
 
Regards
 
Vivek