This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] Apply Allman style to lambdas
ClosedPublic

Authored by HazardyKnusperkeks on Jan 18 2021, 4:20 AM.

Diff Detail

Event Timeline

HazardyKnusperkeks requested review of this revision.Jan 18 2021, 4:20 AM
HazardyKnusperkeks created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJan 18 2021, 4:20 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Formatting corrected.

I think this is LGTM, however..

I've always felt that we shouldn't require BreakBeforeBraces to be Custom in order to be able to use the BraceWrapping.XXXX

Users in my view should be able to say:

BreakBeforeBraces: Allman
BraceWrapping:
   BeforeLambdaBody: true

This would mean users could pick the "Closest" style to theirs and then modify just the fields.

Either that or using BraceWrapping with a non Custom style should be a warning.

It doesn't take much to find .clang-format files on github that look like this

MyDeveloperDay added a comment.EditedJan 18 2021, 10:08 AM

Similarly I see users having to define everything just to turn one thing off

MyDeveloperDay accepted this revision.Jan 18 2021, 10:09 AM
This revision is now accepted and ready to land.Jan 18 2021, 10:09 AM

I think that would have helped me also. I think it's not that easy, but honestly never checked.

curdeius accepted this revision.Jan 18 2021, 12:23 PM

LGTM.
As a note, I agree with @MyDeveloperDay.
It's more user-friendly and IMO less surprising to set the base wrapping style in BreakBeforeBraces and then customize it changing in BraceWrapping.
I'd argue that it won't be a breaking change for .clang-format configs that are "correct" and would just make the examples you brought above work as intended.
Current behaviour is even strange in one case, what values are set to elements *omitted* in BraceWrapping when BreakBeforeBraces is Custom? Those from LLVM style?
Anyway, that goes way beyond this patch. I'd think about implementing this time permitting.

This revision was automatically updated to reflect the committed changes.