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

Issue 266050: Add trim, trimLeft and trimRight methods to String (Closed)

Created:
11 years, 2 months ago by Jan de Mooij
Modified:
9 years, 7 months ago
CC:
v8-dev
Visibility:
Public.

Description

Based on a recent patch for Webkit. trim is defined in ES 5 section 15.5.4.20. BUG=465

Patch Set 1 #

Patch Set 2 : '' #

Total comments: 1

Patch Set 3 : '' #

Total comments: 2

Patch Set 4 : '' #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+151 lines, -0 lines) Patch
M src/runtime.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M src/runtime.cc View 1 2 3 1 chunk +28 lines, -0 lines 0 comments Download
M src/string.js View 1 2 2 chunks +15 lines, -0 lines 0 comments Download
A test/mjsunit/third_party/string-trim.js View 1 2 3 1 chunk +107 lines, -0 lines 2 comments Download

Messages

Total messages: 9 (0 generated)
Jan de Mooij
11 years, 2 months ago (2009-10-12 14:19:23 UTC) #1
Christian Plesner Hansen
I would suggest using two bools instead of encoding the left/right values into a bool. ...
11 years, 2 months ago (2009-10-12 15:14:29 UTC) #2
Christian Plesner Hansen
> [...] instead of encoding the left/right values into a bool. That should have been ...
11 years, 2 months ago (2009-10-12 15:15:09 UTC) #3
Jan de Mooij
On 2009/10/12 15:15:09, Christian Plesner Hansen wrote: > > [...] instead of encoding the left/right ...
11 years, 2 months ago (2009-10-12 15:34:11 UTC) #4
Christian Plesner Hansen
Lgtm. I'll land this tomorrow.
11 years, 2 months ago (2009-10-12 17:57:43 UTC) #5
Erik Corry
LGTM - thanks for the patch. http://codereview.chromium.org/266050/diff/5002/6002 File src/runtime.cc (right): http://codereview.chromium.org/266050/diff/5002/6002#newcode3610 Line 3610: while (right ...
11 years, 2 months ago (2009-10-12 18:51:56 UTC) #6
Jan de Mooij
Thanks for your comments. I am not sure though what you mean by "two spaces ...
11 years, 2 months ago (2009-10-12 21:04:48 UTC) #7
Christian Plesner Hansen
Landed in http://code.google.com/p/v8/source/detail?r=3052 (I fixed Erik's last comments on landing).
11 years, 2 months ago (2009-10-13 08:15:50 UTC) #8
Erik Corry
11 years, 2 months ago (2009-10-13 08:27:43 UTC) #9
http://codereview.chromium.org/266050/diff/8001/7002
File test/mjsunit/third_party/string-trim.js (right):

http://codereview.chromium.org/266050/diff/8001/7002#newcode81
Line 81: rightTrimString = wsString   + testString; // Trimmed from the right.
You 'need' an extra space immediately preceding the double slash (//) here.

http://codereview.chromium.org/266050/diff/8001/7002#newcode99
Line 99: var testValues = [0, Infinity, NaN, true, false, ({}), ['an','array']
Is there a missing comma at the end of this line?  I can't quite parse this, but
perhaps the machine can :-).

Powered by Google App Engine
This is Rietveld 408576698