RFR: [15] JDK-8236030: Cleanup use of String.toCharArray

[Jonathan Gibbons]

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 
>>>> <jonathan.gibbons at oracle.com>:
>>>>
>>>> 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/
>>>>>>>