[PATCH] Update block formatting to behave like Xcode

Daniel Jasper djasper at google.com
Wed Apr 29 04:52:26 PDT 2015


================
Comment at: include/clang/Format/Format.h:225
@@ -224,1 +224,3 @@
 
+  /// \brief Compress the selector parts when more than two inline blocks are specified
+  bool ObjCXcodeBlockFormat;
----------------
I this style documented somewhere? There are many corner cases that you aren't explicitly testing. E.g. what if there are non-block selectors in between? What if the selectors before the first block do not fit on one line?

ObjCXcodeBlockFormat is not a good name. It would be good to have something that actually describes the behavior. Like this it is not really discoverable. And this does not really change the behavior of blocks in general, but the behavior of blocks in method exprs, IIUC.

Also, the comment needs to be quite a bit more elaborate.

================
Comment at: lib/Format/ContinuationIndenter.cpp:419
@@ -418,2 +418,3 @@
       if (NextNonComment->LongestObjCSelectorName == 0) {
+        // This catches keys in Literal NSDictionaries
         State.Stack.back().AlignColons = false;
----------------
Either remove the comment again or make it more elaborate. As is, it seems more confusing.

================
Comment at: lib/Format/ContinuationIndenter.cpp:857
@@ -855,3 +856,3 @@
               (Tok->CanBreakBefore && Tok->NewlinesBefore > 0)) {
-            BreakBeforeParameter = true;
+            BreakBeforeParameter = Style.ObjCXcodeBlockFormat == false;
             break;
----------------
!Style.ObjCXcodeBlockFormat

================
Comment at: unittests/Format/FormatTest.cpp:9803
@@ +9802,3 @@
+TEST_F(FormatTest, FormatsXcodeStyleBlocks) {
+  FormatStyle AppleStyle = getLLVMStyle();
+  AppleStyle.ColumnLimit = 0;
----------------
Again, is there some documentation of what "Apple style" is?

================
Comment at: unittests/Format/FormatTest.cpp:9804
@@ +9803,3 @@
+  FormatStyle AppleStyle = getLLVMStyle();
+  AppleStyle.ColumnLimit = 0;
+  AppleStyle.IndentWidth = 4;
----------------
It seems quite weird that this style would only affect ColumnLimit 0. If you'd want to add it, we need to make it work for styles with column limits, too.

================
Comment at: unittests/Format/FormatTest.cpp:9818-9821
@@ +9817,6 @@
+               "               }\n"
+               "              secondBlock:^(Bar *b) {\n"
+               "                  // ...\n"
+               "                  int i;\n"
+               "              }\n"
+               "               thirdBlock:^Foo(Bar *b) {\n"
----------------
Whether Xcode does this or not, my personal opinion is that this is quite terrible. The alignment of a few colons that are many lines a part is next to useless where as this looks very messy, especially if the difference between block selector names is more than 1 character.

================
Comment at: unittests/Format/FormatTest.cpp:9861
@@ +9860,3 @@
+               AppleStyle);
+  verifyFormat("[UIView animateWithDuration:0 // This will force colon alignment\n"
+               "                 animations:^{\n"
----------------
What is this testing that's not already tested above?

http://reviews.llvm.org/D9237

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list