[PATCH] Update block formatting to behave like Xcode

Brian King brianaking at gmail.com
Tue May 5 10:59:33 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:
> KingOfBrian wrote:
> > 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.
> The fundamental question is, whether we want clang-format to support this style. I don't think it is useful to support it, just because it is what some other tool creates. That would mean that clang-format would have to implement all the flaws of other indentation tools, which we certainly do not want to do.
> 
> That being said, if this is widely used and ideally documented somewhere, it might be worth adding. But we certainly need to define it precisely in the comment (especially is there is no other documentation) and we need to make it work with a non-zero column limit.
After thinking about this some, this does 2 separate style modifications, and if the goal is a discoverable config format, I these two things could be reflected in 2 different configuration options. 

The first change is to not do left compression suggested by the google style. Also note that in the google style it states 'inlined blocks may have their segments left-aligned ... This helps when invocations contain multiple inlined blocks'. All this change does is provide the option to not perform this special case formatting (which is allowed by the style guide). What do you think about this being reflected via a 'ObjcLeftAlignMultipleBlocks` option?

The second change is to not force line breaks on multiple-argument ObjCMethodExpr. This only works with zero column limit, I have to hunt down the other code path for a column limit. 

Your argument of not re-creating bad styles makes a lot of sense to me. I want to make sure that we agree that the style in the test on line 9855 is worth re-creating. This format is used a lot, although it's not explicitly documented anywhere that I can tell.  The other styles are incidental to getting that working, although they have the bonus of matching the current Xcode tool-chain. I'm not going to be disappointed if the other formats don't work, but it's going to be very hard to get adoption in the iOS world if the styles I mentioned above aren't re-creatable. (also -- going to comment on them explicitely incase numbers change)

================
Comment at: unittests/Format/FormatTest.cpp:9860
@@ +9859,3 @@
+               "}];",
+               AppleStyle);
+  verifyFormat("[UIView animateWithDuration:0 // This will force colon alignment\n"
----------------
The style above is the block format I'm looking to re-create.

http://reviews.llvm.org/D9237

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






More information about the cfe-commits mailing list