Bug 27508 - Add checks for switch statement indentation to cpplint
: Add checks for switch statement indentation to cpplint
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: Tools / Tests
: 528+ (Nightly build)
: PC All
: P2 Normal
Assigned To: Nobody
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-07-21 11:54 PDT by Jakob Petsovits
Modified: 2009-07-21 13:06 PDT (History)
1 user (show)

See Also:


Attachments
Add checks for switch statement indentation to cpplint (6.50 KB, patch)
2009-07-21 11:54 PDT, Jakob Petsovits
no flags Details | Formatted Diff | Diff
Add checks for switch statement indentation to cpplint (6.55 KB, patch)
2009-07-21 11:59 PDT, Jakob Petsovits
levin: review-
Details | Formatted Diff | Diff
Add checks for switch statement indentation to cpplint (try 3) (8.66 KB, patch)
2009-07-21 12:39 PDT, Jakob Petsovits
levin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jakob Petsovits 2009-07-21 11:54:22 PDT
Created attachment 33196 [details]
Add checks for switch statement indentation to cpplint

Switch statements have strong similarities to namespaces when it comes to indentation, so I stole from my previous patch (https://bugs.webkit.org/show_bug.cgi?id=27461) and adapted some parts to implement style guideline #5 from http://webkit.org/coding/coding-style.html to cpplint. Looking for a motivated reviewer, hope you like it!
Comment 1 Jakob Petsovits 2009-07-21 11:59:31 PDT
Created attachment 33197 [details]
Add checks for switch statement indentation to cpplint

Note to self: Don't post patch files unless bug report is created and URL is inserted into the ChangeLog.
Apart from that, same patch as the previous one.
Comment 2 David Levin 2009-07-21 12:12:57 PDT
Re changelog: Just use prepare-ChangeLog --bug YourBugNumber
Comment 3 David Levin 2009-07-21 12:16:10 PDT
Comment on attachment 33197 [details]
Add checks for switch statement indentation to cpplint

In general it looks good but I think it will have problems on nested switch statements. What can be done about that?
Comment 4 Jakob Petsovits 2009-07-21 12:39:21 PDT
Created attachment 33200 [details]
Add checks for switch statement indentation to cpplint (try 3)

Good point, I had not thought of that case. Here's a relatively simple solution that'll work in pretty much all common cases, approach explained in the code comments. Plus more test cases. Is that good enough, or does it need to be perfect?
Comment 5 David Levin 2009-07-21 13:01:36 PDT
Comment on attachment 33200 [details]
Add checks for switch statement indentation to cpplint (try 3)

This is an improvement over what we have now and seems to minimize the possibility of false alarms (with respect to nested switch statements), so it seems like a positive step forward.
Comment 6 David Levin 2009-07-21 13:06:45 PDT
Committed as http://trac.webkit.org/changeset/46189