Bug 32137 – [Website] Style guide should allow indentation of _nested_ namespaces

• Component: WebKit Website
• Status: RESOLVED FIXED

[Jens Alfke]

At http://webkit.org/coding/coding-style.html, rule #3 says "Code inside a namespace should not be indented". This is obviously because most source files have a namespace wrapped around them and indenting the whole file is pointless.

However, this shouldn't apply to _nested_ namespace declarations, which are a lot shorter, and by normal C/C++ rules should be indented to distinguish their scope. An example from a patch I'm currently getting reviewed:

// HTTPHeaderMap.h
namespace WebCore {

   class HTTPHeaderMap : public HashMap<AtomicString, String, CaseFoldingHash> {
	...
	...
   };

   namespace HTTPHeaders {
       extern const char* const Accept;
       extern const char* const Authorization;
       ...
       extern const char* const UserAgent;
   }
}

This would look strange if the contents of HTTPHeaders weren't indented. Especially since if HTTPHeaders were made a class instead (which I almost decided to do) the style guide _would_ say to indent it.

   namespace HTTPHeaders {
   extern const char* const Accept;
   extern const char* const Authorization;
   ...
   extern const char* const UserAgent;
   }

So I propose to edit the rule to read:

3. Code inside a namespace that takes up the entire file (apart from its #includes) should not be indented. However, if there are nested namespaces, their contents should be indented normally.

[Jens Alfke]

Created attachment 44276 [details]
patch

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44276 [details] without any errors.

[Eric Seidel (no email)]

We'll have to fix check-webkit-style too if we're changing the guidelines.

[David Levin]

I'll work on a fix fir check-webkit-style.  This issue is a bit rare so we won't get a lot of cases with issues anyway.

[Chris Jerdonek]

(In reply to comment #1)
> Created an attachment (id=44276) [details]
> patch

> -<li>Code inside a namespace should not be indented.
> +<li>Code inside a namespace that takes up the entire file (apart from its
> +#includes) should not be indented. However, if there are nested namespaces,
> +their contents should be indented normally.

Note that there are non-nested namespaces that don't take up the entire file.  For example--

namespace JSC {
...
}

namespace WTF {
...
}

So I think you should use a word like "outermost".

Also, I think consecutive declarations should be treated as a single namespace for the purposes of this rule, rather than as a nesting.  For example--

namespace JSC { namespace WREC {

(This actually occurs in the code base.)

I mentioned these points in an e-mail to the group last month:

https://lists.webkit.org/pipermail/webkit-dev/2009-November/010521.html

Thanks.

[Jens Alfke]

How does this sound:

"When one or more <code>namespace</code> blocks together take up the entire file (apart from its
#includes), their contents should not be indented. However, if there are nested namespaces that occupy only a portion of the file, their contents should be indented normally."

Thumbs-up and I'll roll a new patch.

[David Levin]

"take up the entire file" seems mildly subjective and it needs multiple exceptions: comments, #include's, #ifdef, #endif, #define, using (see JavaScriptCore/wtf/RefCounted.h)

I think outermost namespace is a simpler concept and the right idea.  Consecutive namespaces is the right thing for the other idea.

[Jens Alfke]

Well, I was considering the possibility of files that are mostly non-namespaced but have a short namespace in them; in that case it would make sense to indent the namespace. But maybe that doesn't appear anywhere in WebKit.

How about:

"The contents of an outermost <code>namespace</code> block should not be indented. However, if there are nested namespaces that occupy only a portion of the file, their contents should be indented normally."

[David Levin]

How about this?

"The contents of an outermost <code>namespace</code> (and any nested namespaces with the same scope) block should not be indented. The contents of other namespaces should be indented."

[Jens Alfke]

It wasn't intuitively clear to me what "with the same scope" means, but I think this is about as clear as we're going to be able to get it :) I'll update the patch.

[Jens Alfke]

Created attachment 44329 [details]
patch 2

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44329 [details] without any errors.

[David Levin]

Comment on attachment 44329 [details]
patch 2 

Missing ChangeLog. Please add one and feel free to submit (since this issue has already been discussed on WebKit-dev in the past).

[Eric Seidel (no email)]

Hmm.  Looks like this one got stalled at commit.

[Jens Alfke]

Sorry this fell on the floor ... committed r54258.