[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