Re: [webkit-dev] namespace indent
On Tue, Dec 1, Jeremy Orlow wrote:
Adam's right though: unlike most of the other style changes, if we don't fix this one all at once, only new files will ever match the style guide. This is different than most of the other guidelines where things eventually converge as people touch lines of the file.
I am in favor of using a script to do this change. It looks like we're using a similar approach to do a global rename: http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/do-webcore-rename Once created, the script could be executed on a per folder, or per project basis, etc. depending on what people prefer. I expressed interest a couple weeks ago in writing such a script: https://lists.webkit.org/pipermail/webkit-dev/2009-November/010521.html Since I don't have a time frame though, I'm not suggesting that people hold off on fixing the issue in files they touch. --Chris
On Tue, Dec 1, 2009 at 3:22 PM, David Levin <levin at chromium.org> wrote:
4) (I think it may be possible to) notice that there are other unchanged lines that have this problem, and then just not print the error if that occurs.
dave
Should the namespace-indent rule apply even to nested namespaces? I understand the reasoning behind the rule — it's a waste of horizontal space to indent almost everything in the file — but if a secondary namespace is declared, it would seem weird not to indent its lines either. This comes up because I have a patch out for review that includes the addition of an HTTPHeaders namespace that just contains a bunch of string constants: // 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; } —Jens
On 03.12.2009, at 14:22, Jens Alfke wrote:
This comes up because I have a patch out for review that includes the addition of an HTTPHeaders namespace that just contains a bunch of string constants:
I do not think constants for HTTP headers are part of HTTPHeaderMap - maybe it would be better to add them to a new file. That would resolve the issue with style automatically, although I agree with the rationale you provided. - WBR, Alexey Proskuryakov
btw, that you put it this way :) This issue came up before:
First, it seems like the original motive was to avoid pointlessly indenting nearly the whole file:
https://lists.webkit.org/pipermail/webkit-dev/2009-September/010002.html
So, I was wondering if we can clarify the rule to apply only to the outermost namespace declaration.
So, I was wondering if we can clarify the rule to apply only to the
outermost namespace declaration. [Darin Adler's reply] Yes, I think we can. So far it seems at least two people agree with this and no one objected last time. The next appropriate step if you want the issue fixed is to file a bug on the style guide and update it. dave On Thu, Dec 3, 2009 at 3:33 PM, Alexey Proskuryakov <ap@webkit.org> wrote:
On 03.12.2009, at 14:22, Jens Alfke wrote:
This comes up because I have a patch out for review that includes the
addition of an HTTPHeaders namespace that just contains a bunch of string constants:
I do not think constants for HTTP headers are part of HTTPHeaderMap - maybe it would be better to add them to a new file. That would resolve the issue with style automatically, although I agree with the rationale you provided.
- WBR, Alexey Proskuryakov
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
participants (4)
-
Alexey Proskuryakov
-
Chris Jerdonek
-
David Levin
-
Jens Alfke