RFR - JDK-8200434 - String::align, String::indent (code review)

[Jim Laskey]

The special case for non-multiline strings aside, look at the following example;
	String s = "abc\ndef\nghi\n";
	System.out.println(s.lines().collect(Collectors.joining("\n", "", "\n")).lines().count());
	System.out.println(s.lines().collect(Collectors.joining("\n")).lines().count());
As expected the result is 

3
3

Now try 

	String s = "abc\ndef\n\n";
	System.out.println(s.lines().collect(Collectors.joining("\n", "", "\n")).lines().count());
	System.out.println(s.lines().collect(Collectors.joining("\n")).lines().count());

The result is now

3
2

We started with 3 lines but ended up with 2 lines. This is because String::split(“\\R”), BufferedReader​::lines() and String::lines() have the definition of line-ness as
* A <i>line</i> is either a sequence of zero or more characters
* followed by a line terminator, or it is a sequence of one or
* more characters followed by the end of the string. A
* line does not include the line terminator.
So if we didn’t add the final line terminator, then we would drop a line every time we had an empty line at the end of the string.

Adding to the cases above

	String s = "abc\ndef\n   \n".indent(-3);
	System.out.println(s.lines().collect(Collectors.joining("\n", "", "\n")).lines().count());
	System.out.println(s.lines().collect(Collectors.joining("\n")).lines().count());
Would also result in

3
2

Thus, it is a line terminator and not a line separator. As a bonus. the use cases we see with String::indent(int n) are consistent with having a final line terminator.

	String composite = head.indent(4) + body.indent(8) + tail.indent(4);
vs
	String composite = head.indent(4) + "\n" + body.indent(8) + "\n" + tail.indent(4) + "\n";

Your point about consistency with non-multiline is valid, and we will drop the special case since it is inconsistent with the spec. The correction will treat non-multiline strings as single line.  (Adding indentation to simple strings can easily be done with " ".repeat(n) and removing indentation with s.substring(n).)

Note:

	"a single line".lines().count() == 1
	"".lines().count() == 0

Cheers,

— Jim


> On Aug 29, 2018, at 6:48 PM, Ivan Gerasimov <ivan.gerasimov at oracle.com> wrote:
> 
> Hi Jim!
> 
> Why a new line is always added to the result of indent(int):
> 
> return stream.collect(Collectors.joining("\n", "", "\n"));
> Because of this, a call to str.indent(0) depends on whether the string is single-line or multi-line: In the former case it is unchanged, while in the later case it gets a newline appended.
> Moreover, if a multi-line string already ends with a new line, then a new new-line is not appended, which seems inconsistent.
> 
> I think, it may be better to always preserve the number of existing trailing new lines.
> 
> With kind regards,
> Ivan
> 
> 
> On 8/29/18 7: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 <http://cr.openjdk.java.net/~jlaskey/8200434/webrev/index.html>
>> jbs: https://bugs.openjdk.java.net/browse/JDK-8200434 <https://bugs.openjdk.java.net/browse/JDK-8200434>
>> 
>> Cheers,
>> 
>> — Jim
>> 
>> 
> 
> -- 
> With kind regards,
> Ivan Gerasimov