Closed Bug 1221638 Opened 9 years ago Closed 9 years ago

Remove use of for-each from im/.

Categories

(Instantbird Graveyard :: Other, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Instantbird 45

People

(Reporter: arai, Assigned: arai)

References

Details

Attachments

(2 files, 1 obsolete file)

Need to replace non-standard for-each with one of: * for-of * for-in * array.map/array.filter for array-comprehension as a part of bug 1083470. converting rules are following: * for-each * for each (let x in array) { ... } -> for (let x of array) { ... } * for each (let x in object) { ... } -> for (let key in object) { let x = object[key]; ... } * for each (let [key, value] in Iterator(object)) { ... } -> for (let key in object) { let value = object[key]; ... } * for each (let x in array) { ... } where array can be null or undefined -> if (array) { for (let x of array) { ... } } * legacy array comprehension with for-each (newer array comprehension is now also a non-standard feature, I'd like to go with map/filter) * [EXPR for each (x in array)] -> array.map(x => EXPR) * [EXPR for each (x in array) if (COND)] -> array.filter(x => COND).map(x => EXPR) * [x for each (x in array) if (COND)] -> array.filter(x => COND) * [EXPR for each ([i, x] in Iterator(array)) if (g(x, i)] -> array.filter((x, i) => g(x, i)).map((x => EXPR) * [EXPR for each (x in arraylike)] -> Array.from(arraylike).map(x => EXPR) * [EXPR for each (x in string)] -> Array.prototype.slice.call(string).map(x => EXPR) // Array.from behaves differently for surrogate-pair
Attached patch Remove for-each from im/. (obsolete) (deleted) — Splinter Review
Attachment #8687318 - Flags: review?(aleth)
Attached file Types for some complicated cases. (deleted) —
Comment on attachment 8687318 [details] [diff] [review] Remove for-each from im/. Review of attachment 8687318 [details] [diff] [review]: ----------------------------------------------------------------- Thanks! r+ with questions answered ;) ::: im/content/accounts.js @@ +363,5 @@ > + ["disconnect", isCommandDisabled], > + ["moveup", accountList.selectedIndex == 0], > + ["movedown", accountList.selectedIndex == accountList.itemCount - 1], > + ]; > + for (let [name, state] of disabledItems) { Why not keep it an object and then for (let name in disabledItems) { ...name... ...disabledItems[name]... } ::: im/content/viewlog.js @@ +276,5 @@ > let today = null, yesterday = null; > > // Build a chatLogTreeLogItem for each log, and put it in the right group. > let groups = {}; > + for (let log of getIter(this._logs)) { Are you sure this shouldn't be for...in?
Attachment #8687318 - Flags: review?(aleth) → review+
Thank you for reviewing! :D (In reply to aleth [:aleth] from comment #3) > Comment on attachment 8687318 [details] [diff] [review] > Remove for-each from im/. > > Review of attachment 8687318 [details] [diff] [review]: > ----------------------------------------------------------------- > > Thanks! r+ with questions answered ;) > > ::: im/content/accounts.js > @@ +363,5 @@ > > + ["disconnect", isCommandDisabled], > > + ["moveup", accountList.selectedIndex == 0], > > + ["movedown", accountList.selectedIndex == accountList.itemCount - 1], > > + ]; > > + for (let [name, state] of disabledItems) { > > Why not keep it an object and then > for (let name in disabledItems) { > ...name... > ...disabledItems[name]... > } Okay, reverted to object. it seems to be better for that case. > ::: im/content/viewlog.js > @@ +276,5 @@ > > let today = null, yesterday = null; > > > > // Build a chatLogTreeLogItem for each log, and put it in the right group. > > let groups = {}; > > + for (let log of getIter(this._logs)) { > > Are you sure this shouldn't be for...in? getIter is legacy generator, that behaves samely both with for-in and for-of. then, we will eventually replace legacy generator with ES6 generator, that works only with for-of. So I think using for-of there will be better.
(In reply to Tooru Fujisawa [:arai] from comment #4) > Thank you for reviewing! :D > > (In reply to aleth [:aleth] from comment #3) > > Why not keep it an object and then > > for (let name in disabledItems) { > > ...name... > > ...disabledItems[name]... > > } > > Okay, reverted to object. it seems to be better for that case. Can you attach a new patch so it doesn't bitrot? Thanks.
Here's updated patch.
Attachment #8687318 - Attachment is obsolete: true
Attachment #8687600 - Flags: review+
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Instantbird 45
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: