Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(443)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 677013002: Make dart2js String.split match the VM version. (Closed)

Created:
6 years, 2 months ago by Lasse Reichstein Nielsen
Modified:
6 years, 1 month ago
Reviewers:
Søren Gjesse, floitsch
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Make dart2js String.split match the VM version. The dart2js version used the JS version of string split, which includes captures between the parts. The Dart version doesn't do that. BUG= http://dartbug.com/21395 R=sgjesse@google.com Committed: https://code.google.com/p/dart/source/detail?r=41313

Patch Set 1 #

Patch Set 2 : Fix type. #

Patch Set 3 : TYpo #

Patch Set 4 : Add tests. Fix code. #

Patch Set 5 : Also add documentation to String.split. #

Total comments: 2

Patch Set 6 : Tweak docs. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+207 lines, -96 lines) Patch
M sdk/lib/_internal/compiler/js_lib/interceptors.dart View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M sdk/lib/_internal/compiler/js_lib/js_string.dart View 1 2 3 1 chunk +30 lines, -2 lines 0 comments Download
M sdk/lib/_internal/compiler/js_lib/regexp_helper.dart View 1 2 3 1 chunk +19 lines, -0 lines 0 comments Download
M sdk/lib/core/string.dart View 1 2 3 4 5 2 chunks +31 lines, -9 lines 0 comments Download
D tests/corelib/string_split_reg_exp_test.dart View 1 2 3 1 chunk +0 lines, -47 lines 0 comments Download
M tests/corelib/string_split_test.dart View 1 2 3 1 chunk +126 lines, -38 lines 0 comments Download

Messages

Total messages: 7 (2 generated)
Lasse Reichstein Nielsen
6 years, 2 months ago (2014-10-24 08:22:15 UTC) #2
Lasse Reichstein Nielsen
6 years, 2 months ago (2014-10-24 10:04:58 UTC) #4
Søren Gjesse
lgtm https://codereview.chromium.org/677013002/diff/80001/sdk/lib/core/string.dart File sdk/lib/core/string.dart (right): https://codereview.chromium.org/677013002/diff/80001/sdk/lib/core/string.dart#newcode488 sdk/lib/core/string.dart:488: * Splitting happends at UTF-16 code unit boundaries, ...
6 years, 1 month ago (2014-10-27 08:35:04 UTC) #5
Lasse Reichstein Nielsen
Committed patchset #6 (id:100001) manually as 41313 (presubmit successful).
6 years, 1 month ago (2014-10-27 12:52:45 UTC) #6
Lasse Reichstein Nielsen
6 years, 1 month ago (2014-10-28 10:15:05 UTC) #7
Message was sent while issue was closed. 
https://codereview.chromium.org/677013002/diff/80001/sdk/lib/core/string.dart
File sdk/lib/core/string.dart (right):

https://codereview.chromium.org/677013002/diff/80001/sdk/lib/core/string.dart...
sdk/lib/core/string.dart:488: * Splitting happends at UTF-16 code unit
boundaries,
I try to avoid writing "NOTE" or "NOTICE" unless something is surprisingly
broken. The fact that it is written at all should be enough.
There is no "d" in "happens", though.

Dart 2.0: Remove the word "rune".

Powered by Google App Engine
This is Rietveld 408576698