[webkit-dev] Some new coding style rules

[David Kilzer]

On Wed, August 4, 2010 at 11:56:53 PM, Sam Weinig wrote:
> On Wed, Aug 4, 2010 at 1:29 AM, Nikolas Zimmermann 
><zimmermann at physik.rwth-aachen.de> wrote:
> 
> >namespace WebCore {
> >...
> >} // namespace WebCore
> >
> >2. ENABLE(FOO) #endif comments
> >
> >#if ENABLE(FOO)
> >..
> >#endif // ENABLE(FOO)
> 
> I like these two forms of comments.

As do I.  My preference for rules is:

- Always use "} // namespace Foo" comments with namespaces.

- Always use "#endif // ENABLE(FOO)" comments if there are more than 10-15 
lines  of code between the matching #if/#else/#elif, or if the #endif is 
nested.  But  do not treat it as a warning/error if an #endif macro has a 
well-formed comment  and doesn't obey the 'required' rules in the previous 
sentence (unless this inconsistency would drive anyone nuts).


I like both of these comments for the following reasons:


- You don't have to scroll up or otherwise jump around the source to know 
what the right curly brace or the #endif macro is used for.

In the namespace case, the block almost always spans most of the file, so 
there's rarely a case when it's less than 10 lines away from the left 
curly brace.  Or to put it another way, the "namespace Foo {" line is always 
near the top of the file, and the "} // namespace Foo" line is always near the 
bottom of the file, so it's rare that you see them both at the same time.

In the case of the #endif macro, I find that the comments are useful if 
the corresponding #if/#else/#elif is more than 10-15 lines away (e.g., more than 
a screenful), or if there is any #if macro nesting such that #endif statements 
can appear next to each other (see Platform.h).


- When merging, having these comments are a great help to both the person 
doing the merge and to merge algorithms.

New methods are often added near the end of a namespace, which means the "}" 
for the end of the method looks the same as the "}" for the end of a 
namespace (especially with our current namespace indent rules), so merges with 
conflicts sometimes end up "repurposing" the namespace curly brace for the end 
of a new method.  Providing an explicit comment prevents this behavior, and 
provides a common anchor point in both the original ("ours") and new ("theirs") 
files.

Likewise, #endif macros (especially when grouped together due to nesting) 
will frequently get "repurposed" by merge algorithms.  This can create more 
work (especially the way some macros are nested in Platform.h) if there are 
changes to both files being merged.

--

On a related note, I think we should also define rules for indenting #if 
macros.  I think this is orthogonal to using comments, though, and I prefer 
having comments regardless of the indent rules.

Note that macro indenting has already started (haphazardly) in Platform.h 
and perhaps a few other places.

Here are some examples:

#if ENABLE(FOO)
#if PLATFORM(BAR)
#if USE(BAZ)
...
#else
...
#endif // USE(BAZ)
#endif // PLATFORM(BAR)
#endif // ENABLE(FOO)

#if ENABLE(FOO)
# if PLATFORM(BAR)
#  if USE(BAZ)
...
#  else
...
#  endif // USE(BAZ)
# endif // PLATFORM(BAR)
#endif // ENABLE(FOO)

#if ENABLE(FOO)
#if PLATFORM(BAR)
  #if USE(BAZ)
...
    #else
...
  #endif // USE(BAZ)
#endif // PLATFORM(BAR)
#endif // ENABLE(FOO)

My original opinion was to never indent macros, but after staring at the 
above examples, I do think indenting makes them easier to read.  I'm still 
trying to decide whether I like the "#" anchored on the left or whether I prefer 
it anchored to if/else/elif/endif macro names, though.

Dave