Re: RFR: 8184692: add Pattern.asMatchPredicate

2018-04-12 Thread Roger Riggs 

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.theeyar...@oracle.com>
*Cc:* Core-Libs-Dev <core-libs-dev@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.theeyar...@oracle.com> 
<mailto:vivek.theeyar...@oracle.com>

Cc: Roger Riggs<roger.ri...@oracle.com> <mailto:roger.ri...@oracle.com>; 
Core-Libs-Dev<core-libs-dev@openjdk.java.net>
<mailto:core-libs-dev@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.theeyar...@oracle.com> 
<mailto:vivek.theeyar...@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.ri...@oracle.com> <mailto:roger.ri...@oracle.com>

Cc: Core-Libs-Dev<core-libs-dev@openjdk.java.net>
<mailto:core-libs-dev@openjdk.java.net>

Subject: Re: RFR: 8184692: add Pattern.asMatchPredicate

On Apr 9, 2018, at 8:41 AM, Roger Riggs<roger.ri...@oracle.com> 
<mailto:roger.ri...@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.theeyar...@oracle.com>
<mailto:vivek.theeyar...@oracle.com>

    Cc: Core-Libs-Dev<core-libs-dev@openjdk.java.net>
<mailto:core-libs-dev@openjdk.java.net>

Subject: Re: RFR: 8184692: add Pattern.asMatchPredicate

On Apr 4, 2018, at 10:47 AM, Vivek 
Theeyarath<vivek.theeyar...@oracle.com>
<mailto:vivek.theeyar...@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/

RE: RFR: 8184692: add Pattern.asMatchPredicate

2018-04-12 Thread 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.theeyar...@oracle.com>
Cc: Core-Libs-Dev <core-libs-dev@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.theeyar...@oracle.com;<vivek.theeyar...@oracle.com>
Cc: Roger Riggs HYPERLINK 
"mailto:roger.ri...@oracle.com;<roger.ri...@oracle.com>; Core-Libs-Dev 
HYPERLINK 
"mailto:core-libs-dev@openjdk.java.net;<core-libs-dev@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.theeyar...@oracle.com;<vivek.theeyar...@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.ri...@oracle.com;<roger.ri...@oracle.com>
Cc: Core-Libs-Dev HYPERLINK 
"mailto:core-libs-dev@openjdk.java.net;<core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8184692: add Pattern.asMatchPredicate
 
 
 

On Apr 9, 2018, at 8:41 AM, Roger Riggs HYPERLINK 
"mailto:roger.ri...@oracle.com;<roger.ri...@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.theeyar...@oracle.com;<vivek.theeyar...@oracle.com>
Cc: Core-Libs-Dev HYPERLINK 
"mailto:core-libs-dev@openjdk.java.net;<core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8184692: add Pattern.asMatchPredicate
 
 
 

On Apr 4, 2018, at 10:47 AM, Vivek Theeyarath HYPERLINK 
"mailto:vivek.theeyar...@oracle.com;<vivek.theeyar...@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

 

 

 

 

Re: RFR: 8184692: add Pattern.asMatchPredicate

2018-04-12 Thread Paul Sandoz 
Ok, thanks. When the CSR is approved please directly send me an exported 
changeset and i will push for you.

Paul. 

> On Apr 12, 2018, at 5:49 AM, Vivek Theeyarath <vivek.theeyar...@oracle.com> 
> 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.theeyar...@oracle.com>
> Cc: Roger Riggs <roger.ri...@oracle.com>; Core-Libs-Dev 
> <core-libs-dev@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.

Re: RFR: 8184692: add Pattern.asMatchPredicate

2018-04-12 Thread Roger Riggs 

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.theeyar...@oracle.com>
Cc: Roger Riggs <roger.ri...@oracle.com>; Core-Libs-Dev 
<core-libs-dev@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.theeyar...@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 <roger.ri...@oracle.com>
Cc: Core-Libs-Dev <core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8184692: add Pattern.asMatchPredicate




On Apr 9, 2018, at 8:41 AM, Roger Riggs <roger.ri...@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 <vivek.theeyar...@oracle.com>
Cc: Core-Libs-Dev <core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8184692: add Pattern.asMatchPredicate




On Apr 4, 2018, at 10:47 AM, Vivek Theeyarath <vivek.theeyar...@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

RE: RFR: 8184692: add Pattern.asMatchPredicate

2018-04-12 Thread Vivek Theeyarath 
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.theeyar...@oracle.com>
Cc: Roger Riggs <roger.ri...@oracle.com>; Core-Libs-Dev 
<core-libs-dev@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.theeyar...@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 <roger.ri...@oracle.com>
> Cc: Core-Libs-Dev <core-libs-dev@openjdk.java.net>
> Subject: Re: RFR: 8184692: add Pattern.asMatchPredicate
> 
> 
> 
>> On Apr 9, 2018, at 8:41 AM, Roger Riggs <roger.ri...@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 <vivek.theeyar...@oracle.com>
>>> Cc: Core-Libs-Dev <core-libs-dev@openjdk.java.net>
>>> Subject: Re: RFR: 8184692: add Pattern.asMatchPredicate
>>> 
>>> 
>>> 
>>>> On Apr 4, 2018, at 10:47 AM, Vivek Theeyarath 
>>>> <vivek.theeyar...@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
>> 
>

