RESOLVED FIXED 104774
Change SET_VAR, SET_BORDERVALUE_COLOR macro to require semicolon(;) at the end of the line
https://bugs.webkit.org/show_bug.cgi?id=104774
Summary Change SET_VAR, SET_BORDERVALUE_COLOR macro to require semicolon(;) at the en...
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
Patch (27.56 KB, patch)
2012-12-12 16:17 PST, Jaehun Lim
no flags
Jaehun Lim
Comment 1 2012-12-12 00:33:10 PST
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
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.