Bug 27377

Summary: Assorted cpplint improvements
Product: WebKit Reporter: Adam Treat <manyoso>
Component: WebKit Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Adds new cpplint check for webkit rule violation
levin: review+
Turn off erroneous filters preventing valid checks
levin: review+
Move comment to where it belongs levin: review+

Adam Treat
Reported 2009-07-17 10:12:35 PDT
The following patches are for assorted improvements in cpplint. The ChangeLog describes them.
Attachments
Adds new cpplint check for webkit rule violation (3.39 KB, patch)
2009-07-17 10:27 PDT, Adam Treat
levin: review+
Turn off erroneous filters preventing valid checks (1.66 KB, patch)
2009-07-17 10:28 PDT, Adam Treat
levin: review+
Move comment to where it belongs (1.82 KB, patch)
2009-07-17 10:28 PDT, Adam Treat
levin: review+
Adam Treat
Comment 1 2009-07-17 10:27:34 PDT
Created attachment 32953 [details] Adds new cpplint check for webkit rule violation
Adam Treat
Comment 2 2009-07-17 10:28:10 PDT
Created attachment 32954 [details] Turn off erroneous filters preventing valid checks
Adam Treat
Comment 3 2009-07-17 10:28:34 PDT
Created attachment 32955 [details] Move comment to where it belongs
David Levin
Comment 4 2009-07-17 12:09:52 PDT
Comment on attachment 32954 [details] Turn off erroneous filters preventing valid checks > diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog > index 7b66d46..80f3a75 100644 > --- a/WebKitTools/ChangeLog > +++ b/WebKitTools/ChangeLog > @@ -3,6 +3,29 @@ > Reviewed by NOBODY (OOPS!). > > https://bugs.webkit.org/show_bug.cgi?id=27377 > + Don't filter whitespace at the end of the line. This is not > + explicitly a rule of webkit coding style, but there is no reason > + not to warn of this common style problem. > + > + Don't filter whitespace newline. This prevents cpplint of complaining > + about the following situation: I was confused when I read this because it sounded like removing the filtering prevented the complaining. Here's an idea for an alternate wording: Don't filter whitespace newline. Now, cpplint will complain about the following situation: > + > + if (true) { > + doSomething(); > + doSomethingAgain(); > + } > + else > + doSomething(); > + > + Which is a webkit coding style rule violation. > + > + * Scripts/modules/cpplint.py: > + > +2009-07-17 Adam Treat <adam.treat@torchmobile.com> > + > + Reviewed by NOBODY (OOPS!). > + > + https://bugs.webkit.org/show_bug.cgi?id=27377 > This makes cpplint complain about this for instance: > > if (true) > diff --git a/WebKitTools/Scripts/modules/cpplint.py b/WebKitTools/Scripts/modules/cpplint.py > index 2ef4e3a..cf22e3d 100644 > --- a/WebKitTools/Scripts/modules/cpplint.py > +++ b/WebKitTools/Scripts/modules/cpplint.py > @@ -2786,10 +2786,8 @@ def use_webkit_styles(): > # modify the implementation and enable them. > global _DEFAULT_FILTERS > _DEFAULT_FILTERS = [ > - '-whitespace/end_of_line', > '-whitespace/comments', > '-whitespace/blank_line', > - '-whitespace/newline', # '\r' > '-runtime/explicit', # explicit > '-runtime/virtual', # virtual dtor > '-runtime/printf',
David Levin
Comment 5 2009-07-17 12:30:38 PDT
Comment on attachment 32953 [details] Adds new cpplint check for webkit rule violation > + or search(r'\b(if|for|while|switch)\b', previous_line)) Please add "else" and a test case using else.
Adam Treat
Comment 6 2009-07-17 13:14:54 PDT
Landed with r46051, r46052, and r46053.
Note You need to log in before you can comment on or make changes to this bug.