owenpan added a comment.
FWIW, I think we can use a shorter name `AlignConsecutiveCaseStatements`
instead of `AlignConsecutiveShortCaseStatements`.
================
Comment at: clang/lib/Format/WhitespaceManager.cpp:854
+ return C.Tok->is(TT_CaseLabelColon);
+ } else {
+ // Ignore 'IsInsideToken' to allow matching trailing comments which
----------------
No `else` after `return`.
================
Comment at: clang/lib/Format/WhitespaceManager.cpp:859-860
+ // reflow early due to detecting multiple aligning tokens per line.
+ return (!C.IsInsideToken && C.Tok->Previous &&
+ C.Tok->Previous->is(TT_CaseLabelColon));
+ }
----------------
Please remove the redundant parentheses.
================
Comment at: clang/lib/Format/WhitespaceManager.cpp:909
+
+ if (!Changes[I].Tok->is(tok::comment))
+ LineIsComment = false;
----------------
================
Comment at: clang/lib/Format/WhitespaceManager.cpp:913
+ if (Changes[I].Tok->is(TT_CaseLabelColon)) {
+ LineIsEmptyCase = Changes[I].Tok->Next == nullptr ||
+ Changes[I].Tok->Next->isTrailingComment();
----------------
================
Comment at: clang/unittests/Format/FormatTest.cpp:19284-19293
+ verifyFormat("switch (level) {\n"
+ "case log::info: return \"info\";\n"
+ "case log::warning: return \"warning\";\n"
+ "// comment\n"
+ "case log::critical: return \"critical\";\n"
+ "default: return \"default\";\n"
+ "\n"
----------------
================
Comment at: clang/unittests/Format/FormatTest.cpp:19218
+ // Verify comments and empty lines break the alignment
+ verifyFormat("switch (level) {\n"
+ "case log::info: return \"info\";\n"
----------------
HazardyKnusperkeks wrote:
> galenelias wrote:
> > HazardyKnusperkeks wrote:
> > > If both strings are the same, you only need to supply it once.
> > >
> > > Or are they different and I can't see it?
> > They are the same, but I can't use the single string version, as that one
> > calls test::messUp on the string which will strip blank lines, which then
> > invalidates the test case. It seems pretty much all tests which are trying
> > to validate behavior around empty spaces has to use the two string version.
> Check.
> They are the same, but I can't use the single string version, as that one
> calls test::messUp on the string which will strip blank lines, which then
> invalidates the test case. It seems pretty much all tests which are trying
> to validate behavior around empty spaces has to use the two string version.
We have `verifyNoChange` for that now.
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
