This is an archive of the discontinued LLVM Phabricator instance.

clang-format: add options to merge empty record body
ClosedPublic

Authored by Typz on Jun 20 2017, 4:45 AM.

Details

Summary

This patch introduces a few extra BraceWrapping options, similar to
SplitEmptyFunction, to allow merging empty 'record' bodies (e.g.
class, struct, union and namespace):

  • SplitEmptyClass
  • SplitEmptyStruct
  • SplitEmptyUnion
  • SplitEmptyNamespace

The SplitEmptyFunction option name has also been simplified/
shortened (from SplitEmptyFunctionBody).

These options are helpful when the correspond AfterXXX option is
enabled, to allow merging the empty record:

class Foo
{};

In addition, this fixes an unexpected merging of short records, when
the AfterXXXX options are used, which caused to be formatted like
this:

class Foo
{ void Foo(); };

This is now properly formatted as:

class Foo
{
   void Foo();
};

Diff Detail

Repository
rL LLVM

Event Timeline

Typz created this revision.Jun 20 2017, 4:45 AM
Typz updated this revision to Diff 103202.Jun 20 2017, 6:43 AM

Enable merging records for Mozilla style

djasper edited edge metadata.Jun 25 2017, 11:10 PM

Do you know of a style guide that would actually want to handle class, structs and unions differently? In most of Clang, they are handled as "records" and fundamentally, they are so alike that I'd hope that people always want the same behavior for all of them.

Typz added a comment.Jun 26 2017, 1:28 AM

I don't know if some style would want different styles, and I agree with you on principle; but since the brace wrapping is already configured for each kind of record, I choose to keep things consistent and have flags for each kind of record.
But I can merge the options, and keep only SplitEmptyRecord and SplitEmptyNamespace, if you really think it is better.

Yes merge them into those two, please. I think we introduced the others because of some linux style, but generally lets try not to introduce options that people aren't going to use.

Typz updated this revision to Diff 103939.Jun 26 2017, 6:33 AM

Merge SplitEmptyClass/Struct/Union options into a single SplitEmptyRecord option.

djasper accepted this revision.Jun 26 2017, 6:34 AM

Looks good.

This revision is now accepted and ready to land.Jun 26 2017, 6:34 AM
This revision was automatically updated to reflect the committed changes.