Re: [webkit-dev] unwritten rules of webkit style

2009-09-03 Thread Brent Fulgham


On Sep 2, 2009, at 9:46 AM, Peter Kasting wrote:

Misc
Files who should end with newlines.

s/who//.  In fact it might be clearer to say Files should end with  
a trailing newline.


I always follow this rule, but I don't remember why it came to exist.   
Is this convention needed for source control or something?


-Brent___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] unwritten rules of webkit style

2009-09-03 Thread Eric Seidel
It's a warning spit out from the compiler, diff, and other unix tools.
http://stackoverflow.com/questions/72271/no-newline-at-end-of-file-compiler-warning
is one explanation as to why gcc might output that warning (which turns into
an error due to the mac build treaing all warnings as errors).

-eric

On Thu, Sep 3, 2009 at 7:18 AM, Brent Fulgham bfulg...@gmail.com wrote:


 On Sep 2, 2009, at 9:46 AM, Peter Kasting wrote:

 *Misc*
 Files who should end with newlines.


 s/who//.  In fact it might be clearer to say Files should end with a
 trailing newline.


 I always follow this rule, but I don't remember why it came to exist.  Is
 this convention needed for source control or something?

 -Brent

 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] unwritten rules of webkit style

2009-09-03 Thread Yong Li
I guess gcc complains if file doesn end with new line?

-Yong
  - Original Message - 
  From: Darin Adler 
  To: Brent Fulgham 
  Cc: WebKit Development 
  Sent: Thursday, September 03, 2009 1:11 PM
  Subject: Re: [webkit-dev] unwritten rules of webkit style


  On Sep 2, 2009, at 11:18 PM, Brent Fulgham wrote:


On Sep 2, 2009, at 9:46 AM, Peter Kasting wrote:
Misc
Files who should end with newlines.


  s/who//.  In fact it might be clearer to say Files should end with a 
trailing newline.


I always follow this rule, but I don't remember why it came to exist.  Is 
this convention needed for source control or something?


  It’s not needed for anything. Tools like diff produce slightly unpleasant 
output for files that don’t end in newlines, so we include them.


  -- Darin




--


  ___
  webkit-dev mailing list
  webkit-dev@lists.webkit.org
  http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] unwritten rules of webkit style

2009-09-03 Thread Gavin Barraclough
I believe the C spec requires that files end in a newline, though I  
couldn't comment on the c++ spec.  Possibly redundant to list this as  
WebKit style issue, if required by the language?


G.

On Sep 3, 2009, at 10:11 AM, Darin Adler wrote:


On Sep 2, 2009, at 11:18 PM, Brent Fulgham wrote:


On Sep 2, 2009, at 9:46 AM, Peter Kasting wrote:

Misc
Files who should end with newlines.

s/who//.  In fact it might be clearer to say Files should end  
with a trailing newline.


I always follow this rule, but I don't remember why it came to  
exist.  Is this convention needed for source control or something?


It’s not needed for anything. Tools like diff produce slightly  
unpleasant output for files that don’t end in newlines, so we  
include them.


-- Darin

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] unwritten rules of webkit style

2009-09-03 Thread Darin Fisher
GCC will complain (maybe only a warning?) if C++ files don't end in a
newline.-Darin

On Thu, Sep 3, 2009 at 1:03 PM, Gavin Barraclough barraclo...@apple.comwrote:

 I believe the C spec requires that files end in a newline, though I
 couldn't comment on the c++ spec.  Possibly redundant to list this as WebKit
 style issue, if required by the language?
 G.

 On Sep 3, 2009, at 10:11 AM, Darin Adler wrote:

 On Sep 2, 2009, at 11:18 PM, Brent Fulgham wrote:

 On Sep 2, 2009, at 9:46 AM, Peter Kasting wrote:

 *Misc*
 Files who should end with newlines.


 s/who//.  In fact it might be clearer to say Files should end with a
 trailing newline.


 I always follow this rule, but I don't remember why it came to exist.  Is
 this convention needed for source control or something?


 It’s not needed for anything. Tools like diff produce slightly unpleasant
 output for files that don’t end in newlines, so we include them.

 -- Darin

 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev



 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] unwritten rules of webkit style

2009-09-03 Thread Darin Adler

On Sep 3, 2009, at 1:03 PM, Gavin Barraclough wrote:

I believe the C spec requires that files end in a newline, though I  
couldn't comment on the C++ spec.  Possibly redundant to list this  
as WebKit style issue, if required by the language?


