[PATCH] D42650: [clang-format] New format param BinPackObjCProtocolList

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore requested changes to this revision.
stephanemoore added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/Format/ContinuationIndenter.cpp:1232
+
+    bool BinPackParameters =
+        (State.Line->Type != LT_ObjCDecl && Style.BinPackParameters) ||
----------------
Given that we now have a new setting distinguishing bin packing of Objective-C protocol conformance lists and bin packing of parameters perhaps this local variable should be named more generally? Maybe "BinPackDeclaration" would be appropriate because this local variable takes effect if `State.Line->MustBeDeclaration` is true?


================
Comment at: lib/Format/Format.cpp:765
     GoogleStyle.ColumnLimit = 100;
+    GoogleStyle.BinPackObjCProtocolList = FormatStyle::BPS_Never;
   }
----------------
I propose that we defer this to a future commit while we establish consensus at Google.


================
Comment at: unittests/Format/FormatTestObjC.cpp:325-334
+  Style.ColumnLimit = 40;
+  // BinPackParameters should be true by default.
+  verifyFormat("void eeeeeeee(int eeeee, int eeeee,\n"
+               "              int eeeee, int eeeee);\n");
+  // BinPackObjCProtocolList should be BPS_Never by default.
+  verifyFormat("@interface fffffffffffff () <\n"
+               "    fffffffffffff,\n"
----------------
Similar to my other comment, I propose deferring the changes to Google style to a future commit.


Repository:
  rC Clang

https://reviews.llvm.org/D42650