But please wait for other opinions.

@kadircet shouldn't you at least say why it got reverted? Even better state your problem and give a chance to fix before you revert?

Will be fixed on commit.

In D158363#4606159, @d0nc1h0t wrote:

In D158363#4604468, @HazardyKnusperkeks wrote:

Please upload the patch with the full context.

I'm creating a patch via 'git diff' from the root of the project. What does full context mean?

Please upload the patch with the full context. And add a test case. (From your issue.)

In D156259#4558570, @jp4a50 wrote:

@HazardyKnusperkeks could you merge this for me assuming the build is green please? I don't have merge rights. Thanks.

In D156370#4537033, @gedare wrote:

In D156370#4536793, @HazardyKnusperkeks wrote:

Does this result in a different annotation? Could you add a test for that?

If I understand you correctly, it does, for example:
$ echo "void __attribute__((naked)) foo(int bar);" | clang-format -style=llvm -debug 2>&1 | grep \'foo\'

Without the change:
M=0 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=23 Name=identifier L=31 PPK=2 FakeLParens= FakeRParens=0 II=0x555d17efdce0 Text='foo'

With the change:
M=0 C=1 T=FunctionDeclarationName S=1 F=0 B=0 BK=0 P=80 Name=identifier L=31 PPK=2 FakeLParens= FakeRParens=0 II=0x559855502ce0 Text='foo'

I don't how to test for that.

Does this result in a different annotation? Could you add a test for that?

You need some tests to show that the new options really apply like intended.

In D138263#4535269, @MyDeveloperDay wrote:

In D138263#3936536, @HazardyKnusperkeks wrote:

In D138263#3936007, @owenpan wrote:

I suppose it's fairly easy to annotate the l_brace of a namespace? If so, then wouldn't it be better to do that?

But the r_brace has no MatchingParen, and I didn't want to go into that hole.

How many times has that caught you out though?....I could have used this SO many times to constraint stuff... I wonder how painful a BraceMatchingPass would be, labelling all BraceTypes to Match each others.

All of those variants are fine by me, but I'd stick with the current version, because it is linked to AllowShortCaseLabelsOnASingleLine.

For the replacement part, I refer to @owenpan and/or @MyDeveloperDay hoping they have more insight.
The rest look good to me.

Nice work.

In D155239#4520081, @gedare wrote:

In D155239#4520067, @HazardyKnusperkeks wrote:

Everything is fine, I just need to know how the attribute stuff is formatted with plain LLVM style.

oh, sorry. plain llvm style. I will look for a good place to add that.

Everything is fine, I just need to know how the attribute stuff is formatted with plain LLVM style.

Apart from that I'm okay.

Please provide a diff with the full context (-U999999 will do the trick).

D155529#inline-1505573
But what does clang-format without this patch format? I just want to make sure, that we don't have a regression when this lands, but D155529 not.

If you limit it to Never I don't see any value in the differentiation. You could just always use Custom (by dropping the custom and only having the nested options).

Can you mark this change as closed?
If you'd put "Differential Revision: https://reviews.llvm.org/D153589" in your commit message this would have happened automatically.

In D154484#4483659, @sstwcw wrote:

Did you forget about GNU statement expressions?

if (({
      foo();
      bar();
    })) {
}

Thanks for the patience, I'm really looking forward to use this.

Please wait for other opinions.

In D151761#4474136, @galenelias wrote:

I re-wrote the alignment to stop using AlignTokens so that I can now handle all the edge cases that came up. Specifically:

• Allowing empty case labels (implicit fall through) to not break the alignment, but only if they are sandwiched by short case statements.
• Don't align the colon of a non-short case label that follows short case labels when using 'AlignCaseColons=true'.
• Empty case labels will also now push out the alignment of the statements when using AlignCaseColons=false.

Also, this now avoids having to add the IgnoreNestedScopes parameter to AlignTokens which didn't feel great.

I refactored AlignMacroSequence so I could re-use the core aligning method, since it's the same logic I need, but I removed some of the dead code just to simplify things along the way. But even with that, this version is quite a bit more code (~100 lines vs. ~30 lines).

In D154093#4466597, @sstwcw wrote:

In D154093#4466246, @HazardyKnusperkeks wrote:

I'd really prefer you put what you need to modify mutable instead of removing the const from everything else. But that's just my opinion.

Why should the insertBreak method be const?

Give the author some time is okay, but I think we can and should commandeer at some point and abandon.

I'd really prefer you put what you need to modify mutable instead of removing the const from everything else. But that's just my opinion.

Is this still an issue?

In D153641#4444465, @rymiel wrote:

@HazardyKnusperkeks I'm not sure why it didn't recurse already, given that you even documented that it doesn't, but I chose to trust in the Beyoncé rule.