[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

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

HazardyKnusperkeks added a comment.

In D151761#4416224 <https://reviews.llvm.org/D151761#4416224>, @galenelias wrote:

> Well, I guess I didn't put quite enough thought into the `AlignCaseColons` option.  While this solves the empty case label problem, it will also match in non-short case label scenarios such as the following, which doesn't seem desirable:
>
>   switch (level) {
>   case log::info  :
>   case log::error :
>     return true;
>   case log::none     :
>   case log::warning  :
>   case log::critical :
>     return false;
>   }
>
> In order to solve this (and the other) issues, I think the only solution is to roll a custom alignment method instead of using `AlignTokens`, so that the alignment can be more correctly based on the detection of short case statements.
>
> This is going to take considerably more time and code, so not sure when I'll be able to work on it.

For me this would actually be desired. But from the name `ShortCaseStatements` I can see that it may be not what we want. This could be //fixed// with just renaming to `ConsecutiveCaseStatements`. :)

In D151761#4415755 <https://reviews.llvm.org/D151761#4415755>, @galenelias wrote:

> Ok, I added the ability to align the case label colons.  In your original message you mentioned "I'd like to align the colon (and thus the statement behind that)" which implies actually adding the whitespace after the 'case' token itself.  Not sure if that would still be your preference in an ideal world, or if I just misinterpreted your request.  Aligning the colons themselves is very straightforward.
>
> I opted to make this an option on `AlignConsecutiveStyle`, as that is consistent with how we customize some of the other AlignConsecutive* options, and it seemed awkward to add a floating top level boolean config option which applied to just this scenario - although it has the similar downside that it muddies the AlignConsecutiveStyle options for the other use cases.

I have no problem with that, but the full disclosure is: @MyDeveloperDay  isn't happy with e.g. `PadOperators` and thus will most likely not like this.

I think we need some input what should be able to be aligned - the statement vs. the colon - and only //short// case statements vs all case statements.



================
Comment at: clang/include/clang/Format/Format.h:257
+    /// \endcode
+    bool AlignCaseColons;
     bool operator==(const AlignConsecutiveStyle &R) const {
----------------
I'd also sort this alphabetically.


================
Comment at: clang/lib/Format/WhitespaceManager.cpp:881
+          return C.Tok->is(TT_CaseLabelColon);
+        } else {
+          // Ignore 'InsideToken' to allow matching trailing comments which
----------------
else after return can be dropped.


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

https://reviews.llvm.org/D151761