krasimir added inline comments.
================ Comment at: lib/Format/UnwrappedLineFormatter.cpp:405 + for (; J != E; ++J) { + const AnnotatedLine *Line = J[0]; + if (Line->InPPDirective != InPPDirective) ---------------- I'd change `J[0]` to `*J` and rename `Line` to something else so that it doesn't shadow `Line`. ================ Comment at: lib/Format/UnwrappedLineFormatter.cpp:406 + const AnnotatedLine *Line = J[0]; + if (Line->InPPDirective != InPPDirective) + break; ---------------- Please add a test for this if. ================ Comment at: unittests/Format/FormatTest.cpp:912 + " break;\n" + "}", + Style); ---------------- I'd suggest adding more cases here, like: ``` "case 6: /* comment */ x = 1; break;\n" "case 7: x = /* comment */ 1; break;\n" "case 8:\n" " x = 1; /* comment */\n" " break;\n" "case 9:\n" " break; // comment line 1\n" " // comment line 2\n" "case 10:\n" " return; /* comment line 1\n" " * comment line 2 */\n" "}", ``` I'm interested also why `case 10` here fails. ================ Comment at: unittests/Format/FormatTest.cpp:930 + "}", + Style)); + verifyFormat("switch (a) {\n" ---------------- I'd like to see tests where an overly long comment line appears, like: ``` EXPECT_EQ("switch (a) {\n" "case 0:\n" " return; // long long long long long long long long long long " "long long comment\n" " // line\n" "}", format("switch (a) {\n" "case 0: return; // long long long long long long long " "long long long long long comment line\n" "}", Style)); EXPECT_EQ("switch (a) {\n" "case 0:\n" " return; /* long long long long long long long long long long " "long long comment\n" " line */\n" "}", format("switch (a) {\n" "case 0: return; /* long long long long long long long " "long long long long long comment line */\n" "}", Style)); ``` https://reviews.llvm.org/D35557 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits