[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