Closed Bug 1170419 Opened 4 years ago Closed 4 years ago

Move comments of member fields of struct related to border-collapse to the top side of the fields

Categories

(Core :: Layout: Tables, defect)

defect
Not set

Tracking

()

RESOLVED INVALID
Tracking Status
firefox41 --- affected

People

(Reporter: xidorn, Unassigned)

References

Details

Attachments

(1 file)

A bunch of comment in structs in nsTableFrame are currently put to the right side of the member fields, which makes them longer than our line-length restriction.
Attached patch patchSplinter Review
Also removes some whitespaces.
Attachment #8613861 - Flags: review?(dholbert)
Comment on attachment 8613861 [details] [diff] [review]
patch

Hmm... I don't think I agree with this change.  Most of the lines modified by this patch are under 80 chars already, and IMO they're more readable as they currently stand.

I'd prefer we simply rewrap the few comments that are longer than 80 chars.
Attachment #8613861 - Flags: review?(dholbert) → review-
(In reply to Daniel Holbert [:dholbert] from comment #2)
> Comment on attachment 8613861 [details] [diff] [review]
> patch
> 
> Hmm... I don't think I agree with this change.  Most of the lines modified
> by this patch are under 80 chars already, and IMO they're more readable as
> they currently stand.

They look pretty but are hard to change, especially extend. Also adding whitespaces between type and name doesn't seem to be something we want to keep.

> I'd prefer we simply rewrap the few comments that are longer than 80 chars.

Well, dbaron taught me to wrap comments at 72 characters (bug 966166 comment 129). Although I don't see any official style guide saying that, I generally follow his guide there.
(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #3)
> They look pretty but are hard to change, especially extend.

I suppose, but it's not *too* hard -- and we read code + comments more frequently than we have to edit them.  So, I'm not sure that editability should trump code/comment-readability here.

(If we had concrete plans on rewriting all of these comments imminently, I'd be more inclined to agree, though I'd still probably need some convincing.)

> Also adding
> whitespaces between type and name doesn't seem to be something we want to
> keep.

I don't think there's a strong guideline either way on this, to be honest, and I think this sort of whitespace helps with readability in many cases.

So, while I'm mildly OK with dropping whitespace of this sort, I also don't see a strong reason to do so, for its own sake.

> > I'd prefer we simply rewrap the few comments that are longer than 80 chars.
> 
> Well, dbaron taught me to wrap comments at 72 characters (bug 966166 comment
> 129). Although I don't see any official style guide saying that, I generally
> follow his guide there.

Interesting, I hadn't heard that guideline from him before.  I'm curious as to his reasons, but I suspect they might not apply here. (Also, note that he expressed this simply as a preference, rather than as a strict rule to apply across the board.)

I suspect dbaron didn't intend for this suggestion to apply to 1-liner (or close-to-one-liner) comments that are offset the right of a variable-declaration, like the ones in question here.  I'd bet he was talking about larger comment-blocks which are separate from code.
OK, so let's close this bug.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.