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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
