Re: RFR: JDK-8263261 Extend String::translateEscapes to support unicode escapes [v12]
On Fri, 26 Jan 2024 16:54:14 GMT, Roger Riggs wrote: >> Jim Laskey has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 12 additional >> commits since the last revision: >> >> - Merge remote-tracking branch 'upstream/master' into 8263261 >> - Update unicode to Unicode >> - Requested changes >> - Update String.java >> - Requested changes >> - Update Copyright >> - Update copyright year of test >> - Add JLS Unicode Escapes reference >> - Update comment >> - Update copyright year >> - ... and 2 more: https://git.openjdk.org/jdk/compare/af9bfd62...040bda82 > > src/java.base/share/classes/java/lang/String.java line 4229: > >> 4227: * {@code \u005Cu...u} >> 4228: * Unicode escape >> 4229: * single UTF-16 code unit equivalent > > The `...` makes it less clear what is being shown. It might be clearer to > include the in the resulting value and drop the multiple `u` case. Changed > src/java.base/share/classes/java/lang/String.java line 4245: > >> 4243: * escape sequences and Unicode escapes are translated as >> encountered in one pass and >> 4244: * not done as an Unicode escapes pass followed >> by an escape sequences >> 4245: * pass. > > I would move the description of the compiler behavior to the end and remove > "also". For example, > Suggestion: > > * @implNote As a convenience for use with constructed > * strings, this method translates Unicode escapes. For example, this > * method could be used when ASCII encoded text files need to maintain > Unicode > * content. The translation is done in a single pass and is > non-recursive. That is, > * escape sequences and Unicode escapes are translated as encountered in > one pass and > * not done as an Unicode escapes pass followed by an > escape sequences > * pass. By comparison, the compiler translates all Unicode escapes > before string > * literals are translated. Changed > test/jdk/java/lang/String/TranslateEscapes.java line 97: > >> 95: verifyUnicodeEscape("\\u2022", "\u2022"); >> 96: verifyUnicodeEscape("\\ud83c\\udf09", "\ud83c\udf09"); >> 97: verifyUnicodeEscape("\\u2022", "\u2022"); > > Include the code from the example as a test case too. None present. Was a mis-paste. - PR Review Comment: https://git.openjdk.org/jdk/pull/17491#discussion_r1467926349 PR Review Comment: https://git.openjdk.org/jdk/pull/17491#discussion_r1467926483 PR Review Comment: https://git.openjdk.org/jdk/pull/17491#discussion_r1467929023
Re: RFR: JDK-8263261 Extend String::translateEscapes to support unicode escapes [v12]
On Fri, 26 Jan 2024 15:06:50 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 with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains 12 additional commits since > the last revision: > > - Merge remote-tracking branch 'upstream/master' into 8263261 > - Update unicode to Unicode > - Requested changes > - Update String.java > - Requested changes > - Update Copyright > - Update copyright year of test > - Add JLS Unicode Escapes reference > - Update comment > - Update copyright year > - ... and 2 more: https://git.openjdk.org/jdk/compare/b94b04ff...040bda82 src/java.base/share/classes/java/lang/String.java line 4229: > 4227: * {@code \u005Cu...u} > 4228: * Unicode escape > 4229: * single UTF-16 code unit equivalent The `...` makes it less clear what is being shown. It might be clearer to include the in the resulting value and drop the multiple `u` case. src/java.base/share/classes/java/lang/String.java line 4245: > 4243: * escape sequences and Unicode escapes are translated as > encountered in one pass and > 4244: * not done as an Unicode escapes pass followed by > an escape sequences > 4245: * pass. I would move the description of the compiler behavior to the end and remove "also". For example, Suggestion: * @implNote As a convenience for use with constructed * strings, this method translates Unicode escapes. For example, this * method could be used when ASCII encoded text files need to maintain Unicode * content. The translation is done in a single pass and is non-recursive. That is, * escape sequences and Unicode escapes are translated as encountered in one pass and * not done as an Unicode escapes pass followed by an escape sequences * pass. By comparison, the compiler translates all Unicode escapes before string * literals are translated. test/jdk/java/lang/String/TranslateEscapes.java line 97: > 95: verifyUnicodeEscape("\\u2022", "\u2022"); > 96: verifyUnicodeEscape("\\ud83c\\udf09", "\ud83c\udf09"); > 97: verifyUnicodeEscape("\\u2022", "\u2022"); Include the code from the example as a test case too. - PR Review Comment: https://git.openjdk.org/jdk/pull/17491#discussion_r1467892757 PR Review Comment: https://git.openjdk.org/jdk/pull/17491#discussion_r1467895901 PR Review Comment: https://git.openjdk.org/jdk/pull/17491#discussion_r1467900516
Re: RFR: JDK-8263261 Extend String::translateEscapes to support unicode escapes [v12]
> 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 with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 12 additional commits since the last revision: - Merge remote-tracking branch 'upstream/master' into 8263261 - Update unicode to Unicode - Requested changes - Update String.java - Requested changes - Update Copyright - Update copyright year of test - Add JLS Unicode Escapes reference - Update comment - Update copyright year - ... and 2 more: https://git.openjdk.org/jdk/compare/eea3c2d9...040bda82 - Changes: - all: https://git.openjdk.org/jdk/pull/17491/files - new: https://git.openjdk.org/jdk/pull/17491/files/b57a551d..040bda82 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17491&range=11 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17491&range=10-11 Stats: 8261 lines in 340 files changed: 5165 ins; 1908 del; 1188 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