[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

Galen Elias via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 9 14:51:08 PDT 2023


galenelias added a comment.

In D151761#4404179 <https://reviews.llvm.org/D151761#4404179>, @HazardyKnusperkeks wrote:

> In D151761#4403828 <https://reviews.llvm.org/D151761#4403828>, @galenelias wrote:
>
>> In D151761#4400693 <https://reviews.llvm.org/D151761#4400693>, @HazardyKnusperkeks wrote:
>>
>>> I'd say: align it.
>>
>> If it was straightforward, I would definitely agree to just align across empty case labels, but unfortunately there is no way to do that while using the generic AlignTokens (since the line with just a case has no token which matches the pattern, but needs to not break the alignment streak).  I guess the way to do it generically would be to add some sort of ExcludeLine lambda to allow filtering out lines.  Given that I don't think this is a common pattern with these 'Short Case Labels', and it's fairly easy to work around with `[[fallthrough]];` my plan is just to leave this as is (and add a test to make the behavior explicit).
>
> Leaving it open (and documenting that) I could get behind, but making it explicit as desired behavior will not get my approval. On the other hand I will not stand in the way, if the others approve.

Yah, I think leaving it open would be my preference at this point.  Not sure how to properly document that though?  Just be explicit in the tests?  Mention it in `alignConsecutiveShortCaseStatements`?  Should it be documented in the generated documentation (that feels a bit heavy)?


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

https://reviews.llvm.org/D151761



More information about the cfe-commits mailing list