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