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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to