Ticket #8182 (closed defect: fixed)

Opened 7 months ago

Last modified 6 months ago

Support native String.trim

Reported by: peller Owned by: elazutkin
Priority: normal Milestone: 1.3
Component: String Version: 1.2.1
Severity: normal Keywords:
Cc: jburke

Description

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

Attachments

trim.patch (1.0 kB) - added by peller 7 months ago.
str.trim is shorter than "trim" in str, but either seems reasonable

Change History

Changed 7 months ago by peller

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

  Changed 7 months ago by peller

  • component changed from Storage/Flash to String

  Changed 7 months ago by dante

  • cc jburke added
  • milestone changed from 1.3 to 1.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?

  Changed 7 months ago by 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.

  Changed 7 months ago by dante

  • owner changed from dante to peller

+1

  Changed 6 months ago by peller

  • status changed from new to closed
  • resolution set to fixed

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

  Changed 6 months ago by peller

  • milestone changed from 1.4 to 1.3

follow-up: ↓ 9   Changed 6 months ago by elazutkin

  • status changed from closed to reopened
  • resolution deleted

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.

  Changed 6 months ago by peller

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

in reply to: ↑ 7   Changed 6 months ago by peller

  • status changed from reopened to new
  • owner changed from peller to elazutkin

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.

  Changed 6 months ago by elazutkin

  • status changed from new to closed
  • resolution set to fixed

(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.

  Changed 6 months ago by elazutkin

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

  Changed 6 months ago by peller

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

Note: See TracTickets for help on using tickets.