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/

Reply via email to