Yes, I intended it in the way Claes explained it, and I also assumed this was a bug.
In the case of removeTrailingWhitespace even something like "x " would return unchanged, because the for-loop stops before reaching the first character. I wonder if this case of a single non-whitespace text never occurs, or if just nobody ever noticed it? Hannes > Am 18.12.2019 um 16:18 schrieb Jonathan Gibbons <[email protected]>: > > Claes, > > Thanks for the clarification. It sounds like the old behavior was a bug. > > I note that I did do before/after comparison for the main docs build > and for all the many runs of javadoc performed by the jtreg tests, > and all compared OK. > > -- Jon > > On 12/18/19 7:11 AM, Claes Redestad wrote: >> Sorry for jumping in: I initially misread Hannes' observation in the >> same way, but I think he meant that " ".strip() will return "", while >> removeTrailingWhitespace(" ") will return " ", which is a more >> subtle difference in behavior. >> >> /Claes >> >> On 2019-12-18 15:46, Jonathan Gibbons wrote: >>> Hannes, >>> >>> Thanks for the review. >>> >>> My reading of the code for String.strip.... is that the original string is >>> returned if there is no whitespace to be removed ... in other words, the >>> new methods should have the same behavior as the removed methods. >>> >>> -- Jon >>> >>> On 12/18/19 4:56 AM, Hannes Wallnöfer wrote: >>>> 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/ >>>>>>>>
