Page MenuHomePhabricator

[clang-format] Do not break after ObjC category open paren
ClosedPublic

Authored by benhamilton on Apr 11 2018, 10:05 AM.

Details

Summary

Previously, clang-format would break Objective-C
category extensions after the opening parenthesis to avoid
breaking the protocol list:

% echo "@interface ccccccccccccc (ccccccccccc) <ccccccccccccc> { }" | \
  clang-format -assume-filename=foo.h -style="{BasedOnStyle: llvm, \
  ColumnLimit: 40}"
@interface ccccccccccccc (
    ccccccccccc) <ccccccccccccc> {
}

This looks fairly odd, as we could have kept the category extension
on the previous line.

Category extensions are a single item, so they are generally very
short compared to protocol lists. We should prefer breaking after the
opening < of the protocol list over breaking after the opening (
of the category extension.

With this diff, we now avoid breaking after the category extension's
open paren, which causes us to break after the protocol list's
open angle bracket:

% echo "@interface ccccccccccccc (ccccccccccc) <ccccccccccccc> { }" | \
  ./bin/clang-format -assume-filename=foo.h -style="{BasedOnStyle: llvm, \
  ColumnLimit: 40}"
@interface ccccccccccccc (ccccccccccc) <
    ccccccccccccc> {
}

Test Plan: New test added. Confirmed test failed before diff and

passed after diff by running:
% make -j16 FormatTests && ./tools/clang/unittests/Format/FormatTests

Diff Detail

Repository
rL LLVM

Event Timeline

benhamilton created this revision.Apr 11 2018, 10:05 AM

Both patch and comment inside patch say "break after closing paren" and yet that's not what the test or example in the description actually do. Why is that?

Fair point. I'll update the diff description.

The net effect of the patch is to break after the closing paren; the way I
implemented that was to raise the penalty for breaking after the opening
paren.

I understand that, but the test example does not break after the closing paren. It breaks after the subsequent "<".

True. I will reword the description to clarify.

djasper accepted this revision.Apr 12 2018, 7:55 AM

Looks good.

lib/Format/TokenAnnotator.cpp
2276 ↗(On Diff #142033)

This is still using the old wording.

This revision is now accepted and ready to land.Apr 12 2018, 7:55 AM
benhamilton retitled this revision from [clang-format] Prefer breaking after ObjC category close paren to [clang-format] Do not break after ObjC category open paren.Apr 12 2018, 8:01 AM
benhamilton edited the summary of this revision. (Show Details)
benhamilton marked an inline comment as done.
  • Update comment.
lib/Format/TokenAnnotator.cpp
2276 ↗(On Diff #142033)

Fixed, thanks.

This revision was automatically updated to reflect the committed changes.