Re: RFR: JDK-8263261 Extend String::translateEscapes to support unicode escapes [v7]

2024-01-26 Thread Jim Laskey 
On Fri, 19 Jan 2024 18:28:22 GMT, Roger Riggs  wrote:

>> Jim Laskey has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Update Copyright
>
> test/jdk/java/lang/String/TranslateEscapes.java line 113:
> 
>> 111: }
>> 112: 
>> 113: static void verifyEscape(String string1, String string2) {
> 
> These are unicode escapes too.  The method name should reflect that.

Changed

-

PR Review Comment: https://git.openjdk.org/jdk/pull/17491#discussion_r1467635284

Re: RFR: JDK-8263261 Extend String::translateEscapes to support unicode escapes [v7]

2024-01-26 Thread Jim Laskey 
On Fri, 19 Jan 2024 18:27:24 GMT, Roger Riggs  wrote:

>> Jim Laskey has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Update Copyright
>
> test/jdk/java/lang/String/TranslateEscapes.java line 127:
> 
>> 125: } catch (IllegalArgumentException ex) {
>> 126: }
>> 127: }
> 
> The method name implies valid unicode escape sequences, but they are all 
> invalid.
> The method name could be "verifyIllegalUnicodeEscape`.

Changed

-

PR Review Comment: https://git.openjdk.org/jdk/pull/17491#discussion_r1467632496

Re: RFR: JDK-8263261 Extend String::translateEscapes to support unicode escapes [v7]

2024-01-19 Thread Roger Riggs 
On Fri, 19 Jan 2024 18:23:40 GMT, Jim Laskey  wrote:

>> Currently String::translateEscapes does not support unicode escapes, 
>> reported as a IllegalArgumentException("Invalid escape sequence: ..."). 
>> String::translateEscapes should translate unicode escape sequences to 
>> provide full coverage,
>
> Jim Laskey has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update Copyright

test/jdk/java/lang/String/TranslateEscapes.java line 113:

> 111: }
> 112: 
> 113: static void verifyEscape(String string1, String string2) {

These are unicode escapes too.  The method name should reflect that.

test/jdk/java/lang/String/TranslateEscapes.java line 127:

> 125: } catch (IllegalArgumentException ex) {
> 126: }
> 127: }

The method name implies valid unicode escape sequences, but they are all 
invalid.
The method name could be "verifyIllegalUnicodeEscape`.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/17491#discussion_r1459509112
PR Review Comment: https://git.openjdk.org/jdk/pull/17491#discussion_r1459505939

Re: RFR: JDK-8263261 Extend String::translateEscapes to support unicode escapes [v7]

2024-01-19 Thread Jim Laskey 
> Currently String::translateEscapes does not support unicode escapes, reported 
> as a IllegalArgumentException("Invalid escape sequence: ..."). 
> String::translateEscapes should translate unicode escape sequences to provide 
> full coverage,

Jim Laskey has updated the pull request incrementally with one additional 
commit since the last revision:

  Update Copyright

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/17491/files
  - new: https://git.openjdk.org/jdk/pull/17491/files/79312823..2eb5c194

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=17491&range=06
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17491&range=05-06

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/17491.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17491/head:pull/17491

PR: https://git.openjdk.org/jdk/pull/17491