[webkit-dev] Coding style change - Indentation of forward declarations in headers
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
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
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
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
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
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