This module makes it easier to deal with the arrays/enumerators that c++ xpcom methods are apt to return. This should make it easier for extensions to work with our code in a comfortable way. Instead of getting folders the current way: var iter = folder.GetSubFolders(); while (true) { try { doStuff(iter.currentItem().QueryInterface(Ci.nsIFoo)); iter.next(); } catch (ex) { break; } } we can do var iter = folder.GetSubFolders(); for each (var obj in fixIterator(iter, Ci.nsIFoo)) { doStuff(obj); } Similarly for nsISupportsArray... var count = array.Count() for (var i = 0; i < count; i++) { var object = array.GetElementAt(i).QueryInterface(Ci.nsIFoo); ... } we can just do for each (var object in fixIterator(array, Ci.nsIFoo) { ... }

Comment on attachment 304304 [details] [diff] [review] iteratorUtils v1 Note that there are a lot of other places in the code that can use these utils, this is just a few examples...

Libraries, which are not TB-specific (but MailNews-specific) should reside in /mailnews, not /mail. (So SM etc. do have a chance to use it.) It doesn't make much sense to fork stuff which is not defining UI directly.

Comment on attachment 304304 [details] [diff] [review] iteratorUtils v1 This looks like generally useful code. Any chance to get this module into a core component like XPCOMUtils.jsm, JSON.jsm, etc.? And some drive-by comments: >+EXPORTED_SYMBOLS = ["fixIterator", "toXPCOMIterator"]; That should be |var EXPORTED_SYMBOLS = ...| so as to not cause a strict warning (see bug 422161). >+ // Try to QI our object to each of the known iterator types. If the QI does >+ // not throw, assign our iteration function >+ try { >+ aEnum.QueryInterface(Ci.nsISupportsArray); Is there anything wrong with using instanceof instead? Like so: >+ if (aEnum instanceof Ci.nsISupportsArray) { >+ } catch(ex) {} Not sure what your style guide recommends, but for clarity's sake it might be better for catch blocks not to be both empty _and_ uncommented (so that when the catch block's content is refactored by somebody else, it's obvious whether the try-catch remains necessary).

(In reply to comment #3) > (From update of attachment 304304 [details] [diff] [review]) > This looks like generally useful code. Any chance to get this module into a > core component like XPCOMUtils.jsm, JSON.jsm, etc.? I talked with mconnor about this and he suggested it may be too late in the FF cycle to move this into something more core. > > That should be |var EXPORTED_SYMBOLS = ...| so as to not cause a strict warning > (see bug 422161). Thanks. I saw that bug land after I had made this patch. > > Is there anything wrong with using instanceof instead? Like so: Nope. Just a bad habit of mine. > > >+ } catch(ex) {} > > Not sure what your style guide recommends, but for clarity's sake it might be > better for catch blocks not to be both empty _and_ uncommented And with instanceof, we can drop it entirely!

See also bug 380839, which deals with some of the same things (and some different things).

Comment on attachment 304304 [details] [diff] [review] iteratorUtils v1 Patch has now bitrotted. :-(

Patch updated to fix bitrot and to include the instanceof and strict-warning fixes mentioned in the comments.

No substantive changes, just updated for 4 months of bit-rot.

Comment on attachment 337051 [details] [diff] [review] patch v2.1 this is working fine in the kill-rdf repo. I wonder if SM might want this? If there's demand, we could move it to mailnews/base/utils, perhaps.

do these files need to get added to the packages, like js components?

> I wonder if SM might want this? Yes, see comment #2. :) > we could move it to mailnews/base/utils Sounds reasonable.

Standard8 thinks js modules automatically get picked up because there's a dist/bin/modules/* that picks up all the js modules. I'll tweak things to move this to base/utils before landing.

Three points: 1. What about nsIArray? nsISupportsArray is deprecated. 2. aEnum.GetElementAt(i).QueryInterface(face) should be aEnum.QueryElementAt(i, face) 3. Document that toXPCOMIterator won't work if you extend Array.prototype!

thx, Neil, I'll fix 2, document 3, and file a followup bug on 1

this patch addresses two of Neil's issues - I'll file a follow on bug for the nsIArray issue.

Bug 462083 filed for follow up issue.

I backed out the changes to mailCommands.js due to regressions - see bug 462121, the reply button was broken, and empty junk (and probably empty trash) fails. http://hg.mozilla.org/comm-central/rev/a0c1355972f5 I think Search is working ok, but please test that again as I saw javascript errors on the console: JavaScript error: , line 0: uncaught exception: 2147500034

this is also causing an issue when editing a saved search's properties. Follow-on patch upcoming.

Comment on attachment 345340 [details] [diff] [review] command glue needs to include iteratorUtils.jsm, it seems [Checkin: Comment 25] Yeah, kill-rdf has other people import() it, but it's necessary here, at least for now.

Comment on attachment 345340 [details] [diff] [review] command glue needs to include iteratorUtils.jsm, it seems [Checkin: Comment 25] I see the issue this fixes in the kill-rdf repo as well, so we need to be careful about reverting this...

This has landed, marking fixed.

Is this something worth proposing for toolkit/m-c ?

(In reply to comment #23) > Is this something worth proposing for toolkit/m-c ? Its not entirely obvious, but comment 5 references bug 380839 which is the toolkit bug for getting these in core.

Comment on attachment 345340 [details] [diff] [review] command glue needs to include iteratorUtils.jsm, it seems [Checkin: Comment 25] http://hg.mozilla.org/comm-central/rev/998037e32091

Comment on attachment 345210 [details] [diff] [review] patch that I'll land [Checkin+Backout: Comment 26+17] http://hg.mozilla.org/comm-central/rev/a93d4bef18d2 http://hg.mozilla.org/comm-central/rev/bb04b97bd5a0 http://hg.mozilla.org/comm-central/rev/cb1cabceabff

Comment on attachment 337051 [details] [diff] [review] patch v2.1 >+ return { __iterator__: iter }; Since iter uses yield, you can just return iter(); directly.