Bug 104774 - Change SET_VAR, SET_BORDERVALUE_COLOR macro to require semicolon(;) at the end of the line
Summary: Change SET_VAR, SET_BORDERVALUE_COLOR macro to require semicolon(;) at the en...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-12 00:11 PST by Jaehun Lim
Modified: 2012-12-17 15:09 PST (History)
4 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Jaehun Lim 2012-12-12 00:11:48 PST
Remove redundant semicolons in RenderStyle.h
Comment 1 Jaehun Lim 2012-12-12 00:33:10 PST
Created attachment 178990 [details]
Patch
Comment 2 Alexey Proskuryakov 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.
Comment 3 Eric Seidel (no email) 2012-12-12 11:32:31 PST
Yup.  Generally we write our macros to require semicolons. :)  Please don't remove these.
Comment 4 Jaehun Lim 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?
Comment 5 Eric Seidel (no email) 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.
Comment 6 Jaehun Lim 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.
Comment 7 Jaehun Lim 2012-12-12 16:17:22 PST
Created attachment 179145 [details]
Patch
Comment 8 Eric Seidel (no email) 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.
Comment 9 WebKit Review Bot 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>
Comment 10 WebKit Review Bot 2012-12-17 15:09:29 PST
All reviewed patches have been landed.  Closing bug.