[webkit-dev] Coding style change - Indentation of forward declarations in headers

2010-11-01 Thread Brady Eidson
Currently, the style guidelines specify "The contents of an outermost namespace 
block (and any nested namespaces with the same scope) should not be indented."

I like this rule - *most* of the time.

A common pattern throughout the project is forward declaring types from 
different namespaces in headers.  And traditionally, we've indented these 
declarations.

While it's not difficult to find this pattern in WebCore itself, it is quite 
pervasive in the WebKit layers where WebCore:: types often need to be forward 
declared.  To be clear, I'm talking about situations like this from 
WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm:

namespace WebCore {
class AuthenticationChallenge;
class CachedFrame;
class HistoryItem;
class ProtectionSpace;
class ResourceLoader;
class ResourceRequest;
}

class WebFrameLoaderClient : public WebCore::FrameLoaderClient {
...
}

...which I think is far superior in the header to:

namespace WebCore {
class AuthenticationChallenge;
class CachedFrame;
class HistoryItem;
class ProtectionSpace;
class ResourceLoader;
class ResourceRequest;
}

class WebFrameLoaderClient : public WebCore::FrameLoaderClient {
...
}

I think this pattern increases readability of forward declarations in headers 
and we should change the style guidelines to specify its continued use.

Thoughts?

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


Re: [webkit-dev] Coding style change - Indentation of forward declarations in headers

2010-11-01 Thread Peter Kasting
On Mon, Nov 1, 2010 at 9:40 AM, Brady Eidson  wrote:

> I think this pattern increases readability of forward declarations in
> headers and we should change the style guidelines to specify its continued
> use.
>
> Thoughts?
>

I don't find either one significantly better than the other, personally.

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


Re: [webkit-dev] Coding style change - Indentation of forward declarations in headers

2010-11-01 Thread David Hyatt
Yeah I agree with Peter.  I think blank lines after { and before } would 
improve the readability of the 2nd example even without indentation.

namespace WebCore {

class AuthenticationChallenge;
class CachedFrame;
class HistoryItem;
class ProtectionSpace;
class ResourceLoader;
class ResourceRequest;

}

This doesn't seem worth making a special case for to me.

dave
(hy...@apple.com)

On Nov 1, 2010, at 11:49 AM, Peter Kasting wrote:

> On Mon, Nov 1, 2010 at 9:40 AM, Brady Eidson  wrote:
> I think this pattern increases readability of forward declarations in headers 
> and we should change the style guidelines to specify its continued use.
> 
> Thoughts?
> 
> I don't find either one significantly better than the other, personally.
> 
> PK 
> ___
> 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] Coding style change - Indentation of forward declarations in headers

2010-11-01 Thread Brady Eidson

On Nov 1, 2010, at 10:00 AM, David Hyatt wrote:

> Yeah I agree with Peter.  I think blank lines after { and before } would 
> improve the readability of the 2nd example even without indentation.
> 
> namespace WebCore {
> 
> class AuthenticationChallenge;
> class CachedFrame;
> class HistoryItem;
> class ProtectionSpace;
> class ResourceLoader;
> class ResourceRequest;
> 
> }

I agree this also makes it more readable, but...

> This doesn't seem worth making a special case for to me.

If the general sentiment is "I don't have a strong preference either way", then 
I would argue that we should allow its continued used simply because so much 
code already uses it.

~Brady

> 
> dave
> (hy...@apple.com)
> 
> On Nov 1, 2010, at 11:49 AM, Peter Kasting wrote:
> 
>> On Mon, Nov 1, 2010 at 9:40 AM, Brady Eidson  wrote:
>> I think this pattern increases readability of forward declarations in 
>> headers and we should change the style guidelines to specify its continued 
>> use.
>> 
>> Thoughts?
>> 
>> I don't find either one significantly better than the other, personally.
>> 
>> PK 
>> ___
>> 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] Coding style change - Indentation of forward declarations in headers

2010-11-01 Thread David Levin
On Mon, Nov 1, 2010 at 10:06 AM, Brady Eidson  wrote:

>
> On Nov 1, 2010, at 10:00 AM, David Hyatt wrote:
>
> Yeah I agree with Peter.  I think blank lines after { and before } would
> improve the readability of the 2nd example even without indentation.
>
> namespace WebCore {
>
> class AuthenticationChallenge;
> class CachedFrame;
> class HistoryItem;
> class ProtectionSpace;
> class ResourceLoader;
> class ResourceRequest;
>
> }
>
>
> I agree this also makes it more readable, but...
>
> This doesn't seem worth making a special case for to me.
>
>
> If the general sentiment is "I don't have a strong preference either way",
> then I would argue that we should allow its continued used simply because so
> much code already uses it.
>

On the other hand, less rules/exceptions = less mental clutter for me to
keep track of = easier for folks to get right, imo.

But I do believe that you are simply stating a rule that is already followed
and just not written, so while I prefer less rules/exception, I don't object
to this.

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


Re: [webkit-dev] Coding style change - Indentation of forward declarations in headers

2010-11-03 Thread Darin Adler
On Nov 1, 2010, at 9:40 AM, Brady Eidson wrote:

> Thoughts?

I agree that  we should find a way to express the existing de facto rule 
clearly, rather than changing all the code to match the wording of the rule in 
the guidelines.

I think the rule is something about indenting code inside namespace except when 
it’s the “main namespace” surrounding an entire source file (header or not).

-- Darin

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