RFR: 8184692: add Pattern.asMatchPredicate

Roger Riggs roger.riggs at oracle.com
Fri Apr 13 02:42:09 UTC 2018


Thanks Vivek,

Looks good to me.

On 4/12/18 9:25 PM, Vivek Theeyarath wrote:
>
> Hi Roger,
>
> Please find the updated webrev 
> http://cr.openjdk.java.net/~vtheeyarath/8184692/webrev.04/ 
> <http://cr.openjdk.java.net/%7Evtheeyarath/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<vivek.theeyarath at oracle.com> <mailto:vivek.theeyarath at oracle.com>
>
>     Cc: Roger Riggs<roger.riggs at oracle.com> <mailto:roger.riggs at oracle.com>; Core-Libs-Dev<core-libs-dev at openjdk.java.net>
>     <mailto: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<vivek.theeyarath at oracle.com> <mailto: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
>         <http://cr.openjdk.java.net/%7Evtheeyarath/8184692/webrev.03>  . Please review.
>
>         Regards
>
>         Vivek
>
>         -----Original Message-----
>
>         From: Paul Sandoz
>
>         Sent: Monday, April 09, 2018 9:25 PM
>
>         To: Roger Riggs<roger.riggs at oracle.com> <mailto:roger.riggs at oracle.com>
>
>         Cc: Core-Libs-Dev<core-libs-dev at openjdk.java.net>
>         <mailto:core-libs-dev at openjdk.java.net>
>
>         Subject: Re: RFR: 8184692: add Pattern.asMatchPredicate
>
>             On Apr 9, 2018, at 8:41 AM, Roger Riggs<Roger.Riggs at Oracle.com> <mailto: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/
>                 <http://cr.openjdk.java.net/%7Evtheeyarath/8184692/webrev.02/>
>
>                 Also, I have created a csr for this issuehttps://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<vivek.theeyarath at oracle.com>
>                 <mailto:vivek.theeyarath at oracle.com>
>
>                 Cc: Core-Libs-Dev<core-libs-dev at openjdk.java.net>
>                 <mailto:core-libs-dev at openjdk.java.net>
>
>                 Subject: Re: RFR: 8184692: add Pattern.asMatchPredicate
>
>                     On Apr 4, 2018, at 10:47 AM, Vivek Theeyarath<vivek.theeyarath at oracle.com>
>                     <mailto: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/
>                     <http://cr.openjdk.java.net/%7Evtheeyarath/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
>



More information about the core-libs-dev mailing list