[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

[Galen Elias via Phabricator via cfe-commits]

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 \"info\";\n"
----------------
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.


================
Comment at: clang/unittests/Format/FormatTest.cpp:19257
+
+  Alignment.ColumnLimit = 80;
+  Alignment.SpaceBeforeCaseColon = true;
----------------
HazardyKnusperkeks wrote:
> For what is this?
Oops, I was trying to set ColumnLimit = 0 before to stop it from merging multi-line case statements, but it wasn't working as expected, so I ended up inserting comments instead, and forgot to remove resetting this.  Good catch!


================
Comment at: clang/unittests/Format/FormatTest.cpp:19206
+
+  // Make sure we don't incorrectly align correctly across nested switch cases
+  EXPECT_EQ("switch (level) {\n"
----------------
HazardyKnusperkeks wrote:
> 
I'll get used to adding full stops eventually... sorry for not catching this.


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

https://reviews.llvm.org/D151761