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/