This is possibly not relevant to the discussion, but we follow this  
guideline for all source files, including ones in languages such as  
HTML and JavaScript.


-- Darin

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] unwritten rules of webkit style

2009-09-02 Thread Peter Kasting
On Wed, Sep 2, 2009 at 8:40 AM, David Levin le...@chromium.org wrote:

 *Comments
 * There should be a *single* space after punctation and before the next
 sentence.

 There should only be a single space before end of line comments.


I don't think either of these are unwritten rules.  Both (especially the
second) are frequently seen with two spaces.  I frequently see close of #if
or namespace EOL comments with one space before //, and other EOL comments
with two.  I wouldn't mind standardizing (my preference is two spaces) but
I'm not sure we can yet.

*Indentation*
 Additional clauses in a conditional may be indented 4 extra spaces to
 visually separate them from the statement to be executed.


 Like this

 if (condition1  condition2)
 statement;


I note you used may instead of should or must.  It seems like we
should pick the right thing and say must.  My reading of the current style
guide is that the current recommended indentation for this case is:
if (long_condition1
 long_condition2
 long_condition3)
statement;

I don't believe I've seen cases like the one you give.

*Misc*
 Files who should end with newlines.


s/who//.  In fact it might be clearer to say Files should end with a
trailing newline.

PK
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] unwritten rules of webkit style

2009-09-02 Thread Peter Kasting
On Wed, Sep 2, 2009 at 9:44 AM, Adam Barth aba...@webkit.org wrote:

 On Wed, Sep 2, 2009 at 11:31 AM, Alexey Proskuryakova...@webkit.org wrote:
  As an aside, is there any practical difference between static const and
  const in C++? The only difference I'm aware of is that the former is
  deprecated in the standard.

 I bet they're different in the light of const_cast, but const_cast
 shines a lot of light on things that shouldn't be visible.


Darin and I have commented on this topic in a separate thread.  However I'll
say here that the constness of the two objects is precisely the same (no
const_cast() difference).

PK
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] unwritten rules of webkit style

2009-09-02 Thread David Levin
On Wed, Sep 2, 2009 at 9:31 AM, Alexey Proskuryakov a...@webkit.org wrote:


 02.09.2009, в 8:40, David Levin написал(а):

 Use enums instead of bools for parameters.  The one exception is function
 names that start with set and take one parameter (e.g. setAllowHeaders).

 The purpose of this rule is to avoid having illegible function calls like
 doSomething(myData, true, 0, false, true). If a function is always called
 with a named variable, there is no practical reason to invent an enum for
 it.



Yes, this is a C/C++ rule.  I do Objective C so seldom that I didn't think
about that, but the guideline should mention that.



 *Indentation*
 Additional clauses in a conditional may be indented 4 extra spaces to
 visually separate them from the statement to be executed.

 Like this

 if (condition1 condition2)
 statement;


 It doesn't look like there's a prevailing style for this currently.


I've seen Darin Adler do this a few times.  I even unindented one instance
and got corrected before checkin, but perhaps this happens so seldom that it
isn't worth mentioning.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] unwritten rules of webkit style

2009-09-02 Thread Darin Adler

On Sep 2, 2009, at 9:46 AM, Peter Kasting wrote:

On Wed, Sep 2, 2009 at 8:40 AM, David Levin le...@chromium.org  
wrote:

Comments
There should be a single space after punctation and before the next  
sentence.


There should only be a single space before end of line comments.

I don't think either of these are unwritten rules.  Both (especially  
the second) are frequently seen with two spaces.  I frequently see  
close of #if or namespace EOL comments with one space before //,  
and other EOL comments with two.  I wouldn't mind standardizing (my  
preference is two spaces) but I'm not sure we can yet.


We did standardize on one space for both of these years ago. But this  
rule is one that people often forget and that a few others disagree  
with and disobey intentionally.



Indentation
Additional clauses in a conditional may be indented 4 extra spaces  
to visually separate them from the statement to be executed.



Like this
if (condition1
 condition2)
statement;

I note you used may instead of should or must.  It seems like  
we should pick the right thing and say must.  My reading of the  
current style guide is that the current recommended indentation for  
this case is:

if (long_condition1
 long_condition2
 long_condition3)
statement;

I don't believe I've seen cases like the one you give.


The style where the  and the statement have the same indentation  
doesn’t work well. David is correct that I recommend and use the extra  
indenting level, and have included it in new code added to this  
project for years. We may not have consensus on this, though. Some  
people still put the operators on the ends of lines, in fact.


-- Darin

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] unwritten rules of webkit style

