[PATCH] D35557: clang-format: merge short case labels with trailing comments
Krasimir Georgiev via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 21 02:38:08 PDT 2017
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
More information about the cfe-commits
mailing list