This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] Fix putting ObjC message arguments in one line for multiline receiver
ClosedPublic

Authored by jolesiak on May 22 2018, 6:19 AM.

Details

Summary

Reapply reverted changes from D46879.

Currently BreakBeforeParameter is set to true everytime message receiver spans multiple lines, e.g.:

[[object block:^{
  return 42;
}] aa:42 bb:42];

will be formatted:

[[object block:^{
  return 42;
}] aa:42
   bb:42];

even though arguments could fit into one line. This change fixes this behavior.

Test Plan:
make -j12 FormatTests && tools/clang/unittests/Format/FormatTests

Diff Detail

Event Timeline

jolesiak created this revision.May 22 2018, 6:19 AM
jolesiak edited the summary of this revision. (Show Details)May 22 2018, 6:23 AM
krasimir added inline comments.May 22 2018, 7:59 AM
unittests/Format/FormatTestObjC.cpp
815

What's the receiver's scope in this comment referring to?
Also, how would the old test cases be formatted?

benhamilton accepted this revision.May 22 2018, 8:38 AM

Please address @krasimir's comments, but looks good to me.

This revision is now accepted and ready to land.May 22 2018, 8:38 AM
jolesiak added inline comments.May 22 2018, 8:50 AM
unittests/Format/FormatTestObjC.cpp
815

For a receiver: [obj a:42] I meant ] as a token closing the scope.
I'll rephrase the comment to be more precise.

Old tests were introduced in D46879. After this change the formatting will be the same as it was before D46879, i.e. the same as for last test touched in this change:

[[obj aaaaaa:42
           b:42]
    cc:42
     d:42];

even if

[[obj aaaaaa:42
           b:42]
    cc:42 d:42];

satisfies the column limit.

krasimir added inline comments.May 22 2018, 8:56 AM
unittests/Format/FormatTestObjC.cpp
815

Ah, I think get it now: the new code should only apply to after object blocks and not after object receivers? Is this the intention?

jolesiak added inline comments.May 22 2018, 9:44 AM
unittests/Format/FormatTestObjC.cpp
815

The intention is to put arguments into one line if they fit but only in the same line as last character of a receiver expression, e.g.:

[[object block:^{
  return 42;
}] aa:42 bb:42];

instead of

[[object block:^{
  return 42;
}] aa:42
   bb:42];

but not

[[obj a:42]
    a:42 b:42];

I think it gets a little bit too complicated for no reason. Let me revert D46879 and rebase this change.

Can you update the diff description to reflect that it includes the original change?

jolesiak retitled this revision from [clang-format] Fix ObjC message arguments handling to [clang-format] Fix putting ObjC message arguments in one line for multiline receiver.May 22 2018, 10:09 AM
jolesiak edited the summary of this revision. (Show Details)
jolesiak removed a reviewer: djasper.
jolesiak added a subscriber: djasper.
krasimir added inline comments.May 22 2018, 10:19 AM
unittests/Format/FormatTestObjC.cpp
815

In that case, would this be allowed?

//      limit:       V
[[obj a:42
      b:42
      c:42
      d:42] e:42 f:42]
jolesiak updated this revision to Diff 148169.May 23 2018, 1:05 AM
  • Add test
jolesiak added inline comments.May 23 2018, 1:06 AM
unittests/Format/FormatTestObjC.cpp
815

Yes, I added this test.

Does it look fine now @krasimir ?

krasimir accepted this revision.May 24 2018, 3:42 AM
This revision was automatically updated to reflect the committed changes.