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

[Maxime Beaulieu via cfe-commits]

mxbOctasic added inline comments.

================
Comment at: lib/Format/UnwrappedLineParser.cpp:789-790
@@ -773,4 +788,4 @@
       if (FormatTok->Tok.is(tok::l_brace)) {
-        if (Style.BraceWrapping.AfterObjCDeclaration)
-          addUnwrappedLine();
+        CompoundStatementIndenter Indenter(
+            this, Style, Line->Level, Style.BraceWrapping.AfterObjCDeclaration);
         parseBlock(/*MustBeDeclaration=*/false);
----------------
mxbOctasic wrote:
> We may not want this.
Type is set to TT_CompoundStatementLBrace by the CompoundStatementIndenter.

Should it be TT_ObjCBlockLBrace instead?

Should the braces be affected by BraceWrapping.IndentBraces?

================
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();
----------------
djasper wrote:
> 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)?
Something like "applyBraceWrappingFlags"?
The preset styles do not set incompatible flags (yet?).
Do we want to perform fix-ups like this only on custom styles?

I will update the \brief in Format.h to mention that some flags can be overridden in certain edge cases.

================
Comment at: unittests/Format/FormatTest.cpp:474
@@ +473,3 @@
+  EXPECT_EQ("if (true) { f(); }\nelse { f(); }",
+            format("if (true)\n"
+                   "{\n"
----------------
djasper wrote:
> I'd remove all the line breaks and "\n"s here.
Should we consider the BeforeElse flag when merging this statement?




http://reviews.llvm.org/D19069