[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