[PATCH] Update block formatting to behave like Xcode

Brian King brianaking at gmail.com
Wed Apr 29 09:44:19 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;
----------------
djasper wrote:
> 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.
I haven't been able to find it documented anywhere, I looked around for a while. But it's been the same formatting in Xcode since blocks were introduced. I'll add some more tests for long selector-parts and see how it behaves.

I'm a bit stumped on the naming for behavior so I opted to name it based on the origin -- similar in approach to `allman` vs `soustroup`. I'm not sure there's a name that is discoverable, since the difference between this and the current google style is pretty nuanced. Would changing it to a style enumeration be more acceptable?

I added the prefix ObjC to imply that this only affects the behavior of blocks in method exprs, obviously that was not clear enough.

Also, reading the comment again, it's wrong, sorry about that. The comment explains the inverse of one of the two behavior changes - I'll take another stab at it, but without a good name for the flag, I'm a bit stumped. Let me think about it.

================
Comment at: unittests/Format/FormatTest.cpp:9804
@@ +9803,3 @@
+  FormatStyle AppleStyle = getLLVMStyle();
+  AppleStyle.ColumnLimit = 0;
+  AppleStyle.IndentWidth = 4;
----------------
djasper wrote:
> 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.
Right, let me look into that.

================
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"
----------------
djasper wrote:
> 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.
I think Apple developers may have a degree of Stockholm syndrome here, but it is a style that people want at this point. I dislike it personally too, and usually don't do line breaks so it left aligns. If I do, I keep the blocks very short, or break them out into a variable. 

Multi-block obj-c can get ugly no matter what way you cut it. For instance, I thought the multi-block google style was so terrible that it was a bug!





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

http://reviews.llvm.org/D9237

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






More information about the cfe-commits mailing list