[PATCH] D19069: clang-format: Fixed various brace wrapping and block merging bugs

[Daniel Jasper via cfe-commits]

djasper added inline comments.

================
Comment at: lib/Format/UnwrappedLineParser.cpp:1388-1390
@@ -1372,3 +1387,5 @@
     parseBlock(/*MustBeDeclaration=*/false);
-    if (Style.BraceWrapping.BeforeElse)
+    if (Style.BraceWrapping.BeforeElse ||
+        Style.BraceWrapping.AfterControlStatement &&
+            Style.BraceWrapping.IndentBraces)
       addUnwrappedLine();
----------------
mxbOctasic wrote:
> IndentBraces can only work properly when both AfterControlStatement and BeforeElse are active.
> The opening brace can only be indented when it is wrapped on its own line, and the else keyword has to be on another line too.
> We have to decide which option has precedence over the others.
> Right now I'm overriding BeforeElse when the opening brace is wrapped.
Maybe we should put logic like this into expandPresets (possibly renaming it)?

================
Comment at: unittests/Format/FormatTest.cpp:427
@@ +426,3 @@
+
+  AllowSimpleBracedStatements.BreakBeforeBraces = FormatStyle::BS_Custom;
+  AllowSimpleBracedStatements.BraceWrapping.AfterControlStatement = true;
----------------
I think we need a better way to structure these tests. And reusing a style with the name "AllowSimpleBracedStatements" doesn't make sense here.

================
Comment at: unittests/Format/FormatTest.cpp:474
@@ +473,3 @@
+  EXPECT_EQ("if (true) { f(); }\nelse { f(); }",
+            format("if (true)\n"
+                   "{\n"
----------------
I'd remove all the line breaks and "\n"s here.

================
Comment at: unittests/Format/FormatTest.cpp:2454-2456
@@ -2340,3 +2453,5 @@
   // Function-level try statements.
-  verifyFormat("int f() try { return 4; } catch (...) {\n"
+  verifyFormat("int f() try {\n"
+               "  return 4;\n"
+               "} catch (...) {\n"
                "  return 5;\n"
----------------
mxbOctasic wrote:
> This test probably should not have been changed.
> However, it's strange to have the try on one line but not the catch.
Yeah, I agree.


http://reviews.llvm.org/D19069