Bug 30538 - Replace boolean operator indentation example with an if statement.
Summary: Replace boolean operator indentation example with an if statement.
Status: UNCONFIRMED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Website (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-10-19 14:40 PDT by Andrew Scherkus
Modified: 2010-06-10 19:21 PDT (History)
4 users (show)

See Also:


Attachments
Round 1 (1.52 KB, patch)
2009-10-19 14:42 PDT, Andrew Scherkus
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Scherkus 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.
Comment 1 Andrew Scherkus 2009-10-19 14:42:12 PDT
Created attachment 41453 [details]
Round 1
Comment 2 Eric Seidel (no email) 2009-10-19 14:57:12 PDT
Comment on attachment 41453 [details]
Round 1

OK.  :shrug:
Comment 3 WebKit Commit Bot 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>
Comment 4 WebKit Commit Bot 2009-10-19 15:10:50 PDT
All reviewed patches have been landed.  Closing bug.
Comment 5 Darin Adler 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;
Comment 6 Eric Seidel (no email) 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.
Comment 7 Andrew Scherkus 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?