1634911 - Simplify loops over lines in block layout to use ranged for.

Status: RESOLVED FIXED

(Core :: Layout: Block and Inline, task, P5)

Description

[Emilio Cobos Álvarez [:emilio]]

Loops like this would read so much nicer if they used ranged for.

Comment 1

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

Yeah, there are many static helper functions in nsBlockFrame.cpp, so we might need to define a helper methods in nsBlockFrame like

nsLineList Lines() { return mLines; }

Then, we can forward iterate lines via

for (nsLineBox& line : Lines()) {
  ...
}

Or backward:

for (nsLineBox& line : Reversed(Lines())) {
  ...
}

Comment 2

[Shishir Jaiswal]

Hi Ting-Yu,

I agree with what you. I'd discussed the same with emilio and am currently working over the patch. Please assign this bug to me.

Thanks,
Shishir

Comment 3

[Shishir Jaiswal]

Attachment: Bug 1634911 - Simplify loops over lines in block layout to use ranged for. r=emilio

Comment 4

[Shishir Jaiswal]

Hi Emilio,

There were a few suggestions given by reviewbot after your approval of the patch. I'd addressed those comments and submitted the updated patch. Could you approve those changes as well and land the patch?

Many thanks. Have a nice day.

Regards,
Shishir

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Thanks, this looks great!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/39dff7adfc30
Simplify loops over lines in block layout to use ranged for. r=emilio

Comment 7

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

Re comment 4:

For a future reference, in Firefox's workflow, after a patch is approved it means there are only minor nits left to address, and the reviewer trusts the patch author to fix those nits. The author can land the patch whenever it's ready. (In Phabricator, use Add Action -> Change Project Tags -> add a "Check-in Needed" tag.) Someone or a bot will pick up your patch and land the patch.

Comment 8

[Dorel Luca [:dluca]]

Backed out changeset 39dff7adfc30 (bug 1634911) for build bustage in layout/generic/nsBlockFrame.cpp. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=302938445&repo=autoland&lineNumber=43994

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=39dff7adfc302a9fd6cbcfa93d53acc481823828

Backout:
https://hg.mozilla.org/integration/autoland/rev/14402768268589b5bc16acf68509349bab62bc7a

Comment 9

[Shishir Jaiswal]

I've fixed the error in the latest patch pushed via moz-phab.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e708c9a6b38f
Simplify loops over lines in block layout to use ranged for. r=emilio

Comment 11

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/e708c9a6b38f