[PATCH] D158795: Fix AlignArrayOfStructures + Cpp11BracedListStyle=false

galenelias marked 4 inline comments as done. galenelias added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:20888 + " { 7, 5,\"!!\" }\n" + "};\n", + Style); owenpan wrote: > galenelias

[PATCH] D158795: Fix AlignArrayOfStructures + Cpp11BracedListStyle=false

galenelias updated this revision to Diff 553809. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158795/new/ https://reviews.llvm.org/D158795 Files: clang/lib/Format/WhitespaceManager.cpp clang/unittests/Format/FormatTest.cpp Index: clang/unittests/Format/FormatTest.cpp

[PATCH] D158795: Fix AlignArrayOfStructures + Cpp11BracedListStyle=false

galenelias updated this revision to Diff 553783. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158795/new/ https://reviews.llvm.org/D158795 Files: clang/lib/Format/WhitespaceManager.cpp clang/unittests/Format/FormatTest.cpp Index: clang/unittests/Format/FormatTest.cpp

[PATCH] D158795: Fix AlignArrayOfStructures + Cpp11BracedListStyle=false

galenelias marked an inline comment as done. galenelias added inline comments. Comment at: clang/lib/Format/WhitespaceManager.cpp:1247 if (Previous && Previous->isNot(TT_LineComment)) { - Changes[Next->Index].Spaces = 0; + Changes[Next->Index].Spaces =

[PATCH] D158795: Fix AlignArrayOfStructures + Cpp11BracedListStyle=false

galenelias created this revision. galenelias added a project: clang-format. Herald added projects: All, clang. Herald added a subscriber: cfe-commits. Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay. galenelias requested review of this revision. Fixes:

[PATCH] D156705: [clang-format] Fix braced initializer formatting with templated base class initializer

galenelias added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:13461 " A() : a{} {}\n" + " A() : Base{} {}\n" " A(int b) : b(b) {}\n" HazardyKnusperkeks wrote: > Please also add nested templates. Ok, added `A() :

[PATCH] D156705: [clang-format] Fix braced initializer formatting with templated base class initializer

galenelias updated this revision to Diff 545798. galenelias marked an inline comment as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156705/new/ https://reviews.llvm.org/D156705 Files: clang/lib/Format/UnwrappedLineParser.cpp clang/unittests/Format/FormatTest.cpp Index:

[PATCH] D156705: [clang-format] Fix braced initializer formatting with templated base class initializer

galenelias created this revision. Herald added projects: All, clang, clang-format. Herald added a subscriber: cfe-commits. Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay. galenelias requested review of this revision. This fixes

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

galenelias added a comment. In D150403#4539874 , @owenpan wrote: > This seems to cause a regression. See > https://github.com/llvm/llvm-project/issues/64134. @galenelias any idea? I will take a look. The logic I added is trying to distinguish `{ } {`

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

galenelias updated this revision to Diff 543558. galenelias added a comment. Addresses latest review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151761/new/ https://reviews.llvm.org/D151761 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/ReleaseNotes.rst

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

galenelias marked 6 inline comments as done. galenelias added a comment. In D151761#4524653 , @owenpan wrote: > FWIW, I think we can use a shorter name `AlignConsecutiveCaseStatements` > instead of `AlignConsecutiveShortCaseStatements`. My only

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

galenelias added a comment. In D151761#4483050 , @HazardyKnusperkeks wrote: > Thanks for the patience, I'm really looking forward to use this. > > But please wait for other opinions. Thanks so much @HazardyKnusperkeks for all the feedback and help.

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

galenelias updated this revision to Diff 538288. galenelias marked 2 inline comments as done. galenelias added a comment. Addressed outstanding review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151761/new/ https://reviews.llvm.org/D151761 Files:

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

galenelias marked 6 inline comments as done. galenelias added inline comments. Comment at: clang/include/clang/Format/Format.h:380 +} +bool operator!=(const ShortCaseStatementsAlignmentStyle ) const { + return !(*this == R); HazardyKnusperkeks

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

galenelias updated this revision to Diff 538228. galenelias marked an inline comment as done. galenelias edited the summary of this revision. galenelias added a comment. This iteration switches away from using `AlignConsecutiveStyle` and instead uses a new `ShortCaseStatementsAlignmentStyle`.

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

galenelias marked an inline comment as done. galenelias added inline comments. Comment at: clang/include/clang/Format/Format.h:327 + /// \version 17 + AlignConsecutiveStyle AlignConsecutiveShortCaseStatements; HazardyKnusperkeks wrote: > Since you are not

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

galenelias updated this revision to Diff 537383. galenelias edited the summary of this revision. galenelias added a comment. I re-wrote the alignment to stop using AlignTokens so that I can now handle all the edge cases that came up. Specifically: - Allowing empty case labels (implicit fall

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

galenelias added a comment. 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

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

galenelias updated this revision to Diff 530707. galenelias edited the summary of this revision. galenelias added a comment. 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

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

galenelias added a comment. In D151761#4404179 , @HazardyKnusperkeks wrote: > In D151761#4403828 , @galenelias > wrote: > >> In D151761#4400693 , >>

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

galenelias updated this revision to Diff 529365. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151761/new/ https://reviews.llvm.org/D151761 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/ReleaseNotes.rst clang/include/clang/Format/Format.h clang/lib/Format/Format.cpp

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

galenelias added a comment. In D151761#4400693 , @HazardyKnusperkeks wrote: > In D151761#4400056 , @galenelias > wrote: > >> In D151761#4394758 , >> @MyDeveloperDay

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

galenelias marked an inline comment as done. galenelias added a comment. In D151761#4394758 , @MyDeveloperDay wrote: > did you consider a case where the case falls through? (i.e. no return) > > "case log::info :return \"info\";\n" > "case

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

galenelias marked 3 inline comments as done. galenelias added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:798 + +case log::info: return "info:"; +case log::warning: return "warning:"; MyDeveloperDay wrote: > Do you think the

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

galenelias marked 3 inline comments as done. galenelias added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:19218 + // Verify comments and empty lines break the alignment + verifyFormat("switch (level) {\n" + "case log::info:return

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

galenelias updated this revision to Diff 528118. galenelias added a comment. Fixup up review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151761/new/ https://reviews.llvm.org/D151761 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/ReleaseNotes.rst

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

galenelias updated this revision to Diff 528072. galenelias marked 3 inline comments as done. galenelias retitled this revision from "clang-format: Add AlignConsecutiveShortCaseLabels" to "clang-format: Add AlignConsecutiveShortCaseStatements". galenelias edited the summary of this revision.

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseLabels

galenelias added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:790 + +**AlignConsecutiveShortCaseLabels** (``AlignConsecutiveStyle``) :versionbadge:`clang-format 17` :ref:`¶ ` + Style of aligning consecutive short case labels.

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseLabels

galenelias added a comment. In 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

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseLabels

galenelias created this revision. galenelias added a project: clang-format. Herald added projects: All, clang. Herald added a subscriber: cfe-commits. Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay. galenelias requested review of this revision. This resolves

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

galenelias updated this revision to Diff 524484. galenelias edited the summary of this revision. galenelias added a comment. Thanks for the additional review comments. Hopefully everything if fixed. In D150403#4358845 , @owenpan wrote: > Can you put

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

galenelias added a comment. In D150403#4347323 , @HazardyKnusperkeks wrote: > We'll wait a bit, if someone might have a comment. And (at least I) need name > and email for the commit. Name: Galen Elias Email: galenelias at gmail dot com CHANGES

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

galenelias added a comment. Thanks @HazardyKnusperkeks! I don't have commit access, so will need someone to land this for me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150403/new/ https://reviews.llvm.org/D150403 ___ cfe-commits

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

galenelias marked 3 inline comments as done. galenelias added a comment. Thanks for the review and feedback. Sorry about the unfortunate timing between @sstwcw and my fix - we submitted our fixes less than 24 hours apart. I didn't think there would be another simultaneous fix for this 5 year

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

galenelias updated this revision to Diff 522245. galenelias edited the summary of this revision. galenelias added a comment. Addressed remaining review feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150403/new/ https://reviews.llvm.org/D150403 Files:

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

galenelias added a comment. @HazardyKnusperkeks, thanks for the feedback. I added the TokenAnnotatorTests from @sstwcw's review. I also updated the design to use a single stack instead of the two parallel stacks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150403/new/

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

galenelias updated this revision to Diff 521828. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150403/new/ https://reviews.llvm.org/D150403 Files: clang/lib/Format/UnwrappedLineParser.cpp clang/unittests/Format/FormatTest.cpp clang/unittests/Format/TokenAnnotatorTest.cpp Index:

[PATCH] D150452: [clang-format] Recognize nested blocks

galenelias added a comment. Coincidentally I also sent out a review to fix this issue yesterday, but went with a different approach of trying to scope the ProbablyBracedList logic by just looking at the lbrace previous token. https://reviews.llvm.org/D150403 Repository: rG LLVM Github

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

galenelias added a subscriber: sstwcw. galenelias added a comment. Looks like @sstwcw also submitted a fix for the same issue, with a bit of a different approach: https://reviews.llvm.org/D150452 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

galenelias created this revision. galenelias added reviewers: cpplearner, HazardyKnusperkeks, MyDeveloperDay. Herald added projects: All, clang, clang-format. Herald added a subscriber: cfe-commits. Herald added reviewers: rymiel, owenpan. galenelias requested review of this revision. This is a