[cfe-commits] r172333 - in /cfe/trunk: lib/Format/Format.cpp unittests/Format/FormatTest.cpp

[Nico Weber]

On Mon, Jan 14, 2013 at 9:33 AM, Jordan Rose <jordan_rose at apple.com> wrote:
> The ultimate goal in Apple-style is to switch to one selector piece per line, aligned by colons, when a message expr needs to be broken up. (I will say this wasn't even usually my personal style before joining Apple, but it is what our docs usually do and what Xcode currently does when autoindenting code.)

This is the ultimate goal for google style too. It's tracked in PR14939.

>
> Still, nice improvement!
>
> Jordan
>
>
> On Jan 12, 2013, at 14:48 , Nico Weber <nicolasweber at gmx.de> wrote:
>
>> Author: nico
>> Date: Sat Jan 12 16:48:47 2013
>> New Revision: 172333
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=172333&view=rev
>> Log:
>> Formatter: Prefer breaking before ObjC selector names over breaking at their ':'
>>
>> Before:
>>  if ((self = [super initWithContentRect:contentRect styleMask:
>>                  styleMask backing:NSBackingStoreBuffered defer:YES])) {
>>
>> Now:
>>  if ((self = [super initWithContentRect:contentRect styleMask:styleMask
>>                  backing:NSBackingStoreBuffered defer:YES])) {
>>
>>
>>
>> Modified:
>>    cfe/trunk/lib/Format/Format.cpp
>>    cfe/trunk/unittests/Format/FormatTest.cpp
>>
>> Modified: cfe/trunk/lib/Format/Format.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=172333&r1=172332&r2=172333&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Format/Format.cpp (original)
>> +++ cfe/trunk/lib/Format/Format.cpp Sat Jan 12 16:48:47 2013
>> @@ -170,7 +170,7 @@
>>
>> /// \brief Checks whether the (remaining) \c UnwrappedLine starting with
>> /// \p RootToken fits into \p Limit columns.
>> -bool fitsIntoLimit(const AnnotatedToken &RootToken, unsigned Limit) {
>> +static bool fitsIntoLimit(const AnnotatedToken &RootToken, unsigned Limit) {
>>   unsigned Columns = RootToken.FormatTok.TokenLength;
>>   bool FitsOnALine = true;
>>   const AnnotatedToken *Tok = &RootToken;
>> @@ -188,6 +188,15 @@
>>   return FitsOnALine;
>> }
>>
>> +/// \brief Returns if a token is an Objective-C selector name.
>> +///
>> +/// For example, "bar" is a selector name in [foo bar:(4 + 5)]
>> +static bool isObjCSelectorName(const AnnotatedToken &Tok) {
>> +  return Tok.is(tok::identifier) && !Tok.Children.empty() &&
>> +         Tok.Children[0].is(tok::colon) &&
>> +         Tok.Children[0].Type == TT_ObjCMethodExpr;
>> +}
>> +
>> class UnwrappedLineFormatter {
>> public:
>>   UnwrappedLineFormatter(const FormatStyle &Style, SourceManager &SourceMgr,
>> @@ -479,6 +488,14 @@
>>     if (Left.is(tok::semi) || Left.is(tok::comma) ||
>>         Left.ClosesTemplateDeclaration)
>>       return 0;
>> +
>> +    // In Objective-C method expressions, prefer breaking before "param:" over
>> +    // breaking after it.
>> +    if (isObjCSelectorName(Right))
>> +      return 0;
>> +    if (Right.is(tok::colon) && Right.Type == TT_ObjCMethodExpr)
>> +      return 20;
>> +
>>     if (Left.is(tok::l_paren))
>>       return 20;
>>
>> @@ -1188,6 +1205,8 @@
>>       return false;
>>     if (Left.is(tok::colon) && Left.Type == TT_ObjCMethodExpr)
>>       return true;
>> +    if (isObjCSelectorName(Right))
>> +      return true;
>>     if (Left.ClosesTemplateDeclaration)
>>       return true;
>>     if (Left.Type == TT_PointerOrReference || Left.Type == TT_TemplateCloser ||
>>
>> Modified: cfe/trunk/unittests/Format/FormatTest.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=172333&r1=172332&r2=172333&view=diff
>> ==============================================================================
>> --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
>> +++ cfe/trunk/unittests/Format/FormatTest.cpp Sat Jan 12 16:48:47 2013
>> @@ -1537,12 +1537,18 @@
>>   verifyFormat("throw [self errorFor:a];");
>>   verifyFormat("@throw [self errorFor:a];");
>>
>> -  // The formatting of this isn't ideal yet. It tests that the formatter doesn't
>> -  // break after "backing" but before ":", which would be at 80 columns.
>> +  // This tests that the formatter doesn't break after "backing" but before ":",
>> +  // which would be at 80 columns.
>>   verifyFormat(
>>       "void f() {\n"
>> -      "  if ((self = [super initWithContentRect:contentRect styleMask:\n"
>> -      "                  styleMask backing:NSBackingStoreBuffered defer:YES]))");
>> +      "  if ((self = [super initWithContentRect:contentRect styleMask:styleMask\n"
>> +      "                  backing:NSBackingStoreBuffered defer:YES]))");
>> +
>> +  verifyFormat("[foo setasdfasdffffffffffffadfasdfasdf:\n"
>> +               "    [bar dowith:asdfdsfasdfasdfasfasfasfsafasdfsfad]];");
>> +
>> +  verifyFormat("[foo checkThatBreakingAfterColonWorksOk:\n"
>> +               "    [bar ifItDoes:reduceOverallLineLengthLikeInThisCase]];");
>>
>> }
>>
>> @@ -1582,6 +1588,7 @@
>>   verifyFormat("@'c'");
>>   verifyFormat("@true");
>>   verifyFormat("NSNumber *smallestInt = @(-INT_MAX - 1);");
>> +  // FIXME: Array and dictionary literals need more work.
>>   verifyFormat("@[");
>>   verifyFormat("@{");
>>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits