Thank you!

@wanders, thank you for digging through code to get a feeling of the usage!

• try to update Phabricator diff

• refresh Phabricator diff

• add a FIXME

• add a regression test

Thank you!

Thank you!

Another weak argument about why the current rule makes sense (break before << between two string literals) is: suppose you don't want such a break. Then you can change your code to concatenate the two string literals into a single one, avoiding the problem altogether, if possible (this can be unacceptable in cases where the combined length of the two string literals is excessive, but at that point we will likely break because of the column limit; or when there is a chain of more than 2 string literals and the user wishes to keep them fluently reflowing as the code evolves.)

In D50078#2053324, @krasimir wrote:

I'm happy with this patch!

We found a couple of rough edges:

• alignment after ?: and
• new formatting of _ ? _ ? _ : _ : _ patterns is bad

These are illustrated as examples D and E below (A, B and C look good to me). test.cc is how I'd expect clang-format to behave with this patch with BreakBeforeTernaryOperators = true, like in LLVM style.

• alignment after ?:: this is a GNU extension and we've seen it used in C++ and ObjC: https://stackoverflow.com/questions/24538096/in-objective-c. I think this is special enough for us to consider an occurrence of ?: to break a ternary operator chain for the purposes of this alignment. I'd expect a +4 alignment of the last 2 lines of example D for consistency with A.

• new formatting doesn't work for _ ? _ ? _ : _ : _ patterns; old formatting is better in that case. I think the new chained alignment works very well for _ ? _ : _ ? _ : _ ... cases, but we should attempt it otherwise.

% cat test.cc
//-- column 80 ----------------------------------------------------------------V
// A
int i = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa 
            ? bbbbbbbbbbbbbbbbbbb
            : cccccccccccccccccccc;
 
//-- column 80 ----------------------------------------------------------------V
// B
int i = aaaaaaaaaaaaaaaaaaaaaaaaaa ? bbbbbbbbbbbbbbbbbbb
        : cccccccccccccccccccc     ? dddddddddddddddddddd
                                   : eeeeeeeeeeeeeeeee;
 
//-- column 80 ----------------------------------------------------------------V
// C
int i = aaaaaaaaaaaaaaaaaaaaaaaaaa
            ?: bbbbbbbbbbbbbbbbbbb ? ccccccccccccc
                                   : ddddddddd;
 
//-- column 80 ----------------------------------------------------------------V
// D
int i = aaaaaaaaaaaaaaaaaaaaaaaaaa
            ?: bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
                   ? bbbbbbbbbbbbbbbbbbb
                   : cccccccccccccccccccc;
 
//-- column 80 ----------------------------------------------------------------V
// E 
return temp[0] > temp[1] ? temp[0] > temp[2] ? 0 : 2
                         : temp[1] > temp[2] ? 1 : 2;

% bin/clang-format -style=llvm test.cc
//-- column 80 ----------------------------------------------------------------V
// A
int i = aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
            ? bbbbbbbbbbbbbbbbbbb
            : cccccccccccccccccccc;
 
//-- column 80 ----------------------------------------------------------------V
// B
int i = aaaaaaaaaaaaaaaaaaaaaaaaaa ? bbbbbbbbbbbbbbbbbbb
        : cccccccccccccccccccc     ? dddddddddddddddddddd
                                   : eeeeeeeeeeeeeeeee;
 
//-- column 80 ----------------------------------------------------------------V
// C
int i = aaaaaaaaaaaaaaaaaaaaaaaaaa
            ?: bbbbbbbbbbbbbbbbbbb ? ccccccccccccc
                                   : ddddddddd;
 
//-- column 80 ----------------------------------------------------------------V
// D
int i = aaaaaaaaaaaaaaaaaaaaaaaaaa
            ?: bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
               ? bbbbbbbbbbbbbbbbbbb
               : cccccccccccccccccccc;
 
//-- column 80 ----------------------------------------------------------------V
// E
return temp[0] > temp[1]   ? temp[0] > temp[2] ? 0 : 2
       : temp[1] > temp[2] ? 1
                           : 2;

This is nice! Thank you!

In D80144#2054451, @MyDeveloperDay wrote:

Will this help? D80547: [clang-format] Fix an ObjC regression introduced with new [[likely]][[unlikely]] support in if/else clauses

In D80144#2053543, @MyDeveloperDay wrote:

maybe for now we just do

if (FormatTok->is(tok::l_square) && !Style.isObjC())
    parseSquare();

Unfortunately this introduced a bad regression with ObjC method expressions in if bodies:

if (argc > 5) [obj func:arg];

is now formatted as:

if (argc > 5)[obj func:arg]
  ;

I'm happy with this patch!

Just some thoughts.

Thank you!

Awesome! Thank you!