Re: RFR: 8184692: add Pattern.asMatchPredicate

2018-04-11 Thread Paul Sandoz 
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.theeyar...@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 <roger.ri...@oracle.com>
> Cc: Core-Libs-Dev <core-libs-dev@openjdk.java.net>
> Subject: Re: RFR: 8184692: add Pattern.asMatchPredicate
> 
> 
> 
>> On Apr 9, 2018, at 8:41 AM, Roger Riggs <roger.ri...@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 <vivek.theeyar...@oracle.com>
>>> Cc: Core-Libs-Dev <core-libs-dev@openjdk.java.net>
>>> Subject: Re: RFR: 8184692: add Pattern.asMatchPredicate
>>> 
>>> 
>>> 
>>>> On Apr 4, 2018, at 10:47 AM, Vivek Theeyarath 
>>>> <vivek.theeyar...@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
>> 
>

RE: RFR: 8184692: add Pattern.asMatchPredicate

2018-04-09 Thread Vivek Theeyarath 
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 <roger.ri...@oracle.com>
Cc: Core-Libs-Dev <core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8184692: add Pattern.asMatchPredicate



> On Apr 9, 2018, at 8:41 AM, Roger Riggs <roger.ri...@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 <vivek.theeyar...@oracle.com>
>> Cc: Core-Libs-Dev <core-libs-dev@openjdk.java.net>
>> Subject: Re: RFR: 8184692: add Pattern.asMatchPredicate
>> 
>> 
>> 
>>> On Apr 4, 2018, at 10:47 AM, Vivek Theeyarath <vivek.theeyar...@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
>

Re: RFR: 8184692: add Pattern.asMatchPredicate

2018-04-09 Thread Paul Sandoz 


> On Apr 9, 2018, at 9:17 AM, Xueming Shen  wrote:
> 
> 
> Paul,
> 
> I don't recall any particular reason why we did Predicate 
> asPredicate() instead
> of Predicate asPredicate() back 1.8?  mutability?
> 

Hmm… me neither i would like to think it was not an oversight :-)

Mutability should not be an issue *locally* for the predicate implementation 
since it captures no mutable state. I suspect we erred on the side of 
immutability in general, so the predicate would be safe to use if there were 
any concurrent operation going on.

I believe we could revisit this decision (as a separate issue) and a change to 
Predicate would be both source and binary compatible. However, i 
would be inclined to leave things as they are and stick to the overall 
immutability story.

Paul.

Re: RFR: 8184692: add Pattern.asMatchPredicate

2018-04-09 Thread Xueming Shen 


Paul,

I don't recall any particular reason why we did Predicate 
asPredicate() instead

of Predicate asPredicate() back 1.8?  mutability?

sherman

On 4/9/18, 8:41 AM, Roger Riggs 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.


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 <vivek.theeyar...@oracle.com>
Cc: Core-Libs-Dev <core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8184692: add Pattern.asMatchPredicate



On Apr 4, 2018, at 10:47 AM, Vivek Theeyarath 
<vivek.theeyar...@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

Re: RFR: 8184692: add Pattern.asMatchPredicate

2018-04-09 Thread Paul Sandoz 


> On Apr 9, 2018, at 8:41 AM, Roger Riggs <roger.ri...@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 <vivek.theeyar...@oracle.com>
>> Cc: Core-Libs-Dev <core-libs-dev@openjdk.java.net>
>> Subject: Re: RFR: 8184692: add Pattern.asMatchPredicate
>> 
>> 
>> 
>>> On Apr 4, 2018, at 10:47 AM, Vivek Theeyarath <vivek.theeyar...@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
>

Re: RFR: 8184692: add Pattern.asMatchPredicate

2018-04-09 Thread Roger Riggs 

Hi Vivek,

As with Pattern.asPredicate the first sentence can be improved.

   Creates a predicate that tests if this pattern matches the entire region.

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 <vivek.theeyar...@oracle.com>
Cc: Core-Libs-Dev <core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8184692: add Pattern.asMatchPredicate




On Apr 4, 2018, at 10:47 AM, Vivek Theeyarath <vivek.theeyar...@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

RE: RFR: 8184692: add Pattern.asMatchPredicate

2018-04-09 Thread Vivek Theeyarath 
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 <vivek.theeyar...@oracle.com>
Cc: Core-Libs-Dev <core-libs-dev@openjdk.java.net>
Subject: Re: RFR: 8184692: add Pattern.asMatchPredicate



> On Apr 4, 2018, at 10:47 AM, Vivek Theeyarath <vivek.theeyar...@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

Re: RFR: 8184692: add Pattern.asMatchPredicate

2018-04-05 Thread Paul Sandoz 


> On Apr 4, 2018, at 10:47 AM, Vivek Theeyarath  
> 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