2009-09-02 Thread Geoffrey Garen
Comments should look like sentences by beginning with a capital and  
ending with a period (punctation).


I think this is a generally good recommendation, but sometimes a  
sentence fragment makes for a better comment, e.g.:


if (x == y) // false for NaN

Don't add explicit line breaks in the middle of a statement unless  
it is severely illegible even at wide editor window width (which  
current code tends to treats as about 120-180 characters).


Meh. We have a lot of code that breaks around  and || operators,  
and I think it's OK.


Additional clauses in a conditional may be indented 4 extra spaces  
to visually separate them from the statement to be executed.



Like this
if (condition1
 condition2)
statement;


I think the majority of our code disagrees with this guideline. I  
disagree with it too. I would prefer the  to be evenly indented  
with condition1, or the  to be on the same line as condition1, and  
condition2 evenly indented with condition1.




Geoff___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] unwritten rules of webkit style

2009-09-02 Thread David Levin
On Wed, Sep 2, 2009 at 11:23 AM, Geoffrey Garen gga...@apple.com wrote:

 Like this

 if (condition1  condition2)
 statement;

 I think the majority of our code disagrees with this guideline. I disagree
 with it too. I would prefer the  to be evenly indented with condition1,
 or the  to be on the same line as condition1, and condition2 evenly
 indented with condition1.


This was meant to list an acceptable style, not suggest it but it
is obviously confusing and controversial.

Since my goal is to document what is standard practice, this should just be
omitted for now.  Once the conversation dies down I'll send a revised
version without this (and with other corrections).

Dave
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] unwritten rules of webkit style

2009-09-02 Thread David Levin
On Wed, Sep 2, 2009 at 11:54 AM, Yong Li yong...@torchmobile.com wrote:

  {} should be added in this case:


 if (condition1  condition2)
 statement;


Not according to current WebKit style because it is a single line statement.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] unwritten rules of webkit style

2009-09-02 Thread Yong Li
Current guideline also contains these 2 cases that {} should be used. I think 
when the condition is multi-lined, this should also apply. (BTW, hate the 
existing rule no braces for one line. it doesn't give any benefit. worse than 
always use braces)if (condition) {
// Some comment
doIt();
}

if (condition) {
myFunction(reallyLongParam1, reallyLongParam2, ...
reallyLongParam5);
}
- Original Message - 
  From: David Levin 
  To: Yong Li 
  Cc: WebKit Development 
  Sent: Wednesday, September 02, 2009 2:56 PM
  Subject: Re: [webkit-dev] unwritten rules of webkit style





  On Wed, Sep 2, 2009 at 11:54 AM, Yong Li yong...@torchmobile.com wrote:

{} should be added in this case:

  if (condition1 
   condition2)
  statement;


  Not according to current WebKit style because it is a single line statement.

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] unwritten rules of webkit style

2009-09-02 Thread Yong Li
For multi-line condition, the following style is most readable to me.

if (condition1 
 condition2)
{
 // code...
}

  - Original Message - 
  From: Yong Li 
  To: David Levin 
  Cc: WebKit Development 
  Sent: Wednesday, September 02, 2009 3:05 PM
  Subject: Re: [webkit-dev] unwritten rules of webkit style


Current guideline also contains these 2 cases that {} should be used. I think 
when the condition is multi-lined, this should also apply. (BTW, hate the 
existing rule no braces for one line. it doesn't give any benefit. worse than 
always use braces)if (condition) {
// Some comment
doIt();
}

if (condition) {
myFunction(reallyLongParam1, reallyLongParam2, ...
reallyLongParam5);
}
- Original Message - 
From: David Levin 
To: Yong Li 
Cc: WebKit Development 
Sent: Wednesday, September 02, 2009 2:56 PM
Subject: Re: [webkit-dev] unwritten rules of webkit style





On Wed, Sep 2, 2009 at 11:54 AM, Yong Li yong...@torchmobile.com wrote:

  {} should be added in this case:

if (condition1 
 condition2)
statement;


Not according to current WebKit style because it is a single line statement.




--


  ___
  webkit-dev mailing list
  webkit-dev@lists.webkit.org
  http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] unwritten rules of webkit style

2009-09-02 Thread Dimitri Glazkov
I don't think the intent of this thread was to create new rules. Let's
stick with documenting existing style practices.

:DG

