[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseLabels

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

HazardyKnusperkeks added a comment.

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

> In D151761#4385163 <https://reviews.llvm.org/D151761#4385163>, @HazardyKnusperkeks wrote:
>
>> When I read the title I thought "Yay, some one is doing what I want and don't find the time.", but (for me) sadly you're not.
>> I'd like to align the colon (and thus the statement behind that). Do you think you can add that option too?
>
> Hmm, it had not occurred to me that there are folks who would want to insert the spaces after the 'case'.  I can think about how that might be accomplished, but it doesn't seem straightforward given the current alignment infrastructure since the token you want to align (the colon) isn't the token which should be padded (the token after the case keyword).  I think I'd have to write custom alignment code to allow for the separation of these two.

After sleeping over it I want to revoke my request. Aligning the statements is sufficient, we don't need too much options. It just has to work with `SpaceBeforeCaseColon`, which I assume it already does.



================
Comment at: clang/docs/ClangFormatStyleOptions.rst:790
+
+**AlignConsecutiveShortCaseLabels** (``AlignConsecutiveStyle``) :versionbadge:`clang-format 17` :ref:`¶ <AlignConsecutiveShortCaseLabels>`
+  Style of aligning consecutive short case labels.
----------------
MyDeveloperDay wrote:
> galenelias wrote:
> > MyDeveloperDay wrote:
> > > Did you generate this by hand or run the dump_format_style.py function? Format.h and the rst look out of sync
> > This was generated from dump_format_style.py.  What looks out of sync?  My guess is the confusing thing is all the styles which use `AlignConsecutiveStyle` get the same documentation pasted for them which specifically references `AlignConsecutiveMacros` and makes it look like it's for the wrong option.
> that doesn't feel right to me.. not saying its not dump_format_style.py but it shouldn't do that in my view
What shouldn't it be doing?
The struct is used for multiple options and the documentation is that ways since we have the struct.


================
Comment at: clang/include/clang/Format/Format.h:308
+  /// \version 17
+  AlignConsecutiveStyle AlignConsecutiveShortCaseLabels;
 
----------------
galenelias wrote:
> HazardyKnusperkeks wrote:
> > We need another name, you're not aligning the label, but the statement.
> I was taking the name from the GitHub issue, which while I agree is a slight misnomer seemed to yield what I assumed most people would expect.  I guess `AlignConsecutiveStatementsAfterShortCaseLabels` is more correct, but is also getting pretty verbose.
Maybe just `AlignConsecutiveShortCaseStatements`?


================
Comment at: clang/unittests/Format/FormatTest.cpp:19195
 
+TEST_F(FormatTest, AlignConsecutiveShortCaseLabels) {
+  FormatStyle Alignment = getLLVMStyle();
----------------
Could you add test cases with not short cases between/at the begin of/at the end of the short ones?


================
Comment at: clang/unittests/Format/FormatTest.cpp:19255
+               Alignment);
+}
+
----------------
galenelias wrote:
> HazardyKnusperkeks wrote:
> > Add a test with `AcrossEmptyLines` and `AcrossComments`.
> There is a test for both `AcrossEmptyLines` and `AcrossComments`.  Maybe I'm not understanding your comment?
A test where both are `true`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151761