[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D151761#4474136 <https://reviews.llvm.org/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).

Nice work!



================
Comment at: clang/include/clang/Format/Format.h:327
+  /// \version 17
+  AlignConsecutiveStyle AlignConsecutiveShortCaseStatements;
 
----------------
Since you are not using AlignTokens anymore, I'd say use your own struct. You don't have the not used memberand you don't have to add a new member which is not used by AlignTokens.


================
Comment at: clang/lib/Format/WhitespaceManager.cpp:108
   alignConsecutiveAssignments();
+  alignConsecutiveShortCaseStatements();
   alignChainedConditionals();
----------------
Is this the right position?
Could you add a test with aligning assignments and case statements?

```
case XX: xx = 2; break;
case X: x = 1; break;
```
It should end up as
```
case XX: xx = 2; break;
case X:  x  = 1; break;
``` 


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

https://reviews.llvm.org/D151761