Looks good. I noticed that the removed methods would return the original string if they didn’t encounter a non-whitespace character, but I guess that wasn’t actually part of their intended behaviour.
Hannes > Am 16.12.2019 um 23:47 schrieb Jonathan Gibbons <[email protected]>: > > Updated webrev based on Ivan's suggestion to use one of stripLeading(), > stripTrailing(), or strip() instead of the old methods, which are now removed > > -- Jon > > http://cr.openjdk.java.net/~jjg/8236030/webrev.01/ > > > > On 12/16/2019 01:33 PM, Jonathan Gibbons wrote: >> Ivan, >> >> Great suggestion, and sets up the possibility to use strip() when both >> leading and trailing whitespace should be removed. >> >> The only slight change in semantics would be that these methods work in >> terms of code-points, not characters, and javadoc has not (yet?) been >> adapted to use code-points throughout. >> >> -- Jon >> >> >> On 12/16/2019 01:23 PM, Ivan Gerasimov wrote: >>> Hello! >>> >>> Can String.stripLeading()/stripTrailing() methods be used instead, or is >>> there a reason to avoid them? >>> >>> With kind regards, >>> >>> Ivan >>> >>> >>> On 12/16/19 1:10 PM, Jonathan Gibbons wrote: >>>> Please review a tiny change to eliminate a string copy into a temporary >>>> character buffer when removing leading or trailing whitespace from a >>>> string. >>>> >>>> The affected methods are called within the main code to translate doc >>>> comments to HTML. This is noreg-cleanup/no-reg-trivial, so no new >>>> additional tests. >>>> >>>> Webrev below, but the patch is also included here: >>>> >>>> --- >>>> a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java >>>> Mon Dec 16 15:33:03 2019 -0500 >>>> +++ >>>> b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java >>>> Mon Dec 16 13:07:18 2019 -0800 >>>> @@ -1604,22 +1604,19 @@ >>>> } >>>> >>>> private String removeTrailingWhitespace(String text) { >>>> - char[] buf = text.toCharArray(); >>>> - for (int i = buf.length - 1; i > 0 ; i--) { >>>> - if (!Character.isWhitespace(buf[i])) >>>> - return text.substring(0, i + 1); >>>> + int i = text.length() - 1; >>>> + while (i >= 0 && Character.isWhitespace(text.charAt(i))) { >>>> + i--; >>>> } >>>> - return text; >>>> + return i == text.length() - 1 ? text : text.substring(0, i + 1); >>>> } >>>> >>>> private String removeLeadingWhitespace(String text) { >>>> - char[] buf = text.toCharArray(); >>>> - for (int i = 0; i < buf.length; i++) { >>>> - if (!Character.isWhitespace(buf[i])) { >>>> - return text.substring(i); >>>> - } >>>> + int i = 0; >>>> + while (i < text.length() && >>>> Character.isWhitespace(text.charAt(i))) { >>>> + i++; >>>> } >>>> - return text; >>>> + return i == 0 ? text : text.substring(i); >>>> } >>>> >>>> /** >>>> >>>> >>>> -- Jon >>>> >>>> >>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8236030 >>>> Webrev: http://cr.openjdk.java.net/~jjg/8236030/webrev/ >>>> >> >
