[PATCH] D11851: clang-format: Add "AllowShortNamespacesOnASingleLine" option
Daniel Jasper via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 10 00:29:50 PDT 2015
djasper added inline comments.
================
Comment at: include/clang/Format/Format.h:115
@@ -114,1 +114,3 @@
+ /// \brief If \c true, <tt> namespace a { class b; } </tt> can be put
+ /// on a single line
----------------
Don't use markup here. Use a \code block instead.
================
Comment at: lib/Format/UnwrappedLineFormatter.cpp:204
@@ +203,3 @@
+ TheLine->First->is(tok::kw_namespace)) {
+ if (unsigned result = tryMergeNamespace(I, E, Limit)) {
+ return result;
----------------
No braces.
================
Comment at: lib/Format/UnwrappedLineFormatter.cpp:267
@@ -260,1 +266,3 @@
+ unsigned tryMergeNamespace(
+ SmallVectorImpl<AnnotatedLine *>::const_iterator I,
----------------
How is this different from tryMergeSimpleBlock, in particular, which behavior differences do you want?
================
Comment at: unittests/Format/FormatTest.cpp:10441
@@ +10440,3 @@
+ verifyFormat("namespace foo { class bar; }", style);
+ verifyFormat("namespace foo {\nclass bar;\nclass baz;\n}", style);
+ verifyFormat("namespace foo {\nint f() { return 5; }\n}", style);
----------------
Please insert a pair of quotes ("") after each '\n' and format, so that this becomes a concatenated multiline string.
What about:
namespace A { namespace B { class C; }}
I think this is what many people would use. It's fine not to fix this in this patch, but at least denote the behavior with a corresponding test.
Repository:
rL LLVM
http://reviews.llvm.org/D11851
More information about the cfe-commits
mailing list