<i18n dev> [9] RFR: 8159781: jlink --include-locales fails with java.util.regex.PatternSyntaxException

Masayoshi Okutsu masayoshi.okutsu at oracle.com
Wed Jun 22 15:13:30 UTC 2016


Looks good.

Masayoshi


On 6/22/2016 1:53 PM, Naoto Sato wrote:
> Modified line 243-246 to avoid generating a new List instance with 
> Collectors.toList():
>
> http://cr.openjdk.java.net/~naoto/8159781/webrev.03/
>
> Naoto
>
> On 6/21/16 1:51 PM, Naoto Sato wrote:
>> Thank you for the review, Masayoshi.
>>
>> I don't think that "-verbose:gc" has anything to do with the long
>> pattern string (the option had been there before I wrote the locale
>> plugin), but I see your point here. I modified the fix according to your
>> suggestion:
>>
>> http://cr.openjdk.java.net/~naoto/8159781/webrev.02/
>>
>> Naoto
>>
>> On 6/20/16 7:53 PM, Masayoshi Okutsu wrote:
>>> If the long pattern string is avoided, it'll be unnecessary to use "%%"
>>> and replaceAll? I'm also concerned to keep concatenating strings to
>>> produce a long string (rather than using a StringBuilder). Would 
>>> that be
>>> the reason to put -verbose:gc in the test program? Use of List<String>
>>> will simplify the processing as Mandy suggests.
>>>
>>> Masayoshi
>>>
>>>
>>> On 6/21/2016 7:00 AM, Naoto Sato wrote:
>>>> Here is the updated webrev:
>>>>
>>>> http://cr.openjdk.java.net/~naoto/8159781/webrev.01/
>>>>
>>>> Naoto
>>>>
>>>> On 6/20/16 8:18 AM, Naoto Sato wrote:
>>>>> Hi Mandy, thank you for the review.
>>>>>
>>>>> INCLUDE_LOCALE_FILES is actually a pattern template, and the real
>>>>> filter
>>>>> patterns are constructed at the runtime with the passed argument, by
>>>>> replacing "%%" with locale patterns. So I think it is less complex to
>>>>> handle it as the current way.
>>>>>
>>>>> I will modify the "regex:" prepending as you suggested.
>>>>>
>>>>> Naoto
>>>>>
>>>>> On 6/17/16 3:50 PM, Mandy Chung wrote:
>>>>>>
>>>>>>> On Jun 17, 2016, at 3:29 PM, Naoto Sato <naoto.sato at oracle.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>> Decided to fix this separately from the other include locales 
>>>>>>> issues.
>>>>>>> Here is the bug and the proposed fix:
>>>>>>>
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8159781
>>>>>>> http://cr.openjdk.java.net/~naoto/8159781/webrev.00/
>>>>>>
>>>>>> The change looks okay.   This can be converted to use
>>>>>> ResourceFilter::includeFilter(List<String> patterns) rather than
>>>>>> building a comma-separated patterns string.  That means 
>>>>>> META_FILES and
>>>>>> INCLUDE_LOCALE_FILES can be made as List<String>. You have an option
>>>>>> to prepend “regex:” when constructing the patterns to pass to
>>>>>> ResourceFilter.  I think that’s a good clean up to do.
>>>>>>
>>>>>> Mandy
>>>>>>
>>>



More information about the i18n-dev mailing list