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.
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. 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 > >
clang-format-IndentInNamespaces.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
