[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

[Galen Elias via Phabricator via cfe-commits]

galenelias marked an inline comment as done.
galenelias added inline comments.


================
Comment at: clang/include/clang/Format/Format.h:327
+  /// \version 17
+  AlignConsecutiveStyle AlignConsecutiveShortCaseStatements;
 
----------------
HazardyKnusperkeks wrote:
> 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.
Sure. Will post an iteration with this - hopefully I'm getting all the plumbing correct.


================
Comment at: clang/lib/Format/WhitespaceManager.cpp:108
   alignConsecutiveAssignments();
+  alignConsecutiveShortCaseStatements();
   alignChainedConditionals();
----------------
HazardyKnusperkeks wrote:
> 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;
> ``` 
Great point, I hadn't actually really fully considered the ordering dependency here.  I agree that aligning short case statements before assignments (and probably declarations?) seems more correct.  However, these other alignConsecutive* methods don't actually align across scopes, so won't align declarations/assignments within the body of a case statement.  I verified that these don't align today using AlignConsecutiveAssignments.  I will however move this up to try to make it as logically correct as possible.


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

https://reviews.llvm.org/D151761