729069 - Move comm-central's iteratorUtils.jsm module to the toolkit

Status: RESOLVED WONTFIX

(Toolkit :: General, enhancement)

Description

[Florian Quèze [:florian]]

Attachment: Patch

comm-central has a quite useful JS module to deal with arrays and enumerators. I would like to use it for instant messaging code shared between Thunderbird (bug 714733) and Instantbird, but as this module is in comm-central instead of mozilla-central, I would have to fork it in Instantbird's code repository in order to do this, which would be quite sad.

Comment 1

[Florian Quèze [:florian]]

Attachment: Same with tests

I forgot to also move the tests in the previous patch...

Comment 2

[Dave Townsend [:mossop]]

A few comments that could do with being discussed before starting reviews:

I'm trying to justify the existence of toArray to myself. Why would you ever use it instead of just directly using [a for (a of foo)]? I'd expect the caller to know what it is dealing with and save us the hassle of flakey duck-typing.

fixIterator should be called toJSIterator (as we may well want a toXPCOMIterator at some point).

Otherwise this stuff looks sensible, I think we'd want to put it in an IteratorUtils object to match the normal jsms we have in tree (and name the file the same).

Comment 3

[Dave Townsend [:mossop]]

Comment on attachment 599094 [details] [diff] [review]
Same with tests

Review of attachment 599094 [details] [diff] [review]:
-----------------------------------------------------------------

I'd like to figure out particularly the first part of my last comment before reviewing this.

Comment 4

[Mark Banner (:standard8)]

nsISupportsArray/nsIEnumerator are dead. nsISimpleEnumerator now has js iterator support (bug 1484496) - from what I can tell it is also available to nsIArray.

So I think that makes this obsolete now. Florian, what do you think?

Comment 5

[Florian Quèze [:florian]]

(In reply to Mark Banner (:standard8) from comment #4)

So I think that makes this obsolete now. Florian, what do you think?

Agreed!