[PATCH] D93938: [clang-format] Fixed AfterEnum handling

Sat Jan 2 06:14:39 PST 2021

curdeius added a comment.

In D93938#2476186 <https://reviews.llvm.org/D93938#2476186>, @MyDeveloperDay wrote:

> I think we are missing some clarity in this bug as to what the actual problem is, I do agree the test looks wrong,

I agree on this. If like to see a more exhaustive test suite for all the combinations of AfterEnum and AllowShortEnumsOnASingleLine, not only fixing the single wrong test.

> This seems to be that in "Attach" mode, then AllowShortEnumsOnASingleLine=false doesn't attach the brace.

That's my understanding as well.

> I'm somewhat struggling to understand how the code change fixes that.. given that Style.BraceWrapping.AfterEnum should be true correct?

In Attach mode, AfterEnum is false. Cf. `expandPresets` https://github.com/llvm/llvm-project/blob/47877c9079c27f19a954b660201ea47717c82fec/clang/lib/Format/Format.cpp#L752.

Said all that, it *seems* to me that the fix is correct apart from the strangely looking `if (!Style.isCpp()) {` change that I don't really understand. Why should C++ be handled differently in this regard? What am I missing?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93938/new/

https://reviews.llvm.org/D93938