Hi Jim,
Overall it looks fine. Some quibbles on the wording and the test.
The spec for the align() and align(n) methods in String.java show a
possibly misleading inconsistency.
The first line says:
Removes vertical and horizontal white space margins.
But later align() says:
Trailing spaces are preserved.
The former implies all 4 margins but then it seems only to apply to 3 of
the 4. (top, left, bottom).
I'm not sure if there some wording that can clear that up in the first line.
AlignIndent.java:
- Line 28: this test should not need /othervm nor the explicit @compile
- There should be tests of align(n) with negative values.
- The for for for for structure doesn't follow the style guide.
Thanks, Roger
(Sorry for the duplicates, I missed core-libs the first time)
On 8/29/18 10:00 AM, Jim Laskey wrote:
Please review the code for String::align and String::indent at the link below.
Notes:
Includes a private version of String::isMultiline() which may be made
into a public method at some future date
Includes minor correctness clean up of StringLatin1.java,
StringUTF16.java
webrev: http://cr.openjdk.java.net/~jlaskey/8200434/webrev/index.html
jbs: https://bugs.openjdk.java.net/browse/JDK-8200434
Cheers,
— Jim
