Add String.repeat, String.padLeft, String.padRight.

Add String.repeat, String.padLeft, String.padRight.

We keep seeing reimplementations everywhere, and reimplementing it ourselves.

BUG= http://dartbug.com/16919
R=sgjesse@google.com

Committed: https://code.google.com/p/dart/source/detail?r=32784

[floitsch, 2014-02-19 10:20:36]

FYI.

https://codereview.chromium.org/172153002/diff/1/runtime/lib/string_patch.dart
File runtime/lib/string_patch.dart (right):

https://codereview.chromium.org/172153002/diff/1/runtime/lib/string_patch.dar...
runtime/lib/string_patch.dart:333: for (int i = 0; i < delta; i++) {
In theory one could do the same trick as for efficient exponentiation: double
the padding at each iteration and only look at the bits.

Not sure it warrants a TODO.

https://codereview.chromium.org/172153002/diff/1/sdk/lib/_internal/lib/js_str...
File sdk/lib/_internal/lib/js_string.dart (right):

https://codereview.chromium.org/172153002/diff/1/sdk/lib/_internal/lib/js_str...
sdk/lib/_internal/lib/js_string.dart:186: // Start by doing JS trim. Then check
if it leaves a
Unfinished sentence.

https://codereview.chromium.org/172153002/diff/1/sdk/lib/core/string.dart
File sdk/lib/core/string.dart (right):

https://codereview.chromium.org/172153002/diff/1/sdk/lib/core/string.dart#new...
sdk/lib/core/string.dart:331: * If [separator] is provided, it is insert between
the copies
it is inserted

https://codereview.chromium.org/172153002/diff/1/sdk/lib/core/string.dart#new...
sdk/lib/core/string.dart:336: String repeat(int times, [String separator = ""]);
Do we really need this one?

The only good use-case I can think of (that isn't handled by padLeft/padRight)
would be several new lines...

https://codereview.chromium.org/172153002/diff/1/sdk/lib/core/string.dart#new...
sdk/lib/core/string.dart:367: String padLeft(int newSize, String padding, { int
size });
I'm not convinced that dealing with surrogate pairs is necessary. In Utf-16 they
are very rare, and once you deal with non-standard unicode characters you
probably don't deal with strings of specific sizes anymore. (Why would you want
to pad with three spaces, if the runes in your string merge together...)

Most of the time padding will be " ".

I would have had the following signature:
String padLeft(int newSize, [String padding = " "])

I would also require the padding to be of size 1.

All other uses can easily written by hand.

[Søren Gjesse, 2014-02-19 10:44:10]

LGTM, but I think we should hold back repeat.

https://codereview.chromium.org/172153002/diff/1/sdk/lib/_internal/lib/js_str...
File sdk/lib/_internal/lib/js_string.dart (right):

https://codereview.chromium.org/172153002/diff/1/sdk/lib/_internal/lib/js_str...
sdk/lib/_internal/lib/js_string.dart:186: // Start by doing JS trim. Then check
if it leaves a
Leaves a...?

https://codereview.chromium.org/172153002/diff/1/sdk/lib/_internal/lib/js_str...
sdk/lib/_internal/lib/js_string.dart:187: Strint result = JS("String",
"#.trim()", this);
What type is Strint?

https://codereview.chromium.org/172153002/diff/1/sdk/lib/core/string.dart
File sdk/lib/core/string.dart (right):

https://codereview.chromium.org/172153002/diff/1/sdk/lib/core/string.dart#new...
sdk/lib/core/string.dart:367: String padLeft(int newSize, String padding, { int
size });
I find the option of padding with size larger than one useful. Regarding the
names size argument actually using this requires some way of getting the actual
"rendering length", which we don't really have except for .runes.length - which
does not handle combining characters.

https://codereview.chromium.org/172153002/diff/1/tests/corelib/string_test.dart
File tests/corelib/string_test.dart (right):

https://codereview.chromium.org/172153002/diff/1/tests/corelib/string_test.da...
tests/corelib/string_test.dart:465: Expect.equals('aaaaa', ''.padLeft(5, 'a'));
Lines 463 and 465 are the same tests - did you intend something else?

https://codereview.chromium.org/172153002/diff/1/tests/corelib/string_test.da...
tests/corelib/string_test.dart:492: Expect.equals('aaaaa', ''.padRight(5, 'a'));
Lines 490 and 492 are the same tests - did you intend something else?

[Lasse Reichstein Nielsen, 2014-02-19 11:43:16]

https://codereview.chromium.org/172153002/diff/1/runtime/lib/string_patch.dart
File runtime/lib/string_patch.dart (right):

https://codereview.chromium.org/172153002/diff/1/runtime/lib/string_patch.dar...
runtime/lib/string_patch.dart:333: for (int i = 0; i < delta; i++) {
It is cute, though :)

https://codereview.chromium.org/172153002/diff/1/sdk/lib/_internal/lib/js_str...
File sdk/lib/_internal/lib/js_string.dart (right):

https://codereview.chromium.org/172153002/diff/1/sdk/lib/_internal/lib/js_str...
sdk/lib/_internal/lib/js_string.dart:186: // Start by doing JS trim. Then check
if it leaves a
On 2014/02/19 10:20:36, floitsch wrote:
> Unfinished sentence.

Done.

https://codereview.chromium.org/172153002/diff/1/sdk/lib/_internal/lib/js_str...
sdk/lib/_internal/lib/js_string.dart:187: Strint result = JS("String",
"#.trim()", this);
On 2014/02/19 10:44:10, Søren Gjesse wrote:
> What type is Strint?

Done.

https://codereview.chromium.org/172153002/diff/1/sdk/lib/core/string.dart
File sdk/lib/core/string.dart (right):

https://codereview.chromium.org/172153002/diff/1/sdk/lib/core/string.dart#new...
sdk/lib/core/string.dart:331: * If [separator] is provided, it is insert between
the copies
On 2014/02/19 10:20:36, floitsch wrote:
> it is inserted

Done.

https://codereview.chromium.org/172153002/diff/1/sdk/lib/core/string.dart#new...
sdk/lib/core/string.dart:336: String repeat(int times, [String separator = ""]);
If we have padLeft, you can do str.repeat(n) by doing "".padLeft(n, str). I'd
rather have a dedicated repeat than have to watch that hack.
I do believe repeating a string is useful in general.

https://codereview.chromium.org/172153002/diff/1/sdk/lib/core/string.dart#new...
sdk/lib/core/string.dart:367: String padLeft(int newSize, String padding, { int
size });
This is not (just) dealing with surrogate pairs. In fact, it doesn't deal with
surrogates pairs at all. They are used as examples in the same way as the HTML
entities.
What it does do is leaving an opening for the user to deal with strings that
have a logical size different from their physical length.

The padding should generally have a logical size of 1, but that doesn't mean it
must be a string of length 1. The "&nbsp;" example isn't mine, it's from the
feature request.

From looking at our own code, padding is usually '0' or ' ', probably with '0'
being slightly more common.

https://codereview.chromium.org/172153002/diff/1/tests/corelib/string_test.dart
File tests/corelib/string_test.dart (right):

https://codereview.chromium.org/172153002/diff/1/tests/corelib/string_test.da...
tests/corelib/string_test.dart:465: Expect.equals('aaaaa', ''.padLeft(5, 'a'));
No, I just ended up with the same test twice, when I was thinking up cases.

https://codereview.chromium.org/172153002/diff/1/tests/corelib/string_test.da...
tests/corelib/string_test.dart:492: Expect.equals('aaaaa', ''.padRight(5, 'a'));
On 2014/02/19 10:44:10, Søren Gjesse wrote:
> Lines 490 and 492 are the same tests - did you intend something else?

Done.

[Lasse Reichstein Nielsen, 2014-02-19 12:33:05]

Committed patchset #3 manually as r32784 (presubmit successful).