On Wed, 17 May 2023 09:19:10 GMT, Raffaello Giulietti <rgiulie...@openjdk.org>
wrote:
>> When appropriate and useful, copies only the relevant portion of the
>> `CharSequence` to the match result.
>
> Raffaello Giulietti 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 five additional
> commits since the last revision:
>
> - Adjusted incorrect logic in results().
> Added tests for results().
> Refined capturing logic when there's no match.
> - Merge branch 'master' into JDK-8132995
> - Adjusted erroneous logic in results().
> Added tests for results().
> - Added some randomness in tests.
> - 8132995: Matcher should be optimized to reduce space usage
LGTM.
Consider simplifying `text instanceof String s ? s.substring(first, last) :
text.subSequence(first, last).toString()` to just `text.subSequence(first,
last).toString()`
src/java.base/share/classes/java/util/regex/Matcher.java line 278:
> 276: public MatchResult toMatchResult() {
> 277: String capturedText = hasMatch()
> 278: ? text instanceof String s
This `instanceof` became a little gratuitous now since `text.subSequence(..)`
on `String` delegates to `substring` and the `toString()` will be a no-op.
-------------
Marked as reviewed by redestad (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/13231#pullrequestreview-1430876173
PR Review Comment: https://git.openjdk.org/jdk/pull/13231#discussion_r1196633626
