Hi,
Found a bug introduced with BraceWrappingFlags AfterControlStatement MultiLine. This feature conflicts with the existing BeforeCatch and BeforeElse flags.
For example, our team uses BeforeElse.
if (foo || bar) { doSomething(); } else { doSomethingElse(); }
If we enable MultiLine (which we'd really love to do) we expect it to work like this:
if (foo || bar) { doSomething(); } else { doSomethingElse(); }
What we actually get is:
if (foo || bar) { doSomething(); } else { doSomethingElse(); }
I added two tests that show this issue. One for if/else and one for try/catch (which is affected in the same way as if/else).
One more test with ColumnLimit set to 40 is to check that a suggested fix doesn't break existing cases. This test is the copy of an existing test from the same case, but it removes line wrap from the stage.
I suggest the fix, but I'm new to this code, so maybe you could suggest something better.
As far as I understood, the problem is caused by the following code:
CompoundStatementIndenter(UnwrappedLineParser *Parser, const FormatStyle &Style, unsigned &LineLevel) : CompoundStatementIndenter(Parser, LineLevel, Style.BraceWrapping.AfterControlStatement, // <----- here Style.BraceWrapping.IndentBraces) {} CompoundStatementIndenter(UnwrappedLineParser *Parser, unsigned &LineLevel, bool WrapBrace, bool IndentBrace) : LineLevel(LineLevel), OldLineLevel(LineLevel) { if (WrapBrace) // <----- and here Parser->addUnwrappedLine(); if (IndentBrace) ++LineLevel; }
Style.BraceWrapping.AfterControlStatement used to be boolean, now it's an enum. MultiLine and Always both turn into true, thus MultiLine leads to a call of addUnwrappedLine().
I tried to place Style.BraceWrapping.AfterControlStatement == FormatStyle::BWACS_Always right in the CompoundStatementIndenter constructor, but that breaks MultiLine.
As I said, I'm new to this code, so maybe my fix is not at the right place, so I would be glad if we find a better fix.