Summary: | [Website] Style guide should allow indentation of _nested_ namespaces | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jens Alfke <jens> | ||||||
Component: | WebKit Website | Assignee: | Jens Alfke <jens> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Minor | CC: | cjerdonek, eric, hamaji, levin, webkit.review.bot | ||||||
Priority: | P3 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Jens Alfke
2009-12-03 16:00:01 PST
Created attachment 44276 [details]
patch
style-queue ran check-webkit-style on attachment 44276 [details] without any errors.
We'll have to fix check-webkit-style too if we're changing the guidelines. 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. (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. 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. "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. 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." 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." 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. Created attachment 44329 [details]
patch 2
style-queue ran check-webkit-style on attachment 44329 [details] without any errors.
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).
Hmm. Looks like this one got stalled at commit. Sorry this fell on the floor ... committed r54258. |