I have extended your patch and submitted it as r187540. Cheers, Daniel
On Mon, Jul 29, 2013 at 3:08 AM, Daniel Jasper <[email protected]> wrote: > > > > On Mon, Jul 29, 2013 at 12:00 PM, Curdeius Curdeius <[email protected]>wrote: > >> Hi, >> thanks for your comments. >> I've fixed the patch. >> File formatting should be ok now. >> I've changed the name to IndentInNamespaces as suggested. >> > > Thank you. Do you have commit access or should I commit this for you? > > What about the indenting of inner namespaces: take a look for instance at >> WebKit coding style about namespaces: >> http://www.webkit.org/coding/coding-style.html#indentation-namespace. >> It's not exactly what I proposed, but it's close still. >> > > I will implement what is required for WebKit. Lets see how we go from > there. > > Cheers, > Daniel > > Cheers, >> Marek Kurdej >> >> >> 2013/7/29 Daniel Jasper <[email protected]> >> >>> Please attach patches as plain text, not as zip-files. Or alternatively, >>> use phabricator (http://llvm-reviews.chandlerc.com/). >>> >>> Comments inline. >>> >>> + /// \brief Indent the content of namespaces by one level. >>>> + /// >>>> + /// When false, use the same indentation level as outside block >>>> (file or namespace). >>>> >>> >>> Please adhere to the 80 column limit. Or ideally, just use clang-format >>> ;-). >>> >>> >>>> + bool IndentNamespaces; >>>> >>> >>> IndentInNamespaces might be slightly more precise, but I am not sure. >>> >>> >>>> - parseBlock(/*MustBeDeclaration=*/true, 0); >>>> + parseBlock(/*MustBeDeclaration=*/true, >>>> /*AddLevels=*/Style.IndentNamespaces); >>>> >>> >>> Please adhere to the 80 column limit. >>> >>> >>>> + FormatStyle getLLVMStyleWithIndentNamespaces(bool IndentNamespaces = >>>> true) { >>>> + FormatStyle Style = getLLVMStyle(); >>>> + Style.IndentNamespaces = IndentNamespaces; >>>> + return Style; >>>> + } >>>> + >>>> + FormatStyle getGoogleStyleWithIndentNamespaces(bool IndentNamespaces >>>> = true) { >>>> + FormatStyle Style = getGoogleStyle(); >>>> + Style.IndentNamespaces = IndentNamespaces; >>>> + return Style; >>>> + } >>>> >>> >>> I don't think it is worth adding these methods. Just define a local >>> style in your test: >>> >>> TEST_F(..., ...) { >>> FormatStyle Style = getLLVMStyle(); >>> Style.IndentNamespace = true; >>> ... >>> } >>> >>> >>>> On Fri, Jul 26, 2013 at 2:17 PM, Curdeius Curdeius >>>> <[email protected]>wrote: >>>> Hi, >>>> Clang-format was lacking an option to add indentation within namespaces. >>>> I've created a patch that adds this option to >>>> clang::Format::FormatStyle. >>>> There is also a basic test. >>>> You'll find it in the attachment. >>>> I think it will be nice to have a possibility to indent only the >>>> content of namespaces, but excluding the inner namespaces, i.e. to have >>>> something like: >>>> >>>> class Indented1; >>>> namespace inner1 { >>>> class Indented2; >>>> } // namespace inner1 >>>> namespace inner2 { >>>> } // namespace inner2 >>>> } // namespace outer >>>> instead of: >>>> >>>> class Indented1; >>>> namespace inner1 { >>>> class Indented2; >>>> } // namespace inner1 >>>> namespace inner2 { >>>> } // namespace inner2 >>>> } // namespace outer >>>> >>> >>> Really? I have never seen the latter before and it would confuse me >>> quite a bit. >>> >>> Cheers, >>> Daniel >>> >>> >> >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