On Wed, Sep 2, 2009 at 12:16 PM, Yong Liyong...@torchmobile.com wrote:
 For multi-line condition, the following style is most readable to me.

 if (condition1
      condition2)
 {
  // code...
 }


 - Original Message -
 From: Yong Li
 To: David Levin
 Cc: WebKit Development
 Sent: Wednesday, September 02, 2009 3:05 PM
 Subject: Re: [webkit-dev] unwritten rules of webkit style

 Current guideline also contains these 2 cases that {} should be used. I
 think when the condition is multi-lined, this should also apply.

 (BTW, hate the existing rule no braces for one line. it doesn't give any
 benefit. worse than always use braces)

 if (condition) {
 // Some comment
 doIt();
 }

 if (condition) {
 myFunction(reallyLongParam1, reallyLongParam2, ...
 reallyLongParam5);
 }

 - Original Message -
 From: David Levin
 To: Yong Li
 Cc: WebKit Development
 Sent: Wednesday, September 02, 2009 2:56 PM
 Subject: Re: [webkit-dev] unwritten rules of webkit style


 On Wed, Sep 2, 2009 at 11:54 AM, Yong Li yong...@torchmobile.com wrote:

 {} should be added in this case:


 if (condition1
          condition2)
     statement;

 Not according to current WebKit style because it is a single line statement.

 

 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] unwritten rules of webkit style

2009-09-02 Thread Cameron McCormack
David Levin:
 In copyright entries, don't use ranges for years. Use capital (C) for the
 copyright and no comma after the last year.  Example of a well formed
 copyright entry:
  * Copyright (C) 2004, 2005, 2006, 2007, 2008 Apple Inc. All rights
 reserved.

I’ve noticed a couple of different ways of writing subsequent copyright
lines, e.g.:

  Copyright (C) 2005 Somebody email
2006, 2007 Somebody Else email

  — http://trac.webkit.org/browser/trunk/WebCore/svg/SVGSVGElement.cpp

and:

  Copyright (C) 2005 Somebody email
  Copyright (C) 2006, 2007 Somebody Else email

  — http://trac.webkit.org/browser/trunk/WebCore/svg/SVGFilterElement.cpp

-- 
Cameron McCormack ≝ http://mcc.id.au/
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] unwritten rules of webkit style

2009-09-02 Thread Patrick Mueller

Darin Adler wrote:

On Sep 2, 2009, at 5:00 PM, Cameron McCormack wrote:


I’ve noticed a couple of different ways of writing subsequent copyright
lines, e.g.:

 Copyright (C) 2005 Somebody email
   2006, 2007 Somebody Else email

  — http://trac.webkit.org/browser/trunk/WebCore/svg/SVGSVGElement.cpp

and:

 Copyright (C) 2005 Somebody email
 Copyright (C) 2006, 2007 Somebody Else email

  — http://trac.webkit.org/browser/trunk/WebCore/svg/SVGFilterElement.cpp



I think the second format is the one we should use.


IBM actually has rules about what the copyright statements should look 
like (surprise surprise) for our own copywritten material, and so I 
applied it for a recent set of changes to 
WebCore/inspector/front-end/ResourceView.js:


 * Copyright (C) 2007, 2008 Apple Inc.  All rights reserved.
 * Copyright (C) IBM Corp. 2009  All rights reserved.

I'll be first to say ... ick ...  And I'll also note that it's unlikely 
anything horrible will happen to me if I was forced to use some other 
format instead (like what everyone else is using, for instance).  But 
other folks may not be so flexible.


I'm thinking we probably be need to be flexible about the format of the 
line, perhaps strongly suggest the current format as the format to use.


And I certainly agree that the second form is better.  I can imagine a 
lawyer getting upset if they don't see the Copyright (C) on a line 
where they expect it (OMG - YOU DIDN'T COPYRIGHT IT!!!)


--
Patrick Mueller - http://muellerware.org

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] unwritten rules of webkit style

2009-09-02 Thread TAMURA, Kent




{} should be added in this case:




if (condition1  condition2)
 statement;




Not according to current WebKit style because it is a single line
statement.



I don't like this rule. We need to be careful to add/remove a sentence in an
existing block, need to guess how many sentences to be added when we add
if.

A few days ago, this rule brought a real bug to me.

Existing code:
  if (attr-name() == fooAttr)
  doSometing();
  else if (attr-name() == barAttr) ...

I added if like:

  if (attr-name() == fooAttr)
  if (isBaz())
  doSometing();
  else if (attr-name() == barAttr) ...

The else clause unexpectedly attached to if (isBaz())
I wasted about an hour to found this bug.

--
TAMURA Kent
Software Engineer, Google



___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev