Bug 104774

Summary:

Change SET_VAR, SET_BORDERVALUE_COLOR macro to require semicolon(;) at the end of the line

Product:

WebKit

Reporter:

Jaehun Lim <ljaehun.lim>

Component:

Layout and Rendering

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ap, eric, ojan.autocc, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none

Jaehun Lim

Reported 2012-12-12 00:11:48 PST

Remove redundant semicolons in RenderStyle.h

Attachments
Patch (30.78 KB, patch) 2012-12-12 00:33 PST, Jaehun Lim	no flags	Details Formatted Diff Diff
Patch (27.56 KB, patch) 2012-12-12 16:17 PST, Jaehun Lim	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Jaehun Lim

Comment 1 2012-12-12 00:33:10 PST

Created attachment 178990 [details] Patch

Alexey Proskuryakov

Comment 2 2012-12-12 10:47:41 PST

Comment on attachment 178990 [details] Patch I don't think that this is an improvement. Statements should end with semicolons for consistency, otherwise the code looks wrong to humans. If you really want to avoid redundant semicolons, please change the macros to require them.

Eric Seidel (no email)

Comment 3 2012-12-12 11:32:31 PST

Yup. Generally we write our macros to require semicolons. :) Please don't remove these.

Jaehun Lim

Comment 4 2012-12-12 13:37:48 PST

Thank you for your comments. Some lines ends with semicolons, others not. I think it's not consistent. How about to change all lines end with semicolons? Or no need to change?

Eric Seidel (no email)

Comment 5 2012-12-12 13:43:42 PST

I'd be happy to r+ such a change. But better than doing it manually would be to make sure check-webkit-style knows how to catch macro-lines w/o a ;, and to make sure that the WebKit style guide makes this explicit. W/o changing those patches to fix the code are only a temporary fix at best.

Jaehun Lim

Comment 6 2012-12-12 14:29:37 PST

(In reply to comment #5) > I'd be happy to r+ such a change. But better than doing it manually would be to make sure check-webkit-style knows how to catch macro-lines w/o a ;, and to make sure that the WebKit style guide makes this explicit. W/o changing those patches to fix the code are only a temporary fix at best. I'll make a patch to add ; first. And then make another bug for what you said. Thanks.

Jaehun Lim

Comment 7 2012-12-12 16:17:22 PST

Created attachment 179145 [details] Patch

Eric Seidel (no email)

Comment 8 2012-12-17 14:54:15 PST

Comment on attachment 179145 [details] Patch :shrug: I'm not sure this really helps. But I don't see it really hurting either.

WebKit Review Bot

Comment 9 2012-12-17 15:09:25 PST

Comment on attachment 179145 [details] Patch Clearing flags on attachment: 179145 Committed r137950: <http://trac.webkit.org/changeset/137950>

WebKit Review Bot

Comment 10 2012-12-17 15:09:29 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.