mitsuhiko/templatetk — comment on commit 08e0f8d4d1614657cc3cf6aa4a23ed125ddb0f55
File: templatetk/res/templatetk.runtime.js line 162
By chrisdickinson:

I really urge you to reconsider this. Requiring customization of the library for such a common use case is... bad. Also, as is, the `for(var obj in iterable)` loop will not always behave as expected on all `Array`'s. It'd probably be best to build up a couple of shortcut functions in the preamble, including:

````javascript
var toString = {}.toString // instead of the more verbose "Object.prototype.toString"
var isArray = Array.isArray || function(x) { return /Array/.test(toString.call(x)) }
var hasOwn = {}.hasOwnProperty
````

And then reworking the `sequenceFromIterable` function to adhere more closely to JS standards (using the `for(;;)` syntax for arrays, and using the `for(var x in y) if(y.hasOwnProperty(x))` for objects):

````javascript
sequenceFromIterable : function(iterable) {
var rv = []
if(isArray(iterable)) {
for(var i = 0, len = iterable.length; i < len; ++i)
rv.push(i)
} else for(var i in iterable) if (hasOwn.call(iterable, i)) {
rv.push(i)
}
return rv
}
````

This change would make the sequenceFromIterable work as expected with [sugar](http://sugarjs.com/), [prototype](http://www.prototypejs.org/), and the venerable [es5shim](https://github.com/kriskowal/es5-shim), not to mention any array with annotations (re: the result of `RegExp.prototype.match` from the standard library). In short, there's all sorts of cases where the existing method falls down, and changing it brings it more in line with JS standards.

If you'd like, I could fork and add some failing test cases plus a patch that makes them pass.