Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#8182 closed defect (fixed)

Support native String.trim

Reported by: Adam Peller Owned by: Eugene Lazutkin
Priority: high Milestone: 1.3
Component: String Version: 1.2.1
Keywords: Cc: James Burke
Blocked By: Blocking:

Description

ECMAScript 3.1 will support String.trim, and FF 3.1 is advertising support for this. Other browsers will follow.

Attachments (1)

trim.patch (1.0 KB) - added by Adam Peller 10 years ago.
str.trim is shorter than "trim" in str, but either seems reasonable

Download all attachments as: .zip

Change History (13)

Changed 10 years ago by Adam Peller

Attachment: trim.patch added

str.trim is shorter than "trim" in str, but either seems reasonable

comment:1 Changed 10 years ago by Adam Peller

Component: Storage/FlashString

comment:2 Changed 10 years ago by dante

Cc: James Burke added
Milestone: 1.31.4

are we sure prototype doesn't add a .trim() method to String.prototype ? For some reason, this doesn't feel right ... are there any subtle differences in FF's .trim() impl and ours?

comment:3 Changed 10 years ago by Adam Peller

I suggest we get it in SRTL. This one's a lot simpler than query. The only possible discrepancy is whether we use the same set of chars -- whitespace should match what RegExp? uses, iirc; not sure about LineSeparator?.

From the ES 3.1 08-11-2008 draft:

15.5.4.20 String.prototype.trim ( ) If this object is not already a string, it is converted to a string. The result is a copy of the string with both leading and trailing white space removed. The definition of white space is the union of WhiteSpace? and LineTerminator?. The result is a string value, not a String object. NOTE The trim function is intentionally generic; it does not require that its this value be a String object. Therefore, it can be transferred to other kinds of objects for use as a method.

I don't see a trim() method in prototype-js (kinda surprising... they must have one?) but I do see one in MS Ajax.net, and sure, other toolkits could define one. So what? I'm sure there are plenty of other places where buggy overrides could mess us up, but this one is important enough to be in a new standard, and the behavior is pretty straight-forward.

comment:4 Changed 10 years ago by dante

Owner: changed from dante to Adam Peller

+1

comment:5 Changed 10 years ago by Adam Peller

Resolution: fixed
Status: newclosed

(In [16231]) Use native String.trim() where available. Fixes #8182

comment:6 Changed 10 years ago by Adam Peller

Milestone: 1.41.3

comment:7 Changed 10 years ago by Eugene Lazutkin

Resolution: fixed
Status: closedreopened

Why not implement it like that (pseudo code):

dojo.trim = String.prototype.trim || function(s){ /* our trim */ };

This way we don't check if the standard trim is available on every single call.

comment:8 Changed 10 years ago by Adam Peller

(In [16238]) Update docs to indicate native String.trim() is used. Refs #8182

comment:9 in reply to:  7 Changed 10 years ago by Adam Peller

Owner: changed from Adam Peller to Eugene Lazutkin
Status: reopenednew

Replying to elazutkin:

Why not implement it like that (pseudo code):

dojo.trim = String.prototype.trim || function(s){ /* our trim */ };

This way we don't check if the standard trim is available on every single call.

Good suggestion -- I should have done something like this -- but it seems at a minimum, you need a call (though I'm not sure why offhand) at which point, we start to add some bloat. I worry a bit less about bloating dojo/string.js than base, though I'm starting to think with native methods dojo.string.trim is just bloat itself.

comment:10 Changed 10 years ago by Eugene Lazutkin

Resolution: fixed
Status: newclosed

(In [16239]) Restructuring dojo.trim() and dojo.string.trim() to make a choice (native vs. manual) only once during the loading time. !strict. Fixes #8182.

comment:11 Changed 10 years ago by Eugene Lazutkin

(In [16240]) Reducing the code by several bytes and improving the load time a little, !strict, refs #8182.

comment:12 Changed 10 years ago by Adam Peller

(In [16241]) Remove unneeded call to str.trim(). Refs #8182

Note: See TracTickets for help on using tickets.