[PATCH] D99840: [clang-format] Correctly attach enum braces with ShortEnums disabled

MyDeveloperDay added a comment. Possible bug https://bugs.llvm.org/show_bug.cgi?id=51640 Comment at: clang/include/clang/Format/Format.h:547 /// false: - /// enum - /// { + /// enum { /// A, Post commit Nit: @lunasorcery Rule of thumb in

[PATCH] D99840: [clang-format] Correctly attach enum braces with ShortEnums disabled

lunasorcery added a comment. Looking at this again I think the change is flawed. The `InitialToken` is taken *after* the enum token, and additionally the invocation of `ShouldBreakBeforeBrace` is falling through to the `return false;` case every time. As such the call to `addUnwrappedLine()`

[PATCH] D99840: [clang-format] Correctly attach enum braces with ShortEnums disabled

This revision was automatically updated to reflect the committed changes. Closed by commit rG71616722d409: [clang-format] Correctly attach enum braces with ShortEnums disabled (authored by lunasorcery, committed by curdeius). Changed prior to commit:

[PATCH] D99840: [clang-format] Correctly attach enum braces with ShortEnums disabled

lunasorcery updated this revision to Diff 362220. lunasorcery added a comment. Sorry it took me so long to get back to this - fell off my radar somewhat. I've updated the release notes, I think it's ready to commit now? I lack commit rights (first time contributor!) so for attribution it'd be

[PATCH] D99840: [clang-format] Correctly attach enum braces with ShortEnums disabled

HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. In D99840#2718959 , @curdeius wrote: > LGTM. I also consider it a bug. LLVM should not be affected as it uses > `AllowShortEnumsOnASingleLine: true` whereas this problem

[PATCH] D99840: [clang-format] Correctly attach enum braces with ShortEnums disabled

curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM. I also consider it a bug. LLVM should not be affected as it uses `AllowShortEnumsOnASingleLine: true` whereas this problem arises only with `AllowShortEnumsOnASingleLine: false`.

[PATCH] D99840: [clang-format] Correctly attach enum braces with ShortEnums disabled

MyDeveloperDay added a comment. > I don't think we should change the LLVM style I'm not 100% convinced we are changing the LLVM style per say. LLVM always "cuddles" the "{" and Allow short Enums on a single line is true, this means if the "{" is on a new line then I think its actually a bug.

[PATCH] D99840: [clang-format] Correctly attach enum braces with ShortEnums disabled

HazardyKnusperkeks added a comment. While the fix seems to be right, I don't think we should change the LLVM style (and possibly other styles). So my suggestion is to apply the fix, but actually change the style so that it behaves like before, so that braces of enums are wrapped. And maybe

[PATCH] D99840: [clang-format] Correctly attach enum braces with ShortEnums disabled

lunasorcery created this revision. lunasorcery added reviewers: djasper, klimek. lunasorcery added a project: clang-format. lunasorcery requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Previously, with AllowShortEnumsOnASingleLine disabled,