Bug 30538

Summary: Replace boolean operator indentation example with an if statement.
Product: WebKit Reporter: Andrew Scherkus <scherkus>
Component: WebKit WebsiteAssignee: Nobody <webkit-unassigned>
Status: UNCONFIRMED    
Severity: Normal CC: commit-queue, darin, eric, levin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Round 1 none

Andrew Scherkus
Reported 2009-10-19 14:40:38 PDT
As Dave Levin pointed out in https://bugs.webkit.org/show_bug.cgi?id=30529 my || should have been on the next line. The style guide gives an example using a "return" statement, however there are many instances in WebKit today that contradicts that rule and place the operators at the end of the line. If "if" statements are not an exception, then I'd suggest replacing the example to use an if statement. You're more likely to write if statements than return statements so this example should prevent further style violations from happening.
Attachments
Round 1 (1.52 KB, patch)
2009-10-19 14:42 PDT, Andrew Scherkus
no flags
Andrew Scherkus
Comment 1 2009-10-19 14:42:12 PDT
Created attachment 41453 [details] Round 1
Eric Seidel (no email)
Comment 2 2009-10-19 14:57:12 PDT
Comment on attachment 41453 [details] Round 1 OK. :shrug:
WebKit Commit Bot
Comment 3 2009-10-19 15:10:46 PDT
Comment on attachment 41453 [details] Round 1 Clearing flags on attachment: 41453 Committed r49816: <http://trac.webkit.org/changeset/49816>
WebKit Commit Bot
Comment 4 2009-10-19 15:10:50 PDT
All reviewed patches have been landed. Closing bug.
Darin Adler
Comment 5 2009-10-19 17:17:53 PDT
I specifically chose a return statement rather than an if statement, because of the problem where the boolean operator lines end up lined up with the statement in the body of the if. In the past we have discussed indenting one additional level to avoid that problem, but this was contentious enough that I wanted to leave it out until we discussed it. I am *not* sure that what you checked in here represents our style recommendation, and I wish we had some discussion of it before it was landed! I would write: if (a || b) return; Not: if (a || b) return;
Eric Seidel (no email)
Comment 6 2009-10-19 17:23:10 PDT
Happy to revert it. We can also make an additional checkin which makes specific all of these cases.
Andrew Scherkus
Comment 7 2009-10-19 17:23:57 PDT
Eeek -- sorry :( I'm fine reverting -- I was just confused at what the proper style should be and grepping the code base didn't give any consistent answers. How about we start the discussion on webkit-dev?
Note You need to log in before you can comment on or make changes to this bug.